code-review

代码审查

价值： 反馈与沟通——结构化审查可以发现作者看不到的缺陷；将审查分成多个阶段，则可以避免一个领域的全面检查挤占其他领域的审查时间。

目的

讲解一种系统化的三阶段代码审查方法，分别检查规范遵循情况、代码质量和领域完整性。通过确保每个维度都得到重点关注，避免综合审查时遗漏问题。

实践

按顺序分三个阶段

按顺序分三个阶段审查代码。不要合并阶段。每个阶段只关注一个方面。前一阶段未通过，就不能进入后续阶段——审查不符合规范的代码质量毫无意义。

阶段 1：规范遵循情况。 代码是否按要求实现？不能多，也不能少。

对于每项验收标准或要求：

1. 找到实现该项的代码
2. 找到验证该项的测试
3. 确认实现与规范完全一致

将每项标准标记为：PASS、FAIL（缺失／不完整／不一致）或 CONCERN（已实现，但可能有误）。将任何超出要求的实现标记为 OVER-BUILT。

如果任何标准为 FAIL，请停止。返回实现阶段后再继续。

架构遵循情况检查（在逐项检查后、进入阶段 2 前执行）：

• 如果存在 docs/ARCHITECTURE.md：验证此次变更是否符合其中记录的所有约束和模式（Components、Patterns、Constraints 部分）。不符合即为 FAIL——严重程度与缺失验收标准相同。
• 如果不存在 docs/ARCHITECTURE.md：将其标记为阶段 2 的 CONCERN：“未找到架构文档；无法验证架构遵循情况。”

在阶段 1 的输出中包含：Architecture Compliance: PASS / FAIL / N/A (no ARCHITECTURE.md)

垂直切片层覆盖

对于实现垂直切片（新增用户可观察行为）的任务，请按顺序执行以下检查：

1. 入口点接线检查（基于差异）： 检查变更集是否包含对应用入口点或其接线／路由层的修改。如果切片声称新增了用户可观察行为，但差异中没有涉及任何接线或入口点代码，则评审不通过，除非作者明确说明现有接线为何已能路由到新行为。

2. 端到端可追溯性： 确认能否从应用的外部入口点，经由任意基础设施或集成层，追踪到新的领域逻辑，并追踪回可观察的输出。如果这条路径的任何环节未包含在变更集中，且代码库中也尚不存在，则标记此缺口。

3. 边界级测试覆盖： 确认至少有一个测试通过应用的外部边界（例如 HTTP 请求、CLI 调用、队列消息）来测试新行为，而不是直接调用内部函数。如果应用架构支持自动化边界测试，则缺少此类测试应作为评审关注点。

4. 测试级异味检查： 如果变更集中的每项测试都是对孤立内部函数的单元测试，且没有集成级或验收级测试，则将此标记为关注点。该切片可能实现了领域逻辑，却未证明它能通过运行中的应用触达。

阶段 2：代码质量。 代码是否清晰、易于维护且经过充分测试？

检查每个变更文件：

• 清晰度： 无需额外上下文就能理解代码的作用吗？ 命名是否清晰？结构是否直观？
• 领域类型： 在适合使用语义类型的地方是否使用了语义类型？你必须遵循 domain-modeling 技能来检测原始类型偏执。
• 错误处理： 是否使用带类型的错误处理错误？是否覆盖了所有路径？
• 测试质量： 测试验证的是行为，而非实现吗？变更代码的覆盖是否充分？
• YAGNI： 是否存在未使用的代码、推测性功能或过早抽象？

按严重程度对问题分类：

• CRITICAL：缺陷风险，可能导致错误
• IMPORTANT：可维护性问题，应在合并前修复
• SUGGESTION：风格或轻微改进，可选

如果存在任何 CRITICAL 问题，请停止。返回实现阶段。

阶段 3：领域完整性。 最后一道关卡——代码是否遵守领域边界？

检查以下内容：

1. 编译时约束机会： 是否有测试在检查本可由类型系统约束的事项？
2. 领域类型一致性： 所有边界是否都使用语义类型，还是有原始类型泄漏？
3. 验证位置： 验证是否在构造时进行（解析而非验证），而不是散布在业务逻辑中？
4. 状态表示： 类型能否表示无效状态？布尔字段是否掩盖了状态机？（参见 domain-modeling 中的布尔值作为状态检查。）
5. 约定合规性： 类型和模式是否符合项目约定？ 应用“约定优先于先例”规则——现有代码违反约定不能作为辩护理由。

标记问题，但建议不应阻止合并；但约定违规除外——根据“约定优先于先例”规则，此类问题会阻止合并。

评审输出

完成全部三个阶段后，生成结构化摘要：

REVIEW SUMMARY
Stage 1 (Spec Compliance): PASS/FAIL
Stage 2 (Code Quality): PASS/FAIL/PASS with suggestions
Stage 3 (Domain Integrity): PASS/FAIL/PASS with flags

Overall: APPROVED / CHANGES REQUIRED

If CHANGES REQUIRED:
  1. [specific required change]
  2. [specific required change]

结构化审查证据

完成全部三个阶段后，生成一个 REVIEW_RESULT 证据包，包含：各阶段的判定 {stage, verdict (PASS/FAIL), findings [{severity, description, file, line?, required_change?}]}, overall_verdict, required_changes_count, blocking_findings_count。

当上下文元数据中提供 pipeline-state 时，代码审查技能将以流水线模式运行，并将证据保存到 .factory/audit-trail/slices/<slice-id>/review.json。单独运行时，证据仅供参考（不会保存）。

在工厂模式下，流水线推送代码前由完整团队进行审查——这是取代构建期间共识机制的质量检查点。推送前必须解决所有阻断性审查反馈。工厂模式审查小节请参阅 references/mob-review.md。

基于文件的审查产物

审查结果必须默认写入 .reviews/ 文件进行持久化。消息仅用于补充协调，不会在上下文压缩后保留。

• 命名：<reviewer-name>-<task-slug>.md（例如，kent-beck-user-login.md）
• **内容：**完整的结构化审查结果（全部三个阶段）
• 位置：.reviews/ 目录（添加到 .gitignore）
• 消息中注明“审查已发布到 .reviews/”——实质性反馈仅保存在文件中

这样可确保审查结果在上下文压缩、Agent 重启以及缺少 Agent 间消息机制的运行框架中得以保留。

非阻断性反馈升级

在不同切片的连续 2 次或更多次审查中出现的非阻断性事项（SUGGESTION 严重级别），必须升级为阻断性事项（IMPORTANT 严重级别）。 通过检查 .reviews/ 中之前的审查文件来跟踪重复出现的情况。

这样可避免反复出现的质量问题一直被推迟，并被视为“只是建议”。

面向用户的行为验证

当 GWT 场景描述用户可见的行为（UI 元素、显示的消息、视觉变化）时，变更集必须包含能产生相应可见输出的代码。如果场景描述了 UI 交互，却只实现 API，则不符合规范——该切片尚未完成。

规范优先于先例

书面规范优先于观察到的模式。当审查结果与项目规范（CLAUDE.md、AGENTS.md、crate 级文档、架构决策记录）相冲突，但与代码库中的现有代码一致时，该审查结果仍然有效。违反规范的现有代码属于技术债，而非先例。

规则：

1. 绝不因现有代码而降低严重级别。 无论已有多少文件犯了同样的错误，违反规范都属于阻断性问题。
2. 将新代码标记为阻断性问题。 即使不存在此前的代码，也应采用相同的严重级别。
3. 将现有违规项列为重构候选。 在审查结果中另附说明，列出存在相同反模式的文件，以便团队安排清理。

示例：项目规范要求“状态机使用类型状态模式”。新代码却使用 struct Foo { is_active: bool }，理由是已有三个文件也采用了这种写法。审查必须阻止新代码，并指出这三个现有文件属于技术债。

处理分歧

当你的审查意见与实现方案相冲突时：

1. 使用具体的代码引用说明问题
2. 解释风险——可能会出现什么问题
3. 提出替代方案
4. 如果一轮沟通后仍未达成一致，则升级给用户处理

你的职责是发现作者遗漏的问题，而不是阻碍进展。

业务价值与用户体验意识

在阶段 1 中，还应考虑：

• 这个切片是否为用户带来可见价值？
• 验收标准是否具体且可测试（而非含糊不清）？
• 此次变更后，用户旅程是否仍然连贯？
• 是否从用户视角处理了边界情况和错误状态？

这些不是阻断性问题，但在相关时应予以指出。

强制执行说明

• 独立模式：仅提供建议。Agent 无法通过机械方式阻止未经过审查的合并。
• 流水线模式：设置门禁。审查结论会阻止流水线继续推进——FAIL 或 CHANGES REQUIRED 会中止当前切片。

硬性约束：

• 阶段 1 的 FAIL 会阻止后续阶段：[H]
• 违反约定（Convention Over Precedent 规则）：[RP]

有关流水线返工预算冲突，请参阅模板目录中的 CONSTRAINT-RESOLUTION.md。

约束

• 非阻塞升级：当同一个问题（而非仅仅同一类别）在针对不同切片的 2 次或更多连续审查中出现时，将触发升级。为了避免触发升级而改变同一问题的措辞，属于违规行为。判断标准是：人工审查者是否会认为这些问题源自同一个根本问题？
• 约定优先于先例：“书面约定”是指记录在 CLAUDE.md、AGENTS.md、ADR 或 crate/package 级文档中的约定。仅在现有代码中观察到的模式属于先例，而非约定。但如果某种模式具有普遍性（100% 的文件都遵循该模式），且没有书面约定与之矛盾，就应将其标记为待记录的候选项，而不是直接忽略。
• 垂直切片覆盖范围：当变更会修改用户可观察到的行为时，才适用层覆盖检查。“用户可观察到”是指使用应用程序的人会注意到差异。不会改变行为、只改善性能的基础设施变更，不属于用户可观察到的变更。新的 API 端点属于用户可观察到的变更，即使目前还没有 UI。

验证

按照此技能完成审查后，请确认：

• 三个阶段均已分别按顺序执行
• 阶段 1 中有一项明确检查是否符合 docs/ARCHITECTURE.md
• 阶段 1 中，每条验收标准都已映射到代码和测试
• 阶段 2 中，已评估每个变更文件的清晰度和领域类型用法
• 阶段 3 中，已检查领域完整性是否有机会通过编译时机制来强制执行
• 未因现有代码中存在相同模式而降低约定违规的严重程度
• 已生成结构化摘要，并明确列出各阶段的 PASS/FAIL
• 任何 CHANGES REQUIRED 项都列出了具体、可执行的修复措施
• 审查结果已写入 .reviews/ 文件（而不仅仅是消息）
• 在 2 次或更多审查中出现的重复非阻塞事项已升级为阻塞事项
• 已针对描述 UI 的场景，验证面向用户的行为

如果未满足任何一项标准，请在最终确定前重新检查相关阶段。

依赖项

此技能可独立使用。如需增强工作流，可与以下内容集成：

• **domain-modeling：**提供阶段 2 和阶段 3 中提及的原始类型过度使用和解析而非验证原则
• **tdd：**审查通常会在 TDD 周期之后进行；此技能会验证该周期的产出
• **mutation-testing：**可在代码审查后作为额外的质量门禁

缺少依赖项？使用以下命令安装：

npx skills add jwilger/agent-skills --skill domain-modeling