Code Review Recent Changes

Code Review: Recent Changes

Review the diff between HEAD and a fixed point the user supplies, along three independent axes:

• Standards - does the code conform to this repo's documented conventions?
• Spec - does the code faithfully implement the originating issue / PRD / spec?
• Maintainability - is the change structurally healthy, or did it leave the codebase harder to change?

Each axis runs as its own parallel sub-agent so they don't pollute each other's context, then this skill aggregates their findings. Keep them separate: a change can pass one axis and fail another - code that follows every convention but implements the wrong thing (Standards pass, Spec fail), or does exactly what the issue asked while leaving the codebase messier (Spec pass, Maintainability fail). Separate reporting stops one axis from masking another.

Review stance

Two ideas shape how the sub-agents work, so build them into the briefs:

• Read outward from the diff. An agent handed a diff tends to treat it as the edge of the world. Tell each sub-agent to look past it: for a changed symbol, read its surrounding function/file and the modules that call it or that it calls. A hunk that looks fine in isolation can duplicate an existing helper, contradict a sibling module's pattern, or leave a half-finished migration two files over.
• Be ambitious on Maintainability; precise on Standards and Spec. Standards and Spec are close to binary - a documented rule is violated or it isn't, a requirement is met or it isn't - so favour precision and don't manufacture findings. Maintainability is where the valuable, easy-to-miss findings live, so favour recall: propose a restructuring even when you're not fully sure, because a wrong suggestion costs the reader one quick "no", while a worthwhile one you never raise is one nobody gets to consider. To keep the wrong ones cheap to dismiss, every Maintainability finding carries a confidence label and concrete evidence.

Process

1. Pin the fixed point

Whatever the user named is the fixed point - a commit SHA, branch name, tag, main, HEAD~5. Pass it through; don't be opinionated. If they didn't give one, ask: "Review against what - a branch, a commit, or main?" Don't proceed without it.

Capture three things once and reuse them across all sub-agents:

• Diff: git diff <fixed-point>...HEAD (three-dot, so the comparison is against the merge-base).
• Commits: git log <fixed-point>..HEAD --oneline.
• Size deltas: git diff <fixed-point>...HEAD --stat - the Maintainability axis uses this to spot files crossing a size boundary.

2. Check for a spec

Scan the captured commit list and the user's request for a spec signal: an issue reference (#123, Closes #45, !67), a spec/PRD path the user passed, or an issue/PRD the user named. This is a cheap check - the commit list is already in context.

• A signal is present → read references/spec-review.md and follow it to locate the spec and build the Spec sub-agent brief.
• No signal → skip the Spec axis. Don't open the reference; note "no spec available" in the final report.

3. Identify the standards sources

Anything in the repo that documents how code should be written:

• CLAUDE.md, AGENTS.md, CONTRIBUTING.md
• CONTEXT.md / CONTEXT-MAP.md (including per-directory ones); docs/adr/ (architectural decisions are standards)
• STYLE.md, STANDARDS.md, STYLEGUIDE.md at the repo root or under docs/
• .editorconfig, eslint.config.*, biome.json, prettier.config.*, tsconfig.json - machine-enforced; note them but don't re-check what tooling already enforces.

Collect the list; the Standards sub-agent reads them.

4. Spawn the sub-agents in parallel

Send a single message with the Agent calls, all using the general-purpose subagent: Standards and Maintainability always, plus Spec if step 2 found a signal (use the brief from references/spec-review.md).

Standards sub-agent - include the diff command, commit list, and the standards-source file list from step 3. Brief:

Read the standards docs, then the diff. Read beyond the diff where context matters - e.g. to tell whether a new helper duplicates a canonical one or breaks a sibling module's pattern. Report, per file/hunk, every place the diff violates a documented standard. Cite the standard (file + the rule). Separate hard violations from judgement calls. Skip anything tooling already enforces. Order findings worst-first. Under 400 words.

Maintainability sub-agent - include the diff command, commit list, and the --stat output. Brief:

Audit the change for structural health and future changeability. Read beyond the diff: for any changed symbol, read its surrounding file and the modules it calls or is called by. Be ambitious - hunt for a "code judo" move that preserves behaviour while making the change dramatically simpler, deleting whole branches, helpers, or layers rather than rearranging them.

>

Report two groups:

>

(a) Structure (highest value). Concrete triggers: a file crossing the repo's documented size limit (or ~1000 lines / 5k tokens if none is documented) where the new code could be split out; a new conditional or special case bolted onto an unrelated flow that belongs behind its own abstraction; logic duplicated instead of reusing an existing canonical helper; a thin wrapper or pass-through that adds indirection without clarity; feature-specific logic leaking into a shared module; an optional prop the diff adds even though every call site supplies it (agents reach for optionality to shrink a change's blast radius), or an any/unknown/cast that hides an invariant that is actually fixed; a half-finished decomposition or migration.

>

(b) Tests and seams. Did the change make the code easier or harder to test and change? Is new behaviour covered? Did it add or remove a seam - a point where behaviour can be substituted or observed? Swallowed errors and silent fallbacks belong here.

>

For each finding: state the problem, cite file + line, and give the specific restructuring you would make. Hold that restructuring to a bar - it must remove moving pieces, not relocate the same complexity or offer a tidier version of the same messy idea; if your fix doesn't make the code simpler on net, drop the finding. Label confidence (high or worth-checking). Order findings worst-first, structural regressions before legibility nits. Prefer a few high-conviction findings over a long list of cosmetic notes. Under 500 words.

5. Aggregate

Present the reports under ## Standards, ## Spec, and ## Maintainability, verbatim or lightly cleaned. If the Spec axis was skipped, keep its heading and write "no spec available". Don't merge or rerank across axes - the separation is the point.

Close with:

• A one-line tally per axis (number of findings, worst severity).
• An explicit verdict: approve, approve with nits, or request changes. Reserve request changes for blocker-class findings: a hard standard violation, a missing or incorrect spec requirement, or a high-confidence structural regression. A worth-checking Maintainability finding is advice, not a blocker - on its own it shouldn't drop the verdict below approve with nits. Name the single worst issue. Don't approve on correct behaviour alone - a change can work and still leave the codebase meaningfully messier.

Gotchas

• Stale base. Reviewing against main while local main is behind origin/main computes the wrong merge-base, so the diff silently includes or drops the wrong commits. Fetch first, or pin the fixed point to origin/main.
• Inline instead of parallel. Running the axis briefs in this conversation rather than spawning them as real sub-agents collapses the context isolation that keeps the axes from polluting each other. Spawn them as separate Agent calls.

Spec axis: locating the spec and briefing the sub-agent

Read this only when the review has a spec to check against - i.e. step 2 of SKILL.md found a spec signal (an issue reference in the commits, a path the user passed, or an issue/PRD the user named). With no such signal, skip the Spec axis and report "no spec available"; there's no need to open this file.

Locate the spec

Look for the originating spec, in this order:

1. Issue references in the commit messages (#123, Closes #45, GitLab !67). To fetch one, detect how this repo's tracker works, trying in order:

• docs/agents/issue-tracker.md exists → follow the workflow it documents.
• gh repo view succeeds → GitHub issues; fetch with gh issue view <number> --comments (the always-on github skill covers the commands).
• docs/issues/ or docs/BACKLOG.md exists → the item may live there as a local backlog entry; read it directly.
• none of the above → ask the user how to fetch the issue.

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.

If a signal pointed at a spec but it can't be located or fetched, ask the user where it is. If after that nothing turns up, skip the Spec axis and note "no spec available" in the final report.

Spec sub-agent brief

Spawn a general-purpose sub-agent. Include the diff command, the commit list, and the spec path or fetched contents. Brief:

Read the spec, then the diff. Read beyond the diff where it helps you judge whether a requirement is genuinely wired up rather than stubbed or partial. 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. Order findings worst-first. Under 400 words.