/deadeye-review

name: deadeye-review
description: Four-lens self-review (over-engineering, correctness, performance, security) of the working diff, or the whole repo with --repo.
license: MIT
argument-hint: "[--repo]"

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

Deadeye Review

Review code through four lenses — over-engineering, correctness, performance, security — the same rubric `/deadeye-pr` runs on a whole pull request, scoped instead to your working diff or the whole repo. This is the local, pre-PR self-review: catch what would otherwise wait for a PR (or a bot) to find. For a real GitHub PR, use `/deadeye-pr`. For a deeper, dependency-audit-backed security-only pass, use `/deadeye-guard`.

Two scopes:

• **default** — the current working diff.
• **`--repo`** (or "audit the whole repo") — the entire repository, ranked

worst-first across all four lenses. See "Whole-repo mode" below.

Scope (default: the working diff)

Get the diff with `git diff` (or `git diff --staged` if the user says staged, or `git diff <ref>` for a named base). 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.

• Empty diff (nothing changed or staged): say so plainly and stop — do

not substitute a different scope.

• Not a git repo: say so plainly and stop -- same as an empty diff above.

Before tagging `yagni:`/`delete:`, or claiming an `authz`/nil/sanitizer check is MISSING, grep for implementers/callers/guards OUTSIDE the diff — an "interface with one impl" whose second impl lives in a test file, or a guard that lives one call down, is a false positive. Report only what you confirmed.

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?
• **The diff's own claims are claims.** A comment ("no-op if absent, still valid", "safe because of the guarantee above"), 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 a list aliased from a default arg, shared config, or module cache; every `await` — can it never resolve, and does pre-await state still hold after (abort, concurrent completion)?
• **Sweep the cheap layer:** dead scaffolding, unused imports, placeholder secrets, unpinned deps, a `default:` giving a CPU host a GPU image; a test that mocks its own unit proves nothing.

The four lenses

The tags classify what a trace found; they are not the search. For each value the diff creates, re-derives, or newly trusts: name who chooses it, follow it to every consumer (indexed, parsed, compiled, matched, loaded, allocated), and at each ask what the worst chooser sends — absent, empty, the wrong type that still passes the check, the costliest to process. What breaks there is the finding, on the standard line with its tag. Four lenses are four questions at each consumer of ONE trace, not four passes; a clean footer names the value traced and the consumer that survived — "no known sink in the diff" earns nothing.

One line per finding, ranked most-severe first within each lens:

Each finding is one comment — write