/meta-reviewing-reviewing

name: meta-reviewing-reviewing
description: Code review process — reading order, findings, severity, scope and the approval decision. Load when reviewing a diff or PR, writing review comments, or deciding whether to approve or request changes.

Code Review Patterns

> **Quick Guide:** A review is worth what the author can act on. Start from the diff's purpose and the file the change is about — a design problem there leads the review. Must Fix blocks approval, Should Fix is strongly recommended, Nice to Have is optional, and a finding that earns none of them is not written. Approve once the diff meets its success criteria and leaves the code healthier, even where you would have written it differently; a clean diff gets a short approval, not manufactured findings.

**Detailed Resources:**

• [examples/core.md](examples/core.md) — worked findings, labelled and unlabelled reviews, scope findings, the verdict's evidence, praise, a complete clean review
• [reference.md](reference.md) — inline comment prefixes, and how verdicts map to platform review states

---

<critical_requirements>

Before writing a review

**Read the specification or description, then every changed file in full, before writing a finding.** The purpose is the yardstick for every judgement, and a finding written from a partial read is about code you have not seen — the check it calls missing may sit below where you stopped.

**Judge the diff against its purpose, not against an ideal codebase.** The author can act on what the diff changed; a finding about code it never touched, or a refactor its specification never asked for, is work with nothing to weigh it against.

**Give every finding a `file:line`, the consequence of leaving it, and a fix pointed at an existing pattern where the codebase has one.** Without the location the author hunts for it, and without the consequence it is weighed as a style note.

**Label every finding with its severity, and leave unwritten anything that earns none.** Authors read unlabelled comments as mandatory, so a typo and an injection compete for the same round and the trivia buries the defect.

**Verify each success criterion with evidence before approving.** An approval that skips them ratifies whatever shipped.

**Name what the diff did well, specifically and sincerely.** Praise that names a choice tells the author what to repeat; generic praise tells them nothing and reads as filler.

</critical_requirements>

---

**Auto-detection:** code review, review this PR, pull request review, review the diff, review comments, approve, request changes, LGTM, nit:, blocking, non-blocking, Must Fix, Should Fix, Nice to Have, success criteria, review verdict

**Applies to:**

• Reviewing a diff, PR or implementation against the specification it was written to
• Deciding a finding's severity, and whether it is worth writing at all
• Writing a finding the author can act on without a follow-up question
• Choosing between approve, request changes and major revisions

**Handled elsewhere:**

• What to look for in a particular kind of code — UI, server, command-line, model-calling or infrastructure — is a domain checklist's job; this skill decides what a finding from any of them is worth and how it is written
• Formatting, lint and type errors belong to the tools that report them; a review restating their output spends the author's attention on what a machine already said
• Auditing a whole system's security — threat models, penetration tests — is its own discipline; this skill catches the security defects a diff makes visible
• Choosing an architecture before code exists — a review judges the design a diff carries, not the one a specification should have picked

---

<philosophy>

Philosophy

Review exists to keep a codebase getting healthier while changes keep landing. That makes it a filter rather than an audit: the bar is "better than before and fit for its purpose", not "how I would have written it", because there is no perfect code to hold a diff against — only better code.

**Restraint is part of rigour.** A speculative suggestion sends the author to work the diff's purpose never asked for, as surely as a missed bug does by omission, and a review padded with them teaches authors to skim the next one, including the finding that matters.

**The reviewer reports; the author fixes.** A finding names the problem and the pattern that solves it, and leaves the change to the person who owns it.

</philosophy>

---

<decision_framework>

How severe is this finding?

Would leaving it break something — behaviour, security, data, a critical
performance problem, a required success criterion, or a major violation of
a convention the codebase documents?
├─ YES → MUST FIX
└─ NO → Is it an improvement to code the diff changed?
    ├─ YES → Is the churn worth the diff's purpose, and does the spec ask for it?
    │   ├─ NO to either → don't write it
    │   └─ YES → Is the cost concrete in this diff — an edge case the spec implies,
    │            a swallowed error, a measured slowdown, a minor convention
    │            deviation in the changed lines?
    │       ├─ YES → SHOULD FIX
    │       └─ NO → don't write it — it is speculation
    └─ NO → Is it an addition the author may take or leave — tests beyond adequate,
            documentation, a clearly labelled future idea?
        ├─ YES → NICE TO HAVE
        └─ NO → don't write it — a style preference, a wording preference, an unrequested refactor

Which verdict

**Approve** — every success criterion met with evidence, tests adequate for what changed and passing, no Must Fix. Should Fix findings travel with the approval as strong recommendations, and a minor one can become a follow-up rather than hold the merge.

**Request changes** — a Must Fix exists, or a criterion is unmet or unevidenced, and the fixes are patches to this approach.

**Major revisions needed** — the approach itself has to change: a design that cannot meet the s