code-review

!bash "${CLAUDE_SKILL_DIR}/../../hooks/yaml-helper.sh" resolve_config --keys automation

代码审查

每次调用 AskUserQuestion 都遵循 .claude/docs/automation-modes.md （协作模式始终提问 · 引导模式仅针对重大事项提问 · 自主模式记录并继续； automation_always_ask 类别始终提问）。

输入不足——生成任何报告前先检查

如果此技能所需的输入不存在，答案应为“无法运行”—— 而不是一份填好的报告。 先检查；如果检查失败，就停止。

1. 列出此技能会读取的输入（数据文件、先前的报告、性能分析器输出、 测试结果、注册表、源代码）。
2. 对每项记录 FOUND 或 ABSENT——不要写“假定存在”。
3. 如果某个章节所需的任何输入为 ABSENT，该章节应标记为 NOT ASSESSED — NO DATA。不要估算，不要根据相邻产物推断，也不要将 必填单元格留给之后阅读模板的人填写。
4. 如果所有必需输入都为 ABSENT，就停止，并将 NOT ASSESSED — NO DATA 作为整体结论，注明缺少哪些内容以及由哪个技能生成。

结论为 NOT ASSESSED 也算成功。 当没有数据时，这是对 “数据说明了什么？”这一问题正确且有用的回答。它要防止的故障模式很明确：如果报告的结论枚举中没有“无法运行”这一状态，就会产生 虚假的通过结果——例如，在项目没有资产也没有标准时，资产审计仍返回 COMPLIANT；或者在没有任何性能分析器数据、也从未设置预算时，性能分析仍报告“相对于 16.67ms 预算有 >99% 的余量”。

没有证据绝不等于证明不存在。 如果没有可扫描的文件，因此扫描未发现匹配项，那就不代表完成了任何验证。请说明具体是哪种情况——读者无法仅凭绿色结果判断。

---

阶段 1：加载目标文件

完整读取目标文件。读取 CLAUDE.md，了解项目编码标准。

---

阶段 2：识别引擎专家

读取 project.yaml 中的 specialists 块；如果不存在，则回退到 .claude/docs/technical-preferences.md 的 ## Engine Specialists 部分。记录：

• 主要专家——根据 engine.name 推导出的 <engine>-specialist（Godot→godot-specialist、Unity→unity-specialist、Unreal→unreal-specialist）；用于架构和广泛的引擎相关问题
• 语言/代码专家——specialists.code——用于审查项目的主要语言文件
• 着色器专家——specialists.shader——用于审查着色器文件
• UI 专家——specialists.ui——用于审查 UI 代码

null 表示未设置——将该键视为不存在，并跳过对应的专家。绝不要将其作为 Agent 名称启动。 v1.0 迁移会为旧文件中未指定的专家写入 null，并且只要有一个成员已设置，就会写入整个块——因此，只配置了代码专家的项目会包含 shader: null 和 ui: null。配置读取器会为这些值返回四字符字符串 "null"，它并非空值，因此会被视为已配置。在这里，null、空值和缺失状态相同。

如果未配置引擎（project.yaml 中没有 engine.name，且 technical-preferences.md 显示 [TO BE CONFIGURED] 或文件缺失），则跳过引擎专家步骤。在本次运行的输出中记录 Engine validation: NOT ASSESSED — no engine configured (`engine.name` unset in `project.yaml`)。 没有说明的跳过检查与通过的检查无法区分；读者无法判断是否根本没有寻求引擎方面的指导。

---

第 3 阶段：ADR 合规性检查

参数： /code-review [file(s)] 可选择性地将故事文件路径作为最后一个参数（例如，/code-review Assets/Scripts/Combat/Attack.cs production/epics/combat/story-001.md）。如果提供了故事路径，请读取该文件以提取适用的 ADR 引用。

按以下优先级搜索 ADR 引用：

1. 故事文件（如果作为参数提供）
2. 实现文件顶部的头部注释
3. 提及这些文件的提交消息（git log --oneline -- [file]）

查找 ADR-NNNN 或 docs/architecture/adr- 等模式（不区分大小写）。

如果未找到 ADR 引用，请注明：“未找到 ADR 引用 — 已跳过 ADR 合规性检查。若要进行完整的 ADR 合规性审查，请提供故事路径：/code-review [files] [story-path]。”

对于每个引用的 ADR，只加载此检查所需的章节，绝不无界地读取整个文件。 较长的 ADR 会超过 25k-token 的 Read 上限，而读取被截断后唯一的补救方法是分段读取剩余内容——这是读取文件最耗时的方式。请使用与 /dev-story 和 /create-stories 相同的模式：

1. 映射标题（开销低——仅获取行号）：Grep pattern="^## " path="[adr-file]" output_mode="content" -n
2. 仅对 ## Decision 和 ## Consequences 进行有界读取，根据行号设置 Read(offset, limit) 范围，使其在下一个标题开始处结束。如果标题映射为空（即 ADR 早于模板制定，格式不标准），则改为完整 Read 一次；如果内容在上限处被截断，则直接 grep 决策/后果内容，而不要分段读取剩余内容。

如果引用的 ADR 文件缺失或无法读取，则 ADR 合规性为 NOT ASSESSED — [ADR] could not be read：请注明其名称。完全没有引用时，结果为 NO ADRS FOUND，这是不同的结果。

根据这两个章节，将偏差分类为：

• 架构违规（阻断）：使用 ADR 明确拒绝的模式
• ADR 偏离（警告）：与选定方案存在实质性差异，但未使用禁止的模式
• 轻微偏差（信息）：与 ADR 指引存在微小差异，不影响整体架构

---

第 4 阶段：标准合规性

确定系统类别（引擎、游戏玩法、AI、网络、UI、工具）并评估：

• 公共方法和类都有文档注释
• 每个方法的圈复杂度低于 10
• 方法长度不超过 40 行（不包括数据声明）
• 通过注入依赖项（游戏状态不使用静态单例）
• 配置值从数据文件加载
• 系统公开接口（而非依赖具体类）

---

第 5 阶段：架构与 SOLID

架构：

• 依赖方向正确（engine <- gameplay，而非相反）
• 模块之间没有循环依赖
• 分层合理（UI 不拥有游戏状态）
• 使用事件/信号进行跨系统通信
• 与代码库中已确立的模式一致

SOLID：

• 单一职责：每个类只有一个变更原因
• 开闭原则：可扩展而无需修改
• 里氏替换原则：子类型可替换基类型
• 接口隔离原则：没有臃肿的接口
• 依赖倒置原则：依赖抽象，而非具体实现

---

第 6 阶段：游戏特定注意事项

• 与帧率无关（使用 delta time）
• 热路径（更新循环）中没有内存分配
• 妥善处理 null/空状态
• 在需要时确保线程安全
• 资源清理（无泄漏）

---

阶段 7：专家审查（并行）

通过 Agent 同时启动所有适用的专家——不要等一个启动后再启动下一个。

报告前验证每项专家发现。不要未经核查就转述发现。 对每项发现，在报告中记录：

• 文件和行号。
• 证据——引用的代码，或会触发问题的具体输入／状态。
• 置信度——VERIFIED（你已自行核查）或 UNVERIFIED — specialist claim（你无法核查）。

无法核查的发现应标为未验证或直接舍弃，绝不能仅凭专家笃定的措辞就认定为缺陷。

此步骤为何是强制要求。 专家在推导方面可靠，在诊断现有代码方面却不可靠：对一个短函数的错误断言可能说得头头是道，让人粗略一看便信以为真；而声称存在的 bug，一旦枚举各种输入，可能就站不住脚。如果没有这一步，父级审查就会成为洗白渠道：猜测以专家发现的名义进入，最后却作为经过审查的缺陷输出。

引擎专家

如果已配置引擎，请确定每个文件适用哪位专家，并行启动：

• 主要语言文件（.gd、.cs、.cpp）→ 语言／代码专家
• 着色器文件（.gdshader、.hlsl、着色器图）→ 着色器专家
• UI 屏幕／组件代码 → UI 专家
• 涉及多个领域或不明确 → 主要专家

对于任何涉及引擎架构（场景结构、节点层级、生命周期钩子）的文件，也要启动主要专家。

QA 可测试性审查

对于逻辑和集成故事，还要通过 Agent 并行启动 qa-tester，并提供：

• 正在审查的实现文件
• 故事中的 ## QA Test Cases 部分（由 qa-lead 预先编写的测试规格）
• 故事中的 ## Acceptance Criteria

请让 qa-tester 评估：

• 是否暴露了所有测试钩子和接口（未隐藏在 private/internal 访问限制之后）？
• 故事中 ## QA Test Cases 部分的 QA 测试用例是否对应可测试的代码路径？
• 是否有验收标准在当前实现下无法测试（例如硬编码值、没有用于注入的缝隙）？
• 实现是否引入了现有 QA 测试用例未涵盖的新边界情况？
• 是否存在应该测试但没有测试的可观察副作用？

对于视觉／体验和 UI 故事：qa-tester 应审查 ## QA Test Cases 中的手动验证步骤能否按当前实现执行，例如，手动检查人员需要进入的状态是否确实可达。

输出前收集所有专家发现。

---

阶段 8：输出审查

## Code Review: [File/System Name]

### Engine Specialist Findings: [N/A — no engine configured / CLEAN / ISSUES FOUND]
[Findings from engine specialist(s), or "No engine configured." if skipped]

### Testability: [N/A — no story path given / N/A — Visual/Feel or Config story / TESTABLE / GAPS / BLOCKING]
[qa-tester findings: test hooks, coverage gaps, untestable paths, new edge cases]
[If BLOCKING: implementation must expose [X] before tests in ## QA Test Cases can run]

### ADR Compliance: [NOT ASSESSED / NO ADRS FOUND / COMPLIANT / DRIFT / VIOLATION]
[List each ADR checked, result, and any deviations with severity]

### Standards Compliance: [X/6 passing]
[List failures with line references; a missing doc comment names the method or class]

### Architecture: [NOT ASSESSED / CLEAN / MINOR ISSUES / VIOLATIONS FOUND]
[List specific architectural concerns]

### SOLID: [NOT ASSESSED / COMPLIANT / ISSUES FOUND]
[List specific violations]

### Game-Specific Concerns
[List game development specific issues]

### Positive Observations
[What is done well -- always include this section]

### Required Changes
[Must-fix items before approval — ARCHITECTURAL VIOLATIONs always appear here]

### Suggestions
[Nice-to-have improvements]

### Verdict: [NOT ASSESSED / APPROVED / APPROVED WITH SUGGESTIONS / CHANGES REQUIRED]

选择结论，按顺序匹配，命中第一项即停止：

• CHANGES REQUIRED——“Required Changes”下有任何内容
• NOT ASSESSED——没有可审查的内容（上文的无数据路径）、引擎专家审查未运行——未配置引擎，或无法启动某位专家——或者报告中的任何部分标为 NOT ASSESSED（ADR 合规性中引用的 ADR 无法读取、架构、SOLID）；请注明是哪一项。NOT ASSESSED 的优先级低于 CHANGES REQUIRED，因为已知缺陷比跳过检查更值得采取行动；但高于两个批准结论：跳过引擎检查的审查不能算是批准了代码。（NO ADRS FOUND 不属于 NOT ASSESSED：没有 ADR 可供检查，不会阻止批准。）
• APPROVED WITH SUGGESTIONS——只有建议
• APPROVED——没有必需修改，也没有建议

此技能为只读——不会写入任何文件。

---

阶段 9：后续步骤

使用 AskUserQuestion：

• 提示词："代码审查已完成——结论：[NOT ASSESSED / APPROVED / APPROVED WITH SUGGESTIONS / CHANGES REQUIRED]。你想如何继续？"（逐字使用阶段 8 的结论）
• 选项（根据结论调整）：
  • 如果为 APPROVED：
    • [A] Run /story-done to mark the story complete
    • [B] Stop here
  • 如果为 APPROVED WITH SUGGESTIONS：
    • [A] Apply the suggestions, then run /story-done
    • [B] Run /story-done now — suggestions noted for later
    • [C] Stop here
  • 如果为 CHANGES REQUIRED：
    • [A] Fix the issues and re-run /code-review
    • [B] Run /story-done anyway with noted exceptions
    • [C] Stop here
  • 如果为 NOT ASSESSED：
    • [A] Fix what stopped the review (point me at the code, run /setup-engine, or fix the unreadable ADR reference), then re-run /code-review
    • [B] Stop here

如果发现 ARCHITECTURAL VIOLATION：

• 如果该违规与现有 ADR 相矛盾：修改实现，使其符合 docs/architecture/[adr-file].md。如果设计确实发生了变化，运行 /architecture-decision，正式修订现有 ADR——不要另建一份与之冲突的 ADR。
• 如果该违规模式没有对应的 ADR：运行 /architecture-decision，在修复代码之前记录正确的处理方式。