code-review-patterns

代码审查模式

概述

代码审查能在漏洞发布前发现它们。但在确认功能之前先审查代码质量，顺序是颠倒的。

**核心原则：**先验证代码能正常工作，再验证代码质量。

参考文件

仅阅读当前审查所需的参考文件：

• references/review-order-and-checkpoints.md，了解优先关注问题的阅读顺序、审查检查点、无发现时的停止条件和重新审查循环
• references/security-review-checklist.md，检查身份验证、输入/输出、密钥、网络、存储和依赖项
• references/code-review-heuristics.md，快速检查可维护性、性能、隐性故障、边缘情况、草率写法和 UI

信号质量规则

仅在确定时才标记问题。误报会削弱信任，并浪费修复周期。

标记	不要标记
无法编译/解析（语法、类型、导入错误）	项目指南中未规定的风格偏好
导致所有输入都产生错误结果的逻辑错误	取决于特定输入/状态的潜在问题
明确违反指南（引用具体规则）	主观改进建议或吹毛求疵

快速审查清单（参考模式）

快速审查时，检查以下 8 项：

• 代码简单易读
• 函数和变量命名恰当
• 没有重复代码
• 错误处理得当
• 没有暴露密钥或 API 密钥
• 已实现输入验证
• 测试覆盖充分
• 已考虑性能问题

铁律

NO CODE QUALITY REVIEW BEFORE SPEC COMPLIANCE

如果尚未验证代码符合要求，就不能审查代码质量。

两阶段审查流程

阶段 1：规范符合性审查

它是否完成了要求的任务？

1. 阅读需求

   • 要求是什么？
   • 验收标准是什么？
   • 有哪些边缘情况？

2. 追踪实现

   • 代码是否实现了每项要求？
   • 是否处理了所有边缘情况？
   • 是否与规范完全一致？

3. 测试功能

   • 运行测试
   • 必要时进行手动测试
   • 验证输出是否符合预期

**关卡：**只有阶段 1 通过后，才能进入阶段 2。

阶段 2：代码质量审查

代码写得好吗？

按优先级顺序审查：

1. 安全性 - 可能被利用的漏洞
2. 正确性 - 逻辑错误、遗漏的边缘情况
3. 性能 - 不必要的缓慢
4. 可维护性 - 难以理解或修改
5. 用户体验 - 用户体验问题（涉及 UI 时）
6. 无障碍性 - A11y 问题（涉及 UI 时）

审查顺序

逐行扫描代码之前，请阅读 references/review-order-and-checkpoints.md，并按关注点而非原始差异顺序 还原此次变更。

安全审查

对于身份验证、数据、网络、存储或可从外部访问的代码，在整理审查发现之前，请阅读 references/security-review-checklist.md。

LSP 驱动的代码分析

审查时使用 LSP 进行语义理解：

任务	LSP 工具	比 grep 更好的原因
查找函数的所有调用方	lspCallHierarchy(incoming)	查找实际调用，而非字符串匹配
查找类型/变量的所有用法	lspFindReferences	基于语义，而非文本
跳转到定义	lspGotoDefinition	跳转到实际定义
了解函数调用了什么	lspCallHierarchy(outgoing)	映射调用链

使用 LSP 的审查工作流：

1. localSearchCode → 查找符号并获取 lineHint
2. lspGotoDefinition(lineHint=N) → 理解实现
3. lspFindReferences(lineHint=N) → 检查所有用法是否一致
4. lspCallHierarchy(incoming) → 确认调用方能处理变更

**重要：**始终先从 localSearchCode 获取 lineHint。切勿猜测行号。

审查启发式

对于性能、可维护性、边缘情况、隐性故障、类型设计偏差或 UI 专项检查，请阅读 references/code-review-heuristics.md。

错误/正确——静默可选链：

// WRONG: silently swallows null user
const name = user?.profile?.name ?? 'Unknown';

// RIGHT: log the gap, then degrade
const name = user?.profile?.name;
if (!name) {
  logger.warn('user profile missing name', { userId: user?.id });
}
return name ?? 'Unknown';

边缘情况分类体系

用于在审查过程中系统扫描边缘情况的参考清单：

类别	示例	检测方法
缺少 else/default	switch 没有 default；可空值的 if 没有 else	检查 switch/if 是否穷尽所有情况
未防护的输入	未验证用户输入，缺少 null 检查	检查是否直接使用参数而未验证
差一错误	循环边界、数组索引、分页	检查所有 < 与 <=、array[length] 与 array[length-1]
算术边缘情况	除以零、整数溢出、浮点数	检查 / 运算符是否验证除数
隐式类型转换	使用 == 而非 ===，字符串转数字，真值/假值	检查 ==（而非 ===）以及混合类型的 +
竞态条件	共享可变状态，异步操作未加锁	检查异步路径中是否修改共享变量
超时/重试缺口	网络调用没有超时，无重试耗尽机制	检查 fetch/axios 是否配置超时

在第 2 阶段质量审查期间使用。只检查与变更代码相关的类别。

清晰优先于简洁

• 嵌套三元表达式 a ? b ? c : d : e → 使用 if/else 或 switch
• 为了少写两行而写成密集的单行代码 → 写成 3 行清晰代码，而非 1 行巧妙代码
• 带有复杂回调的链式 .map().filter().reduce() → 使用命名的中间变量

模式识别标准

审查期间，识别值得记录的模式：

标准	查找内容	示例
经验性	新开发者可能不了解的知识	“所有 API 响应都采用封装结构”
约定性	可能存在不同选择的具体约定	“数据库使用 snake_case，JS 使用 camelCase”
非典型	不符合框架常见模式	“自定义退避重试逻辑”
一致性	在多个文件中重复出现	“所有服务都提供健康检查端点”

如果在审查期间发现这些模式：

1. 在审查反馈中记录该模式
2. 将其纳入记忆笔记（模式部分）——路由器会通过 Memory Update task 将其持久化到 patterns.md
3. 标记与既定模式不一致之处

严重性分类

严重性	定义	操作
CRITICAL	安全漏洞或阻碍功能运行	合并前必须修复
MAJOR	影响功能或存在重大质量问题	合并前应修复
MINOR	风格问题、小幅改进	可以合并，之后修复
NIT	纯粹的风格偏好	可选

多信号审查方法

每次第 2 阶段审查都会产生独立信号。分别评估每个维度。

HARD 信号（任何一项失败都会阻止批准）：

• 安全性： 存在一个真实漏洞 = 维度得分为 0
• 正确性： 存在一个会导致输出错误的逻辑错误 = 维度得分为 0

SOFT 信号（记录问题，但单独出现时不阻止批准）：

• 性能： 存在扩展性隐患，但没有即时影响
• 可维护性： 代码复杂，但功能正常
• UX/A11y： 缺少状态，但核心流程可用

聚合规则：

1. 如果任意 HARD 信号 = 0 → STATUS: CHANGES_REQUESTED（不可协商）
2. 具体的 CONFIDENCE 公式和严重性术语由 code-reviewer Agent 合约规定——此处不要重复说明计算方式（重复副本会产生偏差）。应用 Agent 的 CONFIDENCE = min(HARD scores) capped by avg(SOFT scores) - 10；单个 HARD:0 会将 CONFIDENCE 置为 0。
3. 在 Router Handoff 中提供各信号的明细，以便有针对性地修复

零发现停止

如果评审在所有维度（安全性、正确性、性能、可维护性、UX/A11y）均未发现任何问题，则评审 MUST 停止并重新检查。对于非平凡的更改，零发现意味着评审深度不足，而非代码完美。操作：重新阅读每个已更改的文件。重新运行 references/code-review-heuristics.md 中的启发式扫描。重新运行 references/security-review-checklist.md 中的安全性分类。如果经过有意的重新检查后仍然零发现，则记录：“经强制重新检查 [N 个文件，M 行更改] 后确认零发现。已检查：[列出具体执行的检查]。”没有重新检查证据的单纯“没有问题”属于 INVALID。

每条信号的证据要求： 每条信号 MUST 引用具体的文件:行号。没有证据的信号 = 不予报告。

两次隔离评估 + WEAVE 调和

cc10x 会有意并行运行 code-reviewer 和 silent-failure-hunter。

独立性为何不可妥协： 将两轮评估合并到一个推理头脑中，会悄然造成锚定效应——先读到的问题会影响第二轮评估，全面评审者会把隐藏失败的迹象合理化，而两个“独立”信号也会悄然趋同为一种意见。独立性正是整个流程的意义所在。每轮评估都必须在看不到另一轮评估结果的情况下形成自己的发现。只有在双方都提交各自的列表后，才能进行调和。

WEAVE 协议（取代简单的“采用更严格的判定”合并方式）： 在路由任何发现之前，先对其分类。

类别	定义	路由
AGREED	两轮评估独立发现了同一问题（相同的文件:行号、相同的根本原因）	最高可信度 — 报告；结论趋同是强烈信号
DETECTOR-ONLY	恰好只有一轮评估发现的问题（评审者 XOR silent-failure-hunter）	保留 — 另一轮未发现的问题正是运行两轮评估的价值所在，并非噪声。原样保留发现该问题的评估所提供的证据和严重程度
FALSE-POSITIVE	分类时可疑：没有文件:行号、问题已存在、属于 linter 检查范围，或被另一轮评估的证据驳斥	根据“不要标记”和信号质量规则丢弃

Weave 顺序：AGREED → DETECTOR-ONLY → FALSE-POSITIVE 分类。绝不要仅仅因为另一轮评估没有发现问题，就降低 DETECTOR-ONLY 发现的级别——沉默不等于反驳。

回退规则 — 采用更严格的判定： 如果 WEAVE 后，两轮评估对同一问题仍存在真正矛盾（例如，评审者认为安全，hunter 认为失败被吞掉，且双方证据均未能驳斥对方），则采用更严格的判定。平局时采用 CHANGES_REQUESTED。这是剩余情况下的规则，不是首选步骤——先进行 WEAVE，仅在真正僵持时才升级处理。

不要标记（避免误报）

• 此次更改未引入的既存问题
• 只是看起来可疑的正确代码
• 资深工程师不会指出的吹毛求疵
• linter 已能发现的问题（不要重复工具的检查）
• 项目指南未要求的一般质量问题
• 已通过 lint-ignore 注释明确忽略的问题

优先级输出格式（反馈分组）

按优先级组织反馈（参考模式）：

## Code Review Feedback

### Critical (must fix before merge)
- [95] SQL injection at `src/api/users.ts:45`
  → Fix: Use parameterized query `db.query('SELECT...', [userId])`

### Warnings (should fix)
- [85] N+1 query at `src/services/posts.ts:23`
  → Fix: Batch query with WHERE IN clause

### Suggestions (consider improving)
- [70] Function `calc()` could be renamed to `calculateTotal()`
  → More descriptive naming

始终为每个问题提供具体的修复示例。 不要只说“这里不对”——展示正确的做法。

危险信号 — 停止并重新评审

如果你发现自己正在：

• 检查功能之前先评审代码风格
• 不运行测试
• 跳过安全检查清单
• 给出笼统的反馈（“看起来不错”）
• 不提供文件:行号引用
• 不解释问题所在
• 不提供修复建议

停止。从第 1 阶段重新开始。

防止找借口

借口	事实
“测试通过了，所以没问题”	测试可能遗漏需求。请检查是否符合规范。
“代码看起来很整洁”	代码整洁也可能有错误。请验证功能。
“我信任这位开发者”	信任，但也要验证。每个人都会犯错。
“这只是一个小改动”	小改动也可能引发大问题。请彻底审查。
“没时间全面审查”	修复 Bug 比审查更耗时。请认真做好。
“安全审查太过了”	一个漏洞就可能拖垮公司。请检查。

可选：锚定式 0-4 质量评分标准（高严谨度审查）

CONFIDENCE 分数衡量的是对审查发现的确信程度，而非结果的质量。在高严谨度审查中，审查者 MAY 在判定结果的同时给出一个锚定式 0-4 质量分数。每个整数都对应明确的标准，确保两位审查者给出相同分数：

分数	标准
0	已损坏或不安全。不符合规范、存在 HARD 信号缺陷（安全漏洞、输出错误），或隐瞒失败。不得合并
1	在理想情况下可用，但很脆弱。错误处理、边界情况或验证方面存在实际缺陷，在正常使用中会造成问题
2	扎实且正确，但并无特别之处。存在一些可维护性或清晰度方面的欠账；没有会阻碍合并的问题
3	良好。正确、易读、测试充分，并考虑了边界情况。这是你会乐意负责维护的代码应达到的标准
4	真正出色。正确、清晰、防御性完善，而且设计让后续改动变得容易。非常罕见

总分区间 → 操作：

分数	操作
0	CHANGES_REQUESTED — 不容妥协（对应 HARD-signal:0）
1	CHANGES_REQUESTED — 明确指出脆弱之处
2	APPROVE，并注明后续事项
3-4	APPROVE

校准： 大多数实际代码的分数都处于中间区间（2）。4 分代表真正出色——不要虚高打分。如果你写的每份审查都打 3-4 分，说明有问题的是你的评分标准，而不是代码。

这是附加项——它不会取代由 code-reviewer agent 负责的现有判定结果/CONFIDENCE 约定。它的作用是为路由器提供比二元 APPROVE 更清晰的门槛输入：经过分级的 0-4 分，让路由器区分“勉强通过，需要留意”（1-2）和“可以发布”（3-4），并据此安排修复工作。

输出格式

## Code Review: [PR Title/Component]

### Stage 1: Spec Compliance ✅/❌

**Requirements:**
- [x] Requirement 1 - implemented at `file:line`
- [x] Requirement 2 - implemented at `file:line`
- [ ] Requirement 3 - NOT IMPLEMENTED

**Tests:** PASS (24/24)

**Verdict:** [Meets spec / Missing requirements]

---

### Stage 2: Code Quality

**Security:**
- [CRITICAL] Issue at `file:line` - Fix: [recommendation]
- No issues found ✅

**Performance:**
- [MAJOR] N+1 query at `file:line` - Fix: Use batch query
- No issues found ✅

**Quality:**
- [MINOR] Unclear naming at `file:line` - Suggestion: rename to X
- No issues found ✅

**UX/A11y:** (if UI code)
- [MAJOR] Missing loading state - Fix: Add spinner
- No issues found ✅

---

### Summary

**Decision:** Approve / Request Changes

**Critical:** [count]
**Major:** [count]
**Minor:** [count]

**Required fixes before merge:**
1. [Most important fix]
2. [Second fix]

审查循环协议

请求修改后：

1. 等待修复 - 开发者处理问题
2. 重新审查 - 检查修复是否确实解决了问题
3. 确认没有回归 - 再次运行测试
4. 批准或请求更多修改 - 如有需要，重复以上步骤

未经验证修复有效，绝不批准。

部分阶段审查

审查多阶段计划中单个阶段的代码时：

范围问题	规则
只审查本阶段的改动？	YES — 不要将范围扩展到后续阶段
审查时发现未改动代码中的问题，要标记出来吗？	记录为后续事项；不要因此阻碍本阶段
验证阶段退出标准？	YES — 计划为每个阶段定义了退出标准；验证这些标准，而非最终产品
审查与后续阶段的集成点？	仅标记接口方面的问题 — 不要要求实现后续阶段的内容

核心原则： 当阶段退出标准已满足且没有回归时，部分阶段审查即为成功。如果计划还有更多阶段，“功能尚未完成”不是有效的拒绝理由。

最终检查

批准前：

• 阶段 1 已完成（已验证是否符合规范）
• 阶段 2 已完成（已审查所有清单）
• 所有关键/主要问题均已解决
• 测试通过
• 未引入回归
• 已为每项声明收集证据