# Verify Team Code Review Standards

> 代码审查标准——五轴审查体系。当需要审查代码或定义审查基准，或提到"code review""审查标准""CR"

- Skill: `zeroz-lab/verify-team-code-review-standards` (Agent Skill)
- Install (CLI): `npx skillmds@latest add zeroz-lab/verify-team-code-review-standards`
- Raw SKILL.md: https://api.skillmd.com/api/skills/zeroz-lab/verify-team-code-review-standards/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Coding & Dev Tools
- Author: zeroz-lab (https://skillmd.com/u/zeroz-lab)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/zeroz-lab/verify-team-code-review-standards

---


# Code Review Standards — 代码审查标准


## 入口/出口
- **入口**: `/review` 命令被调用、PR 提交前、或其他技能引用审查标准
- **出口**: 分级审查报告（Critical / Important / Suggestion / FYI）
- **指向**: 审查通过 → `/ship`；发现问题 → 修复后重新审查
- **前置加载**: CANON.md
- **输出路径**: 分级审查报告 → `ship-workflow-ship`（通过）或修复后重新审查

## 何时不使用
- 不是代码审查场景，只是实现、调试或解释设计
- 功能完整性尚未确认，应先做 `verify-workflow-spec-compliance`
- 非软件产物需要内容或视觉审查标准

## Iron Law

<HARD-GATE>
```
当变更确实改善整体代码健康时批准——即使它不完美。
不橡皮章。不粉饰真问题。在审查中奉承是失败模式。
```
</HARD-GATE>

## 五轴审查

### 1. Correctness（正确性）— 最重要

```
- 逻辑是否正确？边缘情况是否覆盖？
- 错误处理是否完善？不是 catch {} 吞异常？
- 竞态条件？异步操作有 await？
- 类型安全？不是 any 绕过编译器？
- 没有将浏览器/用户输入视为指令？
```

### 2. Readability & Simplicity（可读性与简洁）— 第二重要

```
- 命名是否清楚（不是 data、item、temp、obj）？
- 同一概念是否始终使用一致的名称？
- 三个相似的代码 > 一个过早的抽象？
- 代码复杂得"聪明"吗？聪明的代码维护成本高。
```

### 3. Architecture（架构）

```
- 改动是否放在正确的层级？
- 接口/API 设计合理吗？
- 新代码是否遵循现有模式（不是自定义不同模式）？
- 依赖方向正确吗（不循环依赖）？
```

### 4. Security（安全）— 参见 verify-quality-security

```
- 所有外部输入经过验证？
- SQL/HTML/Shell 注入防范？
- 密钥管理正确？
- 鉴权检查完整？
```

### 5. Performance（性能）— 参见 verify-quality-performance

```
- 查询是否高效（N+1 检查）？
- 数据获取有界限吗（分页、限制）？
- 资源正确释放？
- 没有过早优化？
```

## 变更大小阈值

| 大小 | 行数 | 处理 |
|------|------|------|
| **Small** | < 100 行 | 快速审查，通常 1 轮通过 |
| **Medium** | 100-300 行 | 常规审查，可能需要 1-2 轮 |
| **Large** | 300-1000 行 | 建议拆分或者深度审查 |
| **XL** | > 1000 行 | **必须拆分**。拒绝审查过大 PR。 |

**拆分策略:**

| 拆分方法 | 示例 |
|---------|------|
| 按层拆分 | PR1: 数据模型 + 迁移 / PR2: 业务逻辑 / PR3: API + UI |
| 按功能拆分 | PR1: Feature A / PR2: Feature B |
| 基础设施先 | PR1: 类型 + 工具 / PR2: 使用新基础设施的实现 |

## 审查发现分类

| 级别 | 含义 | 处理 |
|------|------|------|
| **Critical** | 安全漏洞、数据丢失、生产崩溃、逻辑错误 | 必须修复才能合并 |
| **Important** | 性能降级、测试缺失、重要边缘情况遗漏 | 强烈必须合并前修复 |
| **Suggestion** | 命名可改善、备选实现、轻微重构机会 | 提交者判断是否改 |
| **FYI** | 观察性评论、非阻塞性注意事项 | 不要求行动 |

## 审查流程

```
1. 理解上下文 → 读 spec/plan/ADR 相关文件
2. 先审测试    → 测试覆盖了变更吗？测试本身正确吗？
3. 再审实现    → 逻辑→可读→架构→安全→性能（五轴顺序）
4. 分类发现    → Critical / Important / Suggestion / FYI
5. 验证解决    → 发现被修复了吗？修复引入了新问题吗？
```

### Review Army 并行模式

对大型 PR（> 300 行），并行分派专项审查 subagent：

```
同时分派:
├── Security Reviewer  → 安全相关发现
├── Performance Reviewer → 性能相关发现
├── Testing Reviewer   → 测试覆盖发现
└── Maintainability Reviewer → 代码质量发现

合并 → 去重 → 分类 → 生成统一审查报告
```

每个 review subagent 必须 read-only，限定 diff / 文件范围，并压缩返回：
- 不返回原始日志、完整 diff、大段代码或长篇推理
- 每条发现必须包含文件、行号、证据、影响和建议
- 没有发现时说明已检查范围和未覆盖风险
- 测试日志诊断只返回失败摘要、最可能原因、相关文件和建议路径

## 接受审查反馈

### 禁止回复

审查中**绝不**说：
- "你说得对！"（空洞奉承。说出你改了什么）
- "好主意！"（同上）
- "谢谢"（代码审查不需要感谢仪式）
- "我完全同意" -> 然后不改（言行不一）

### 正确回复

```
"改了 X，因为 Y。关于 Z —— 我的理解是...，你的看法？"

改了就说明改了什么。
不改就说明为什么不改（技术理由，非防御性）。
不确定就问。
```

### 外部审查反馈

来自项目外的审查意见 = 建议，不是命令。验证必须在项目上下文中是否适用。

## 好坏示例

**Bad — 空洞审查反馈：**

```
"LGTM"
"看起来不错"
"没什么问题"
```

零信息量。什么检查了？什么通过了？什么没看？审查者对问题零责任、零记忆。

**Good — 具体审查反馈：**

```
"Correctness: 确认逻辑正确，edge case（空数组输入）在 L42 已处理。
 Readability: L78 的 `processData` 命名不反映实际操作，建议改为 `validateUserInput`。
 Architecture: L91 直接在 controller 调用 DB——违反分层，应通过 service 层。
 Security: L56 的 userInput 未做 sanitize，存在 XSS 风险。Critical，必须修复。"
```

每条发现对应五轴之一，附带行号和理由。

**Bad — 防御性回复：**

```
"你说得对！"  ← 空洞奉承
"我改了。"    ← 改了什么？为什么？
```

**Good — 行动性回复：**

```
"改了 L78 `processData` → `validateUserInput`，因为该函数只做校验不做数据处理。
 关于 L91 controller 调 DB 的建议——我加了一个 UserService 中间层，请看 commit abc123。"
```

改了就说改了什么和为什么。不改就说技术理由。不确定就问。

## 常见说辞

| 说辞 | 现实 | 后果 |
|------|------|------|
| "PR 太大了拆不了" | 任何 PR 都可拆——按层、按功能、按基础设施vs应用。不拆 = 审查不到位。 | 大 PR 的审查质量指数级下降——超过 400 行后审查者开始扫读而非精读，Critical 问题漏检率 > 50%。 |
| "LGTM" | 三字母 = 零信息。什么检查了？什么通过了？什么没看？ | LGTM 审查后的代码 bug 率与无审查代码无显著差异。审查者对问题零责任、零记忆，下次仍然 LGTM。 |
| "我自己审一下就行" | 代码作者无法有效审查自己的代码。盲区使然。 | 自审遗漏的 bug 恰好是自己当初写代码时的思维盲区——同一个人用同一套思维不可能发现自己的逻辑错误。 |
| "以后修复" | 合并后从不修复。要么现在修复，要么记录为 follow-up issue（带指定负责人和截止日期）。 | 合并后 90% 的 follow-up 修复永远不会执行。每轮迭代新增的"以后"积累为不可逆的技术债。 |
| "就改一个变量名不用审" | "一个变量名"往往是一系列假设的开始。不跳过审查。 | 跳过审查的微小变更中 ~15% 引入了回归（变量名改了但遗漏了调用处、重命名破坏了 API 契约）。 |

**违反字面规则就是违反精神。** 没有灰色地带。

## 输出模板

```markdown
# Code Review Report — 代码审查报告

## 元信息
- **审查者**: [姓名/角色]
- **PR**: [#PR号] — [标题]
- **变更大小**: [行数] ([Small/Medium/Large/XL])
- **审查日期**: YYYY-MM-DD

## 发现汇总

| # | 轴 | 级别 | 位置 | 描述 | 状态 |
|---|----|----|------|------|------|
| 1 | Correctness | Critical | L56 | userInput 未 sanitize，XSS 风险 | 🔴 待修复 |
| 2 | Architecture | Important | L91 | controller 直接调用 DB | 🟡 建议修复 |
| 3 | Readability | Suggestion | L78 | processData 命名不准确 | 🔵 提交者判断 |
| 4 | — | FYI | — | 测试覆盖比上月提升 12% | ⚪ 观察 |

## Critical 发现详情

### #1 — Correctness: XSS 风险
- **位置**: `src/controllers/user.ts:L56`
- **描述**: `userInput` 直接拼接进 HTML 模板，未经 sanitize
- **修复建议**: 使用 DOMPurify 或模板引擎自动转义
- **修复状态**: [待修复 / 已修复 / 已验证]

## 审查结论
- **通过**: 所有 Critical 已修复并验证，Important 已处理
- **阻止**: Critical 发现未修复，需修复后重新审查

## 红旗 — STOP

<HARD-GATE>
以下任何一个出现，立即停止：

- PR > 1000 行未被拆分
- 评审者只看了 diff，没拉下代码跑测试
- Critical 发现被标记为 "以后修复" 而通过
- 审查意见全是 "LGTM" 或 "Nice!"（未真正审查）
- 安全相关的变更未被专项审查
- 测试缺失但审查放行
</HARD-GATE>

## 验证失败处理

| 失败场景 | 处理方式 |
|---------|---------|
| Critical 发现未修复 | 阻止合并。要求修复后重新提交审查。不得降级为 Important 或 FYI。 |
| 审查意见全是 LGTM/Nice | 审查无效。要求审查者重新按五轴逐项检查并给出具体发现。 |
| PR > 1000 行且未拆分 | 拒绝审查。要求按层/功能/基础设施拆分为多个 PR。 |
| 测试缺失但审查放行 | 回退审查。要求补充测试覆盖后再重新审查。测试缺失 = Important 以上。 |
| 安全发现被标记"以后修复" | 不可接受。安全 Critical 必须"现在修复"。无法立即修复时提供缓解方案并创建 follow-up issue（带负责人和截止日期）。 |

## 验证清单

- [ ] 五轴全部覆盖（Correctness > Readability > Architecture > Security > Performance）
- [ ] 发现按严重性分类（Critical / Important / Suggestion / FYI）
- [ ] Critical 发现全部解决
- [ ] 变更大小合理（< 300 行理想；> 1000 行被拆分）
- [ ] 测试覆盖了变更
- [ ] 审查报告可追溯（谁审的、审了什么、什么发现）