en un clic
code-review-and-quality
// 执行多维度代码审查。用于合并任何变更之前;用于审查自己、其他 agent 或人类编写的代码;用于在代码进入主分支前从多个维度评估代码质量。
// 执行多维度代码审查。用于合并任何变更之前;用于审查自己、其他 agent 或人类编写的代码;用于在代码进入主分支前从多个维度评估代码质量。
指导稳定的 API 和接口设计。设计 API、模块边界或任何公共接口时使用。创建 REST 或 GraphQL endpoint、定义模块之间的类型契约,或建立前后端边界时使用。
在真实浏览器中测试。构建或调试任何在浏览器中运行的内容时使用。当你需要通过 Chrome DevTools MCP 检查 DOM、捕获 console 错误、分析网络请求、分析性能,或用真实运行时数据验证视觉输出时使用。
自动化 CI/CD pipeline 设置。用于设置或修改构建和部署 pipeline 时;用于需要自动化质量门禁、在 CI 中配置 test runners,或建立部署策略时。
为清晰度简化代码。用于在不改变行为的前提下重构代码以提升清晰度;用于代码能运行但比应有状态更难阅读、维护或扩展时;用于审查已累积不必要复杂度的代码时。
优化 agent 上下文设置。当开始新会话、agent 输出质量下降、在任务之间切换,或需要为项目配置规则文件和上下文时使用。
指导系统化根因调试。当测试失败、构建中断、行为不符合预期,或遇到任何意外错误时使用。当你需要系统化地找到并修复根因,而不是猜测时使用。
| name | code-review-and-quality |
| description | 执行多维度代码审查。用于合并任何变更之前;用于审查自己、其他 agent 或人类编写的代码;用于在代码进入主分支前从多个维度评估代码质量。 |
带质量门禁的多维度代码审查。每个变更在合并前都必须经过审查,没有例外。审查覆盖五个轴:正确性、可读性、架构、安全性和性能。
批准标准: 当一个变更明确改善了整体代码健康度时,就批准它,即使它并不完美。完美代码不存在,目标是持续改进。不要因为它和你自己的写法不完全一致就阻止它。如果它改善了代码库并遵循项目约定,就批准它。
每次审查都从这些维度评估代码:
代码是否做了它声称要做的事?
另一个工程师(或 agent)能否在作者不解释的情况下理解这段代码?
temp、data、result)_unused)、向后兼容 shim,或 // removed 注释?这个变更是否适合系统设计?
详细安全指导见 security-and-hardening。这个变更是否引入了漏洞?
详细 profiling 和优化指导见 performance-optimization。这个变更是否引入了性能问题?
小而聚焦的变更更容易审查、更快合并,也更安全部署。目标大小如下:
~100 lines changed → Good. Reviewable in one sitting.
~300 lines changed → Acceptable if it's a single logical change.
~1000 lines changed → Too large. Split it.
什么算“一个变更”: 一个自包含的修改,只解决一件事,包含相关测试,并且提交后系统仍可运行。它是一个功能的一部分,而不是整个功能。
变更过大时的拆分策略:
| 策略 | 做法 | 何时使用 |
|---|---|---|
| Stack | 先提交一个小变更,再基于它开始下一个变更 | 顺序依赖 |
| By file group | 对需要不同审查者的文件组拆分变更 | 横切关注点 |
| Horizontal | 先创建共享代码/stubs,再接入消费者 | 分层架构 |
| Vertical | 将功能拆成更小的 full-stack 切片 | 功能开发 |
何时可以接受大变更: 完整删除文件,以及自动化重构。这类变更中,审查者只需要验证意图,而不是逐行检查。
将重构和功能开发分开。 一个既重构现有代码又添加新行为的变更,其实是两个变更,应分别提交。小型清理(例如变量重命名)可由审查者判断是否一起包含。
每个变更都需要一段能在版本控制历史中独立成立的描述。
第一行: 简短、祈使句、可独立理解。写 "Delete the FizzBuzz RPC",不要写 "Deleting the FizzBuzz RPC."。它必须足够有信息量,让搜索历史的人不读 diff 也能理解变更。
正文: 说明改变了什么以及为什么。包含代码本身看不出来的上下文、决策和推理。必要时链接 bug 编号、benchmark 结果或设计文档。如果方案存在不足,要明确承认。
反模式: "Fix bug," "Fix build," "Add patch," "Moving code from A to B," "Phase 1," "Add convenience functions."
看代码之前,先理解意图:
- What is this change trying to accomplish?
- What spec or task does it implement?
- What is the expected behavior change?
测试会揭示意图和覆盖范围:
- Do tests exist for the change?
- Do they test behavior (not implementation details)?
- Are edge cases covered?
- Do tests have descriptive names?
- Would the tests catch a regression if the code changed?
带着五个轴逐步检查代码:
For each file changed:
1. Correctness: Does this code do what the test says it should?
2. Readability: Can I understand this without help?
3. Architecture: Does this fit the system?
4. Security: Any vulnerabilities?
5. Performance: Any bottlenecks?
为每条评论标注严重程度,让作者知道哪些是必需修改,哪些是可选建议:
| 前缀 | 含义 | 作者动作 |
|---|---|---|
| (no prefix) | 必需变更 | 合并前必须处理 |
| Critical: | 阻塞合并 | 安全漏洞、数据丢失、功能损坏 |
| Nit: | 轻微、可选 | 作者可以忽略,通常是格式或风格偏好 |
| Optional: / Consider: | 建议 | 值得考虑,但不是必须 |
| FYI | 仅供参考 | 无需动作,是供未来参考的上下文 |
这可以防止作者把所有反馈都当成强制要求,并在可选建议上浪费时间。
检查作者的验证说明:
- What tests were run?
- Did the build pass?
- Was the change tested manually?
- Are there screenshots for UI changes?
- Is there a before/after comparison?
使用不同模型提供不同审查视角:
Model A writes the code
│
▼
Model B reviews for correctness and architecture
│
▼
Model A addresses the feedback
│
▼
Human makes the final call
这能捕捉单个模型可能漏掉的问题,因为不同模型有不同盲点。
审查 agent 的示例 prompt:
Review this code change for correctness, security, and adherence to
our project conventions. The spec says [X]. The change should [Y].
Flag any issues as Critical, Important, or Suggestion.
任何重构或实现变更之后,都要检查孤立代码:
不要把死代码留在周围,它会迷惑未来的读者和 agent。但也不要默默删除你不确定的东西。有疑问就问。
DEAD CODE IDENTIFIED:
- formatLegacyDate() in src/utils/date.ts — replaced by formatDate()
- OldTaskCard component in src/components/ — replaced by TaskCard
- LEGACY_API_URL constant in src/config.ts — no remaining references
→ Safe to remove these?
缓慢的审查会阻塞整个团队。切换上下文进行审查的成本,低于让别人等待所造成的成本。
解决审查争议时,按这个优先级处理:
不要接受“以后再清理”。 经验表明,推迟的清理很少发生。除非是真正紧急情况,否则要求在提交前清理。如果周边问题无法在本次变更中处理,要求创建 bug 并自我指派。
审查代码时,不管代码是你自己、另一个 agent 还是人类写的:
代码审查的一部分是依赖审查:
添加任何依赖之前:
npm audit)规则: 优先使用标准库和现有工具,而不是新增依赖。每个依赖都是负债。
## 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
### 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.mdreferences/performance-checklist.md| 合理化借口 | 现实 |
|---|---|
| “它能跑,就够了” | 不可读、不安全或架构错误的可运行代码会制造不断复利的债务。 |
| “这是我写的,所以我知道它是对的” | 作者会看不见自己的假设。每个变更都受益于另一双眼睛。 |
| “以后再清理” | 以后不会到来。审查就是质量门禁,要用起来。要求合并前清理,而不是合并后。 |
| “AI 生成的代码大概没问题” | AI 代码需要更多审查,而不是更少。它即使错了,也会显得自信且合理。 |
| “测试通过了,所以没问题” | 测试是必要但不充分的。它们抓不到架构问题、安全问题或可读性问题。 |
审查完成后: