Communicate with the user and produce review reports in Traditional Chinese.
Review the diff between HEAD and a fixed point provided by the user, using two axes:
- Standards: Does the code follow the documented writing conventions of this repository?
- Spec: Does the code faithfully implement the original issue/spec?
Both axes are executed by parallel sub-agents so they do not pollute each other's context. This skill then consolidates their findings.
The issue tracker should already be provided to you. If not, tell the user to run /setup-matt-pocock-skills first.
Process
1. Determine the fixed point
The user specifies the fixed point (commit SHA, branch name, tag, main, HEAD~5, etc.). If they do not specify one, ask.
Capture the diff command: git diff <fixed-point>...HEAD (three dots, so it compares against the merge-base). Also capture the commit list with git log <fixed-point>..HEAD --oneline.
Before proceeding, verify that the fixed point can be resolved (git rev-parse <fixed-point>) and that the diff is not empty. Invalid references or empty diffs should fail here, not in the two parallel sub-agents.
2. Find the spec source
Search for the original spec in this order:
- Issue references in commit messages (
#123,Closes #45, GitLab's!67, etc.), obtained through the process in the tracker document. - A path provided by the user as a parameter.
- A spec file in
docs/,specs/, or.scratch/that matches the branch name or feature. - If nothing is found, ask the user where the spec is. If they say there is no spec, the Spec sub-agent is skipped, and the report notes "No spec available."
3. Find the standards source
Search the repository for all files documenting how code should be written. If CODING_STANDARDS.md or CONTRIBUTING.md exist, they must be included. For Python projects, also check pyproject.toml for linter/formatter settings (ruff, black, mypy, etc.), but skip rules that the tools already enforce.
Beyond the documented conventions, the Standards axis always carries the bad smell baseline below: a fixed set of Fowler code smells (from Chapter 3 of Refactoring), applicable even if the repo has no documentation. Two rules constrain it:
- Repo first. Documented repo conventions always win; when a convention approves something the baseline would flag, do not flag that smell.
- Always judgment calls. Each smell is a labeled heuristic ("might have Feature Envy"), never a hard violation. Like all conventions here, skip anything already enforced by tools.
Each smell follows the pattern "What it is → How to fix it"; compare them against the diff:
- Mysterious Name: A function, variable, or type name doesn't reveal what it does or holds. → Rename; if you can't think of an honest name, the design is vague.
- Duplicated Code: The same logical shape appears in multiple blocks or files within this change. → Extract the common shape and call it from both places.
- Feature Envy: A method accesses another object's data more than its own. → Move the method to the data it envies.
- Data Clumps: The same fields or parameters always appear together (a type wanting to be born). → Wrap them into a type, then pass that type.
- Primitive Obsession: Using a primitive type or string for a domain concept that deserves its own type (e.g.,
strfor part numbers,floatfor amounts and currencies). → Give the concept a small type (e.g.,dataclass). - Repeated Switches: The same
match/ifchain on the same type appears repeatedly in this change. → Use polymorphism or a shared lookup table. - Shotgun Surgery: One logical change forces scattered modifications across many files in the diff. → Centralize things that change together into one module.
- Divergent Change: A file or module is modified for several unrelated reasons. → Split it so each module changes for only one reason.
- Speculative Generality: Abstractions, parameters, or hooks added for needs not in the spec. → Delete them; inline back until real needs appear.
- Message Chains: Long
a.b().c().d()navigation that callers shouldn't depend on. → Hide the traversal behind one method on the first object. - Middle Man: A class or function that mostly delegates downward. → Remove it and call the real target directly.
- Refused Bequest: A subclass or implementer ignores or overrides most of what it inherits. → Replace inheritance with composition.
4. Spawn two sub-agents in parallel
Spawn two sub-agent calls (in the foreground) and consolidate the reports they return.
The Standards sub-agent prompt should include:
- The full diff command and commit list.
- The list of standards source files you found in step 3, plus the bad smell baseline from step 3 pasted in full (the sub-agent has no other way to get it).
- Task description: "Report in Traditional Chinese, noting the file/block for each finding: (a) every place the diff violates a documented convention: cite the convention (file + rule); and (b) any baseline bad smells you find: name the smell and cite the block. Distinguish hard violations from judgment calls: violations of documented conventions can be hard, but baseline bad smells are always judgment calls, and documented repo conventions take priority over the baseline. Skip anything already enforced by tools. Limit to 400 words."
The Spec sub-agent prompt should include:
- The diff command and commit list.
- The spec path or the content you have retrieved.
- Task description: "Report in Traditional Chinese: (a) requirements the spec asks for but are missing or only partially implemented; (b) behavior in the diff that was not asked for (scope creep); (c) requirements that appear implemented but in a way that seems wrong. For each finding, cite the corresponding text from the spec. Limit to 400 words."
If there is no spec, skip the Spec sub-agent and note it in the final report.
5. Consolidate
Place the two reports under headings ## Standards and ## Spec, reproduced verbatim or lightly cleaned. Do not merge or reorder findings, because the two axes are intentionally separate (see "Why two axes").
End with a one-line summary: the total number of findings per axis and the most severe problem within each axis (if any). Do not pick a single winner across axes; that is exactly the reordering the two-axis separation prevents.
Why two axes
A change may pass one axis but not the other:
- Follows all conventions but implements the wrong thing → Standards pass, Spec fail.
- Does exactly what the issue asked for but breaks project conventions → Spec pass, Standards fail.
Reporting separately prevents one axis from overshadowing the other.