mattac21/skills

质量审查

目标：$ARGUMENTS

审查目标的设计和测试质量。报告发现；不编辑代码。

1. 加载评估标准

在查看代码之前，先阅读以下内容：

• ~/.claude/CODING_STANDARDS.md 是评估标准。每个发现都应引用其一个章节标题。
• design-vocabulary.md 包含用于表述发现的术语，以及下文用于验证这些发现的测试。

如果仓库中存在 AGENTS.md / CLAUDE.md，也要阅读它们。如果这些文件说明仓库故意采取了一些标准不鼓励的做法，请在报告中说明一次，而不是标记每个实例。如果代码中观察到的是一种习惯（如每个模块都读取环境变量，每个测试都进行 monkeypatch），而不是有文档记录的选择，那么遵循该习惯的新代码仍然是一个发现。

2. 解析目标

• 无参数：当前分支与其基础分支的差异。运行 git merge-base HEAD origin/main（或 master），然后运行 git diff <base>...HEAD，加上来自 git diff HEAD 的未提交更改。
• PR 编号或 URL：运行 gh pr diff <n> 和 gh pr view <n> --json baseRefName,headRefName,files。
• 分支：运行 git diff <base>...<branch>。
• 路径，或 .（整个仓库）：按原样审查该路径下的现有代码，无差异。这是代码库模式，请参见下文。

3. 充分阅读以判断设计

变更模式（差异、分支、PR）

仅凭差异无法判断模块是否深层。对于每个发生变更的模块，阅读整个文件，并使用 grep 查找其调用者和测试。深度、所有权和重复性是需要根据调用点来判断的，因此请查看它们。

将发现的范围限定在变更引入或恶化的问题上。如果差异触及的代码中存在预先存在的问题，且该变更是修复它的自然时机，则该问题属于发现范围；如果代码未被触及，则不属于。

新模块需要进行模块级审查，如代码库模式所述：像其他部分一样全面地检查类型和可读性。对于每个新增或大幅重写的模块，在评判之前列出每个函数及其参数和返回类型，并记录它接受、返回或从中读取键的每个未类型化的映射、字典或元组，以及每个单字母名称。

在评判可读性之前，使用该语言常用的工具测量每个发生变更的源文件的圈复杂度：例如，Python 用 radon cc -s，Go 用 gocyclo，JavaScript/TypeScript 用 eslint 的 complexity 规则，Rust 用 clippy 的 cognitive_complexity，Java 用 PMD。如果仓库有自己的工具链，则使用它；否则使用一次性运行器（uvx、npx、go run）。优先使用能统计短路布尔操作数的工具；有些工具（例如 ruff 的 C901）会跳过它们，从而低估最差函数的情况。如果没有可用工具，则按照评估标准的定义手动计算决策点。差异新增且分数高于 10 的函数，或者现有函数因差异而超过 10 或进一步增加，则视为候选。对于现有函数，也要对基础版本进行评分（将 git show <base>:<file> 写入临时文件并测量），以便发现可以说明变更增加了什么。

当差异新增超过约 150 行源代码时，在您审查设计部分的同时，并行启动两个 general-purpose 子代理，每个代理都获得差异、变更文件列表及其部分的步骤 4 验证规则：一个负责类型、可读性和“在收益显著时倾向于不可变数据”，另一个负责测试，并将断言检查应用于每个新测试。在保留结果之前，验证每个代理返回的内容。

代码库模式（路径或 .）

路径下的所有内容都在范围内，但随着范围变大，发现的门槛会提高。单个模块或小目录（大致低于 1500 行源代码）会收到模块、函数和命名级别的发现：像其他部分一样全面地检查类型和可读性。一个包或整个仓库只关注大问题：阻碍代码库发展的设计决策和架构模式。当用户命名一个功能或工具（例如“整个 CLI”）但结果只是一个模块时，也同样适用：按模块规模进行评判。对于包或仓库规模的每个候选，请询问：

• 有多少模块、测试或未来的变更需要为其付出代价？如果代价仅限于一个函数，那么在这个规模下这是一个吹毛求疵的问题。
• 修复它是否会改变此仓库中新代码的编写方式？如果修复只是几行本地代码，对调用者没有影响，则放弃。
• 这是一个模式还是一次性的问题？单独的实例仅作为模式的证据。

大问题的例子：一个没有拥有模块的外部系统，因此每个调用者都重复进行身份验证和分页；入口点中融合了策略和编排，导致决策只能通过端到端测试；状态所有权分散在多个模块之间，每个模块都知道存储布局；在导入时构建客户端和配置，导致没有真实环境就无法测试；因为不存在接缝而到处修补内部实现的测试套件。一个长函数、一个命名错误的变量、一个被吞没的异常或一个弱断言本身都不是大问题。

这些问题跨越多个模块，因此在深入阅读文件之前先构建一个映射：

• 模块（文件或包）及其大致功能，来自 git ls-files <path> 和对每个模块顶部的浏览
• 主要部分的连接方式：谁调用谁，以及状态和数据如何在入口点、模块和外部系统之间移动
• 每个外部系统（API、数据库、队列、文件格式、模型）以及每个直接与之交互的模块
• 每个读取环境变量或配置的地方（os.environ、getenv、process.env、标志解析）
• 模块级可变状态和导入时副作用（在模块作用域内构建的客户端、连接、缓存、锁）
• 入口点，以及测试目录，其中每个测试文件练习了哪些模块

大部分工作用 grep 就够了。保持映射简洁；它是一个工作索引，而不是报告的一部分。

如果范围很小（大致低于 3000 行源代码），直接从映射进行审查，并并行启动两个 general-purpose 子代理，每个都获得文件列表及其部分的步骤 4 验证规则。一个只获取评估标准的类型、可读性和“在收益显著时倾向于不可变数据”部分。另一个只获取测试部分，并且必须将断言检查应用于每个测试。这样，这些部分就能得到充分关注，而不是在早期部分之后剩下的精力。在保留之前验证每个代理返回的内容。否则，将其拆分为相关模块的区域，每个区域带其测试，并将每个区域交给一个并行启动的 general-purpose 子代理。给每个代理提供评估标准路径、词汇路径、映射、其文件列表、步骤 4 验证规则以及上述大问题门槛，并告知不要编辑任何内容。一个只看到自己区域的子代理会倾向于局部的吹毛求疵，因此要求它提供其区域中看到的设计模式以及每个模式的证据（文件:行），而不是逐行的发现列表。跨模块问题来自映射，因此由您自己解决这些问题，然后将它们与子代理的模式结合起来，合并那些来自不同区域但相同底层决策的模式，并在报告之前重新检查每个模式的证据。

4. 对照每个部分进行审查

按顺序遍历标准：模块、状态、类型、依赖项和配置、错误、可读性、测试。对于每个候选发现，在保留之前进行验证：

• 浅层模块或直通模块：应用删除测试。命名调用者并说明哪些复杂性会重新出现在它们中，或者说明什么也不会发生。
• 未能证明其存在的抽象：统计接缝处的适配器数量。一个测试假实现算一个。没有隐藏任何内容的一个适配器是一个发现；两个适配器是一个真正的接缝。
• 缺失抽象：同样严格地检查相反方向。寻找一个在三个或更多位置上以内联方式处理的问题，无论是在一个模块内部还是跨模块。
• 缺失所有者 / 重复的问题：列出每个编写该问题的位置（文件:行），而不是说“这出现在多个地方”。
• 无所有者的概念：列出每个表示、解析或构建它的地方（文件:行），以及任何签名中同一类型的参数具有不同含义并排出现的位置。标识符也算作概念，不仅仅是结构化数据。不同种类的名称和 ID 作为相同的基本类型传递（源仓库名和目标仓库名、分支名和引用、两种 ID）是需要捕捉的主要情况。然后说出哪种类型的所有者合适。如果调用者持有并传递该概念，建议为其创建一个类型。如果他们只需要用它来完成某事，建议将其隐藏在完成工作的模块内部。不要默认使用领域类型。
• 排序约束或手动构建的键/路径/ID：展示调用者进行排序或构造的过程。
• 共享可变状态、加载时副作用、深层配置读取：命名全局变量或导入时调用，以及它阻止的测试或第二种配置。
• 快照写入：展示读取、长时间运行的步骤和回写。
• 最好改为不可变的可变数据：仅在可变性具有具体代价时报告。展示第二个持有者看到了意外的更改，它使缓存或键变得不安全，或者更改发生在远离值创建的地方。一个具有唯一所有者的可变结构，该所有者通过自己的代码更改它，这不是一个发现。如果不可变版本会损失性能，说明要记忆化或共享什么，以及该缓存位于何处。
• 可以表示的无效状态：命名绝不应发生的状态，并展示允许该状态的类型（必须同时设置的可选字段、布尔值的组合、具有少量允许值的字符串）。列出仅因为此而存在的守卫或 None 检查（文件:行）。提出一个使该状态不可能的形态，并说明该更改会删除哪些守卫。
• 作为概念的字典或元组：列出读取的键以及读取它们的每个函数（文件:行），以及构建字典的每个位置。以仓库现有的风格（pydantic 模型或 dataclass、TypeScript 接口、Go 结构体，只要是仓库已使用的）提出记录类型，只包含代码使用的字段，并说明负载在何处被解析为它。
• 拥有外部系统的模块：检查其函数是否通过模块全局变量或每次调用的环境读取来共享凭证、身份、基础 URL 或缓存，以及测试是否修补了其多个函数。如果是，建议使用一个具有领域命名方法的客户端类，在启动时构建并传入，并命名假实现将替换的修补。
• 缺失文档注释：列出每个没有注释的非平凡函数（文件:行），以及其文档注释需要说明而签名未说明的内容（回退、错误模式、它接触的状态、输入形态）。将它们分组为每个模块的一个可读性发现。
• 单字母名称：将它们报告为一个分组的可读性发现，列出每个（文件:行）并提出一个名称。它们作为一个发现计入可读性上限，绝不作为删除它们的理由。
• 被吞没的错误：展示从失败到结果发生变化而只有日志行记录它的路径。
• 具有多个含义的名称：使用 grep 查找它，并列出它在每个位置所持有的内容。
• 隐藏其层级或对象的名称：展示调用链或调用点，其中它读起来含糊不清，并提出替换方案。
• 做多件事的函数：尝试写出它的一句话总结。如果不能，引用阻止您的分支、try 块或模式参数。
• 过于复杂的函数：对于来自复杂性度量的每个候选，给出其分数，然后按顺序列出它运行的阶段（过滤输入、获取、声明、验证、构建、创建、记录、通知……），附带它们的行范围和每个阶段的失败规则。建议将拆分作为命名函数，每个阶段或决策一个，附带签名，并说明每个提取的部分如何在不进行修补的情况下变得可测试；通常决策成为纯函数，原始函数读起来像是一个命名步骤的简短序列。每个函数一个发现。高于 20 分时，它与设计发现同级；11 到 20 分是一个可读性发现，不记入可读性上限。不要因为该函数是入口点或仓库有更大的函数而放弃拆分。
• 测试断言：命名会使断言失败的产品缺陷。如果不能，这就是发现。对于参数化或表驱动的情况，跟踪每个输入穿过代码，并说明输入的哪一部分到达了该情况命名的行为。其区分性输入被早期分支消耗的情况，即使存在一个针对它的测试，也不覆盖该行为。同样，检查夹具或假实现是否实际上能够产生触发每个守卫或拒绝路径的输入。如果它永远不能，则该守卫未经测试。
• 测试替身：说明测试是关于什么的，然后说明替身是位于该边界之外还是重新实现了产品逻辑。
• 修补内部实现的测试：指向应该被提取出来的决策或应该被传入的依赖项，因为这才是真正的修复。
• 在任何发现的修复中提出接缝或端口：使用领域中的操作来命名它（close_ticket(id)），而不是传输动词（get/post/put）。一个只转发请求的端口会失败删除测试，并且它的假实现将不得不重新实现路由。在报告之前，对照删除测试检查每个提出的接缝。

赋予测试与之前部分相同的深度，即使早期部分产生了许多发现。针对每个测试文件运行上述每个测试检查：对于每个断言，命名会使它失败的产品缺陷。一个没有失败断言的与安全相关的行为（编辑、身份验证检查、审批关卡）与设计发现同级。

删除任何无法具体化的内容。删除标准未涵盖的风格吹毛求疵。除非直接由质量问题引起（例如，丢失数据的快照写入），否则不在此处报告正确性缺陷；用一行提及并指出 /code-review。

……

[为翻译而截断]