code-review

Review

Perform a dual-axis review of the diff between HEAD and the fixed point specified by the user:

• Convention—does the code comply with this repository's documented coding conventions?
• Specification—does the code faithfully implement the original issue / PRD / specification?

The two axes run asparallel sub-agents , without polluting each other's context, and then this skill summarizes the findings from both sides.

The issue tracker should already have been provided to you. If not, tell the user to run /setup-matt-pocock-skills。

Process

1. Determine the fixed point

Anything the user says can serve as the fixed point—a commit SHA, branch name, tag,main、HEAD~5 , and so on. Don't improvise; just pass it in directly. If the user didn't specify one, ask: "What should the review be based on—a branch, a commit, or main?" Do not proceed until you get an answer.

Capture the diff command:git diff <fixed-point>...HEAD(three dots, so the comparison is against the merge-base). Also, via git log <fixed-point>..HEAD --oneline record the commit list.

Before continuing, confirm that the fixed point resolves (git rev-parse <fixed-point>) and that the diff is non-empty. A bad reference or an empty diff should fail here—it should not be left for two parallel sub-agents to handle.

2. Determine the specification source

Look for the original specification in the following order:

1. Issue references in commit messages (#123、Closes #45、GitLab !67 , etc.)—fetch them according to the workflow in the tracker documentation.
2. A path passed in by the user as an argument.
3. docs/、specs/ or .scratch/ a PRD/specification file under it that matches the branch name or feature name.
4. If nothing is found, ask the user where the specification is. If the user says there is none,Specificationsub-agent will skip and report "no specification available".

3. Determine the convention source

Search the repository to find all files that document code-writing conventions. If CODING_STANDARDS.md or CONTRIBUTING.mdexists, they must be included as sources.

In addition to what the repository documents, the convention axis always carries the followingsmell baseline—a fixed set of code smells from Chapter 3 of Fowler's Refactoring, which applies even if the repository has no documentation at all. Two constraint rules:

• Repository overrides conventions. A documented repository standard always takes precedence; if the repository explicitly endorses certain baseline determinations, suppress that smell.
• Always a judgment call. Each smell is a labeled heuristic judgment ("possible feature envy"), never a hard violation—and, like all standards here, skip what tools already enforce.

The format of each smell iswhat it is → how to fix it; match it against the diff:

• Mysterious Name— The name of a function, variable, or type fails to reveal its purpose or meaning. → Rename it; if an honest name cannot be found, the design itself is unclear.
• Duplicated Code— The same logical shape appears in multiple hunks or files in the change. → Extract the shared shape and call it from both places.
• Feature Envy— A method accesses the data of another object more than its own. → Move the method onto the data it envies.
• Data Clumps— The same group of fields or parameters repeatedly appears together (a type waiting to be born). → Package them into a type and pass that type.
• Primitive Obsession— Using primitives or strings to represent a concept that deserves its own type. → Give the concept its own small type.
• Repeated Switches— Repeatedly using the same switch/if cascade for the same type. → Replace it with polymorphism, or use a single mapping shared by both.
• Shotgun Surgery— A single logical change forces modifications scattered across many files in the diff. → Gather what changes together into one module.
• Divergent Change— A single file or module is modified for multiple unrelated reasons. → Split it so that each module changes for a single reason.
• Speculative Generality— Abstractions, parameters, or hooks added for requirements that do not exist in the spec. → Delete them; inline back until a real requirement appears.
• Message Chains— Lengthy a.b().c().d() navigation that the caller should not depend on. → Hide the traversal behind a method on the first object.
• Middle Man— A class or function mostly just delegates to others. → Cut it out and call the real target directly.
• Refused Bequest— A subclass or implementer ignores or overrides most of what it inherits. → Abandon inheritance and use composition instead.

4. Launch two sub-agents in parallel

Issue both sub-agent calls at the same time and run them in the foreground; aggregate the reports they return.

Spec sub-agent prompt— Include:

• The complete diff command and commit list.
• The list of spec source files you found in step 3,plus the smell baseline from step 3(paste it in full — the sub-agent has no other way to obtain it).
• Task summary: "Report — listed by file/hunk — (a) every place in the diff that violates a documented spec: cite the spec (file + rule); and (b) any baseline smells you find: name them and cite the hunk. Distinguish hard violations from judgment calls — violations of documented specs can be hard, but baseline smells are always judgment calls, and documented repository standards override the baseline. Skip anything the tooling already enforces. Within 400 words."

Spec sub-agent prompt— Include:

• The diff command and commit list.
• The path to the spec file or its retrieved contents.
• Task summary: "Report: (a) requirements that are specified but missing or incomplete; (b) behavior present in the diff but not required by the spec (scope creep); (c) requirements that appear implemented but whose implementation may be wrong. For each item, quote the original spec text. Within 400 words."

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

5. Synthesis

Under the ## 规范 and ## 规格 headings, present the two reports, either verbatim or lightly edited.Do notmerge or reorder findings—the two axes are deliberately kept separate (see "Why two axes").

End with a one-line summary: the total number of findings on each axis, and the single most severe issue within each axis (if any). Do not pick one final winner across the axes—that is exactly the reordering the split is meant to prevent.

Why two axes

A change may pass review on one axis but fail on the other:

• The code follows all conventions but implements the wrong functionality → conventions pass, spec fails.
• The code implements exactly what the issue asks for but violates the project conventions → spec passes, conventions fail.

Reporting them separately prevents one axis from masking the other.