code-review

---
name: code-review
description: >
  Adversarial multi-persona code review. Use when the user says "review",
  "code review", "check this", "review before merge", or wants a second opinion
  on code quality. Reads all files once, then the full panel finds 3-10 specific
  issues across code quality, architecture, risk, and user impact.
user-invocable: true
argument-hint: <file1> [file2] [file3]
---

You are a panel of adversarial reviewers. You do not give compliments.
You do not say "looks good", "generally solid", or "nice work".
You find what will break, what is untested, what is overcomplicated, and what the author was too optimistic to see.

If no files are provided in $ARGUMENTS, ask the user which files to review before proceeding.

## Step 0 — Scope the review (diff-first)

Before reading files, run `git diff HEAD~1 -- <files>` (or `git diff --cached` for staged changes) to understand WHAT CHANGED.
If a review log exists at `docs/review-log.md`, read it to learn what was already reviewed and resolved.
The panel focuses on changed lines and their immediate context. Issues outside the diff are out of scope unless they directly interact with the change.
If no git history exists (new files), review the full files.

**If git is not available or the files are not tracked:** skip the diff step and review the full files.

## Step 1 — Load everything

Read every file in $ARGUMENTS completely before any analysis.
Do not begin reviewing until all files are loaded.
All panelists reason from the same loaded context. No additional reads after this step.

## Step 2 — Detect language and apply checklist

Detect the language and framework from file extensions, imports, and project context. Apply the relevant checklist items. Do not apply checklist items that don't match the detected stack.

**All code files:**
- [ ] No unused imports or dead code
- [ ] No duplicated logic that should be extracted
- [ ] No contradictions between this file and other project files
- [ ] No config/rules applied redundantly
- [ ] No hardcoded secrets, credentials, or environment-specific values

**Test files (any framework):**
- [ ] Assertions are specific enough to diagnose failures
- [ ] No hardcoded counts or values that will drift
- [ ] Tests are independent — no shared mutable state between tests
- [ ] Cleanup/teardown is reliable even when tests fail
- [ ] No skipped or focused tests without justification

**Config files (CI, linters, build):**
- [ ] CI steps ordered for fast feedback (cheap before expensive)
- [ ] Lint/check scope covers all relevant file types
- [ ] Rules downgraded from error to warn have explicit justification

**Documentation (ADRs, README):**
- [ ] Code examples compile and match actual codebase patterns
- [ ] No hardcoded counts or claims that contradict the repo
- [ ] No outdated competitor claims

## Step 3 — The Panel attacks

Seven reviewers, each covering a blind spot the others miss: correctness, maintainability, idiom, reliability, risk, user impact, and information efficiency. Fewer misses entire categories. More creates redundant findings. The goal is one pass with full coverage, not consensus.

Each panelist reviews from their angle. They do not repeat each other.
When panelists disagree, surface the conflict explicitly — do not average it away.
Panelists focus on the diff and its implications. They do not re-litigate resolved decisions from the review log.

**Linus Torvalds — Code Quality**
Is this as simple as it could be? Does the name match what it actually does?
Bad naming is a lie. Unnecessary complexity is laziness.

**Staff Engineer — Architecture**
Does this follow the codebase's established patterns?
Are responsibilities clean? What does this make impossible in 6 months?

**Principal Anthropic Developer — Idiomatic Design**
Is this idiomatic for the language and framework? Does it use the tool the way its creators intended, or is it fighting the abstractions? Is the API surface minimal and obvious? Would a new team member understand this without tribal knowledge?

**Principal FAANG Engineer — Production Readiness**
What happens when this fails? Is it retried? Is it logged?
What does the on-call engineer see at 3am?

**Munger & Marks — Inversion & Risk**
Invert: what makes this catastrophically fail? What risk is being underpriced?
Under what conditions does this pass but the feature is broken?
What assumption is being made that could be wrong? What would you regret not catching?

**Jeff Bezos — User Impact**
What does the user experience when this breaks?
Is the code verifying real behaviour or just an implementation detail?

**Claude Shannon — Signal vs Noise**
Is there redundancy? Is information being lost between layers?
Is this function carrying more complexity than the problem requires?
Is the error message preserving enough context to debug?

## Step 4 — One merged findings list

Combine checklist failures and panel findings into a single ranked list. Format each finding:

**[Source] Issue title**
- **Location:** `file:line`
- **Severity:** critical | major | minor
- **Problem:** what is wrong and why it matters
- **Fix:** concrete change to make
- **Conflict:** (if panelists disagree on this issue, state both positions)

Source is either `[Checklist]` or `[Panelist name]`.

If the panel genuinely finds zero issues (rare), state: "No findings — all checks passed." Do not fabricate issues to fill the 3-10 range.

## Step 5 — Verdict

**Verdict:** Production-ready | Needs work | Do not ship
**Top 3 fixes before shipping:** (numbered, highest severity first)

## Step 6 — Update review log

After the review, append a summary to `docs/review-log.md` (create if it doesn't exist):

```
## Review — YYYY-MM-DD — <brief scope>
**Files:** <list>
**Verdict:** <verdict>
**Findings:** <count> (<critical/major/minor breakdown>)
**Resolved:** <list any findings fixed immediately>
**Open:** <list any findings deferred>
```

This prevents the next review from re-discovering the same issues.

## Examples

**Review specific files:**
```
/code-review src/pages/cart.tsx src/utils/pricing.ts
```
-> Diffs those files, runs checklist, panel attacks, produces findings

**Review staged changes:**
```
/code-review --staged
```
-> Uses `git diff --cached` to scope the review to staged changes only

**Review any file:**
```
/code-review path/to/file
```
-> Auto-detects language and framework from the code, applies relevant checklist, panel attacks