Menu
进行多维度代码审查。在合并任何变更之前使用。在审查由你自己、另一个智能体或人类编写的代码时使用。当需要在代码进入主分支之前评估其跨多个维度的质量时使用。当需要评估变更的正确性、可读性、架构与性能，或审查 PR/diff 时使用。
Install with Codex or Claude Copy this prompt, paste it into Codex, Claude, or another assistant, and let it review the skill page and install it for you.
Based on SOC occupation classification
| name | code-review-and-quality |
| description | 进行多维度代码审查。在合并任何变更之前使用。在审查由你自己、另一个智能体或人类编写的代码时使用。当需要在代码进入主分支之前评估其跨多个维度的质量时使用。当需要评估变更的正确性、可读性、架构与性能，或审查 PR/diff 时使用。 |
带有质量门禁的多维度代码审查。每个变更在合并之前都要经过审查——无例外。审查涵盖四个维度：正确性、可读性、架构和性能。
批准标准： 当一个变更确实能改善整体代码健康度时批准它，即使它并不完美。完美的代码不存在——目标是持续改进。不要因为变更没有完全按照你写代码的方式来写而阻止它。如果它改进了代码库并遵循了项目的约定，就批准它。
每次审查都按以下维度评估代码：
代码做了它声称要做的事情吗？
另一个工程师（或智能体）能在没有作者解释的情况下理解这段代码吗？
temp、data、result）_unused）、向后兼容适配层或 // removed 注释？变更是否符合系统的设计？
any/unknown/可选/强制转换以及掩盖不清晰不变量的静默回退——使边界显式通常会让周围的流程控制变得更简单。详细分析和优化见 performance-optimization。变更是否引入了性能问题？
当你标记一个结构性问题时，提出具体的改动方向——而不仅仅是问题本身。只说"这很复杂"的审查会让作者猜测。应提出有命名的重组方案：
优先选择能消除活动部件的方案，而非将相同的复杂度摊开到更多的代码中。
小而专注的变更更容易审查，合并更快，部署更安全。以这些规模为目标：
~100 行变更 → 好。可以一次性审查完。
~300 行变更 → 如果是单一逻辑变更，可以接受。
~1000 行变更 → 太大。拆分它。
关注文件大小，而不仅仅是 diff 大小。 小的 diff 仍然可能将文件推过健康的边界——单个文件中约 1000 行的总行数（与上述约 1000 行变更行数的阈值不同）是一个常见的检查信号，而非硬性上限。当变更显著增长一个已经很大的文件时，应先考虑是否提取辅助函数、子组件或模块，再继续堆砌更多代码。先分解，再添加。
什么算"一个变更"： 一个自包含的修改，处理一件事，包含相关测试，并在提交后保持系统功能完整。它是功能的一部分——而非整个功能。
变更过大时的拆分策略：
| 策略 | 如何操作 | 何时使用 |
|---|---|---|
| 堆叠 | 提交一个小变更，在此基础上开始下一个 | 顺序依赖 |
| 按文件组 | 为需要不同审查者的文件组分拆变更 | 横切关注点 |
| 水平 | 先创建共享代码/桩，然后是消费者 | 分层架构 |
| 垂直 | 拆分为更小的完整功能切片 | 功能开发 |
可接受的大变更： 完全的文件删除和自动化重构，审查者只需验证意图而非每行代码。
将重构与功能工作分开。 一个既重构现有代码又添加新行为的变更是两个变更——分开提交。小型清理（变量重命名）可以根据审查者的判断包含在内。
每个变更都需要一个能在版本控制历史中独立存在的描述。
第一行： 简短、祈使语气、独立完整。"删除 FizzBuzz RPC"而不是"正在删除 FizzBuzz RPC"。必须足够有信息量，让搜索历史的人能在不阅读 diff 的情况下理解这个变更。
正文： 变更了什么以及为什么。包含代码本身不可见的上下文、决策和推理。引用相关的缺陷编号、基准测试结果或设计文档。当方法存在不足时，承认这些不足。
反模式： "修复缺陷"、"修复构建"、"添加补丁"、"将代码从 A 移到 B"、"第一阶段"、"添加便利函数"。
在看代码之前，理解意图：
- 这个变更试图完成什么？
- 它实现了什么规格或任务？
- 预期的行为变更是什么？
测试揭示意图和覆盖率：
- 是否有针对该变更的测试？
- 它们是否测试行为（而非实现细节）？
- 边界情况是否覆盖？
- 测试是否有描述性名称？
- 如果代码变更，测试是否能捕获回归？
带着五维度视角浏览代码：
对每个变更的文件：
1. 正确性：这段代码是否做了测试说它应该做的事情？
2. 可读性：我能在没有帮助的情况下理解它吗？
3. 架构：这适合系统吗？
4. 安全性：是否有漏洞？
5. 性能：是否有瓶颈？
用严重程度标记每个评论，让作者知道什么是必须的、什么是可选的：
| 前缀 | 含义 | 作者行动 |
|---|---|---|
| (无前缀) | 必须的变更 | 必须在合并前处理 |
| Critical（严重）: | 阻止合并 | 安全漏洞、数据丢失、功能损坏 |
| Nit（小问题）: | 次要、可选 | 作者可以忽略——格式化、风格偏好 |
| Optional（可选）: / Consider（考虑）: | 建议 | 值得考虑但不必须 |
| FYI（供参考） | 仅信息性 | 无需行动——供将来参考的上下文 |
这可以防止作者将所有反馈视为强制的，并在可选建议上浪费时间。
先讲重要的。 按影响力排序发现：正确性和安全性优先，然后是结构性退化和遗漏的简化，最后是其他所有内容。不要将真正的问题埋藏在表面的小问题之下——几个高确信度的评论胜过一长串列表。如果你有一个结构调整建议和十个小问题，结构调整本身就是审查的核心。
检查作者的验证记录：
- 运行了哪些测试？
- 构建通过了吗？
- 变更是否手动测试过？
- 配置变更是否有前后对比？
- 是否有前后对比？
使用不同的模型获取不同的审查视角：
模型 A 编写代码
│
▼
模型 B 审查正确性和架构
│
▼
模型 A 处理反馈
│
▼
人类做出最终决定
这可以捕获单个模型可能遗漏的问题——不同模型有不同的盲点。
给审查智能体的示例提示词：
审查这段代码变更的正确性、安全性和对我们项目约定的遵守。
规格要求 [X]。变更应该 [Y]。
将所有问题标记为 Critical、Required、Optional 或 Nit。
在任何重构或实现变更之后，检查孤立的代码：
不要让死代码到处散落——它会混淆未来的读者和智能体。但不要默默地删除你不确定的事情。有疑问时，先问。
已识别的死代码：
- internal/util/date.go 中的 FormatLegacyDate() ——已被 FormatDate() 替代
- src/storage/ 中的 OldBlockCache 类型 ——已被 BlockCache 替代
- src/config.go 中的 kLegacyApiUrl 常量 ——没有剩余的引用
→ 可以安全移除这些吗？
缓慢的审查会阻塞整个团队。上下文切换以进行审查的成本低于施加给其他人的等待成本。
解决审查争议时，应用以下层次：
不要接受"我之后会清理"。 经验表明，推迟的清理很少会发生。在提交之前要求清理，除非是真正的紧急情况。如果周围问题无法在此次变更中处理，要求提交一个缺陷并自分配。
审查代码时——无论是由你、另一个智能体还是人类编写的：
代码审查的一部分是依赖项审查：
在添加任何依赖项之前：
cargo audit / govulncheck ./... / 供应链扫描工具）规则： 优先使用标准库和现有工具而非新依赖项。每个依赖项都是一个负担。
升级现有依赖项和其他代码变更一样，最危险的升级是那些批量合并的、带有"bump deps"之类消息的升级。用同样的纪律审查它们：
Cargo.toml / go.mod；单次直接升级可能引入数十项间接变更。对于筛选 cargo audit / govulncheck 发现和供应链风险（拼写仿冒、维护者被攻陷），请遵循项目的依赖审计策略——本节涵盖升级工作流；安全性裁决应由用户或项目安全策略决定。
## 审查：[PR/变更标题]
### 上下文
- [ ] 我理解这个变更做了什么以及为什么
### 正确性
- [ ] 变更与规格/任务需求匹配
- [ ] 边界情况已处理
- [ ] 错误路径已处理
- [ ] 测试充分覆盖了变更
### 可读性
- [ ] 命名清晰且一致
- [ ] 逻辑直接明了
- [ ] 无不必要的复杂度
### 架构
- [ ] 遵循现有模式
- [ ] 无不必要的耦合或依赖项
- [ ] 抽象层次适当
- [ ] 重构减少了复杂度而非移动了它
- [ ] 共享模块中没有功能逻辑；文件保持在合理的大小范围内
### 安全性
- [ ] 代码中没有机密
- [ ] 输入在边界处进行了验证
- [ ] 无注入漏洞
- [ ] 认证检查到位
- [ ] 外部数据来源被视作不可信数据
### 性能
- [ ] 无 N+1 模式
- [ ] 无无界操作
- [ ] 列表端点有分页
### 验证
- [ ] 测试通过
- [ ] 构建成功
- [ ] 手动验证已完成（如适用）
### 裁决
- [ ] **批准**——可合并
- [ ] **要求变更**——问题必须处理
references/performance-checklist.md| 合理化借口 | 现实 |
|---|---|
| "它能用，这就够了" | 能用但不可读、不安全或架构错误的代码会产生复利债务。 |
| "是我写的，所以我知道它是正确的" | 作者对自己假设的盲点是不可避免的。每个变更都受益于另一双眼睛。 |
| "我们之后会清理" | "之后"永远不会到来。审查就是质量门禁——使用它。要求在合并前清理而非合并后。 |
| "AI 生成的代码应该没问题" | AI 代码需要更多而不是更少的审查。它自信且看似合理，即使错误时也是如此。 |
| "测试通过了，所以没问题" | 测试是必要的但不是充分的。它们不能捕获架构问题、安全问题或可读性问题。 |
| "这次重构让它更干净了" | 重新定位复杂度不是减少它。如果读者仍然需要掌握相同数量的概念，结构就没有改进——寻找能让分支消失的版本。 |
| "这只是对文件的一个小补充" | 小 diff 仍然可能将文件推过健康大小，并将分支拼接到不相关的流程上。评判结果结构，而非 diff 大小。 |
| "这只是一个版本升级" | 版本升级是你没有编写的行为变更。阅读变更日志；semver 不能保证没有破坏性。 |
| "我在一个 PR 中升级所有东西以节省时间" | 破坏构建的批量升级隐藏了哪个包导致的。每次一个依赖项，使原因和回滚都保持干净。 |
审查完成后：
推定阻塞项： 对以下每一项指出并提出更简单的设计；仅在变更主动使结构恶化时才升级为 Required：重构重新定位复杂性而非减少它；将文件推过大小边界而没有分解的变更；添加到共享模块的功能逻辑；现有规范辅助函数的近乎重复；隐藏不清晰不变量的静默回退。
指导稳定的 RPC、存储协议和接口设计。在设计节点间 RPC、存储协议语义、模块边界或任何公共接口时使用。在定义 gRPC service、对象存储或文件系统语义、节点间的类型契约，或确立组件边界时使用。
自动化 CI/CD 流水线设置。在设置或修改构建和部署流水线时使用。当需要自动化质量门禁、在 CI 中配置测试运行器，或建立部署策略时使用。当需要处理 CI 上不稳定（flaky）的测试时使用。
为清晰性而简化代码。在重构代码以提高可读性而不改变行为时使用。当代码可以正常工作但比应有的更难阅读、维护或扩展时使用。在审查已积累不必要复杂性的代码时使用。当组件过度设计、过于炫技而难以维护时使用。当需要清理越来越难读懂的代码时使用。
验证系统的一致性与持久性承诺。在设计或修改复制协议、写路径、崩溃恢复逻辑时使用。当需要证明已确认的写入不丢失、副本间不发散、崩溃后能恢复到一致状态，或评审 fsync/校验和/事务语义时使用。
优化智能体上下文设置。在开始新会话、智能体输出质量下降、在不同任务之间切换，或需要为项目配置规则文件和上下文时使用。
指导系统化的根因调试。当测试失败、构建中断、行为不符合预期，或遇到任何意外错误时使用。当需要系统化地找到并修复根本原因而非猜测时使用。当服务崩溃（panic）、空指针异常、或之前通过的测试突然挂掉时使用。当生产环境间歇性报错、需要定位根因时使用。当测试或构建昨天还通过、今天就挂了时使用。