Code Review
Use this capability whenever you review a code change — a pull request, a branch or commit-range diff, or your own work before you call it done — whatever the change type, language, or domain. The skill is self-contained: every rule it needs lives here, so it stays correct when installed on its own without any companion skill. When the host project ships its own code-review guideline or posted-review policy, defer to it for project-specific rules and precedence; the methodology here still applies in full wherever that project guidance is silent.
A review has one job — decide whether a change is safe to merge and say why, with evidence — and one output: a report. It does not rewrite the code. Keep finding problems separate from fixing them.
Review target. The target is the change already under review: the working tree against the base branch by default, or a specific pull-request reference, branch name, or commit range when one is named. Establish the diff for that target before reading anything else (see scoping.md).
The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this document are to be interpreted as described in RFC 2119.
The Review Loop
Every review runs the same loop: reset into reviewer mode, scope the change from its diff, assess that diff through the review lenses, classify each finding by severity, report with evidence, and escalate anything too risky to self-approve. The reset is what makes the rest trustworthy — the reviewer inspects what the code does, as if someone else wrote it, instead of re-affirming the reasoning that produced it; this matters most in self-review, where the author and reviewer are the same agent. Do not read any code before the reset — it is the first normative step, owned by scoping.md. The sections below route to the reference that owns each step, ordered as a review applies them.
Review Scoping
See scoping.md for:
- performing the reviewer-mode reset and establishing scope from
git status / git diff / a PR diff
- distinguishing in-scope (the diff) from out-of-scope (pre-existing) code
- reading the full file and every caller/callee around a changed hunk
- checking a change's boundary claims — the neighbours it names as owners — against what those neighbours actually state, not only against the diff
- handling untracked files, an empty or unclear diff, and generated / tool-managed files
Severity Classification
See severity.md for:
- the Critical / Major / Minor / Nit definitions, each pairing a merge impact with a defect class
- the fixed severity floors for categories such as committed secrets, missing access control, unsanitized input, and introduced test or lint failures
- mapping severity counts to an Approve / Approve with Nits / Request Changes verdict
- resolving uncertain severity by escalating upward and stating the assumption
What to Flag: Review Lenses
See review-lenses.md for:
- the correctness lens: logic errors, edge cases, error and async handling, contract changes
- the maintainability lens: naming, organization, abstraction boundaries, complexity, dead code, scope discipline, and naming what a content-adding change should cut
- the security and privacy lens: secrets, input validation, access control, injection, SSRF, auth, data exposure, supply chain
- the testing and verification lens: coverage, stable test hooks, snapshots, flakiness, manual checks
- the performance and reliability lens: data-access cost, concurrency, caching, asset and bundle weight, failure modes
Evidence and Reporting
See evidence-and-reporting.md for:
- the mandatory
file:line citation on every finding and quoting the offending code
- diff-style (
-/+) fix snippets for every Critical and Major finding
- the exact review-report section order, from Summary through Recommended Actions
- what counts as evidence versus assertion, and how to mark findings the reviewer could not verify
- what the author's own account of a change — a verification table, self-authored acceptance criteria, a disclosed figure — does and does not establish
Review Tone
See tone.md for:
- addressing the code, not the author, and stating the concrete risk behind each finding
- acknowledging real strengths without inflating trivial ones, an obligation the internal review report carries and a posted review's summary does not
- keeping style and preference out of blocking severities
- flagging assumptions explicitly and leaving human-authored copy to its authors
Escalation and Decisions
See escalation.md for:
- keeping the review reporting-only — no code mutation, no delegating the review away
- making each fix trivially applicable and pairing it with its verification step
- escalating high-risk changes to an external gate instead of self-approving them
- deferring genuine trade-offs back to the caller as enumerated Decision-needed entries
Posted and CI Reviews
See posted-review-policy.md for:
- when a review is posted to a pull request (a CI reviewer or a managed review product) rather than kept as internal self-review
- collapsing the internal four-tier triage to a two-label Important / Nit report with a one-line tally
- running the repository's mandatory checks and honoring its do-not-report exclusions, built as an enumerated list of checks each coextensive with the finding it silences, never a blanket "anything CI enforces" clause
- posting the whole review as one submission of the platform's review mechanism able to carry diff-anchored comments, chosen before the diff is read, with the submission's verdict kept non-gating for an advisory reviewer as a separate decision that never falls back to a looser container
- keeping the summary to a closed three-entry allowlist — the tally, what could not be checked, and a finding with no line to anchor to — with one exception for a host-mandated per-round enumeration
1---2name: code-review3description: Reviewing a code change — a pull request, a branch or commit-range diff, or a post-implementation self-review of your own work before calling it done. The methodology for judging whether a change already written is safe to merge, and for reporting why. Not for writing the change, only for judging one that already exists. Covers the reviewer-mode reset, diff scoping, a four-tier severity scale with fixed floors, file-line evidence with fix snippets, escalation for high-risk changes, and lenses for correctness, maintainability, security, testing, and performance. Self-contained, so it works installed on its own.4---56# Code Review78Use this capability whenever you review a code change — a pull request, a branch or commit-range diff, or your own work before you call it done — whatever the change type, language, or domain. The skill is self-contained: every rule it needs lives here, so it stays correct when installed on its own without any companion skill. When the host project ships its own code-review guideline or posted-review policy, defer to it for project-specific rules and precedence; the methodology here still applies in full wherever that project guidance is silent.910A review has one job — decide whether a change is safe to merge and say why, with evidence — and one output: a report. It does not rewrite the code. Keep finding problems separate from fixing them.1112**Review target.** The target is the change already under review: the working tree against the base branch by default, or a specific pull-request reference, branch name, or commit range when one is named. Establish the diff for that target before reading anything else (see [scoping.md](./references/scoping.md)).1314The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this document are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119.html).1516## The Review Loop1718Every review runs the same loop: reset into reviewer mode, scope the change from its diff, assess that diff through the review lenses, classify each finding by severity, report with evidence, and escalate anything too risky to self-approve. The reset is what makes the rest trustworthy — the reviewer inspects what the code _does_, as if someone else wrote it, instead of re-affirming the reasoning that produced it; this matters most in self-review, where the author and reviewer are the same agent. Do not read any code before the reset — it is the first normative step, owned by [scoping.md](./references/scoping.md). The sections below route to the reference that owns each step, ordered as a review applies them.1920## Review Scoping2122See [scoping.md](./references/scoping.md) for:2324- performing the reviewer-mode reset and establishing scope from `git status` / `git diff` / a PR diff25- distinguishing in-scope (the diff) from out-of-scope (pre-existing) code26- reading the full file and every caller/callee around a changed hunk27- checking a change's boundary claims — the neighbours it names as owners — against what those neighbours actually state, not only against the diff28- handling untracked files, an empty or unclear diff, and generated / tool-managed files2930## Severity Classification3132See [severity.md](./references/severity.md) for:3334- the Critical / Major / Minor / Nit definitions, each pairing a merge impact with a defect class35- the fixed severity floors for categories such as committed secrets, missing access control, unsanitized input, and introduced test or lint failures36- mapping severity counts to an Approve / Approve with Nits / Request Changes verdict37- resolving uncertain severity by escalating upward and stating the assumption3839## What to Flag: Review Lenses4041See [review-lenses.md](./references/review-lenses.md) for:4243- the correctness lens: logic errors, edge cases, error and async handling, contract changes44- the maintainability lens: naming, organization, abstraction boundaries, complexity, dead code, scope discipline, and naming what a content-adding change should cut45- the security and privacy lens: secrets, input validation, access control, injection, SSRF, auth, data exposure, supply chain46- the testing and verification lens: coverage, stable test hooks, snapshots, flakiness, manual checks47- the performance and reliability lens: data-access cost, concurrency, caching, asset and bundle weight, failure modes4849## Evidence and Reporting5051See [evidence-and-reporting.md](./references/evidence-and-reporting.md) for:5253- the mandatory `file:line` citation on every finding and quoting the offending code54- diff-style (`-`/`+`) fix snippets for every Critical and Major finding55- the exact review-report section order, from Summary through Recommended Actions56- what counts as evidence versus assertion, and how to mark findings the reviewer could not verify57- what the author's own account of a change — a verification table, self-authored acceptance criteria, a disclosed figure — does and does not establish5859## Review Tone6061See [tone.md](./references/tone.md) for:6263- addressing the code, not the author, and stating the concrete risk behind each finding64- acknowledging real strengths without inflating trivial ones, an obligation the internal review report carries and a posted review's summary does not65- keeping style and preference out of blocking severities66- flagging assumptions explicitly and leaving human-authored copy to its authors6768## Escalation and Decisions6970See [escalation.md](./references/escalation.md) for:7172- keeping the review reporting-only — no code mutation, no delegating the review away73- making each fix trivially applicable and pairing it with its verification step74- escalating high-risk changes to an external gate instead of self-approving them75- deferring genuine trade-offs back to the caller as enumerated Decision-needed entries7677## Posted and CI Reviews7879See [posted-review-policy.md](./references/posted-review-policy.md) for:8081- when a review is _posted_ to a pull request (a CI reviewer or a managed review product) rather than kept as internal self-review82- collapsing the internal four-tier triage to a two-label Important / Nit report with a one-line tally83- running the repository's mandatory checks and honoring its do-not-report exclusions, built as an enumerated list of checks each coextensive with the finding it silences, never a blanket "anything CI enforces" clause84- posting the whole review as one submission of the platform's review mechanism able to carry diff-anchored comments, chosen before the diff is read, with the submission's verdict kept non-gating for an advisory reviewer as a separate decision that never falls back to a looser container85- keeping the summary to a closed three-entry allowlist — the tally, what could not be checked, and a finding with no line to anchor to — with one exception for a host-mandated per-round enumeration