review-pr

name	review-pr
description	Critically review an aireceipts PR as an independent critic — different model family from whoever built it, audits test-diffs hardest. Use when asked to review a PR, act as the critic, or before landing/merging any change.
trigger	/review-pr

/review-pr — the critic (generator ≠ critic)

Run as a different model family from whoever built the PR — a same-family review tends to share the same blind spots as the build. For coding PRs the default critic is Codex at deep reasoning effort (maintainer directive 2026-07-02): invoke codex exec --sandbox read-only -C <repo> "<review brief>" with sections 1–4 below as the brief; Codex's reasoning effort should be its deepest available tier for anything touching src/. If you built the PR yourself, say so explicitly and flag the review as same-author, not independent — then STILL run the Codex pass before merge.

No coding PR merges without a recorded deep review. The review (or its verdict summary) is pasted into the PR as a comment so the record survives.

1. Run the gates yourself, silently, unmasked

Don't trust the PR description's claim that gates pass. Re-run:

npx tsc --noEmit;                    echo $?
npx eslint . --max-warnings 0;       echo $?
npx vitest run;                      echo $?
node scripts/verify-goldens.mjs;     echo $?

Any non-zero exit is a blocking finding, full stop — cite the failing command's output.

2. Audit the test diff hardest

The most common way a PR fakes green is by weakening the assertion, not fixing the bug. For every test file touched:

• Was an assertion loosened, deleted, or replaced with toBeDefined()/expect(true)?
• Does a new test actually exercise the changed code path, or just import it?
• For a golden change: is the new golden byte-diffed and explained in the PR, or silently regenerated?

3. Check the invariants (AGENTS.md)

I1 (no model/network calls slipped into the product path), I2 (no fallback price / fabricated $), I3 (new numbers traceable / cited), I5 (goldens intentionally updated), I6 (no ranking language snuck into receipt copy).

4. Scope discipline

Does the diff match the spec it claims to implement — nothing more? Flag unrelated refactors, drive-by renames, or scope creep as separate findings, not blockers unless they're risky. Docs staleness is a finding: if the diff changes user-visible behavior (flags, commands, output) and no README/docs/help update rides in the PR, flag it (build-spec 6.5 requires docs in the same PR).

4.5 Redundancy audit

Does the diff duplicate an existing primitive (search before you approve — same logic under a new name is the tell)? Single-use helpers, dead code, speculative abstraction = findings. The question is always: could this diff be smaller and reuse more?

4.6 Description conformance

The PR description must follow the template: missing "What to review" ordering, or a user-visible change without a "See it" output capture, is a finding — a PR you can't evaluate in 30 seconds wastes every reviewer after you.

4.7 Record the review (unblocks gh pr create)

On completion write the reviewed sha: git rev-parse HEAD > .review-ok (untracked; any new commit invalidates it — re-review the delta). The PreToolUse hook BLOCKS gh pr create without it.

5. Post the review

State pass/fail per section above with specifics (file:line), not a vague "looks good." If everything is clean, say that plainly too — a review with nothing to add is a valid outcome, don't invent nitpicks to seem thorough.