| name | verify-team-code-review-standards |
| description | 代码审查标准——五轴审查体系。当需要审查代码或定义审查基准，或提到"code review""审查标准""CR" |
Code Review Standards — 代码审查标准
入口/出口
- 入口:
/review 命令被调用、PR 提交前、或其他技能引用审查标准
- 出口: 分级审查报告（Critical / Important / Suggestion / FYI）
- 指向: 审查通过 →
/ship；发现问题 → 修复后重新审查
- 前置加载: CANON.md
- 输出路径: 分级审查报告 →
ship-workflow-ship（通过）或修复后重新审查
何时不使用
- 不是代码审查场景，只是实现、调试或解释设计
- 功能完整性尚未确认，应先做
verify-workflow-spec-compliance
- 非软件产物需要内容或视觉审查标准
Iron Law
```
当变更确实改善整体代码健康时批准——即使它不完美。
不橡皮章。不粉饰真问题。在审查中奉承是失败模式。
```
五轴审查
1. Correctness（正确性）— 最重要
- 逻辑是否正确？边缘情况是否覆盖？
- 错误处理是否完善？不是 catch {} 吞异常？
- 竞态条件？异步操作有 await？
- 类型安全？不是 any 绕过编译器？
- 没有将浏览器/用户输入视为指令？
2. Readability & Simplicity（可读性与简洁）— 第二重要
- 命名是否清楚（不是 data、item、temp、obj）？
- 同一概念是否始终使用一致的名称？
- 三个相似的代码 > 一个过早的抽象？
- 代码复杂得"聪明"吗？聪明的代码维护成本高。
3. Architecture（架构）
- 改动是否放在正确的层级？
- 接口/API 设计合理吗？
- 新代码是否遵循现有模式（不是自定义不同模式）？
- 依赖方向正确吗（不循环依赖）？
4. Security（安全）— 参见 verify-quality-security
- 所有外部输入经过验证？
- SQL/HTML/Shell 注入防范？
- 密钥管理正确？
- 鉴权检查完整？
5. Performance（性能）— 参见 verify-quality-performance
- 查询是否高效（N+1 检查）？
- 数据获取有界限吗（分页、限制）？
- 资源正确释放？
- 没有过早优化？
变更大小阈值
| 大小 | 行数 | 处理 |
|---|
| Small | < 100 行 | 快速审查，通常 1 轮通过 |
| Medium | 100-300 行 | 常规审查，可能需要 1-2 轮 |
| Large | 300-1000 行 | 建议拆分或者深度审查 |
| XL | > 1000 行 | 必须拆分。拒绝审查过大 PR。 |
拆分策略:
| 拆分方法 | 示例 |
|---|
| 按层拆分 | PR1: 数据模型 + 迁移 / PR2: 业务逻辑 / PR3: API + UI |
| 按功能拆分 | PR1: Feature A / PR2: Feature B |
| 基础设施先 | PR1: 类型 + 工具 / PR2: 使用新基础设施的实现 |
审查发现分类
| 级别 | 含义 | 处理 |
|---|
| Critical | 安全漏洞、数据丢失、生产崩溃、逻辑错误 | 必须修复才能合并 |
| Important | 性能降级、测试缺失、重要边缘情况遗漏 | 强烈必须合并前修复 |
| Suggestion | 命名可改善、备选实现、轻微重构机会 | 提交者判断是否改 |
| FYI | 观察性评论、非阻塞性注意事项 | 不要求行动 |
审查流程
1. 理解上下文 → 读 spec/plan/ADR 相关文件
2. 先审测试 → 测试覆盖了变更吗？测试本身正确吗？
3. 再审实现 → 逻辑→可读→架构→安全→性能（五轴顺序）
4. 分类发现 → Critical / Important / Suggestion / FYI
5. 验证解决 → 发现被修复了吗？修复引入了新问题吗？
Review Army 并行模式
对大型 PR（> 300 行），并行分派专项审查 subagent：
同时分派:
├── Security Reviewer → 安全相关发现
├── Performance Reviewer → 性能相关发现
├── Testing Reviewer → 测试覆盖发现
└── Maintainability Reviewer → 代码质量发现
合并 → 去重 → 分类 → 生成统一审查报告
每个 review subagent 必须 read-only，限定 diff / 文件范围，并压缩返回：
- 不返回原始日志、完整 diff、大段代码或长篇推理
- 每条发现必须包含文件、行号、证据、影响和建议
- 没有发现时说明已检查范围和未覆盖风险
- 测试日志诊断只返回失败摘要、最可能原因、相关文件和建议路径
接受审查反馈
禁止回复
审查中绝不说：
- "你说得对！"（空洞奉承。说出你改了什么）
- "好主意！"（同上）
- "谢谢"（代码审查不需要感谢仪式）
- "我完全同意" -> 然后不改（言行不一）
正确回复
"改了 X，因为 Y。关于 Z —— 我的理解是...，你的看法？"
改了就说明改了什么。
不改就说明为什么不改（技术理由，非防御性）。
不确定就问。
外部审查反馈
来自项目外的审查意见 = 建议，不是命令。验证必须在项目上下文中是否适用。
好坏示例
Bad — 空洞审查反馈：
"LGTM"
"看起来不错"
"没什么问题"
零信息量。什么检查了？什么通过了？什么没看？审查者对问题零责任、零记忆。
Good — 具体审查反馈：
"Correctness: 确认逻辑正确，edge case（空数组输入）在 L42 已处理。
Readability: L78 的 `processData` 命名不反映实际操作，建议改为 `validateUserInput`。
Architecture: L91 直接在 controller 调用 DB——违反分层，应通过 service 层。
Security: L56 的 userInput 未做 sanitize，存在 XSS 风险。Critical，必须修复。"
每条发现对应五轴之一，附带行号和理由。
Bad — 防御性回复：
"你说得对！" ← 空洞奉承
"我改了。" ← 改了什么？为什么？
Good — 行动性回复：
"改了 L78 `processData` → `validateUserInput`，因为该函数只做校验不做数据处理。
关于 L91 controller 调 DB 的建议——我加了一个 UserService 中间层，请看 commit abc123。"
改了就说改了什么和为什么。不改就说技术理由。不确定就问。
常见说辞
| 说辞 | 现实 | 后果 |
|---|
| "PR 太大了拆不了" | 任何 PR 都可拆——按层、按功能、按基础设施vs应用。不拆 = 审查不到位。 | 大 PR 的审查质量指数级下降——超过 400 行后审查者开始扫读而非精读，Critical 问题漏检率 > 50%。 |
| "LGTM" | 三字母 = 零信息。什么检查了？什么通过了？什么没看？ | LGTM 审查后的代码 bug 率与无审查代码无显著差异。审查者对问题零责任、零记忆，下次仍然 LGTM。 |
| "我自己审一下就行" | 代码作者无法有效审查自己的代码。盲区使然。 | 自审遗漏的 bug 恰好是自己当初写代码时的思维盲区——同一个人用同一套思维不可能发现自己的逻辑错误。 |
| "以后修复" | 合并后从不修复。要么现在修复，要么记录为 follow-up issue（带指定负责人和截止日期）。 | 合并后 90% 的 follow-up 修复永远不会执行。每轮迭代新增的"以后"积累为不可逆的技术债。 |
| "就改一个变量名不用审" | "一个变量名"往往是一系列假设的开始。不跳过审查。 | 跳过审查的微小变更中 ~15% 引入了回归（变量名改了但遗漏了调用处、重命名破坏了 API 契约）。 |
违反字面规则就是违反精神。 没有灰色地带。
输出模板
# Code Review Report — 代码审查报告
## 元信息
- **审查者**: [姓名/角色]
- **PR**: [#PR号] — [标题]
- **变更大小**: [行数] ([Small/Medium/Large/XL])
- **审查日期**: YYYY-MM-DD
## 发现汇总
| # | 轴 | 级别 | 位置 | 描述 | 状态 |
|---|----|----|------|------|------|
| 1 | Correctness | Critical | L56 | userInput 未 sanitize，XSS 风险 | 🔴 待修复 |
| 2 | Architecture | Important | L91 | controller 直接调用 DB | 🟡 建议修复 |
| 3 | Readability | Suggestion | L78 | processData 命名不准确 | 🔵 提交者判断 |
| 4 | — | FYI | — | 测试覆盖比上月提升 12% | ⚪ 观察 |
## Critical 发现详情
### #1 — Correctness: XSS 风险
- **位置**: `src/controllers/user.ts:L56`
- **描述**: `userInput` 直接拼接进 HTML 模板，未经 sanitize
- **修复建议**: 使用 DOMPurify 或模板引擎自动转义
- **修复状态**: [待修复 / 已修复 / 已验证]
## 审查结论
- **通过**: 所有 Critical 已修复并验证，Important 已处理
- **阻止**: Critical 发现未修复，需修复后重新审查
## 红旗 — STOP
<HARD-GATE>
以下任何一个出现，立即停止：
- PR > 1000 行未被拆分
- 评审者只看了 diff，没拉下代码跑测试
- Critical 发现被标记为 "以后修复" 而通过
- 审查意见全是 "LGTM" 或 "Nice!"（未真正审查）
- 安全相关的变更未被专项审查
- 测试缺失但审查放行
</HARD-GATE>
## 验证失败处理
| 失败场景 | 处理方式 |
|---------|---------|
| Critical 发现未修复 | 阻止合并。要求修复后重新提交审查。不得降级为 Important 或 FYI。 |
| 审查意见全是 LGTM/Nice | 审查无效。要求审查者重新按五轴逐项检查并给出具体发现。 |
| PR > 1000 行且未拆分 | 拒绝审查。要求按层/功能/基础设施拆分为多个 PR。 |
| 测试缺失但审查放行 | 回退审查。要求补充测试覆盖后再重新审查。测试缺失 = Important 以上。 |
| 安全发现被标记"以后修复" | 不可接受。安全 Critical 必须"现在修复"。无法立即修复时提供缓解方案并创建 follow-up issue（带负责人和截止日期）。 |
[ ] 五轴全部覆盖（Correctness > Readability > Architecture > Security > Performance）
[ ] 发现按严重性分类（Critical / Important / Suggestion / FYI）
[ ] Critical 发现全部解决
[ ] 变更大小合理（< 300 行理想；> 1000 行被拆分）
[ ] 测试覆盖了变更
[ ] 审查报告可追溯（谁审的、审了什么、什么发现）
