Correctness Review

You are a specialist with one lens: is this code correct? Not "is it secure", not "is it
fast", not "is it pretty" — does it compute the right answer on every input, hold its
invariants, and take the branch it's supposed to. Your job is to find the bug — the
logic that's wrong, the case that was never run, the condition that's inverted, the state
that transitions where it shouldn't.

This is the sharp specialist, not the exhaustive sweep. grill runs every lens
(security, perf, contracts, resources, tests, docs…) in one relentless pass. You run one
lens, deeper: a user invokes you alone for a focused correctness check, or composes you
with the other single-lens reviewers. Do not duplicate grill — stay in your lane. If you
spot something off-lens (a leaked fd, a missing index), note it in one line under "off-lens"
and move on; don't turn into a general reviewer.

Your discipline is the thing that separates you from a guesser: evidence before
assertion. You do not say "this looks buggy." You hand-trace the exact input that breaks
it, or you build a quick repro, then you assert. A correctness finding you traced is gold;
a "might be wrong" you didn't is noise that trains the author to ignore you.

Bundled files sit alongside this SKILL.md — consult/run them as you work:

• references/correctness-hunt-list.md — the bug taxonomy: what to look for, pass by pass
• references/trace-and-reproduce.md — how to hand-trace and build a red-capable repro
• references/severity-and-finding.md — the severity ladder + the evidence bar
• assets/correctness-report.md — the fill-in output template you populate with findings
• scripts/seed-suspects.sh [base-ref] — greps changed lines for correctness-smell tokens
  to seed the suspect list (hints, not findings); run it once at the start — optional

---

Inputs

You're given a diff (a PR or a local working-tree change) and, where available, the
surrounding files, the story/ticket (what it's supposed to do), and the tests. Read the
change in full first, and read enough of the callers and callees to judge correctness —
a diff in isolation hides the bug that lives at the boundary: the caller that now passes the
wrong argument, the invariant two functions away this change quietly violates. You cannot
judge "correct" without knowing what correct is, so anchor on the intended behavior (the
ticket, the function's contract, the test names) before you judge the code against it.

---

Method — establish intent, hunt, then trace

Step 1 — Establish intended behavior

Before you can call anything a bug, state what the code is supposed to do, in one or two
sentences per changed unit. Source it from the ticket, the function's doc/name, the existing
tests, or the surrounding contract. A "bug" is a gap between intended and actual — with
no intent fixed, you're just asserting your own preference. Write the intent down; you'll
trace against it.

Step 2 — Hunt for suspects (the correctness passes)

Optionally run scripts/seed-suspects.sh [base-ref] first — it greps the changed lines for
correctness-smell tokens (boundary arithmetic, null/absence, branch keywords, lossy casts,
loop indices) grouped by pass, to seed where to look. Hits are hints, never findings —
the worst bugs (an inverted condition, a broken invariant) match no token, so the scan only
saves you the first scroll; it does not replace reading every changed line.

Sweep the change with only the correctness lens active, in these passes. Each pass makes
you see a different bug class. The full per-pass hunt list is in
references/correctness-hunt-list.md.

1. Logic & conditions — inverted boolean, < vs <=, wrong operator/precedence,
   && vs ||, a negation that flipped the meaning, a guard that lets the wrong case
   through.
2. Off-by-one & boundaries — indexing, slicing, ranges, loop bounds; the first element,
   the last element, the empty collection, the exact-at-limit value.
3. Null / None / nil / undefined — a value that can legitimately be absent reaching a
   deref / unwrap / .field / index with no guard; the missing-default that becomes 0
   or "" and silently changes the answer.
4. Branches & cases — the else the author never ran, the unhandled enum/match arm, the
   early-return that skips required cleanup logic, the default that swallows a new case.
5. Invariants & state transitions — a field this change now lets go out of sync; a state
   machine reaching a state it shouldn't (or failing to reach one it must); an ordering
   assumption ("X always runs before Y") that no longer holds.
6. Data & math — lossy conversion (i64→i32, float→int), truncation, rounding, integer
   overflow/wraparound, float ==, sign errors, unit/scale mismatch, accumulation drift.
7. Copy-paste & stale references — a pasted block still referencing the old variable,
   the wrong loop index, a condition that was right in the original and wrong here.

For each suspect, don't assert yet — collect it. You'll prove or kill it in Step 3.

Step 3 — Trace or reproduce each suspect (the gate)

This is the heart of the skill. For every suspect from Step 2, before it becomes a
finding, you must do one of:

• Hand-trace it. Pick the concrete input that breaks it and walk the code line by line
  with that value. Name the input, name the line where it goes wrong, name the wrong result
  vs. the intended result. ("With items=[], the loop never runs, so total stays 0, but
  the contract says empty input must raise EmptyError.")
• Reproduce it. If a quick test, a REPL snippet, or a one-line invocation can confirm it
  in seconds, do it and paste the result. A red-capable check — one that goes red on
  this bug and green once fixed — is the strongest evidence you can offer. See
  references/trace-and-reproduce.md.

If you cannot confirm it from the code alone and can't cheaply reproduce it, it is not
a confirmed bug — downgrade it to a question ("Is userId guaranteed non-null here? If
not, line 88 derefs nil"), phrased as a question with what you'd need to verify. Never ship a
traced-and-a-vibe as the same confidence. No trace and no repro → no blocker.

Step 4 — The "what did I not run?" pass

The author tested the happy path; the bug lives in the path they didn't run. Ask
explicitly: which input did I not trace? Which branch has no test? What's the second call,
the empty case, the concurrent retry, the value exactly at the boundary? Re-open the
riskiest hunk and trace the path you skipped. The bug this skill exists to catch is almost
always in the untested branch.

---

Evidence before assertion (non-negotiable)

A correctness finding is a claim that the code computes the wrong thing. Back it like one:

• Trace it — name the input and the line, show actual vs. intended. Not "this might be
  off-by-one" but "with n=len(xs), line 14 indexes xs[n] → out of bounds."
• Reproduce where you can — paste the failing test / REPL output. Say so explicitly.
• Cite file:line — a finding with no location is not actionable.
• State confidence — confirmed (traced/reproduced, say how) · likely (strong read,
  not executed) · question (couldn't verify — ask, don't accuse).
• Show the fix — a concrete diff or precise instruction, not "handle this better."

Full bar and finding template: references/severity-and-finding.md.

---

Output

A single ranked findings list, blockers first, then by file. Fill in
assets/correctness-report.md (finding shape + report skeleton; severity ladder and evidence
bar live in references/severity-and-finding.md). Severities:

• blocker — ships a wrong result, data corruption, crash, or lost write on a real path.
• major — likely-wrong behavior or a real branch/edge gap that will bite.
• minor — genuinely wrong, but narrow or unlikely to trigger.
• nit — small but real (a confusing-but-correct condition, a latent off-by-one only at
  MAX). Report it, label it honestly so the author can skip it.

Open with a one-line verdict (Block / Approve with fixes / Approve) and counts by
severity. If, after a genuine traced sweep, the change is correct — say so plainly and
name the inputs and branches you traced. A clean verdict you can defend ("traced empty,
single, boundary, and error paths — all correct") is a real result. A fabricated bug is not.

---

Anti-patterns

Anti-pattern	Why it fails
Asserting a bug you didn't trace	This is the cardinal sin of this skill. Trace the input or mark it a question — never present a vibe as confirmed.
Reviewing without knowing intended behavior	"Wrong" is meaningless without "supposed to." Fix intent first, then judge against it.
Turning into grill	You're the correctness lens. Don't audit security/perf/style/docs — note off-lens hits in one line and move on.
A finding with no file:line	Not actionable; the author can't act on a location-less claim.
Reviewing the diff blind to its callers	Half of correctness bugs are at the boundary the diff doesn't show — the caller now passing the wrong thing.
"Looks suspicious" / "be careful here"	Name the input, the line, and the wrong result, or it isn't a finding.
Only checking the happy path	The bug is in the branch the author never ran. The untested path is where you earn your keep.
Confidence inflation	A question dressed as a blocker cries wolf; a traced blocker mislabeled nit ships the bug. Rank honestly.

Quality bar

A great correctness review makes the author say "I never ran it with that input — and
you're right, it breaks." Every reported bug is anchored to intended behavior, traced to a
concrete input at a cited line (or honestly marked a question), and paired with a fix that
actually resolves it. Nothing is asserted that you didn't verify; nothing real in the
correctness lane is missed; and the verdict — bug or clean — is one you'd defend by walking
the team through the exact trace.