| name | pre-commit-review |
| description | 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.
|
| metadata | {"author":"liuhean","email":"allsmy.com@gmail.com"} |
提交前修改总结与代码审查
本技能用于在提交前对本次修改做详细总结与代码审查,确保功能测试通过、不引入新问题、不影响需求外的功能,并且开发者对改动有充分理解。
审查的本质: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 共用此阈值,改阈值只需改这里。
第二步:详细总结修改
对每个修改文件写出:
- 文件路径:相对项目根的路径。
- 修改类型:新增 / 修改 / 删除。
- 修改要点:用列表说明改了什么(逻辑、条件、顺序、依赖等),可引用关键代码或行号。
- 修改目的:与需求或问题的对应关系(修复什么、优化什么、限制在什么场景)。
格式示例:
### 文件路径
- **类型**:修改
- **要点**:① …;② …
- **目的**:…
第 2.5 步:Scope Drift(范围漂移)检测
在审查代码质量之前,先判断本次改动是否在应有范围内。
- 读取
TODOS.md(如果存在)、commit messages(git log --oneline)、以及用户提供的需求/问题描述
- 确定本次改动的原始意图(要解决什么问题、实现什么功能)
- 对比实际改动文件列表与原始意图
输出格式(在审查报告中靠前展示):
Scope Check(范围检查): [CLEAN(正常) / DRIFT DETECTED(范围漂移) / MISSING REQUIREMENTS(需求缺失)]
Intent(意图): <1 行总结改动的原始意图>
Delivered(实际改动): <1 行总结 diff 实际在做什么>
[IF drift(范围漂移):逐条列出超出范围的改动]
[IF missing(需求缺失):逐条列出未实现的需求]
此检查为参考信息——不阻塞后续审查,但帮助审查者和开发者意识到范围漂移。
第 2.6 步:Diff Scope(改动范围)自动检测
在进入代码审查前,自动分析改动涉及的领域,决定执行哪些专项清单。
检测结果驱动专项清单:
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 步架构定性检查,应**合并到一份统一的「审查前置分析报告」**中,置于审查报告顶部,分节呈现。避免将每个步骤作为独立输出造成信息碎片化:
## 审查前置分析报告
### 🔔 负载告警
- 改动行数: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/样式改动可跳过。
安全定级规则:安全相关发现最低 🟡;疑似可利用漏洞、可导致数据泄露或越权者,默认 🔴。
3.3 性能专项清单
涉及循环、数据库查询、API 调用时必查;简单逻辑修改可跳过。
3.4 测试质量清单
涉及核心业务逻辑变更时必查;文档/配置改动可跳过。
3.5 项目技术栈专项(TypeScript / Vue3 / NestJS)
项目涉及对应技术栈时核查。
TypeScript
Vue3 / 前端
NestJS / 后端
3.6 复杂度审查
涉及新增函数/类/模块、条件分支较多时必查;纯配置/数据改动可跳过。
3.7 命名与注释审查
3.8 文档同步检查
涉及对外接口、构建流程、部署方式、使用方式变更时必查;纯内部逻辑改动可跳过。
3.9 枚举与状态完整性检查(Cross-Reference)
涉及新增枚举值、状态、类型常量时必查;纯内部逻辑改动可跳过。
3.10 架构与归属审查(放对了吗)
涉及新增模块 / 跨模块调用 / 依赖调整时必查;纯单文件内部逻辑可跳过。
第四步:审查结论
纯审查,不改代码:本技能仅提供审查结论、问题定位与修改建议,绝不直接修改代码。所有代码修改应由开发者本人完成。
审查结论是本次审查的最终决策点,分以下步骤执行:
4.1 输出审查结论
- 🔴 阻塞:标记问题 + 给出可应用的修改方案。修复前使用「建设性反馈原则」引导讨论。
- 🟡 重要:标记问题 + 给出修改方案。
- 🟢 建议:标记优化方向,供开发者后续迭代中酌情处理。
- 💡 思路:标记备选方案,供讨论。
使用以下结构化格式输出结论:
## 审查结论
### 亮点
🎉 [描述值得肯定的设计或实现]
### 必须修复(阻塞提交)
🔴 [阻塞] 文件路径:行号 — 问题描述
→ 原因:为什么这是问题
→ 建议:具体的修改方案
### 应当修复
🟡 [重要] 文件路径:行号 — 问题描述
→ 建议:…
### 优化建议(不阻塞)
🟢 [建议] …
💡 [思路] …
### 问题统计
🔴 阻塞: 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,而是改代码的人不理解为什么那些旧逻辑存在。
执行流程
-
AI 根据本次改动生成一份《改动理解测试题》,包含:
- 改动的背景与目的(1 问)—— 确保开发者知道为什么改
- 核心实现逻辑(2-3 问)—— 验证是否理解关键变更
- 影响范围与潜在风险(1-2 问)—— 验证是否知道可能出问题的地方
- 回退方案(1 问)—— 验证是否知道出问题了怎么恢复
-
开发者逐题作答,AI 逐题检查是否正确。答对有误时 AI 使用引导式提问帮助开发者理解(复用「建设性反馈原则」),而非直接给答案。
-
全部答对 → 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,下次审查自动加载。
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 判断与既有经验的关系:
- 完全重复 → 跳过,不重复写入
- 同向且有新信息(更具体的场景/触发条件)→ 合并进原有条目,不新增
- 结论冲突 → 不覆盖旧经验,标注冲突后保留人工裁决(宁严勿宽:误合并造成的边界污染不可逆)
默认策略:宁漏勿错——把握不足的不记录。误记一条泛泛而谈的经验,比漏记一条具体经验对下次审查的干扰更大。
审查不通过时的处理
- 明确告知用户:审查不通过,并列出原因(逐条,带文件路径与行号)。
- 给出修改建议:使用「情境 + 具体问题 + 建议方案」格式,避免空洞的「请修改」。
- 不执行提交:在用户根据建议修改并再次审查通过前,不进行 git commit;若用户坚持提交,提醒风险并记录原因。
建设性反馈原则
使用引导式提问代替直接否定,促进思考而非施压:
❌ 「这里有 bug,必须改。」
✅ 「如果 items 是空数组,这里会发生什么?」
❌ 「你需要加错误处理。」
✅ 「如果这个 API 调用失败,应该如何处理?」
❌ 「这样写性能很差。」
✅ 「用户量到 10 万时,这里每次都循环查询会有什么影响?」
更好的做法(推荐)
- 先定范围再改:修改前明确「只动哪些文件、只影响哪些场景」,审查时重点核对是否越界。
- 平台/入口显式限制:凡「仅某平台或某入口」才生效的逻辑,用显式条件(如 isWechat、route.query.xxx、配置开关)包裹,避免其他平台误走。
- 场景矩阵:对登录、平台、URL、配置等做一张「场景 → 预期 → 结论」表,避免漏掉边界。
- 关联代码看一眼:修改了 layout/请求/路由相关逻辑时,顺带确认 permission、request 拦截、401 处理未改且兼容。
- 审查文档沉淀:每个分支只维护一个 review 文档,后续审查在原有文档上更新变更部分,避免同一需求多份重复文档。
- Self-Test 双保险:代码审查通过不等于开发者完全理解改动。大面积重构、关键路径或 AI 生成的代码,建议在审查通过后追加开发者理解度校验,避免因不理解旧逻辑而埋下后续隐患。
输出约定
- 总结与审查结论以中文呈现(除非项目约定英文)。
- 审查不通过时,🔴 阻塞问题单独成段,便于用户一眼看到。
- 不通过时必须包含具体原因 + 修改建议,使用「情境 + 具体问题 + 建议方案」格式。
- 如有值得肯定的实现,用 🎉 [亮点] 说明,保持建设性氛围。
小结
- 本质:CR 确认「做对了吗 + 放对了吗」,目标是让人安心发布——它是手段,不是目的。
- 目的:确保每次修改在提交前功能测试通过,不引入新问题,不影响需求外的功能,且开发者对改动有充分理解。
- 通过:无 🔴 阻塞问题,写明符合预期且不影响其他功能,并点出关键保障。
- 不通过:明确原因(文件/行号/场景)+ 修改建议,不执行提交直至用户修复并再次审查通过(或用户明确坚持提交时提醒风险)。
- 审查顺序:先架构定性(§2.7)再逐行专项(§3.2-§3.9),避免在错误架构上精挑细选;安全发现按 §3.2 定级规则处理。
参考:Claude Code 核心工程师爆火攻略:先扫清你的「AI 盲区」