specsfy-specialist-code-review

代码审查

使用时机

• 当有具体的 diff、分支或 PR 需要在合并或发布前评估时使用。
• 即使尚未创建 PR，如果请求是对已编写的更改寻求“第二意见”，也使用。
• 不要用于从零开始设计新的结构性决策——请使用 $specsfy-specialist-software-architecture；审查评估的是已经决定并编写的内容，不能取代决策。
• 当 diff 涉及身份验证、授权、敏感数据或外部输入时，与 $specsfy-specialist-application-security 结合使用；当发现涉及名称、不变量或令人困惑的领域边界时，与 $specsfy-specialist-domain-modeling 结合使用。

流程

1. 确定比较基准和 diff 的确切范围；没有明确基准就审查 diff，会发现一些与更改未涉及的代码有关的问题。
2. 判断之前，先阅读适用的 spec、issue、验收标准和仓库说明；缺少这些依据，“正确”就会沦为个人观点。
3. 将每个变更文件对应到它影响的行为和边界（数据、权限、API 契约、配置、依赖）。
4. 先评估正确性、边界情况、安全性、并发和运行影响，再看风格——只有自动化检查无法覆盖时，风格问题才应阻止合并。
5. 根据测试提供的证据进行检查：如果没有修复，测试会失败吗？还是只覆盖了代码行，却没有证明行为？
6. 报告之前，先阅读实际代码以及调用方／消费者，确认每个可疑发现，减少误报。
7. 按严重性报告，并提供精确位置（文件:行号）、触发故障的条件、影响和可能的修复方式。

准则

• 优先关注 bug 和具体风险；不要把风格偏好变成阻止合并的理由。
• 每条发现都应说明输入条件、可观察到的后果，以及证明问题的证据（代码行、测试、日志）。
• 不仅考虑理想路径，还要考虑与现有客户端的兼容性、并发、回滚计划和更改的可观测性。
• 确认新增测试是否会在实际修复不存在时失败——如果测试在更改前后都能通过，就不能证明任何问题。
• 区分 PR 缺少的范围（阻止合并）和可选的未来改进（仅作评论，不阻止合并）。
• 不要重复报告 lint、formatter 或 type checker 等自动化工具已经覆盖的问题；只指出自动化检查不到的内容。
• 如果适用，明确写出“此审查角度未发现问题”，不要使用暗示观察范围之外也不存在风险的措辞。

反模式

• 当项目已配置 linter 检查时，仍因个人对变量命名或格式的偏好而阻止合并——这会浪费审查的注意力，无法聚焦重要问题。
• 因为“测试都通过了”就批准，却不检查新测试是否确实覆盖更改涉及的行为（同义反复的测试或没有实际断言的测试）。
• 不梳理文件之间的流程，就逐个文件进行审查——这会遗漏交叉影响，例如函数签名变更后，并非所有调用方都已调整。
• 只说“看起来不安全”却不指出具体攻击途径——没有信任边界和具体输入的安全问题无法采取行动。

验证

• 审查完整 diff，包括配置文件、migrations，以及 diff 之外受该行为影响的调用方／消费者。
• 发布之前，根据代码和测试套件的实际状态核实每条报告的问题——未经确认的问题不得加入报告。
• 按严重性（概率 × 影响）排列最终列表，而不是按文件在 diff 中的顺序排列。
• 在最后总结审查覆盖范围（检查了什么）和剩余风险（哪些内容无法确认）。
• 没有上述证据时，不要声称 diff“安全”或“正确”——没有证据支持的绝对措辞是禁止的。

相关技能

• $specsfy-specialist-debugging 可在评审发现尚未解释的缺陷时，提供复现和根因分析。
• 当 diff 中出现相关风险时，$specsfy-specialist-typescript 可深入分析类型契约，$specsfy-specialist-web-accessibility 可深入分析语义、键盘操作和 WCAG。
• 当 diff 涉及身份、授权、敏感数据或外部输入时，使用 $specsfy-specialist-application-security——安全评审会深入分析此技能仅作提示的内容。
• 当发现涉及 diff 所揭示的耦合、边界或结构性决策，而不只是 diff 本身时，使用 $specsfy-specialist-software-architecture。
• 当冲突解决后需要重新评审时，使用 $specsfy-specialist-merge-conflict-resolution，因为冲突解决可能会改变最终行为。

请阅读 references/standards.md，了解评审视角、严重性等级、发现项格式和官方参考资料来源。