Zc Code Review And Quality

代码审查与质量

何时使用

• 合并任何变更之前
• 完成一次功能实现之后
• 评估自己、其他代理或人类写的代码时
• bug 修复和重构之后

输入前提

• 已知道这次变更对应的规格或任务
• 已有测试、构建或其他验证证据可供审查
• 愿意按问题严重度给出明确结论，而不是泛泛评价

执行步骤

1. 先理解变更意图，再看测试和验证证据
2. 按五个维度审查：
   • 正确性
   • 可读性
   • 架构
   • 安全性
   • 性能
3. 把发现按 Critical / Important / Suggestion 分级
4. 对每条问题给出具体位置和修复方向
5. 审查结论给出后，要求作者进入 review 响应闭环，逐条收敛问题
6. 所有 Critical 清零并完成必要回归验证后，才算通过

成功标准

• 评审结论基于代码和证据，而不是个人偏好
• 真正会阻塞合并的问题被明确指出
• 报告能让作者快速定位问题和采取动作
• 评审没有用“看起来还行”替代事实判断
• 审查后的问题处理路径清楚，不把“提了意见”误当成“问题已关闭”

推荐结论格式

审查结论必须给出可执行推荐：

Recommendation: <Approve / Request changes / Defer> because <evidence, risk, and trade-off>.

如果请求修改，说明哪些问题阻塞合并；如果批准，说明剩余风险和已验证证据；如果延后，说明为什么延后比本轮修复更合适。

相关原则

• 先报问题，再做总结
• 只评论代码，不评论作者
• 技术事实高于风格偏好

结构性红旗

以下问题即使 diff 不大，也要作为架构或可维护性风险审查：

• 多个独立调用点重复同一项稳定决策时，先判断删除、复用或局部数据映射能否解决；只有这些方式不能降低复杂度，才把它视为缺少领域模型、策略表或 dispatcher 的证据。
• shared / common 模块里出现 feature-specific 分支，通常说明边界泄漏；优先建议把专属逻辑下沉到调用方、adapter 或策略实现。
• 重构只是把复杂度从一个文件搬到另一个文件，没有减少状态、分支、耦合或重复时，不应算质量改善。
• 文件已经过大、职责混杂或新增逻辑需要读很远的上下文才能判断正确时，即使本次新增行数少，也要指出后续维护风险。
• finding 不能只说“建议抽象一下”；remedy 按删除、复用仓库能力、使用标准库或平台原生能力、内联无效抽象、移动边界、缩小公共 API 的顺序评估，只有当前消费者和稳定边界提供证据时才新增模型或 dispatcher。

与其他技能的衔接

• 接在 incremental-implementation 或 bug 修复之后
• 安全和性能深挖时可转给对应专项 skill
• 对验证证据的审查依赖 verification-before-completion
• 审查输出如果产生待修问题，应进入 review-response-and-resolution

## Review: [PR/Change title]

### Context
- [ ] I understand what this change does and why

### Correctness
- [ ] Change matches spec/task requirements
- [ ] Edge cases handled
- [ ] Error paths handled
- [ ] Tests cover the change adequately

### Readability
- [ ] Names are clear and consistent
- [ ] Logic is straightforward
- [ ] No unnecessary complexity

### Architecture
- [ ] Follows existing patterns
- [ ] No unnecessary coupling or dependencies
- [ ] Appropriate abstraction level
- [ ] New dependencies, configuration, and abstractions are supported by a current requirement or real boundary
- [ ] Does not reimplement an existing repository, standard-library, or platform capability

### Security
- [ ] No secrets in code
- [ ] Input validated at boundaries
- [ ] No injection vulnerabilities
- [ ] Auth checks in place
- [ ] External data sources treated as untrusted

### Performance
- [ ] No N+1 patterns
- [ ] No unbounded operations
- [ ] Pagination on list endpoints

### Verification
- [ ] Tests pass
- [ ] Build succeeds
- [ ] Manual verification done (if applicable)

### Verdict
- [ ] **Approve** — Ready to merge
- [ ] **Request changes** — Issues must be addressed

另见

• 更详细的安全审查指导，请见 references/security-checklist.md
• 更详细的性能审查检查项，请见 references/performance-checklist.md
• 上述上游清单的许可证声明随产物分发于 references/LICENSE-agent-skills.txt

常见合理化说辞

红旗

• PR 在没有审查的情况下合并
• 审查只看测试是否通过，而忽略其他维度
• 没有真正审查就直接说 “LGTM”
• 安全敏感改动没有安全向审查
• 大 PR 大到“没法好好审查”
• bug 修复没有回归测试
• 审查意见没有严重程度标签，导致不清楚哪些必须修
• 接受“我以后再修”——通常不会发生

验证

审查完成后：

• 所有 Critical 问题已解决
• 所有 Important 问题已解决，或已明确延后并给出理由
• 审查意见已逐条进入响应闭环，没有“已读未处理”的项
• 测试通过
• 构建成功
• 已记录验证故事（改了什么、如何验证）