再学 AI
第 51 课 / review
历史对照阅读 → 对照 → 问答 → 场景

历史对照:两条审查轴的旧入口

这是当前 code-review 的历史近似版本。两者都从固定基线出发,分别检查规范与规格。

历史文件,不在当前目录

本课使用下方注明的历史提交原文,帮助理解旧文章和旧提示词。请勿据此认定当前版本仍能直接调用该名称。

还不清楚 Skill、Agent、安装和调用?先读 从零开始的6节入门课

在关系图中查看 review 与其他技能的关联 →

先把必要的概念讲清楚

这是双轴评审的历史版本。规范与规格的基本判断保持相同,但本版本明确指定在同一条消息中调用两个 general-purpose 子 Agent,并把 PRD 列为规格来源。

下面是老师补充的入门说明;原作者的要求保留在中英对照正文中。所有例子均为帮助理解而构造的教学情境。

diff / merge-base|变更差异与共同起点

diff 展示两个版本之间哪些内容增删;merge-base 是两条分支共同的祖先。评审功能分支时从共同起点看差异,有助于聚焦这条分支引入的变化。比较基准错误,可能把别人早已做的修改也当成此次工作。

Git、commit、branch|版本与分支

Git 记录项目随时间的变化;commit 是一次有标识的修改记录;branch 是一条可继续发展的工作线。你可在功能分支试做收藏能力,验证后再合入主分支。保存了文件不等于已提交,提交了也不等于已推到服务器或已上线。

PR / pull request|合并评审请求

把一条分支的修改交给他人检查,并请求合入目标分支的协作对象。PR 通常包含改了什么、为何修改、验证结果和代码差异。草稿 PR 表示还在做;“可评审”表示可以检查;合并完成也不自动等于部署完成。

spec / specification|规格说明

把要解决的问题、预期行为、约束和验收依据写明确的文档。它比“做个好用的学习网站”具体:例如登录用户能收藏课程,刷新后仍保留,重复收藏不会多出记录。它不必规定每个内部函数怎么写,但应让实现者和验收者对同一结果达成一致。

refactoring|重构

在保持约定外部行为的前提下改善内部结构,例如合并重复逻辑、调整职责归属。用户仍能完成同样操作,但代码更容易理解和修改。重构不等于顺便加新需求;测试帮助证明外部行为未被意外改变。

module|模块

承担一组相关职责的代码单元,可以是文件、包或服务,不必等于一个文件。学习进度模块可以公开“记录进度”和“查询进度”,把计算完成比例、保存数据等细节藏在内部。划分是否合理,要看职责和依赖,不能只数文件。

读原文,理解每一步为什么这样做

左右内容按小节对应;窄屏先中文、后英文。两种语言均完整展示,对应讲解紧接在小节之后。译文传达原文要求;老师讲解补充概念、原因、例子与适用边界。

中文译文English · 英文原文
中文译文
name: review
description: "从固定点(提交、分支、标签或合并基点)审查变更,分为规范与规格两个维度:是否遵守仓库编码标准,是否符合原始 issue/PRD。两项评审由并行子 Agent 执行,分别报告。用户希望评审分支、PR、进行中的变更或“从 X 起评审”时使用。"
English · 英文原文
name: review
description: Review the changes since a fixed point (commit, branch, tag, or merge-base) along two axes — Standards (does the code follow this repo's documented coding standards?) and Spec (does the code match what the originating issue/PRD asked for?). Runs both reviews in parallel sub-agents and reports them side by side. Use when the user wants to review a branch, a PR, work-in-progress changes, or asks to "review since X".
中文译文

把用户指定的固定参照点与当前提交 HEAD 比较,从两个独立方向审查差异。

  • 规范 Standards:是否遵循仓库记录的编码规范。
  • 需求 Spec:是否忠实实现原始工单、PRD 或规格要求。

两项由并行子 Agent 分别执行,避免互相影响判断,再汇总结果。

应已提供工单系统说明。如果 docs/agents/issue-tracker.md 缺失,运行 /setup-matt-pocock-skills

English · 英文原文

Two-axis review of the diff between HEAD and a fixed point the user supplies:

  • Standards — does the code conform to this repo's documented coding standards?
  • Spec — does the code faithfully implement the originating issue / PRD / spec?

Both axes run as parallel sub-agents so they don't pollute each other's context, then this skill aggregates their findings.

The issue tracker should have been provided to you — run /setup-matt-pocock-skills if docs/agents/issue-tracker.md is missing.

中文译文

执行过程

English · 英文原文

Process

中文译文
1. 确定比较参照点

采用用户指定的提交 SHA、分支、标签、mainHEAD~5 等。用户未指定时,先询问。

统一使用 git diff <fixed-point>...HEAD。三个点表示与双方共同祖先提交比较。同时用 git log <fixed-point>..HEAD --oneline 记录提交列表。

启动子 Agent 之前,先用 git rev-parse <fixed-point> 确认引用有效,并确认差异非空。不要让两个执行者分别遇到同一个无效参照问题。

English · 英文原文
1. Pin the fixed point

Whatever the user said is the fixed point — a commit SHA, branch name, tag, main, HEAD~5, etc. If they didn't specify one, ask for it.

Capture the diff command once: git diff <fixed-point>...HEAD (three-dot, so the comparison is against the merge-base). Also note the list of commits via git log <fixed-point>..HEAD --oneline.

Before going further, confirm the fixed point resolves (git rev-parse <fixed-point>) and the diff is non-empty. A bad ref or empty diff should fail here — not inside two parallel sub-agents.

老师讲解 · 对应上方原文 · 含教学举例

评审先选对比较范围

固定点可以是主分支、某个提交或标签。三点 diff 从共同祖先比较,意在看当前分支自己引入的变化。若拿错基准,你可能把别人的修改当成这次提交,或漏掉自己的关键变化。

例如分支做了收藏功能,同时主分支增加了别的课程样式。评审应先确认当前比较究竟包含哪些提交。原文要求检查引用有效、差异非空,是让两个后续评审共用正确输入。

工作目录里还没提交的修改,不会天然全部出现在 HEAD 的提交差异中;用户要求评审未提交内容时,范围需要另行明确。不能把这一命令解释为总能覆盖电脑上的全部改动。

中文译文
2. 找到需求来源

按顺序寻找:

  1. 提交消息中的工单引用,如 #123Closes #45、GitLab !67,按工单系统文档获取。
  2. 用户提供的路径。
  3. docs/specs/.scratch/ 中与分支或功能相符的规格。
  4. 找不到就询问用户。用户明确没有规格时,需求审查跳过,并说明没有可用规格。
English · 英文原文
2. Identify the spec source

Look for the originating spec, in this order:

  1. Issue references in the commit messages (#123, Closes #45, GitLab !67, etc.) — fetch via the workflow in docs/agents/issue-tracker.md.
  2. A path the user passed as an argument.
  3. A PRD/spec file under docs/, specs/, or .scratch/ matching the branch name or feature.
  4. If nothing is found, ask the user where the spec is. If they say there isn't one, the Spec sub-agent will skip and report "no spec available".
老师讲解 · 对应上方原文 · 含教学举例

没有规格时,不能假装知道原始要求

优先从提交引用、用户给的路径和项目规格文件找到需求依据。规范来源则来自仓库明确记录的编码标准。二者都应可指出位置,避免评审者把个人偏好当成项目规则。

**例子:**规格要求收藏只能本人看见,代码恰好返回全站收藏列表。代码可能很整齐、类型检查也通过,但需求轴应判定实现错误。

真的没有规格时,可以做规范和一般问题检查,却不能声称“完全符合全部需求”。原文让规格轴报告不可用,就是保持证据边界。

中文译文
3. 找到规范来源

查找记录编码方式的文件,例如 CODING_STANDARDS.mdCONTRIBUTING.md

另外始终使用以下来自 Fowler《重构》第 3 章的代码坏味道参考清单。即使项目没有规范,也可以用它寻找值得检查的问题,但遵守两条规则:

  • 项目规范优先。 项目明确认可的做法,不再因参考清单而报告为坏味道。
  • 坏味道始终需要判断。 应标为“可能存在某问题”等启发性意见,不是硬性违规。已经由工具检查的内容跳过。

每项先说明现象,再说明可能的改善方向:

  • 名称不清楚 Mysterious Name:函数、变量或类型名字无法说明用途。重新命名;若找不到准确名称,可能是设计本身含糊。
  • 重复代码 Duplicated Code:不同改动位置出现相同逻辑结构。可以提取共同部分,让多处调用。
  • 过多使用别处的数据 Feature Envy:方法主要访问另一个对象的数据。考虑将行为移到相关数据所在位置。
  • 总是一起出现的数据 Data Clumps:同几个参数或字段反复成组传递。它们可能应该组成一个明确类型。
  • 用基础类型勉强表示业务概念 Primitive Obsession:值得独立表达的概念一直用字符串或数字表示。考虑建立小的专用类型。
  • 重复分支 Repeated Switches:多处针对同一类型反复写相同 switch 或 if 链。考虑多态,或共享同一映射。
  • 一项变化需要四处修改 Shotgun Surgery:一个逻辑变化牵动许多文件。考虑将一起变化的内容集中到一个模块。
  • 一个模块因无关原因变化 Divergent Change:同一文件因多种不相关需求而修改。考虑拆分责任。
  • 为假想需求提前设计 Speculative Generality:增加规格未要求的抽象、参数或扩展点。去掉多余设计,等真实需求出现再考虑。
  • 过长访问链 Message Chains:调用者依赖 a.b().c().d() 这样的内部导航。考虑把访问过程隐藏在第一个对象的方法后。
  • 只负责转手 Middle Man:类或函数主要向别处转发。考虑删除中间层,直接调用实际目标。
  • 不愿使用继承内容 Refused Bequest:子类忽略或覆盖大部分继承能力。考虑用组合替代继承。
English · 英文原文
3. Identify the standards sources

Anything in the repo that documents how code should be written, such as CODING_STANDARDS.md or CONTRIBUTING.md.

On top of whatever the repo documents, the Standards axis always carries the smell baseline below — a fixed set of Fowler code smells (Refactoring, ch.3) that applies even when a repo documents nothing. Two rules bind it:

  • The repo overrides. A documented repo standard always wins; where it endorses something the baseline would flag, suppress the smell.
  • Always a judgement call. Each smell is a labelled heuristic ("possible Feature Envy"), never a hard violation — and, like any standard here, skip anything tooling already enforces.

Each smell reads what it ishow to fix; match it against the diff:

  • Mysterious Name — a function, variable, or type whose name doesn't reveal what it does or holds. → rename it; if no honest name comes, the design's murky.
  • Duplicated Code — the same logic shape appears in more than one hunk or file in the change. → extract the shared shape, call it from both.
  • Feature Envy — a method that reaches into another object's data more than its own. → move the method onto the data it envies.
  • Data Clumps — the same few fields or params keep travelling together (a type wanting to be born). → bundle them into one type, pass that.
  • Primitive Obsession — a primitive or string standing in for a domain concept that deserves its own type. → give the concept its own small type.
  • Repeated Switches — the same switch/if-cascade on the same type recurs across the change. → replace with polymorphism, or one map both sites share.
  • Shotgun Surgery — one logical change forces scattered edits across many files in the diff. → gather what changes together into one module.
  • Divergent Change — one file or module is edited for several unrelated reasons. → split so each module changes for one reason.
  • Speculative Generality — abstraction, parameters, or hooks added for needs the spec doesn't have. → delete it; inline back until a real need shows.
  • Message Chains — long a.b().c().d() navigation the caller shouldn't depend on. → hide the walk behind one method on the first object.
  • Middle Man — a class or function that mostly just delegates onward. → cut it, call the real target direct.
  • Refused Bequest — a subclass or implementer that ignores or overrides most of what it inherits. → drop the inheritance, use composition.
老师讲解 · 对应上方原文 · 含教学举例

代码气味是值得调查的迹象,不是自动判错

Fowler 的代码气味用于发现结构问题:神秘命名让人猜用途;重复代码让同一规则多处维护;数据泥团提示一组总一起出现的字段可能属于同一概念;霰弹式修改表示一个决定扩散到许多文件。

Feature Envy 可理解为一个方法过度依赖别人拥有的数据;Middle Man 是几乎只转发的中间层。但适配器有时正是需要保留的边界,不能见到转发就删。Speculative Generality 指为尚不存在的需求提前增加复杂扩展结构。

原文明确仓库约定优先、气味只是判断。评审应给出具体代码片段、为什么可能有问题和影响;已由工具准确检查的规则,无需人再逐条重复报一遍。

中文译文
4. 并行运行两个子 Agent

在同一条消息中发起两次 Agent 工具调用,都使用 general-purpose 通用子 Agent。

规范审查的提示包含完整差异命令、提交列表、规范文件清单,以及完整复制的坏味道清单,因为子 Agent 没有别的途径读取它。

任务说明应要求:按文件或改动片段报告违反文档规则的位置,引用规范文件和规则;同时报告疑似坏味道的名称及具体片段。区分硬性违反与判断意见;项目规范优先;工具已检查的事项跳过。报告少于 400 个英文单词。

需求审查的提示包含差异命令、提交列表和规格路径或正文。

任务说明应要求检查:需求缺失或只完成部分;实现了未要求的行为,即范围扩大;看似实现但方式错误的要求。每项都引用对应规格原句。报告少于 400 个英文单词。

没有规格时,不运行需求子 Agent,并在最终报告说明。

English · 英文原文
4. Spawn both sub-agents in parallel

Send a single message with two Agent tool calls. Use the general-purpose subagent for both.

Standards sub-agent prompt — include:

  • The full diff command and commit list.
  • The list of standards-source files you found in step 3, plus the smell baseline from step 3 pasted in full — the sub-agent has no other access to it.
  • The brief: "Report — per file/hunk where relevant — (a) every place the diff violates a documented standard: cite the standard (file + the rule); and (b) any baseline smell you spot: name it and quote the hunk. Distinguish hard violations from judgement calls — documented-standard breaches can be hard, but baseline smells are always judgement calls, and a documented repo standard overrides the baseline. Skip anything tooling enforces. Under 400 words."

Spec sub-agent prompt — include:

  • The diff command and commit list.
  • The path or fetched contents of the spec.
  • The brief: "Report: (a) requirements the spec asked for that are missing or partial; (b) behaviour in the diff that wasn't asked for (scope creep); (c) requirements that look implemented but where the implementation looks wrong. Quote the spec line for each finding. Under 400 words."

If the spec is missing, skip the Spec sub-agent and note this in the final report.

老师讲解 · 对应上方原文 · 含教学举例

分开评审是为了保留两种不同问题

规范评审对照规则,规格评审对照需求。各自独立输入可以减少一种结论影响另一种:代码漂亮,不应让人忽略功能缺失;功能能跑,也不应掩盖违反重要项目约定。

例如一个评审发现取消收藏未实现,另一个发现命名违反项目已有规范。两者不能合并成一个模糊“总体良好”。报告分别保留发现与依据,负责人才能判断怎样处理。

并行子 Agent 提高分工独立性,但不自动保证判断正确。它们仍可能共享同一错误前提,所以每条发现都需要可核对的原句或差异证据。

阅读历史工具指令时,区分方法与宿主语法

同一消息发起两个 Agent 调用,意在让两个评审并行,并保持各自的上下文独立。general-purpose 是这份原文指定的子 Agent 类型,不是所有客户端通用的工具名称。

方法可以迁移:两个评审各自依据规范和需求给证据。语法则必须看当前工具。不能在没有 Agent 工具的环境里,仅写一句“已并行评审”就当作真的执行。

历史文件提到 PRD,也不意味着评审只接受某个固定文件名;它实际寻找的是原始需求依据。

中文译文
5. 汇总报告

分别在 ## Standards## Spec 下呈现两份报告,原样或轻微整理。不要把两个方向混合或重新排序。

最后用一句话说明每个方向各有多少发现,以及各自内部最严重的问题。不要跨两个方向选出一个唯一最严重项,否则会破坏刻意保留的独立判断。

English · 英文原文
5. Aggregate

Present the two reports under ## Standards and ## Spec headings, verbatim or lightly cleaned. Do not merge or rerank findings — the two axes are deliberately separate (see Why two axes).

End with a one-line summary: total findings per axis, and the worst issue within each axis (if any). Don't pick a single winner across axes — that's the reranking the separation exists to prevent.

老师讲解 · 对应上方原文 · 含教学举例

评审完成与修复完成不是同一个状态

此 skill 聚合两份报告,并不因为列出了问题就自动修好它们。读报告应区分:发现了什么、哪些是硬性规范违反、哪些只是设计判断、哪条需求缺失或越界。

每轴分别计数和指出本轴严重问题,避免以一个总分抹平差异。比如规范零问题、规格三项缺失,不能被“整体代码质量高”冲淡。

你可要求后续实现者逐项修复,再提供对应验证。评审是决策输入,不是只需盖一个 APPROVE 就结束的仪式。

中文译文

为什么分开审查

一份改动可能满足其中一边,却不满足另一边:遵循全部规范但做错需求,是规范通过、需求不通过;行为符合需求却破坏项目约定,是需求通过、规范不通过。

分开报告,可以避免一项好表现掩盖另一类问题。

English · 英文原文

Why two axes

A change can pass one axis and fail the other:

  • Code that follows every standard but implements the wrong thing → Standards pass, Spec fail.
  • Code that does exactly what the issue asked but breaks the project's conventions → Spec pass, Standards fail.

Reporting them separately stops one axis from masking the other.

老师讲解 · 对应上方原文 · 含教学举例

代码写得整齐,却仍可能没有实现你要的功能

假设规格要求取消收藏后,刷新也保持未收藏。AI 只改变了页面图标,没有删除服务器记录。代码命名和格式可能完全合规,但刷新后收藏又出现,需求并未实现。

反过来,功能表现正确,却绕过项目规定的权限检查,也不能因“演示成功”就忽略。两份报告分别保留证据,你才能看清不同类型的问题。

固定参照点说明本次究竟检查哪些变化。三点式 diff 通常关注分支从共同祖先以来的改动;该命令本身不包含尚未提交的工作区变化。要审查未提交工作,需要另行明确范围。

坏味道也只是线索。例如转发函数可能多余,也可能是必要权限入口。不能看到 Middle Man 的名字就要求删掉。让审查者说明具体位置、对应规则和判断理由,你才能核对它的意见。

原作者:Matt Pocock · 中文翻译为非官方译本

来源:skills/in-progress/review/SKILL.md ↗

固定版本:0894b3300f1e83cc666ed6ad075489041e3ba291

先作答,再看参考思路

Q1 · 理解

用自己的话说明:它解决什么问题,完成后会留下什么?

请各用一句话回答。若它只做规划或解释,不要把“已开发”“已部署”写成产物。

Q2 · 判断

为何旧审查结果不能直接作为新提交的验收?

我已思考,查看参考思路

代码版本与比较范围已变,结论只覆盖当时审查的差异。应对准确的新版本重新确认相关变化。

Q3 · 追问

原文中哪条要求在你的环境下可能不成立?

说出具体一句及其前提,例如工具不可用、资料缺失、已有项目约定冲突,或它只是作者偏好。把你的答案带回课堂,我们据此继续讨论。

课堂回传格式:第 51 课 / 我的理解 / Q2 回答 / 仍不理解的原句。这里是阅读教材;实时问答在我们的对话中进行。

把方法放进一个具体情境

教学案例:旧评审说“没有发现问题”,但 Spec 轴实际因无规格跳过。迁移到新流程时,应保留这个缺口,不能将旧结论当完整验收。

边界与容易误读的地方

这里描述的并行审查并没有保证真正独立的评价标准。缺原始需求时,更多审查 Agent 也不能凭空补出权威标准。

讨论后再实践:先判断上述情境是否适用,再选择真实任务。现在无需安装、运行命令或修改现有项目。

关联阅读