/deadeye-pr

name: deadeye-pr
description: PR review across four lenses -- over-engineering, correctness, performance, security -- printed locally, opt-in to post.
license: MIT
argument-hint: "[<PR number or URL>] [--post]"

<!-- deadeye-pr: canonical rubric; edit internal/prreview/ruleset.md, the skill and every host rendering are generated from it -->

Deadeye PR Review

One shot over a whole pull request: four lenses, one pass, tagged findings. `/deadeye-review` runs this exact four-lens rubric locally against your working diff or the whole repo — this adds what a PR needs on top: resolving a real PR via `gh`, checking what other reviewers already said, huge-PR fan-out, and an opt-in post back to GitHub. `/deadeye-guard` stays the dedicated deep-security pass this lens is drawn from.

Scope

Resolve the target PR, then review only its diff:

• An argument (a PR number like `123` or a full PR URL, after stripping

`--post`) → that PR.

• No argument → the PR for the current branch.
• Fetch the diff and metadata with the GitHub CLI:
• `gh pr diff <N>` (or `gh pr diff` for the current branch) for the unified diff.
• `gh pr view <N> --json title,body,additions,deletions,files,baseRefName,headRefName` for the header.
• Read the changed hunks **plus enough surrounding context to judge a trust

boundary or a caller contract** — "is this input validated" and "does this break a caller" both need the code around the hunk, not just the `+` lines.

Preconditions and graceful degradation:

• `gh` not installed or not authenticated → say so plainly and stop, or, if

the user has a local branch, offer to review `git diff <base>...HEAD` instead. Do not invent PR contents.

• Not a GitHub repo / no PR for the branch → say so; don't substitute a

different scope. <!-- claude-only -->

• Huge PR (~40+ changed files or a few thousand lines) → review it ALL: fan

out one subagent per ~2,500-line package cluster, in parallel, each returning findings in the standard format. Cheapest tier that fits, floor tier 1 (sonnet) for real logic — tier 0 only for purely mechanical clusters (generated code, lockfiles, vendored deps, renames), top tier for a risky cluster (auth, crypto, concurrency, raw SQL/shell, money). Verify every finding yourself; never truncate or report partial coverage as complete. One integration pass over the combined findings after — an export removed in one cluster, its only caller in another (`break:`/`contract:`). The Premise judgment happens once, in this integration pass, against the PR's own title/body — never per cluster; one cluster's diff isn't the whole PR.

<!-- /claude-only -->

Verify before reporting

Before claiming a check is MISSING — a sanitizer, an authz guard, a nil-check — grep OUTSIDE the diff and follow the value into the callee: the real guard often lives one call down. An `authz`/bypass claim needs a concrete input that reaches the sink, or drop it; one wrong finding erodes trust in all of them.

**Every finding carries its proof.** Append a `proof:` clause naming the concrete thing in THIS repo that makes the finding true — the caller you traced, the grep that came back empty, the auditor line, the test that fails. A finding you cannot prove from the code in front of you is a guess; drop it. Precision is the product: one finding that's true beats ten maybes, and every hosted reviewer drowns in the maybes — that's the gap you win on.

**Run the repo's own checks and fuse them in.** Run what the project ships — `go vet`, `tsc --noEmit`, the linter, the tests the diff touches — and let their output confirm or kill a FUNCTIONAL finding. Mark it `(confirmed)` when a tool or failing test agrees, else `likely`. You can run the code; a diff-only bot can't.

A `deadeye: <shortcut>. ceiling: <limit>. upgrade: <trigger>.` comment over a hunk is a recorded DECISION, not a finding — someone already chose to ship that corner with eyes open. Count those separately as accepted, don't flag them. Never flag the one runnable check coder mode leaves behind for deletion — lean code without its check is unfinished.

Rigor — where reviews miss

Precision is the floor. Five habits separate a real review from a plausible one:

• **Sweep every instance.** One leak, missing registration, or hollow test → check every sibling, in AND out of the diff. A fix with an unfixed twin is a half-fix — name the twin.
• **Disprove your own mitigation.** "X covers it" isn't a pass until X provably runs on the failing path — an early `return`/guard that fires first makes X moot. For a branch gated on a non-null/present field, read the migration: is old data backfilled? Deferred cleanup (mark now, close on the next acquire/release) covers only what reaches that later step: when a diff drops an owner from a cache/pool/registry, or swaps an eager close for a mark, name the line that closes what the owner still holds, on every return path — a mark honoured only on re-use leaks whatever is never re-used.
• **The diff's own claims are claims.** A comment saying a case is harmless or already guaranteed, an assertion in the PR's tests, or "already validated" upstream is a premise to disprove, not evidence — read the validator it points at. For every new or rewritten condition, list the cases it does NOT take — the else, zero/absent, the default arm, the all-absent and one-present combos of a value merged from several optional sources — and follow one of each into its consumer. The bug lives in the case nobody wrote a test for.
• **The bugs a scan slides past:** two arms handling one value (success/error) should mirror — flag the one missing a capture/close/guard; a rewritten condition must keep every predicate it AND-ed (a dropped `ok &&` re-admits what it rejected); a value can pass `isinstance`/`!= undefined`/`in` yet be wrong (`str` subclass, `null` vs `undefined`, a name that exists but isn't the type you assumed); an error branch returning a nil used later; in-place mutation of