skills/docs/engineering/code-review.md
Matt Pocock 14c13c5bf9 Rename review skill to code-review and promote to engineering
Rename the in-progress `review` skill to `code-review` and move it from
`skills/in-progress/` into the promoted `skills/engineering/` bucket.

- Add it to `.claude-plugin/plugin.json` and the Model-invoked sections of
  the top-level and Engineering READMEs; drop it from the in-progress README.
- Add a docs page at `docs/engineering/code-review.md`.
- Point `/implement` and its docs at `/code-review`.
- Update the smell-baseline changeset reference; add a changeset for the move.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-01 12:04:12 +01:00

3.7 KiB

Quickstart:

npx skills add mattpocock/skills --skill=code-review
npx skills update code-review

Source

What it does

code-review reviews the diff between HEAD and a fixed point you supply — a commit, branch, tag, or merge-base — along two separate axes: Standards (does the code follow this repo's documented conventions?) and Spec (does it implement what the originating issue or PRD asked for?). It runs each axis as its own parallel sub-agent and reports them side by side. It never merges or re-ranks the two sets of findings — keeping them separate is the whole point, because a change can pass one axis and fail the other, and a single blended verdict lets one mask the other.

When to reach for it

Type /code-review, or the agent reaches for it automatically when you ask to review a branch, a PR, work-in-progress changes, or anything "since X".

Reach for this when there is a diff to judge against a known-good point and you want the two questions — is it built right? and is it the right thing? — answered independently. It runs at the end of the build loop; for actually writing the code test-first, use tdd, and for building a whole spec into code use implement, which runs its own /code-review pass before committing.

Prerequisites

The Spec axis needs somewhere to find the originating spec — an issue reference in the commit messages, a path you pass in, or a PRD under docs//specs/. That issue-tracker wiring comes from setup-matt-pocock-skills; without a spec the Spec axis simply skips and says so. The Standards axis needs nothing set up — it always carries a built-in Fowler smell baseline even in a repo that documents no conventions.

Two axes, never merged

The defining idea is the two axes. Standards asks whether the diff conforms to how this repo writes code — its CODING_STANDARDS.md or CONTRIBUTING.md, plus a fixed baseline of ~12 Fowler code smells (Mysterious Name, Duplicated Code, Feature Envy, Data Clumps, …). Two rules keep the baseline safe: a documented repo standard always overrides it, and every smell is a judgement call, never a hard violation. Spec asks the orthogonal question — does the code do what the issue or PRD actually asked, without missing requirements or smuggling in scope creep?

They run as parallel sub-agents so neither pollutes the other's context, and the final report presents them under separate ## Standards and ## Spec headings with a per-axis summary. There is deliberately no single winner across axes.

It's working if

  • It pins and confirms the fixed point first (git rev-parse), failing fast on a bad ref or empty diff rather than inside the sub-agents.
  • Standards and Spec findings arrive in two distinct blocks, each citing its source — a repo standard or baseline smell for one, a quoted spec line for the other.
  • When no spec can be found, the Spec axis reports "no spec available" instead of inventing requirements.

Where it fits

code-review is the review step at the tail of the main build chain:

grill-with-docs → to-prd → to-issues → implement → code-review

Its closest neighbour is implement, which drives the build and calls this as its own review pass before committing; upstream, the spec it checks against is produced by to-prd and to-issues. When you're unsure which skill or flow fits, ask-matt routes you.