code-critic

/code-critic — Code Review

Apply this skill to review code changes against project standards.

---

Stance

Treat the implementation as a hypothesis under attack. Your default assumption is that something is wrong; your job is to find what.

Confirmation bias is the dominant failure mode of AI code review — guard against it by actively trying to disconfirm the implementation rather than verifying that it looks reasonable. If you cannot find a fault after genuine effort, that is itself a finding worth stating explicitly (see the Output Format section).

---

Before Reviewing

Selecting the diff

In the /feature workflow the implementation is committed before review, so a bare git diff (working tree) shows nothing — review the branch's committed changes against its base, the default branch named in AGENTS.md → Commands: git diff <default-branch>...HEAD (or git log -p <default-branch>..HEAD). Invoked ad-hoc on uncommitted work, review the working-tree diff instead. If unsure what changed, check git status and git log --oneline first.

Note for ad-hoc use: the push gate records a commit SHA (scripts/review-ok.sh), so a review meant to unlock a push must cover the committed state — commit first, then review.

Architecture Decision Records

Check if a docs/adr/ directory exists. If it does, list the ADRs you read by number in the review output. If none seemed relevant to the diff, say so explicitly — silence is not acceptable, since it is indistinguishable from skipping the step.

If the diff contradicts an ADR, quote the contradicting ADR clause and the offending diff line, and flag it as FAIL.

Implementation Plan

If plan text was provided (inline in the prompt, or via a file path), use it before reviewing. Use it as follows:

• Approval Summary / Acceptance Criteria: the human-approved contract. Verify every AC-<slug>-n has a committed test (via the [AC-<slug>-n] tags in Test Strategy) that would fail if the criterion were broken — a criterion without one is FAIL.
• Contract section (if present): cross-check the diff's routes, form fields/params, response shapes, error rendering, and schema changes against it. An undiscussed deviation from the approved Contract is FAIL.
• Requirements section: the source of truth for what should have been built — used to verify Plan Compliance and that all specified edge cases are handled.
• Approach section: the agreed implementation strategy — used to verify the code follows the intended design rather than an ad-hoc alternative.
• Edge Cases section: the enumerated scenarios that must be handled. Verify every EDGE-<slug>-n has a committed test (via the [EDGE-<slug>-n] tags in Test Strategy) that would fail if the handling strategy were broken — an edge case without one is FAIL.
• Test Strategy section: the agreed test coverage — cross-reference against the actual tests written.
• Files section: the planned file manifest — cross-check against the diff. A file in the diff that is not listed here (or listed but left untouched) is an undiscussed change; flag the mismatch and judge whether it is in scope.
• Out of Scope section: the explicit boundary — do NOT flag as missing anything listed here, and DO flag as scope creep any code that strays into it.