mcpField
C
code-review

code-review

github.com/vectorize-io测试与质量56.6k171 次安装今天更新
代码仓库

关于 code-review

code-review 是用于按项目约定检查代码变更的审查技能,涵盖 Python、TypeScript、API 设计、中间件和 HTTP 调用等规范。

主要功能

  1. 01Python 风格与类型检查
  2. 02Pydantic 模型要求
  3. 03TypeScript 与移动布局规则
  4. 04API 和分页约定
  5. 05ASGI 中间件指南
  6. 06异步 HTTP 客户端规则

适用场景

  1. 01合并前审查代码变更
  2. 02检查 API 端点是否符合约定
  3. 03审查异步 HTTP 调用方式
  4. 04评估控制面板的手机布局

安装

复制下面的命令,在终端中运行。

$npx skills add https://github.com/vectorize-io/hindsight --skill code-review

安装后在 Claude Code 中输入/code-review确认已加载。

详细介绍

代码审查

根据项目的质量标准和编码规范,审查所有变更的代码。

代码标准

编写代码前,请阅读并熟记以下标准。下方的审查步骤会检查代码是否符合这些标准。

支持的解释器

CPython 3.11 是基准版本——docker/standalone/Dockerfile 使用该版本,.python-version 固定的版本和 uv.lock 解析的版本也都是该版本。软件包支持 3.11 至 3.14;CI 中的 build-api-python-versions 会安装并对每个版本执行冒烟测试,因此依赖的最低版本若不支持其中任一解释器,就属于破坏性变更,而不是可以后续处理的问题。

Python 风格
  • Python 3.11+,必须添加类型提示——另请参阅上文的“支持的解释器”
  • 全程使用异步(asyncpg、异步 FastAPI)
  • 请求和响应使用 Pydantic 模型
  • 使用 Ruff 进行代码检查(行长 120)
  • 项目根目录下不得放置 Python 文件——保持目录结构整洁
  • 绝不返回包含多个项的元组——内部函数或私有函数也不例外。始终使用数据类或 Pydantic 模型。没有例外,也不要以“只有两个值”为由走捷径。函数若返回多个值,就为其定义一个具名类型。
使用 Pydantic 模型确保类型安全

绝不对结构化数据使用原始 dict 类型——这适用于所有代码,包括内部辅助函数和私有函数。如果 dict 的键已知,就必须使用数据类或 Pydantic 模型:

  • 函数之间传递的所有数据结构都使用 Pydantic BaseModel
  • 对于不需要 Pydantic 验证的轻量级内部数据容器,使用 @dataclass
  • 使用 @field_validator 进行类型转换(例如,确保日期时间带有时区)
  • 避免使用 dict.get() 模式;改用类型化模型的属性
  • 在边界处将外部数据(JSON、API 响应)解析为 Pydantic 模型
  • 这样可以在解析时捕获类型错误,而不是等到深入业务逻辑后才发现
  • 只有键真正动态或未知的数据才可以使用 dict(例如任意元数据、不符合固定架构的 JSON 数据块)
# BAD - error-prone dict access
def process(data: dict) -> str:
    return data.get("name", "")  # No validation, silent failures

# GOOD - typed and validated
class UserData(BaseModel):
    name: str
    created_at: datetime

    @field_validator("created_at", mode="before")
    @classmethod
    def ensure_tz_aware(cls, v):
        if isinstance(v, str):
            v = datetime.fromisoformat(v.replace("Z", "+00:00"))
        if v.tzinfo is None:
            return v.replace(tzinfo=timezone.utc)
        return v

def process(data: UserData) -> str:
    return data.name  # Type-safe, validated at construction
TypeScript 风格
  • 控制平面使用 Next.js App Router
  • 使用 Tailwind CSS 和 shadcn/ui 组件
手机端控制平面(宽 390px)

globals.css 已处理页面不出现横向滚动、仅悬停时显示的控件在触摸屏上可见,以及表单字段在 iOS 上不会触发缩放。以下检查涵盖共享规则无法处理的部分:

  • 页面子标签使用 components/ui/underline-tabs.tsx,不要手工实现按钮行。
  • 任何筛选器、开关或按钮行都必须换行(flex-wrap),或在自身内部滚动(使用 max-w-full overflow-x-auto,子项使用 shrink-0 whitespace-nowrap)。
  • 固定宽度不得超过约 340px,除非提供手机端备用样式(w-full sm:w-[420px]、w-[min(420px,calc(100vw-1rem))])。并排窗格在 md 以下应堆叠显示。
  • 所有表格都必须通过 components/ui/table.tsx 渲染:表格应在自身容器内滚动,并保留 40rem 的最小宽度,避免列在手机上被挤压。
  • 使用网格布局的表单区域需要设置 min-w-0(form-layout.tsx 中共享的 Section 已设置);并排输入框在 sm 以下应堆叠显示。
  • 全高布局使用 h-dvh/100dvh,而不是 h-screen/100vh(否则 iOS 上页面底部会被 URL 栏遮挡)。
代码注释
  • 始终为重要的技术决策添加注释,说明选择该方案的原因。如果有人会问“为什么要这样做?”,就应该添加注释。
  • 及时更新注释,使其与历史变更保持一致——更改实现方式时,更新注释,说明之前尝试过什么以及为何更改。注释用于记录此前可能存在问题的实现方式。
  • 不要为显而易见的代码添加注释——只在原因无法从代码本身看出时添加。
# BAD - no context for future readers
results = await asyncio.gather(*tasks, return_exceptions=True)

# GOOD - explains the non-obvious choice
# Use return_exceptions=True to avoid cancelling sibling tasks on failure.
# Previously we used TaskGroup but it cancelled all tasks when one failed,
# causing partial writes that left orphaned entity links (see #412).
results = await asyncio.gather(*tasks, return_exceptions=True)
API 层与数据访问
  • api/http.py(或任何 API 路由)中不得直接访问数据库。HTTP 处理程序不得构建 SQL、调用 acquire_with_retry / conn.fetch / conn.fetchrow / conn.execute,或引用 fq_table(...)。所有持久化和查询都位于 MemoryEngine(引擎层)中。处理程序负责解析/验证请求、调用引擎方法、构造 HTTP 响应,并将领域结果映射为状态码(例如返回 None → 404)。
  • 身份验证/租户隔离在每个引擎方法内部强制执行,而不是由处理程序假定。 每个涉及 bank 范围数据的引擎方法都必须通过 request_context 进行身份验证——通常是 await self._authenticate_tenant(request_context)(往往通过 get_bank_profile(...) 间接完成)——以便在执行任何查询之前解析出正确的租户 schema。处理程序必须将 request_context 传递给引擎方法;绝不能在假定 schema 已设置的情况下查询租户范围的表。
  • 引擎方法返回类型化模型(Pydantic/dataclass),而非原始 dict(参见类型安全)。
  • 每个列表端点都要分页,遵循现有的做法。 一个返回集合且集合大小随数据增长的 GET(banks、documents、memories、entities、operations、webhook deliveries、audit logs……)必须接受 limit/offset 并限制其结果——无界列表意味着无界负载加上无界的逐行工作(逐项计数、配置解析、嵌入水合)。照搬 list_documents 使用的形式,不要另创一种:处理程序上使用 limit: int = Query(default=100, ge=0) 和 offset: int = Query(default=0, ge=0),引擎方法上使用匹配的关键字参数,并且响应中携带该页以及 total、limit、offset,以便客户端知道何时停止。当集合是用户在 UI 中选择的内容时,添加 q 搜索参数——客户端过滤只能看到已加载的页面。构造上即有界的端点是例外,而非规则:按设计即为整体结构的树/导出,或在写入时限定的表(例如 observation_history / mental_model_history,在插入时裁剪到 *_max_entries)。如果有界,就分页。
HTTP 中间件
  • 绝不要添加 BaseHTTPMiddleware——这意味着不要使用 @app.middleware("http")(该装饰器会安装一个中间件),也不要使用 add_middleware(SomethingSubclassingBaseHTTPMiddleware)。Starlette 会将每个此类中间件的下游应用运行在一个子任务中,并通过一对 anyio 内存对象流来传递请求和响应。这会带来每个请求一次任务创建外加若干次调度跳转的开销,而由于该开销属于调度开销,它恰好在事件循环已经争用时增长:移除 API 中的两个此类中间件后,在相同并发下 /health/live 从约 2.4k 提升到约 7.9k rps,p99 从约 107ms 降到约 18ms(#4235)。它还会破坏其下层所有内容的 Request.is_disconnected()——http.disconnect 事件永远不会到达路由(#2122)——并且会以微妙的方式吞掉 BackgroundTask / 流式语义。
  • 改为编写纯 ASGI 中间件:一个带有 __init__(self, app) 和 async def __call__(self, scope, receive, send) 的类,当 scope["type"] != "http" 时直接透传,并在需要观察响应时包装 send(从 http.response.start 消息中读取状态码,将头以原始字节对的形式追加到 message["headers"])。在同一个任务中一次 await,没有跳转。hindsight_api/api/observability.py 和 api/disconnect.py 是可以照搬的范例。
  • 当工作需要路由上下文时,优先将工作下移一层。 任何需要知道命中了哪个端点——其参数、其签名、其请求体模型——的内容,都应放在 APIRoute 子类中(app.router.route_class = ...),而不是放在通过遍历 app.routes 并调用 route.matches() 来重新推导路由的中间件中。路由在那里已经解析完毕,且每个路由的事实可以在启动时计算一次,而不是每个请求都计算。参见 api/unknown_params.py。请注意,include_router 不会将应用的 route_class 应用于路由器自身的路由——FastAPI <= 0.140 保留每个源路由的类,>= 0.141 则从源路由器的 route_class 实例化——因此来自被包含/扩展路由器的路由需要在包含之前调用 use_unknown_params_routes(router)。
  • 由客户端输入构建的头值在到达 message["headers"] 之前必须经过净化:查询参数和请求体字段名是经过百分号解码的攻击者输入,因此非 latin-1 的名称会在 send 中途引发异常(因拼写错误而导致 500),而包含 CR/LF 的名称会拆分响应。
出站 HTTP:aiohttp,仅限异步
  • 生产代码使用 aiohttp 发起 HTTP 调用,绝不使用 httpx。 httpx 的异步客户端每次请求的开销明显高于 aiohttp(httpcore 连接池、anyio 层),而这些开销正好落在我们最热门的路径上——每次 retain 和 recall 时的 embeddings、reranking 和 LLM 调用。使用 hindsight_api/engine/aiohttp_session.py 构建客户端:LoopLocalSession(每个事件循环一个 ClientSession,惰性创建——绝不要在 __init__ 中或在另一个线程的 asyncio.run 下可能运行的 initialize() 中创建会话)、per_phase_timeout(httpx 风格的按阶段超时;aiohttp 的 total 会截断长时间流式响应体),以及 raise_for_status(抛出 UpstreamHTTPError,它保留响应体和 status_code,供 remote_retry 据此分类)。
  • 生产代码中禁止同步 HTTP —— 不得使用 requests、urllib.request、urllib3、http.client、httpx.Client,也不得在 SDK 有异步版本时使用同步 SDK 客户端(openai.OpenAI、cohere.Client、litellm.embedding……)。同步调用要么阻塞事件循环,要么需要为每个进行中的请求分配一个线程;应在循环上使用异步形式。将同步调用包装在 asyncio.to_thread / run_in_executor 中并不是解决办法。唯一可接受的例外是完全没有提供异步 API 的 SDK(例如 google.auth 的凭据刷新)——将其放在线程中运行,并在注释中说明原因。
  • 由 ruff 强制执行(hindsight-api-slim/pyproject.toml 中的 TID251、banned-api)。仅当为了配置或分类基于 httpx 构建的第三方 SDK 时,才允许 import httpx(llm_transport.build_sdk_timeout 用于 OpenAI/Anthropic SDK,remote_retry 对其错误进行分类),并附上 # noqa: TID251 以及指明该 SDK 的注释。拒绝任何其他 noqa: TID251。http_probe.py 是唯一一个按文件豁免(仅用标准库的就绪探测,独立进程)。
  • 测试不受此限制。 FastAPI 的 TestClient / httpx.ASGITransport 没问题。要为 aiohttp 客户端打桩上游,请使用 tests/aiohttp_stub.py::stub_server 来提供服务,而不是 mock 会话。
查询中的 Bank/Tenant 隔离
  • Bank 隔离是一条硬性安全不变量:任何查询都不得读取、计数、更新或删除其他 bank 的行。 Tenant 隔离在 schema 层面强制执行(解析后的 search_path / fq_table(...) 限定符,由 _authenticate_tenant 把关);bank 隔离则在 schema 内部通过每条触及多 bank 表的语句上的 bank_id 谓词强制执行。
  • 针对多 bank 表的每条 SQL 语句都必须受 bank_id 约束——直接在 WHERE 中约束,或传递性地约束(见下文)。多 bank 表带有 bank_id 列:memory_units、documents、entities、mental_models、knowledge_pages、memory_links、observation_history 等。
  • 陷阱:按调用方提供的、非全局唯一的键过滤而不带 bank_id。 诸如 document_id 和 mental_models.id 这类键仅在每个 bank 内唯一(其主键是复合主键,例如 (id, bank_id)),因此同一个 id 在每个 bank 中都合法存在。像 UPDATE memory_units SET tags = $1 WHERE document_id = $2 这样的语句——没有 bank_id——会静默地读取/写入每个共享该 id 的 bank 的行。这正是 #3429/#3430 中的确切缺陷。加上 AND bank_id = $n 即可修复。
  • 一条语句合法限定范围的三种方式(接受这些;标记任何不符合其中任何一种的语句):
    1. 显式 WHERE ... AND bank_id = $n。
    2. 全局唯一的单列主键。 按全局 uuid 主键(memory_units.id、entities.id、knowledge_pages.id)或 bank 编码键(chunks.chunk_id,由 engine/chunk_ids.py 构建)过滤不可能跨 bank 冲突。请注意,chunk id 之所以是单射的,只是因为该 helper 对每个组件内部的分隔符进行了转义——它所取代的朴素 {bank_id}_{document_id}_{idx} 拼接会让 ('a', 'b_c') 和 ('a_b', 'c') 指向同一行(#4244),因此在该 helper 之前写入的 id 不能安全地视为受 bank 限定。对比复合主键 id(documents.id/document_id、mental_models.id)——那些是危险的,必须携带 bank_id。
    3. 传递性。 没有 bank_id 列的联结表(unit_entities、entity_cooccurrences、observation_sources)只有在通过全局唯一的 unit/entity id 到达、且这些 id 本身是在同一次调用中从受 bank 限定的查询里选出的,并且边在构造上就是 bank 内的时候才是安全的。如果该 id 集合可能包含另一个 bank 的 id,则它未受限定。
  • 留意两种坏味道:(a)在 WHERE 中使用了调用方提供的 id 且邻近没有 bank_id,而同一方法中相邻的语句却带有 bank_id(这种不对称就是信号);(b)bank_id 谓词只在 if bank_id: 下应用,且默认值为 bank_id: str | None = None——即使当前所有调用方都传入了它,这也是一处潜在隐患。
  • 设计上跨 bank 的操作必须把 bank_id 重写为目标 bank。 transfer/import 路径是唯一合法跨越 bank 的路径;验证每次写入都固定为目标 bank_id,绝不继承源行的 bank_id。
数据库锁
  • 切勿使用 PostgreSQL 咨询锁(pg_advisory_lock、pg_try_advisory_lock、pg_advisory_xact_lock、pg_advisory_unlock 等),无论是在迁移、引擎代码还是其他任何地方。Hindsight 运行在连接池和管理型/兼容 PG 的服务之上,这些环境中的咨询锁不可靠或不受支持:当连接池将会话交给另一个客户端时,会话级锁会悄然泄漏或消失,而调用方可能永远阻塞在一个服务器永远不会授予的锁上。拒绝任何新的出现,包括那些看起来“安全”的、属于事务作用域的用法。
  • hindsight_api/migrations.py 中已有的用法属于历史遗留豁免,而非先例——它已被跟踪以待移除。不要照搬。
  • 应通过设计消除并发,而不是围绕并发加锁:给每个进程分配各自要写入的对象(例如按 schema 进行 DDL,而非共享 public. 对象),使操作幂等,或使用真正的行/表约束(INSERT ... ON CONFLICT、按固定顺序的 SELECT ... FOR UPDATE)。参见 #2690,那是一个曾使用 pg_advisory_xact_lock 而不得不被回滚的迁移。
并发:asyncio 与 threading 原语

一个 Hindsight 进程会运行多个线程(执行器、asyncio.to_thread),并且在测试和工具链中,还会涉及多个事件循环。以下规则在被破坏时是无声的——代码通过测试,却在负载下失败。

选择哪种锁。 这不是风格问题,而是归属问题:

  • asyncio.Lock / Semaphore / Event / Condition / Future 绑定到第一个等待它们的事件循环。 仅将它们用于由单个循环拥有的对象——每个请求、每个连接,或挂在某个由一个循环创建且仅由该循环触碰的对象上的状态。出现 RuntimeError: ... is bound to a different event loop 就是这条规则被破坏的表现。
  • 对于可从一个以上循环访问的任何事物——模块级单例、进程级缓存、注册表——使用 threading.Lock。它与循环无关。它只能保护无 await 的临界区——切勿跨 await 持有它,否则你阻塞的是整个循环,而不是让出。

如果某个临界区必须 await,并且该对象是进程全局的,那么这是设计问题,而不是加锁问题:应将状态按循环划定作用域。

切勿在导入时创建 asyncio 原语。 模块级的 asyncio.Lock()/Semaphore() 会在任何循环存在之前被构造,随后被所有循环共享。当同一进程中出现两个循环时(测试、线程中的 asyncio.run),它甚至会在单循环构建上失败。应在拥有它们的对象内部惰性构建。

合并 future 是按循环的。 一个用 asyncio.Future 去重并发加载的缓存,必须以其进行中映射的键为 (running loop, key)。跨循环共享缓存的数据是可行且可取的;共享 future 则不行。

模块级可变状态。 可从一个以上线程访问的进程全局 dict/set/list 需要保护,并且绝不能在另一个线程可能修改它时对其进行迭代(轻则 RuntimeError: dictionary changed size during iteration;若 C 扩展持有指向一个正在扩容的 dict 的借用引用,则可能发生段错误)。优先选择在锁下只预热一次,而非对热路径加锁。

文档代码示例
  • hindsight-docs/docs/ 中每个调用 Hindsight 的代码示例都必须来自经过测试的示例文件,绝不能使用内联围栏代码块。 这意味着 SDK 调用(Python、Node.js、Go)、hindsight ... 命令行命令,以及对 /v1/... 的 curl 请求。将代码放在 hindsight-docs/examples/api/<page>.{py,mjs,sh,go} 中,并置于 # [docs:<section>] / # [/docs:<section>] 标记之间(JS/Go 中为 //),然后用 <CodeSnippet code={...} section="<section>" language="..." /> 展示(参见 docs/developer/api/memory-banks.mdx)。CI 会针对实时服务运行每个示例文件(scripts/test-doc-examples.sh),因此重命名的方法或标志会破坏构建,而不是悄无声息地破坏文档。
  • 对于其 SDK 或命令行支持该调用的每种语言,都提供标签页,就像页面上相邻的章节那样。需要代码片段的 .md 页面要改为 .mdx。
  • 以下内容可以保留为内联:安装命令、环境/配置代码片段、端点签名行(GET /v1/...)、JSON 请求/响应结构,以及需要 CI 无法运行的基础设施的命令(嵌入式服务启动、管理数据库命令、FUSE 挂载)。
分支卫生
  • 始终从 origin/main 创建新的功能分支 —— 进行 rebase 以确保基线干净。
  • 只包含与 PR/分支/功能相关的提交 —— 不要有不相关的更改。如果分支包含不属于此处的提交,则必须在合并前移除。
通用原则
  • 不要添加超出要求的功能、重构代码或进行“改进”
  • 不要为不可能的场景添加不必要的错误处理
  • 不要为一次性操作创建辅助函数或抽象
  • 不要使用向后兼容的变通方案(未使用的变量、重新导出、“已移除”注释)
  • 三行相似的代码胜过过早的抽象

审查步骤

1. 检查分支卫生
  • 运行 git log --oneline main..HEAD 以列出分支上的所有提交。
  • 验证每个提交都与功能/PR 相关。标记任何不相关的提交。
  • 检查分支是否基于较新的 origin/main(没有过期的基线)。
2. 确定更改的文件

运行 git diff --name-only HEAD(未暂存)和 git diff --cached --name-only(已暂存)以获取所有更改的文件。如果没有本地更改,则使用 git diff main...HEAD --name-only 和 git diff main...HEAD 与基础分支进行 diff,以审查当前分支上的所有提交。

3. 运行 linter
./scripts/hooks/lint.sh

报告任何失败。不要自己修复它们 —— 只需报告。

4. 检查死代码

对于每个更改的 Python 文件,检查:

  • 未使用的导入(Ruff 应该能捕获这些,但要验证)
  • 已添加但从未在任何地方被调用的函数/方法/类
  • 已赋值但从未被读取的变量
  • 应删除的注释掉的代码块

对于每个更改的 TypeScript 文件,检查:

  • 未使用的导入
  • 未使用的变量或函数
  • 注释掉的代码
5. 检查类型安全(Python)

对于每个更改的 Python 文件,检查是否有违规:

  • 结构化数据不得使用原始 dict —— 必须使用 Pydantic 模型或 dataclass,即使对于内部/私有函数也是如此(唯一例外:真正动态/未知的键)
  • 不得返回多项元组 —— 必须使用 dataclass 或 Pydantic 模型,即使对于内部/私有函数也是如此(没有例外)
  • 函数参数和返回类型上缺少类型提示
  • 对于应为时区感知的 datetime 字段,缺少 @field_validator
6. 检查缺失的测试

对于每个新增或发生重大变更的函数/端点/类:

  • 检查是否有对应的测试新增或更新
  • 新增 API 端点必须有集成测试
  • 新增工具函数必须有单元测试
  • Bug 修复应有回归测试

标记任何缺少测试覆盖的新逻辑。

大模型行为变更需要真实的 LLM 评判测试,而不是 MockLLM。 如果变更改变了模型解释提示词的方式——事实/观察提取、fact_type(world/experience)分类、说话人归属、指令遵循、提示词措辞——则必须有一个标记为 pytest.mark.hs_llm_core 的测试运行真实流水线,并通过 tests.llm_judge.assert_meets_criteria 进行断言(而不是字符串/枚举匹配)。将以下情况标记为发现项:

  • 仅由 MockLLM 或字符串断言验证的提示词/分类变更(MockLLM 会回显输入——此类测试会虚假通过)。应修复。
  • 硬断言 fact_type == "world"/"experience"(或其他由模型决定的输出)而不是对其进行评判的测试——这是非确定性的,会在不同提供商/运行之间出现不稳定。应修复(将分类检查移入评判器的 criteria;只保留真正确定性的结构化断言为直接断言)。
  • 仅由慢速 LLM 测试覆盖的确定性机制(提示词组装、抑制/分支逻辑)——这些也应具备快速的非 LLM 单元测试。注意。

完整模式参见 CLAUDE.md → Key Conventions → Testing。

6a. 检查测试是否通过引擎 API 而非原始 SQL 断言记忆状态

测试必须通过调用公共 MemoryEngine 读取 API 来验证 retain / recall / consolidation 产生的结果——list_memory_units(单元及其 metadata / tags;通过 total 获取计数;document_id / fact_type / entity_id 过滤器)、list_entities(规范名称、提及计数)、get_graph_data(节点/边)、get_bank_stats、recall_async——而不是通过 pool.acquire() / conn.fetch* 用原始 SQL 直接访问记忆表(memory_units、memory_links、unit_entities)。对这些表进行断言会使测试与存储层细节耦合,检查的是代理而非可观察属性(参见 General Principles → 测试断言属性,以及 7b 中的处理器规则)。

标记为应修复任何新增或变更的测试,其断言针对 memory_units / memory_links / unit_entities 运行 SELECT / COUNT,而引擎读取方法会返回相同的事实。主要迹象:测试体内出现 async with pool.acquire() as conn: 后跟 SELECT ... FROM memory_units;一个 fetchval("SELECT count(*) FROM memory_units ..."),而 list_memory_units 的 ["total"] 会返回相同结果;一个 canonical_name 查询,而 list_entities 已覆盖。

仅当直接 SQL 用于强制或检查公共 API 无法表达的内部状态时,对这些表使用直接 SQL 才是合理的——例如用 UPDATE documents SET updated_at 伪造竞态,或原始的 memory_links 行数,而去重后的 get_graph_data 边列表无法重现该行数。这些情况必须附带注释说明为何需要直接访问;标记任何没有这样做的。

6b. 检查面向用户的能力是否有系统测试

hindsight-system-tests/ 存放着黑盒故事,它们通过已发布的 Python 客户端驱动真实的 hindsight-api 进程——不访问引擎、不用 SQL、不导入内部模块。它们之所以存在,是因为约 500 个文件的 api-slim 套件是每个机制一个测试,这能抓到机制缺陷,却会漏掉组合缺陷:整合操作把它下面的事实抹掉、增量刷新漏掉一个回溯窗口、传输丢弃思维模型证据、重处理静默地什么也没做。这些缺陷中的每一个都跨越了任何单机制测试都不会触及的多个步骤。跟踪 issue:#4214。

当一项变更新增了用户能叫得出名字的能力,或者让两个既有能力相遇时,它就需要一个系统故事。具体来说,把符合以下情况的 PR 标记为应当修复:

  • 新增或修改 API 端点、retain/recall/reflect 参数,或改变可观察行为的 bank-config 字段;
  • 新增派生层或生命周期步骤(一种 observation 类型、一个刷新触发器、一个后台操作);
  • 让一个既有功能首次与另一个功能交互——新的组合恰恰是单元套件看不到的;
  • 修复组合缺陷。回归测试应放在这里,而不只是放在 api-slim,因为缺陷存在于步骤之间的接缝处。

以下情况不需要系统故事:无可观察变化的内部重构、性能工作、已被现有故事覆盖的机制,或任何仅暴露控制面的东西(本套件按设计仅限 API)。

审查新增或修改的故事时,检查:

  • 它经过客户端。 用 client.aretain(...),而不是 httpx,也不是 MemoryEngine。绕开已发布的客户端会掩盖本套件存在的意义——暴露 SDK 缺陷——import_bank_template 曾无法从任何 SDK 调用(#4232),只有客户端驱动的测试才能发现它。如果故事为让自己通过而绕开客户端,则必须修复。
  • 每次 LLM 调用都被声明。 未脚本化的调用会让测试失败并给出要粘贴的规则;用一个看似合理的默认值来应答某个调用的故事什么也证明不了。绝不要添加兜底规则来压住漏报。
  • 后台工作是被等待,而不是被禁用。 Retain 会入队整合;为保持确定性而把它关掉的故事,已经移除了组合缺陷所栖身的那一半。使用 settled(bank_id)。
  • 它断言整个确定性载荷,而不是某个关键词是否出现。在 LLM、嵌入器和重排器都被打桩的情况下,recall 是其输入的纯函数——排名、分数和渲染文本全都可以固定下来,而"这个词在某处出现过"在融合被反转时同样能通过。
  • 已知缺陷应当失败,而不是被记录在案。 如果 PR 留下一个未满足的契约,故事应断言我们想要的行为,并在修复前保持红色——而不是 xfail,那会让运行保持绿色,从而没有任何东西迫使这个问题被提出,也不是一个固定当前错误答案的测试,那会在有人修复它的那天崩掉,并教下一个读者去删测试。这两种形态都必须修复。

套件的 README 记录了这些约定;tests/test_01_retain_and_recall.py 是断言应当有多完备的参考。

7. 检查 API 一致性

如果 hindsight-api-slim/hindsight_api/api/ 中有任何文件被修改:

  • OpenAPI 规范是否重新生成了?(./scripts/generate-openapi.sh)
  • 客户端 SDK 是否重新生成了?(./scripts/generate-clients.sh)
  • 控制面代理路由是否更新了?(hindsight-control-plane/src/app/api/)
7a. 检查 TS/Python wrapper 客户端对等性

生成的两个 SDK 中,只有这两个在自动生成的底层客户端之上,额外提供了一个手写并持续维护的便捷 wrapper:

  • TypeScript:hindsight-clients/typescript/src/index.ts(HindsightClient)
  • Python:hindsight-clients/python/hindsight_client/hindsight_client.py(Hindsight)

(Rust/Go 等客户端是仅生成的——没有需要同步的 wrapper。)

这些 wrapper 才是大多数第三方使用者实际调用的对象,它们必须暴露相同的接口面。如果某个改动触及了一个 wrapper 的方法——增加/删除参数、更改默认值、转发新的查询/请求体字段——那么另一个 wrapper 中的对应方法必须在同一个(或紧随其后的)PR 中获得相同的改动。 某个参数在生成的 SDK 中存在,却被其中一个 wrapper 丢弃,就会在无声无息中为该语言的所有使用者剥离该参数(这正是 #2975 / #3042 为 listMentalModels/getMentalModel 上的 detail/tags_match/limit/offset 修复的问题)。应当修复——标记任何在一个语言中获得能力而另一个语言中没有的 wrapper 方法,并在两侧添加对应的映射回归测试。

注意:client-coverage-check CI 工具只校验请求体字段,不校验 GET 查询参数——因此查询参数对等性的缺口不会被自动捕获,必须在此处手工检查。

7b. 检查 API 层数据访问边界

对于 hindsight-api-slim/hindsight_api/api/ 中每个被改动的 handler(例如 http.py、mcp.py):

  • 标记 handler 中的任何直接数据库访问——acquire_with_retry、conn.fetch / fetchrow / execute、原始 SQL 字符串或 fq_table(...)。这些是必须修复的:查询必须移入返回类型化模型的 MemoryEngine 方法中,handler 必须调用该方法。
  • 验证认证在引擎中强制执行——handler 必须委托给某个通过 request_context 进行认证(通常经由 get_bank_profile 调用 _authenticate_tenant)的引擎方法。如果某个 handler 读写租户范围内的数据,却没有引擎方法强制执行认证,则是必须修复的(租户数据可能跨 schema 泄露)。
7c. 检查 bank/tenant 查询作用域

对于 diff 中每一条新增或修改的 SQL 语句(在 diff 中 grep conn.fetch、conn.fetchrow、conn.fetchval、conn.execute、executemany 以及任何原始 SELECT/INSERT/UPDATE/DELETE f-string,包括多行的),验证它无法触及另一个 bank 的行——参见上文 Bank/Tenant Isolation in Queries。

对于针对多 bank 表(memory_units、documents、entities、mental_models、knowledge_pages、memory_links、observation_history 等)的每条语句,确认它通过以下三种合法机制之一限定了作用域:

  1. 显式的 AND bank_id = $n;
  2. 全局唯一的单列主键(*.id uuid,或 bank 编码的 chunks.chunk_id)——不是像 documents.id/document_id 或 mental_models.id 这样的复合主键 id;
  3. 传递性地,通过一个本身在同一调用中从 bank 限定查询中选出的全局唯一 id 集合。

标记为必须修复:任何以调用方提供的、非全局唯一的键(document_id、mental_models.id、实体名称等)过滤多 bank 表,且没有 bank_id 谓词的语句——在标记前,构造具体的双 bank 场景(两个 bank 共享该 id;该语句读取/计数/更新/删除了错误 bank 的行或过量报告)以确认问题真实存在。主要迹象:一个携带 bank_id 的兄弟语句紧挨着一个不带 bank_id 的语句;WHERE bank_id 被 if bank_id: 保护,且默认为 None;一个导入/传输写入继承了源 bank_id,而不是固定目标。

7d. 检查列表端点是否分页

对于每个新增或修改的、返回集合的 GET 处理器,确认它接受 limit/offset 并返回 total —— 确切结构见上文 API 层与数据访问。然后端到端确认修复是真实的,因为一个无人强制的参数比没有参数更糟:

  • 限制要作用到实际工作上,而不仅仅是响应上。 验证页大小确实限制了开销大的部分 —— SQL 的 LIMIT/OFFSET,或者(当分页必须发生在进程内过滤之后时,例如 list_banks 中 filter_bank_list 扩展钩子可能丢弃任意 bank)显式切片并只对当前页执行逐项工作 —— 实时存储计数、get_bank_configs、重新嵌入。在一个可能丢弃行的过滤器之前用 SQL 分页是必须修复的问题:它会返回过短或空的页,以及一个把调用方看不到的行也算进去的 total。
  • 仓库内每个消费方都要分页。 一个新的默认 limit 会悄悄截断那些过去能拿到全部数据的调用方:控制平面(src/lib/api.ts + src/app/api/ 代理路由 + 任何持有完整列表的 context/selector)、CLI(hindsight-cli/src/api.rs)、MCP 工具,以及 Zapier 动态下拉框。每一个都必须分页取完,或者在其 UI 中暴露分页 —— 标记任何仍停留在单个默认大小页上的消费方。
  • 搜索也要随之移到服务端。 一个过去在客户端对完整列表做过滤的选择器,现在只会过滤已加载的页。如果端点新增了 q,UI 必须发送它(并禁用其本地过滤,例如 cmdk 的 shouldFilter={false});如果没有,说明为什么该集合足够小、不需要它。
  • 查找自身行的测试不能依赖落在第 1 页 —— 它们应该搜索或传入显式的 limit,而不是依赖默认排序。
8. 检查代码注释

对于每个非平凡的改动:

  • 新的非显而易见逻辑 —— 是否有注释解释其推理?
  • 改变的方法 —— 注释是否包含之前是怎么做的以及为什么改变?
  • 过时的注释 —— 改动代码附近的现有注释是否仍准确描述其行为?
9. 检查集成的完整性

如果 hindsight-integrations/ 中有任何文件被添加或修改,请验证:

  • 测试存在 — 该集成必须有模拟/演练外部框架的测试(而不仅仅是辅助函数的纯单元测试)。检查是否存在包含有意义测试文件的 tests/ 目录。
  • CI 任务存在 — 检查 .github/workflows/test.yml 中是否有对应的 test-<name>-integration 任务。如果缺失,请标记出来。
  • 发布流程 — 检查该集成名称是否位于 scripts/release-integration.sh 中的 VALID_INTEGRATIONS 数组,并且也在 hindsight-dev/hindsight_dev/generate_changelog.py 中的 INTEGRATIONS 字典中(changelog 生成器维护自己的列表;如果该名称在那边缺失,发布会在 changelog 步骤失败)。如果任一缺失,请标记出来。
  • 文档画廊 + 侧边栏条目 — 该集成必须在 hindsight-docs/src/data/integrations.json 中有一个条目。此文件是驱动集成画廊和文档侧边栏的单一事实来源(侧边栏分类会在渲染时从它注入到所有文档版本中)。该条目需要一个内部 /sdks/integrations/<slug> link,并在 hindsight-docs/docs-integrations/<slug>.md(x) 有一个匹配的页面。hindsight-docs/scripts/check-integrations.mjs 构建步骤会强制执行两个方向 — 正向:JSON 中的每个内部条目都有一个文档页面;反向:每个已发布的标签(integrations/<name>/vX.Y.Z)都出现在 JSON 中(像 cloudflare-oauth-proxy 这样的私有基础设施位于脚本的 EXCLUDED 集合中)。标记任何已发布(或正在发布)但缺失于 integrations.json 的集成,以及任何没有文档页面的 JSON 条目。请不要手动编辑 versioned_sidebars/*.json 来添加集成链接 — 它们是位置占位符,由 JSON 填充。
  • 代码规范 — 集成代码必须遵循所有 Python 风格规则(类型提示、不使用原始字典、不使用元组返回等)。
9a. 检查同级实现之间的一致性

当同一项能力在每个变体中都要实现一次时 —— 每个 harness、每种语言、每种方言、每个 provider 各一份 —— 新增或变更的那个变体正是能力悄然缺失的地方。这类缺陷在 diff 中从来都不像 bug:出错的是那段根本不存在的代码,而所有现有测试仍然通过,因为那个被遗忘的同级实现,按定义就是没人给它写测试的那个。dsh 就是这样在 daemon 模式下发布的,却从未真正启动过 daemon(#3524):ensureDaemon 只存在于 hook-only 包装器中,因此全部五个 persistent-plugin harness 都缺少它。

本仓库中已知的同级家族(这不是完整列表 —— 这条规则关注的是其形态):

家族位置
编码 Agent harnesshindsight-integrations/coding-agents/src/(hook harness 与 persistent-plugin harness:dsh、opencode、Kilo、Cline、Prime Agent)
包装器 SDK 客户端TypeScript + Python 包装器 —— 见步骤 7a
Alembic 迁移每个迁移中的 _pg_upgrade / _oracle_upgrade
数据平面 ↔ 控制平面api/http.py 参数与 hindsight-control-plane/src/app/api/** 代理路由 + lib/api.ts
大模型 providerengine/llm_wrapper.py 中的各 provider 分支

操作流程 —— 必须手动执行;没有 linter 能捕获它。 当 diff 新增了一个同级实现,或修改了某个家族中的一个同级实现时:

  1. 枚举该家族。 列出所有现有的同级实现(ls 该目录,grep 注册表)。
  2. 对比能力清单,而不是对比代码。 对于其他同级实现所具备的每一项能力 —— 调用的生命周期钩子、执行的 setup/teardown、遵循的配置开关、尊重的退出选项、注册表/安装器/文档条目 —— 确认被修改的同级实现也具备它,或者其缺失是有意为之并有注释说明。grep 就是工具:grep -rn ensureDaemon src 能证明谁调用了它。
  3. 优先提升而非复制。 如果该项能力现在存在于 N 处,通常的修复方式是把它移入所有同级实现已经共享的那一条路径(例如 RuntimeCore、buildHookOutput),而不是粘贴第 N 份副本,让第 N+1 个同级实现再次遗忘。
  4. 要求结构性的防护测试,而不只是单元测试。 针对那个被遗忘的同级实现的测试按构造就不存在,因此应要求一个覆盖整个家族的断言测试:从文件系统/注册表枚举同级实现,并断言每一个都满足该契约,同时附带一份显式且带注释的豁免清单。先例:registry covers every installable harness(harness/registry.test.ts)、every harness entrypoint reaches a daemon(core/daemon.test.ts)、test_backup_tables_covers_entire_schema、test_migration_shape.py。

将某项在所有同级实现中都存在、唯独一个缺失的能力标记为必须修复 —— 说明哪些同级实现具备它、哪个不具备,以及用户可见的症状是什么(对于 #3524:每次 hindsight_* 工具调用都会因 ECONNREFUSED 而失败,并且没有任何东西会启动 daemon)。新的家族成员上线时没有覆盖全家族的防护测试,则标记为应当修复。

10. 检查 MCP 工具注册的完整性

如果在 hindsight-api-slim/hindsight_api/mcp_tools.py 中新增了任何 MCP 工具,或重命名了现有工具:

  • mcp_tools.py 中的 _ALL_TOOLS 集合 —— 必须包含新的工具名
  • mcp_tools.py 中 register_mcp_tools() 的 tools_to_register 默认集合 —— 必须包含新的工具名
  • hindsight-api-slim/hindsight_api/api/mcp.py 中的 _SINGLE_BANK_TOOLS 集合 —— 如果新工具是 bank 作用域的(而不是像 list_banks/create_bank 这样的 bank 管理工具),则必须包含它
  • hindsight-control-plane/src/components/bank-config-view.tsx 中的 MCP_TOOL_GROUPS —— 必须将新工具纳入 UI 工具选择器对应的合适分组
  • 测试中的工具数量断言(例如 test_mcp_tools.py)—— 必须更新以反映新的数量
11. 检查备份/恢复表覆盖情况

如果某个迁移新增了 PostgreSQL 表(在 hindsight-api-slim/hindsight_api/alembic/versions/ 中查找 CREATE TABLE / op.create_table):

  • BACKUP_TABLES(位于 hindsight-api-slim/hindsight_api/admin/cli.py)——必须包含新表,并放在它通过外键引用的任何表之后(父表在子表之前)。缺失条目会导致静默的数据丢失:该表永远不会被备份,而恢复时的 TRUNCATE banks CASCADE 会清空任何外键指向 banks 的子表(例如 mental_models、directives),即使它从未被保存过。
  • 守卫测试 tests/test_admin_backup_restore.py 中的 test_backup_tables_covers_entire_schema 会强制执行这一点——如果新表未出现在 BACKUP_TABLES 中,请将其标记为 must fix。
  • 仅 Oracle 使用的表(例如 observation_sources)会被有意排除——admin 备份/恢复仅支持 PostgreSQL。
11b. 检查新配置标志是否更新了环境变量模板

如果 diff 新增了配置字段(hindsight-api-slim/hindsight_api/config.py 中新增的 ENV_* / HINDSIGHT_* 环境变量):

  • .env.example(仓库根目录)——必须添加该变量(如果是可选项则注释掉), 并在 hindsight-docs/docs/developer/configuration.mdx 中添加对应文档条目。 如果某个标志被添加到 config.py 但未出现在 .env.example 中,请标记为 should fix。
  • hindsight-embed/hindsight_embed/env.example——捆绑的副本必须与仓库根目录的 .env.example 保持逐字节一致(它用于初始化 embed/profile 配置)。 test_bundled_template_matches_repo_root 同步测试会在出现偏差时失败;如果根文件已更改但未重新复制,请标记为 must fix。
11c. 检查是否存在咨询锁

在 diff 中搜索 advisory(git diff main...HEAD | grep -in advisory)。任何新增的 pg_advisory_lock / pg_try_advisory_lock / pg_advisory_xact_lock / pg_advisory_unlock 调用都是 must fix——参见上文数据库锁定部分。请引导作者 转向替代方案(进程内对象、幂等 DDL、行级约束),而不是仅仅要求他们移除锁。

11e. 检查是否存在 BaseHTTPMiddleware
git diff main...HEAD -- '*.py' | grep -nE "@app\.middleware\(|BaseHTTPMiddleware"

任何新增的 @app.middleware("http") 或 BaseHTTPMiddleware 子类都是 must fix—— 参见上文“HTTP 中间件”部分。请要求使用纯 ASGI 中间件(或者如果逻辑需要路由上下文, 则使用 APIRoute 子类),并检查 send 包装器会对任何源自客户端输入的 header 值进行净化处理。

11f. 检查出站 HTTP 是否为 aiohttp 且为异步
git diff main...HEAD -- '*.py' ':!**/tests/**' | grep -nE "^\+.*(import httpx|from httpx|import requests|urllib\.request|http\.client|noqa: TID251|\.Client\(|asyncio\.to_thread|run_in_executor)"

生产代码中任何新增的 httpx 使用或同步 HTTP 调用都是 must fix——参见上文 “出站 HTTP”部分。ruff 能捕获导入问题;审查则能捕获它无法发现的问题:在存在异步客户端的情况下 使用同步 SDK 客户端、将同步调用推入 to_thread / run_in_executor、在 LoopLocalSession 之外创建 aiohttp.ClientSession,以及任何 并非用于配置或分类第三方 SDK 的 # noqa: TID251。

11g. 检查文档代码示例是否来自经过测试的代码片段

如果 diff 涉及 hindsight-docs/docs/**,请查找新增或变更的围栏代码块(```python、```bash、```typescript、```go 等)中调用 Hindsight 的部分:SDK 客户端方法、hindsight <cmd> CLI 命令,或向 /v1/... 发起的 curl 请求。每一项都必须是由 hindsight-docs/examples/api/ 中某个章节支持的 <CodeSnippet>(参见文档代码示例)。同时检查示例文件是否真正执行了该调用(而不仅仅是打印它),并清理了其 banks。将内联 API 示例标记为 must fix。

线上的 /developer/... URL 提供的是 hindsight-docs/versioned_docs/version-<latest>/ 中的最新快照,而非 docs/(后者对应 /developer/next/);发布时会将 docs/ 同步到其中。若某个文档修复需要在下一次发布前上线,也必须将其复制到该快照中。

11d. 检查并发原语

参见上文“并发”。对 diff 执行 grep:

git diff main...HEAD -- '*.py' | grep -nE "asyncio\.(Lock|Semaphore|Event|Condition|BoundedSemaphore|Future)\(|threading\.(Lock|RLock)\("

必须修复:

  • 在模块作用域创建的 asyncio 原语,或存储在进程级单例上的原语 —— 它会绑定到第一个等待它的循环,并破坏其他所有循环。
  • 在 await 期间持有 threading.Lock —— 这会阻塞事件循环,而不是让出控制权。
  • 一个在途/合并映射持有了 asyncio.Future,而其键未包含正在运行的循环。

还要检查该变更没有悄悄弃用某个解释器:

  • 新增依赖或版本升级,但缺少 3.11-3.14 中某个版本的 wheel —— build-api-python-versions CI 矩阵会安装并对每一个版本进行冒烟测试。
  • 语法或标准库用法比 3.11 更新(uv run ty check 能捕获大部分此类问题)。
  • 某个测试假设每个进程一个事件循环,而它覆盖的却是共享状态。

应当修复:

  • 新增进程级全局可变状态(dict/set/list,基于可变值的 lru_cache)却没有加锁,或在可能被并发修改的地方进行迭代。
12. 对照其他编码规范进行审查

检查 diff 中是否违反上述规范:

  • Python 文件位于项目根目录(不允许)
  • 缺少异步模式(应当全程使用异步)
  • 为请求/响应使用 Pydantic 模型
  • 行长超过 120 个字符
  • 超出要求范围的新功能/代码(过度设计)
  • 针对不可能出现的场景进行不必要的错误处理
  • 过早的抽象或臆测性的辅助函数
  • 向后兼容的临时方案(未使用的变量、重新导出、“已移除”注释)
13. 报告发现结果

按严重程度整理并呈现一份清晰的摘要:

必须修复 — 会导致 CI 失败或违反项目硬性规则的问题:

  • 分支上存在不相关的提交
  • Lint 检查失败
  • 公共函数缺少类型提示
  • 结构化数据使用原始 dict(包括内部代码)
  • 返回多项元组(包括内部代码)
  • 新端点缺少测试
  • 新增面向用户的能力,或现有能力的全新组合,但在 hindsight-system-tests/ 中没有对应故事(见步骤 6b)
  • 系统故事绕过已发布的客户端、静默未脚本化的 LLM 调用,或禁用后台工作以保持确定性
  • 在 api/ 处理函数中直接访问 DB(原始 SQL / acquire_with_retry / fq_table),而不是通过 MemoryEngine 方法
  • 在引擎中未强制执行认证的情况下访问租户范围数据(_authenticate_tenant / get_bank_profile)
  • 对多 bank 表的 SQL 语句按调用方提供的、非全局唯一的键过滤,但没有 bank_id 谓词(跨 bank 读写泄漏——见步骤 7c)
  • 新集成缺少测试、CI 任务或 release-integration.sh 条目
  • 已发布/新增的集成未出现在 hindsight-docs/src/data/integrations.json 中,或 JSON 条目没有对应的 docs-integrations/<slug> 页面(会通过 check-integrations.mjs 导致文档构建失败)
  • 新增 PostgreSQL 表未加入 admin/cli.py 中的 BACKUP_TABLES(恢复时会静默丢失数据)
  • 除 diff 中的实现外,同类实现都具备的某项能力(见步骤 9a)——某个 harness、方言、提供方或语言变体跳过了其他实现都会执行的生命周期步骤
  • 在导入时创建或由进程级单例持有的 asyncio 锁/信号量/事件,或在 await 期间持有 threading.Lock(见步骤 11d)
  • 内联文档代码示例直接调用 Hindsight(SDK、CLI 或 curl /v1/...),而不是使用来自经过测试的 hindsight-docs/examples/api/ 文件的 <CodeSnippet>(见步骤 11g)
  • 仅在部分受支持解释器(3.11 到 3.14)上可用的更改(见受支持解释器)

应当修复 — 影响代码质量的问题:

  • linter 未发现的死代码 / 未使用导入
  • 非平凡工具函数缺少测试
  • 超出任务范围的过度设计

注意 — 可能需要也可能不需要处理的现象:

  • 可能需要重新生成客户端的 API 更改
  • 偏离附近代码风格的模式

对于每项发现,请包含文件路径、行号以及简要说明。

不要自动修复任何问题。报告所有发现结果,让用户决定处理哪些问题。如果没有发现结果,请确认代码看起来没问题。

评论

支持 Markdown