code-review-and-quality

代码审查和质量

概览

带质量门禁的多维度代码审查。每个变更在合并前都必须经过审查，没有例外。审查覆盖五个轴：正确性、可读性、架构、安全性和性能。

批准标准： 当一个变更明确改善了整体代码健康度时，就批准它，即使它并不完美。完美代码不存在，目标是持续改进。不要因为它和你自己的写法不完全一致就阻止它。如果它改善了代码库并遵循项目约定，就批准它。

何时使用

• 合并任何 PR 或变更之前
• 完成功能实现之后
• 当另一个 agent 或模型产出了你需要评估的代码时
• 重构现有代码时
• 修复任何 bug 之后（同时审查修复和回归测试）

五轴审查

每次审查都从这些维度评估代码：

1. 正确性

代码是否做了它声称要做的事？

• 是否符合 spec 或任务要求？
• 是否处理了边界情况（null、empty、边界值）？
• 是否处理了错误路径（不只是 happy path）？
• 是否通过所有测试？测试是否真的在测试正确的事情？
• 是否存在 off-by-one 错误、竞态条件或状态不一致？

2. 可读性和简单性

另一个工程师（或 agent）能否在作者不解释的情况下理解这段代码？

• 命名是否具有描述性，并与项目约定一致？（没有缺少上下文的 temp、data、result）
• 控制流是否直接清晰（避免嵌套三元表达式、深层 callback）？
• 代码组织是否符合逻辑（相关代码放在一起，模块边界清晰）？
• 是否有应该简化的“聪明”技巧？
• 能否用更少的行数完成？（100 行足够却写了 1000 行就是失败）
• 抽象是否配得上它带来的复杂度？（不要在第三个用例之前泛化）
• 注释是否有助于澄清非显而易见的意图？（但不要注释显而易见的代码。）
• 是否存在死代码痕迹：no-op 变量（_unused）、向后兼容 shim，或 // removed 注释？
• 是不是把一个新增的条件分支硬接到了不相干的流程上？ 这是设计异味，不是小问题（nit），把这段逻辑放进它自己的 helper、state 或 policy 里，别去纠缠已有路径。
• 是否出现了对同一形状反复做条件判断？ 这说明缺少一个模型或 dispatcher。“临时”分支通常是永久债务。

3. 架构

这个变更是否适合系统设计？

• 它遵循现有模式，还是引入了新模式？如果是新模式，是否有充分理由？
• 是否保持了清晰的模块边界？
• 是否存在应该共享的代码重复？
• 依赖流向是否正确（没有循环依赖）？
• 抽象层级是否合适（不过度工程化，也不过度耦合）？
• 这次重构是降低了复杂度，还是只是把它挪了个地方？ 数一数读者要跟上这个变更必须同时装进脑中的概念数量。如果所谓“更干净”的版本让这个数量没有变少，它就不干净：优先选择能让整块分支、模式或层消失的重构，而不是把同样的逻辑重新集中到别处。宁可删掉一个抽象，也不要打磨它。
• 功能专属逻辑是否泄漏进了共享或通用模块？ 让逻辑留在它所属的层，复用已有的规范 helper 而不是造一个近似重复品，也不要纵容架构漂移。
• 类型边界是否显式？ 质疑那些随手写下的 any/unknown/可选类型/类型断言，以及掩盖不清晰不变量的静默 fallback。把边界写明，往往能让周围的控制流更简单。

4. 安全性

详细的安全指南见 security-and-hardening。这个变更是否引入了漏洞？

• 用户输入是否经过校验和清洗？
• 密钥是否被排除在代码、日志和版本控制之外？
• 需要认证/授权的地方是否做了检查？
• SQL 查询是否参数化（没有字符串拼接）？
• 输出是否做了编码以防 XSS？
• 依赖是否来自可信来源且无已知漏洞？
• 来自外部来源的数据（API、日志、用户内容、配置文件）是否被视为不可信？
• 外部数据流是否在系统边界处先校验，再用于逻辑或渲染？

5. 性能

详细的 profiling 和优化指南见 performance-optimization。这个变更是否引入了性能问题？

• 有没有 N+1 查询模式？
• 有没有无界循环或不受约束的数据拉取？
• 有没有本该异步的同步操作？
• UI 组件中有没有不必要的重渲染？
• 列表 endpoint 是不是漏了分页？
• 热路径里有没有创建大对象？

结构化补救

指出结构性问题时，要给出改法，而不只是问题本身。只说“这太复杂了”的审查会让作者无所适从。要提出有名字的重构方案：

• 用带类型的模型或显式 dispatcher 替换一长串条件判断。
• 把重复的分支合并成一条更清晰的流程。
• 把编排与业务逻辑分离，让两者各自单独可读。
• 把功能专属逻辑移出共享模块，放进真正拥有这个概念的包（package）。
• 复用规范 helper，而不是自己写一个近似重复的定制版。
• 把类型边界写明，让下游的分支随之消失。
• 删掉只是透传的 wrapper，它增加了间接层却没让 API 更清晰。
• 提取 helper，或把大文件拆成职责聚焦的模块。