# Pre Commit Review

> Summarizes modified files in detail and reviews code to ensure changes do not affect other functionality. Use when the user asks for modification summary, code review, pre-commit review, or to ensure changes pass functional testing without introducing new issues. Trigger terms include: 审查代码、审查修改、review代码、代码审查、代码复审、测试代码、修改总结、提交前审查、 不影响其他功能、审查结果不通过、回归测试、功能测试、修改review、审查变更. If the intent is ambiguous (e.g. 检查一下、看一下代码), ask the user to confirm: "是否需要按提交前审查流程，做修改总结 + 场景矩阵 + 代码审查？" If review fails, state the reason clearly.

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

---


# 提交前修改总结与代码审查

本技能用于在**提交前**对本次修改做详细总结与代码审查，确保功能测试通过、不引入新问题、不影响需求外的功能，并且开发者对改动有充分理解。

## 审查的本质：Code Review 两问

代码审查的价值不在「有人看了代码」或「有人读懂了代码」，而在于确认本次 diff 经得起两个核心问题的检验：

**一问「做对了吗？」** —— 变更是否满足真实需求、覆盖异常与回归风险、符合前置的技术设计。
**二问「放对了吗？」** —— 是否遵守架构边界、避免错误依赖与重复实现、控制长期维护成本。

**CR 的本质**：确保本次修改符合各方面的质量要求，让人安心发布。CR 是**手段**而非**目的**——最终目标是「对发布有信心」，而不是「完成了审查流程」。

> **实验佐证**：曾试验「不 Review 代码，只 Review 需求 + 自动化测试计划与结果」。在测试覆盖足够强的情况下，大量内部功能调整从提交到上生产全自动化，长期运行未出线上事故。结论：**测试足够强时 CR 可显著精简；测试不足时 CR 是质量兜底**——因此 §3.4 测试质量的权重不低。

## 触发场景

用户表达以下**任一含义**时应用本技能（含但不限于）：

- **审查类**：审查代码、审查修改、review代码、代码审查、代码复审、修改审查、审查变更、修改review
- **总结类**：详细总结修改、修改总结、变更总结
- **提交前**：提交前审查、提交前检查、commit 前审查、提交前review
- **质量/测试类**：测试代码、回归测试、功能测试、确保不影响其他功能、不引入新问题、审查结果不通过则告知原因

**若有疑问**：若用户意图不明确（例如仅说「检查一下」「看一下代码」「帮我看下」），先询问确认：「是否需要按提交前审查流程，做修改总结 + 场景矩阵 + 代码审查？」确认后再执行。

## 严重等级标签

在审查意见中使用以下标签区分优先级：

| 标签 | 含义 | 处理要求 |
|------|------|----------|
| 🔴 **[阻塞]** | 必须修复才能提交（安全漏洞、数据丢失、功能中断） | 修复后重新审查 |
| 🟡 **[重要]** | 应当修复，有分歧时讨论 | 原则上修复，或说明原因 |
| 🟢 **[建议]** | 可选优化，不阻塞提交 | 建议项，开发者在后续迭代中酌情处理 |
| 💡 **[思路]** | 备选方案，供讨论 | 无需操作 |
| 📚 **[学习]** | 知识分享，无需操作 | 参考了解 |
| 🎉 **[亮点]** | 值得肯定的做法 | 保持 |

## 执行步骤

### 第一步：收集修改范围（上下文）

- 使用 `git status -s`、`git diff --cached --stat`（或 `git diff --stat`）确定本次修改的文件与行数。
- 若文件超过 10 个或改动超过 400 行，建议按模块分批审查。
- 若用户未指定范围，以当前暂存区或工作区变更为准。
- **加载此前 Learnings（经验沉淀）**：读取 `docs/review/learnings.jsonl`，将之前审查中记录的陷阱/模式载入上下文，避免本次遗漏同类问题。若文件不存在，跳过。
- **CI/测试状态前置检查**：若项目有 CI 或可运行的测试，先确认测试是否通过。测试未通过时先提示开发者修复，再进入审查——在红测基础上做代码审查，审查结论会被污染。

### 第 1.5 步：审查负载告警（轻率审查预警）

**定位**：在收集完修改范围后自动评估审查负载，防止花 30 秒给高复杂度改动点 Approve。

基于第一步收集的数据，AI 自动计算以下指标并输出到审查报告顶部：

| 指标 | 阈值 | 告警 |
|------|------|------|
| 改动行数 | 100-400 行中等负载，> 400 行高负载 | 🔔 高负载建议分批审查或增加审查时间 |
| 涉及安全/支付/权限模块改动行数 | > 50 行 | 🔔 此类改动不应凭直觉通过，建议逐行审查 |
| 跨模块数（**跨模块规则**） | ≥ 3 模块或服务 | 🔔 跨模块改动容易隐含接口不匹配问题，建议重点验证模块间数据传递的一致性 |

> 告警仅是提醒，不阻塞审查。但如果多个告警同时触发，建议 reviewer 仔细检查是否有遗漏后再下结论。
>
> **跨模块规则**：改动涉及 ≥ 3 个模块或服务即命中。本节、§2.6、§4.3 共用此阈值，改阈值只需改这里。

### 第二步：详细总结修改

对**每个修改文件**写出：

- **文件路径**：相对项目根的路径。
- **修改类型**：新增 / 修改 / 删除。
- **修改要点**：用列表说明改了什么（逻辑、条件、顺序、依赖等），可引用关键代码或行号。
- **修改目的**：与需求或问题的对应关系（修复什么、优化什么、限制在什么场景）。

格式示例：

```markdown
### 文件路径
- **类型**：修改
- **要点**：① …；② …
- **目的**：…
```

### 第 2.5 步：Scope Drift（范围漂移）检测

在审查代码质量之前，先判断本次改动是否在应有范围内。

1. 读取 `TODOS.md`（如果存在）、commit messages（`git log --oneline`）、以及用户提供的需求/问题描述
2. 确定本次改动的**原始意图**（要解决什么问题、实现什么功能）
3. 对比实际改动文件列表与原始意图

**输出格式（在审查报告中靠前展示）：**

```
Scope Check（范围检查）: [CLEAN（正常） / DRIFT DETECTED（范围漂移） / MISSING REQUIREMENTS（需求缺失）]
Intent（意图）: <1 行总结改动的原始意图>
Delivered（实际改动）: <1 行总结 diff 实际在做什么>
[IF drift（范围漂移）：逐条列出超出范围的改动]
[IF missing（需求缺失）：逐条列出未实现的需求]
```

> 此检查为**参考信息**——不阻塞后续审查，但帮助审查者和开发者意识到范围漂移。

### 第 2.6 步：Diff Scope（改动范围）自动检测

在进入代码审查前，自动分析改动涉及的领域，决定执行哪些专项清单。

```bash
# 检测改动类型（由 AI 基于 diff 判断）
# 输出：SCOPE_AUTH=true/false  SCOPE_FRONTEND=true/false  SCOPE_BACKEND=true/false
#       SCOPE_CONFIG=true/false  SCOPE_API=true/false  SCOPE_MIGRATIONS=true/false
```

**检测结果驱动专项清单：**
- `SCOPE_AUTH=true` → 必跑 §3.2 安全专项清单
- `SCOPE_FRONTEND=true` → 必跑 §3.5 技术栈前端、视情况跑 §3.6 复杂度
- `SCOPE_BACKEND=true` → 必跑 §3.3 性能专项、§3.5 技术栈后端
- `SCOPE_MIGRATIONS=true` → 必跑 §3.3 性能专项、§3.8 文档同步
- `SCOPE_API=true` → 必跑 §3.8 文档同步
- `SCOPE_CONFIG=true` → 必跑 §3.8 文档同步
- 命中**跨模块规则**（≥ 3 模块或服务） → 必跑 §3.10 架构与归属审查

在审查报告中展示检测结果：`Diff Scope（改动范围）: auth/frontend/backend/config/API/migration`

### 第 2.7 步：审查前置分析报告（统一产出）

第 1.5、2.5、2.6 步的产出及第 2.7 步架构定性检查，应**合并到一份统一的「审查前置分析报告」**中，置于审查报告顶部，分节呈现。避免将每个步骤作为独立输出造成信息碎片化：

```markdown
## 审查前置分析报告

### 🔔 负载告警
- 改动行数：XXX（→ 建议分批审查 / 正常）
- 安全敏感模块改动：XX 行（→ 建议逐行审查）
- 跨模块数：X（→ 建议关注接口一致性）
...

### 🔍 Scope Check（范围检查）
Scope（范围）: [CLEAN（正常） / DRIFT DETECTED（范围漂移） / MISSING REQUIREMENTS（需求缺失）]
Intent（意图）: ...
Delivered（实际改动）: ...

### 📐 Diff Scope（改动范围）
auth/frontend/backend/config/API/migration

### 🏛️ 架构定性（先判方向，再逐行）
设计判断: [方案匹配问题 / 存在方向性风险（先修正方向再逐行）]
- 改动落在职责所属模块？[是 / 否→优先处理]
- 依赖方向符合分层（下层不依赖上层、无循环）？[是 / 否→优先处理]
→ 任一为否：先与开发者确认架构方向，再进入 §3.2-§3.9 逐行审查，避免在错误架构上精挑细选
```

> 前置分析报告中的信息可以引导审查者关注重点，减少 reviewer 在多个步骤产出间跳转的信息断层。
>
> **架构定性**是「先抓大问题」的开关：逐行专项（§3.2-§3.9）前的快速方向判断，不替代 §3.10 的完整架构复核——若此处发现方向性风险，§3.10 仍须执行。

### 第三步：代码审查

**逐行审查原则**：必须逐行审查分配的每一行代码，不得跳过或假定正确。若某段代码无法理解，应要求开发者澄清——你读不懂的地方，其他人也读不懂。

#### 3.1 影响面分析

对每个修改点做**影响面分析**：

| 审查项 | 说明 |
|--------|------|
| **需求内行为** | 本次修改在「目标场景」下是否按预期工作（条件、分支、顺序、依赖是否正确）。 |
| **前置设计符合性** | 实现是否遵循既有技术设计/方案（而非绕过设计另起一套）；设计未覆盖处是否有合理决策记录。 |
| **需求外行为** | 未改动的场景、平台、入口、用户类型是否仍按原逻辑执行（是否有误判、误跳过、误拦截）。 |
| **边界与平台** | 若修改与「平台/环境/登录方式」相关，是否用明确条件（如 isWechat、route.query.xxx）限制，避免其他平台被影响。 |
| **依赖与顺序** | 是否依赖未完成的数据（如 userInfo 未拉取）、执行顺序是否会导致竞态或误报。 |
| **未修改的关联代码** | 路由守卫、请求拦截、401 处理、全局配置等是否未改且与本次逻辑兼容。 |

对**关键分支与条件**做场景矩阵（推荐）：

- 列出：场景（平台/登录态/URL/配置） → 预期行为 → 修改后是否一致 → **对抗性检查（极端输入/异常情况）**[✅已查 / ⬜未查]。
- 重点覆盖：非目标平台、未登录、其他登录方式、异常/401、配置未加载等。
- 对抗性检查示例：对每个场景追问"如果输入是空值/超长值/并发/权限不足/上游超时，这段代码会怎样？"将结果填入对抗性列，并在完成后标记 ✅已查 以确认该场景已做过对抗性评估。

#### 3.2 安全专项清单

> 涉及认证、输入处理、数据存储时必查；纯 UI/样式改动可跳过。
>
> **安全定级规则**：安全相关发现最低 🟡；疑似可利用漏洞、可导致数据泄露或越权者，默认 🔴。

- [ ] 用户输入是否经过校验和过滤（XSS、SQL 注入防护）
- [ ] 认证与鉴权：每个敏感操作前是否有权限校验
- [ ] 接口密钥/Token/密码是否未硬编码在代码中
- [ ] 错误信息是否避免泄露内部细节（堆栈、数据库结构等）
- [ ] CSRF 防护：状态变更操作是否有 CSRF 保护
- [ ] SSRF：后端发起的对外请求，目标 URL 是否校验协议/主机白名单
- [ ] IDOR：资源访问是否校验归属（owner/租户），而非仅校验「已登录」
- [ ] 命令注入：拼接进 shell/子进程的输入是否经转义或参数化
- [ ] 依赖审计：是否运行 `npm audit` / `pnpm audit`；新增依赖是否核验过版本与安全

#### 3.3 性能专项清单

> 涉及循环、数据库查询、API 调用时必查；简单逻辑修改可跳过。

- [ ] 是否存在 N+1 查询问题（循环内调用数据库/接口）
- [ ] 循环是否可以提前终止或用 Map/Set 优化
- [ ] 大列表/响应是否有分页或截断
- [ ] 高频调用的结果是否可以缓存
- [ ] 是否在热路径中存在阻塞性同步操作

#### 3.4 测试质量清单

> 涉及核心业务逻辑变更时必查；文档/配置改动可跳过。

- [ ] 是否测试行为而非实现细节（白盒测试 vs 黑盒测试）
- [ ] 是否覆盖边界情况（空值、空列表、最大值、并发）
- [ ] 是否覆盖失败路径（API 超时、权限拒绝、数据异常）
- [ ] 测试名称是否清晰描述「场景 + 预期结果」
- [ ] 测试是否独立（无共享状态，可任意顺序运行）

#### 3.5 项目技术栈专项（TypeScript / Vue3 / NestJS）

> 项目涉及对应技术栈时核查。

**TypeScript**
- [ ] 避免使用 `any`；若必须使用，是否添加注释说明原因
- [ ] async 函数是否有错误处理（try/catch 或 `.catch()`），避免 UnhandledPromiseRejection
- [ ] 接口/类型是否准确描述数据结构，未使用宽泛的 `object`

**Vue3 / 前端**
- [ ] 是否避免直接修改 props（应通过 emit 通知父组件）
- [ ] 响应式数据依赖是否正确（computed/watch 的依赖项是否完整）
- [ ] `v-for` 是否有唯一且稳定的 `:key`

**NestJS / 后端**
- [ ] 服务注入是否在模块中正确注册（Module providers/imports）
- [ ] 异步操作是否正确使用 `async/await`，避免忘记 `await`
- [ ] 错误是否通过 NestJS 异常过滤器或 `HttpException` 统一抛出

#### 3.6 复杂度审查

> 涉及新增函数/类/模块、条件分支较多时必查；纯配置/数据改动可跳过。

- [ ] 是否存在过度设计（泛化/抽象/配置化超过了当前需求，在解决还不存在的问题）
- [ ] 函数或方法是否过大、职责单一？能否拆分为更小的单元
- [ ] 是否存在嵌套过深（条件嵌套 > 3 层）或可简化的逻辑分支
- [ ] 是否有死代码（未使用的变量/函数/参数/import）
- [ ] 代码是否对后续读者足够自解释——读一遍能理解核心逻辑，无需反复翻看上下文

#### 3.7 命名与注释审查

- [ ] 变量/函数/类/文件名是否准确传达其含义（名称够长到充分表达，不短到需要猜）
- [ ] 注释是否解释 **why**（为什么存在）而非 **what**（代码在做什么）——如果代码不自解释，应优先简化代码而非加注释
- [ ] 是否有已过时的 TODO/FIXME/HACK 残留（已解决的应移除）
- [ ] 若有 reviewer 不理解某段代码，应要求开发者澄清该段代码本身（而非仅在评论串中解释），确保未来读者不再困惑

#### 3.8 文档同步检查

> 涉及对外接口、构建流程、部署方式、使用方式变更时必查；纯内部逻辑改动可跳过。

- [ ] README、API 文档、使用说明是否随代码同步更新
- [ ] 如果删除了代码或接口，是否已删除或标记过期对应的文档
- [ ] 新增的配置项、环境变量、依赖是否有说明或文档记录

#### 3.9 枚举与状态完整性检查（Cross-Reference）

> 涉及新增枚举值、状态、类型常量时必查；纯内部逻辑改动可跳过。

- [ ] 新增的枚举值/状态/类型常量是否在对应的 switch/case、if-else 分支或映射表中被处理
- [ ] 使用 `grep` 或等价方式搜索兄弟值的引用处，检查是否有遗漏处理
- [ ] 数据库中已有的旧数据是否需要迁移或兼容处理
- [ ] API 契约是否同步更新（请求/响应中的可选值列表）

#### 3.10 架构与归属审查（放对了吗）

> 涉及新增模块 / 跨模块调用 / 依赖调整时必查；纯单文件内部逻辑可跳过。

- [ ] **架构边界**：是否遵守分层/模块边界（UI 层不直连存储、跨模块不越权访问内部实现、业务层不散落基建逻辑）
- [ ] **依赖方向**：依赖是否单向、无循环（下层不依赖上层）；新增依赖是否必要、是否引入维护负担
- [ ] **重复实现**：是否存在复制粘贴式重复，或与既有实现并存的「双写」逻辑（应复用既有工具函数/组件/服务）
- [ ] **归属正确**：改动是否落在职责所属的模块，而非塞进「离得近但不对」的地方
- [ ] **长期维护成本**：改动是否让后续修 bug / 加功能更困难；是否引入需要持续同步维护的两处逻辑
- [ ] **接口约定**：模块间入参/出参/错误码是否一致——不同模块对同一个错误场景的处理策略是否相同
- [ ] **数据流向**：数据流是否符合系统架构设计——是否有绕过了原定路由/中间件/API 网关的数据通道
- [ ] **架构假设**：是否引入了「只有一个人知道但团队其他人不知道」的架构假设——如有，需在文档中记录

### 第四步：审查结论

**纯审查，不改代码**：本技能仅提供审查结论、问题定位与修改建议，**绝不直接修改代码**。所有代码修改应由开发者本人完成。

审查结论是本次审查的**最终决策点**，分以下步骤执行：

#### 4.1 输出审查结论

1. **🔴 阻塞**：标记问题 + 给出可应用的修改方案。修复前使用「建设性反馈原则」引导讨论。
2. **🟡 重要**：标记问题 + 给出修改方案。
3. **🟢 建议**：标记优化方向，供开发者后续迭代中酌情处理。
4. **💡 思路**：标记备选方案，供讨论。

使用以下结构化格式输出结论：

```markdown
## 审查结论

### 亮点
🎉 [描述值得肯定的设计或实现]

### 必须修复（阻塞提交）
🔴 [阻塞] 文件路径:行号 — 问题描述
  → 原因：为什么这是问题
  → 建议：具体的修改方案

### 应当修复
🟡 [重要] 文件路径:行号 — 问题描述
  → 建议：…

### 优化建议（不阻塞）
🟢 [建议] …
💡 [思路] …

### 问题统计
🔴 阻塞: N | 🟡 重要: N | 🟢 建议: N | 💡 思路: N | 🎉 亮点: N
> 同一分支多次审查时，统计随 review 文档增量更新（§5），形成「问题数递减曲线」，作为该需求可量化的质量证据。

### 审查反思（收尾两问）
🔍 **眼下最没把握的**：<本次审查中置信度最低、最需要开发者额外验证的 1-2 处；没有则写「无」>
👁 **可能遗漏的**：<本次审查未覆盖但可能影响结论的点（未查的调用方/引用、测试盲区）；没有则写「无」>

> 结论输出前 MUST 完成此两问并如实作答。即使结论全绿，也须标注置信度边界——它不否定结论，而是把 review 的注意力引向最该验证的地方。问题一源自 Sam Altman，问题二源自 Claude。

### 审查结论
❌ 不通过 — 需修复上述 🔴 问题后重新审查
或
✅ 通过 — 进入 §4.2 触发条件判定
```

#### 4.2 审查结论判定

**不通过**：存在 🔴 阻塞问题。必须逐条列出原因，不允许只写「审查不通过」。

**通过**：无 🔴 阻塞问题。通过前先自查：若改动 > 100 行但结论无任何 🔴/🟡 问题，应反思是否漏审——百行以上改动无严重问题极为罕见。同时完成 §4.1「审查反思（收尾两问）」：如实标注最没把握与可能遗漏之处。确认无漏审、反思两问已作答后，**必须检查是否满足 Self-Test 触发条件**（见下方），根据检查结果决定下一步：

| 检查结果 | 下一步 |
|----------|--------|
| Self-Test 不触发 | 直接进入合并流程 |
| Self-Test **SHOULD** 执行 | 执行 §4.4 Self-Test 后进入合并 |
| Self-Test **MUST** 执行 | 必须执行 §4.4，通过后进入合并 |

#### 4.3 Self-Test 触发条件检查

##### 适用场景

| 场景 | 建议 |
|------|------|
| 大面积重构或重写 | **SHOULD** 执行 |
| 涉及关键路径、支付、安全等高风险改动 | **SHOULD** 执行 |
| 改动行数 > 200 行 | **SHOULD** 执行 |
| 命中**跨模块规则**（≥ 3 模块或服务） | **SHOULD** 执行 |
| 涉及数据库迁移或配置变更 | **SHOULD** 执行 |
| AI 自主完成大部分实现且开发者未逐行 review | **SHOULD** 执行 |
| 开发者对改动中复杂逻辑缺乏充分理解 | **SHOULD** 执行 |
| 仅修了一个边界值或单行 bug | 可跳过 |
| 用户明确要求「出测试题」或「你自己做题」 | **MUST** 执行 |

> Self-Test 触发条件以「适用场景」和「用户要求」两者的**较高等级**为准，后者始终优先。

#### 4.4 执行 Self-Test（开发者理解度校验）

**定位**：审查已通过且触发了 Self-Test 条件时，执行开发者理解度校验。不是测试代码，而是测试**开发者是否真正理解改动内容**。

> 多数上线事故不是因为代码有 bug，而是改代码的人不理解为什么那些旧逻辑存在。

#### 执行流程

1. AI 根据本次改动生成一份《改动理解测试题》，包含：
   - **改动的背景与目的**（1 问）—— 确保开发者知道为什么改
   - **核心实现逻辑**（2-3 问）—— 验证是否理解关键变更
   - **影响范围与潜在风险**（1-2 问）—— 验证是否知道可能出问题的地方
   - **回退方案**（1 问）—— 验证是否知道出问题了怎么恢复

2. 开发者逐题作答，AI 逐题检查是否正确。答对有误时 AI 使用引导式提问帮助开发者理解（复用「建设性反馈原则」），而非直接给答案。

3. 全部答对 → Self-Test 通过，进入合并流程。
   答错 → 记录知识盲区，建议开发者返回对应 Phase 重新理解，修复后再测。

#### 通过条件

- 开发者正确回答了改动的背景、逻辑、影响范围和回退方案
- 答错的知识盲区已记录（MAY 记入 `memory/issues.md` 或审查文档中）
- Self-Test 结论不替代第三步的代码审查——Self-Test 通过不代表代码无问题

### 第五步：输出审查文档

- **必须**将本次「修改总结 + 场景矩阵 + 审查结论」写入项目 **`docs/review/`** 下的审查文档。
- **一个分支只需要一个 review 文档**：若当前分支已有对应审查文档（如 `<需求标识>-review.md`），在**原有文档基础上更新**本次变更的修改总结、场景矩阵与审查结论，不新建重复文档；若无，则新建 `<需求或需求标识>-review.md`。
- 文件名建议：`<需求或需求标识>-review.md`（如 `AI-748-review.md`），同一分支后续审查只更新该文件，不新增 `*-re-review.md`、`*-code-review.md` 等分散文档。
- 文档内容至少包含：修改文件列表与要点、场景矩阵、审查结论（通过/不通过及原因）、专项清单检查结果、**Self-Test 题目与结论（如执行过）**。
- 若项目根目录下无 `docs/review/`，先创建该目录再写入；若已有 `docs/review/README.md`，在 README 中维护审查文档索引（每个需求/分支对应一条）。

### 第六步：持久化 Learnings（经验沉淀，可选）

如果本次审查发现了一个**非显而易见的模式/陷阱/反模式**，记录到 `docs/review/learnings.jsonl`，下次审查自动加载。

```bash
echo '{"ts":"'$(date -u +%Y-%m-%dT%H:%M:%SZ)'","type":"TYPE","key":"SHORT-KEY","insight":"DESCRIPTION","confidence":N,"files":["path/to/file"]}' >> docs/review/learnings.jsonl
```

- **type**：`pattern`（可复用做法）、`pitfall`（不要做的事）、`architecture`（结构决策）
- **confidence**：1-10。实际观察到的 8-9，推理的 4-5
- 如果本次审查未发现值得记录的知识，跳过此步

**记录前先做质量判定（先排除，再记录）**。「值得记录」的唯一标准：**下次审查召回后，能改变 Agent 的审查行为**——更早发现问题、避免漏检同类陷阱。达不到这条标准的，不记录。判定分三步：

**① 排除垃圾特征**（命中任一条即不记录）：

| 垃圾特征 | 说明 | 审查场景示例 |
|---------|------|-------------|
| 事实性错误 | 编造不存在的约束/API/方法/配置项 | 「X 方法应返回 Y」但该方法实际不存在 |
| 通用常识 | 任何项目都适用的常规规则 | 「记得加错误处理」 |
| 对话摘要 | 只复述本次审查过程，未沉淀可复用判断 | 「本次审查了 auth 模块」 |
| 一次性 case | 只对当前 diff 有效，不可迁移 | 「此文件第 N 行不该这样写」 |
| 主观偏好 | 个人选择，不代表团队规范 | 「我更喜欢这种写法」 |
| 缺少上下文 | 未关联模块/组件/接口/文件，无法定位复用 | 一条孤立的「这里要异步」 |
| 粒度混用 | 一条经验里通用规则与 case 特定信息混杂 | 「hook 要异步（且仅 stop hook 场景）」 |
| 不可执行 | 只有抽象建议，下次审查不知怎么用 | 「要谨慎处理并发」 |
| 证据不足 | 本次 diff 中依据不足，推理成分过多 | 「感觉这会影响其他功能」 |

**② 源码事实校验（涉及具体 API/类/方法时必做）**：经验中提到的类名、方法名、配置项、路径，先用 `rg`/`grep` 在代码库中检索其存在性，再记录。**纯文本自洽不等于真实存在**——曾有经验声称某类应调用 `attachToQBListView()`，源码检索发现该类根本没有此方法，正确方法是另一个类的方法。**记录一个不存在的方法，比不记录危害更大**：下次审查会按错误指引检查，甚至误导 Agent 怀疑正确代码有误。

**③ 与历史 learnings 去重/合并**：记录前读取 `docs/review/learnings.jsonl`，按 `key`/`insight` 判断与既有经验的关系：
- 完全重复 → 跳过，不重复写入
- 同向且有新信息（更具体的场景/触发条件）→ 合并进原有条目，不新增
- 结论冲突 → 不覆盖旧经验，标注冲突后保留人工裁决（宁严勿宽：误合并造成的边界污染不可逆）

**默认策略：宁漏勿错**——把握不足的不记录。误记一条泛泛而谈的经验，比漏记一条具体经验对下次审查的干扰更大。

## 审查不通过时的处理

1. **明确告知用户**：审查不通过，并列出原因（逐条，带文件路径与行号）。
2. **给出修改建议**：使用「情境 + 具体问题 + 建议方案」格式，避免空洞的「请修改」。
3. **不执行提交**：在用户根据建议修改并再次审查通过前，不进行 git commit；若用户坚持提交，提醒风险并记录原因。

### 建设性反馈原则

使用引导式提问代替直接否定，促进思考而非施压：

```
❌ 「这里有 bug，必须改。」
✅ 「如果 items 是空数组，这里会发生什么？」

❌ 「你需要加错误处理。」
✅ 「如果这个 API 调用失败，应该如何处理？」

❌ 「这样写性能很差。」
✅ 「用户量到 10 万时，这里每次都循环查询会有什么影响？」
```

## 更好的做法（推荐）

1. **先定范围再改**：修改前明确「只动哪些文件、只影响哪些场景」，审查时重点核对是否越界。
2. **平台/入口显式限制**：凡「仅某平台或某入口」才生效的逻辑，用显式条件（如 isWechat、route.query.xxx、配置开关）包裹，避免其他平台误走。
3. **场景矩阵**：对登录、平台、URL、配置等做一张「场景 → 预期 → 结论」表，避免漏掉边界。
4. **关联代码看一眼**：修改了 layout/请求/路由相关逻辑时，顺带确认 permission、request 拦截、401 处理未改且兼容。
5. **审查文档沉淀**：每个分支只维护一个 review 文档，后续审查在原有文档上更新变更部分，避免同一需求多份重复文档。
6. **Self-Test 双保险**：代码审查通过不等于开发者完全理解改动。大面积重构、关键路径或 AI 生成的代码，建议在审查通过后追加开发者理解度校验，避免因不理解旧逻辑而埋下后续隐患。

## 输出约定

- 总结与审查结论以**中文**呈现（除非项目约定英文）。
- 审查不通过时，**🔴 阻塞问题**单独成段，便于用户一眼看到。
- 不通过时必须包含**具体原因 + 修改建议**，使用「情境 + 具体问题 + 建议方案」格式。
- 如有值得肯定的实现，用 🎉 [亮点] 说明，保持建设性氛围。

## 小结

- **本质**：CR 确认「做对了吗 + 放对了吗」，目标是让人安心发布——它是**手段，不是目的**。
- **目的**：确保每次修改在提交前功能测试通过，不引入新问题，不影响需求外的功能，且开发者对改动有充分理解。
- **通过**：无 🔴 阻塞问题，写明符合预期且不影响其他功能，并点出关键保障。
- **不通过**：明确原因（文件/行号/场景）+ 修改建议，不执行提交直至用户修复并再次审查通过（或用户明确坚持提交时提醒风险）。
- **审查顺序**：先架构定性（§2.7）再逐行专项（§3.2-§3.9），避免在错误架构上精挑细选；安全发现按 §3.2 定级规则处理。

---

**参考**：[Claude Code 核心工程师爆火攻略：先扫清你的「AI 盲区」](https://mp.weixin.qq.com/s/uhZCoI-hPS2luilGbv7_DQ)