| name | ai-review |
| description | Judges a diff the way a staff engineer would: does it do what it claims, is it correct at its boundaries, is it safe, is it tested, and is there a smaller version of it. Never claims the result of a mechanical gate — those ran in CI and it says so. Trigger for "review this", "any issues with this", "is this merge-ready", "look over my PR", "what would you change". Not for running the gates — that is just check in CI. Not for finding why something is broken — use /ai-debug. Not for landing the work — use /ai-ship. Not for deciding a request that falls outside a declared boundary — use /ai-verify, because a decision that cannot classify itself reports CANNOT DECIDE and blocks, and a review is not where an out-of-boundary decision gets made. |
| license | Apache-2.0 |
| compatibility | needs git |
| context | fork |
| background | false |
| disable-model-invocation | true |
Judge the diff
The tier to ask for
Review is hard reasoning about a stranger's change; ask the top tier, the model the
repository's [models] section configures for it — never a model name coded here, and
never a provider the repository did not choose. model_router maps review (and plan and
audit) to the top tier; if the pin leaves it unconfigured, fall back to default_tier.
Say which tier you ran on so the command event's tier_model can be read against
reality.
What it produces
Findings, each one at file:line, each with the smallest change that would resolve it.
Steps
- Read the spec and the plan first. Half of all real findings are "this is not what was
agreed", and you cannot see those from the diff alone.
- Read the diff whole before commenting on any part of it. A finding about a line that the
next hunk deletes wastes the author's afternoon. If it does not fit in one pass, say so
in the first line of the report, review it in named parts, and list the parts you have
not read. A truncated pass reads exactly like a complete one, which is the failure this
framework exists to cure, arriving as a confident report about the half that fit.
- Work the checklists in
references/, one lens at a time, and every lens in that
directory is one of them: correctness, security, performance, testing, compatibility,
architecture, simplification, docs, frontend and motion. Each is a separate pass; mixing them
is how the security one gets skipped. Skip a lens the diff cannot touch and name the one
you skipped — a lens nothing routes to is a checklist nobody works. Within each, follow
the data flow: where the value enters, what may change it, where it is read. Then read
the business rule the change encodes, not only the code — a lens applied to lines finds
what is wrong, and only the rule says what is wrong here.
- Never report what a tool already reports. Formatting, lint, secrets, dependency
vulnerabilities — those ran in CI, and repeating them buries the findings only a person
could have made. If the gate did not run, say that instead of standing in for it.
- For each finding: what breaks, the inputs that break it, and the smallest fix. A finding
without a failing scenario is an opinion, and it should be labelled as one.
- Before you call anything blocking, try to kill it, and default to dismissing: re-read
the file around the line, not the hunk, and look for the guard, caller, framework
behaviour or config that makes the scenario impossible. Dismiss it too when the bug is
real and the fix you would propose is wrong, or worse than the code. A real bug you are
unsure of still blocks; a theoretical one you are certain of does not. Say what you
tried to kill and what lived.