Code Review

Code Review (Adversarial + Receiving)

Reference Files

Read only what's needed:

• references/review-order-and-checkpoints.md — review order, checkpoint discipline; load when starting a review that spans multiple files or needs a checkpointed pass
• references/code-review-heuristics.md — heuristics, pattern recognition, false-positive prevention; load when the diff is non-trivial or before reporting CLEAN (Zero-Finding Halt re-scan)
• references/security-review-checklist.md — security review checklist; load whenever the diff touches auth, input handling, network, secrets, or data access

---

Run ADVERSARIAL when producing findings on a diff; run RECEIVING when acting on findings someone else produced. The Code Smells catalog, AI-Generated Anti-Patterns, Metric Honesty Rule, and Deferred Findings handling below apply in both modes.

Mode: ADVERSARIAL REVIEW

Only report issues with confidence ≥80 — below that, a finding is more likely noise than signal, and noise burns the fix loop's time and trust. Do not inflate a score to smuggle a hunch through; a genuine security hunch goes to the Summary as an open question (see the Security exception under Confidence Scoring). Every finding states category, impact, and why it matters. Present a recommendation, not a menu. Be opinionated.

Signal quality rule: One finding with file:line evidence and a fix is worth more than ten generic observations. Never report a pattern without showing where it lives.

Two-Stage Review

Stage 1: Spec Compliance — Does the code do what the plan/spec asked? Check: phase exit criteria met, interfaces match plan's Consumes/Produces, no scope drift, no missing scenarios.

Stage 2: Code Quality — Is the code well-built? Check: correctness, performance, security, clarity, test coverage.

One-fact rule: On a consequential change, find the one fact the change is safe because of — a caller invariant, a lifecycle assumption, a data-shape contract at a boundary — and spend the review proving that fact instead of enumerating maybes. The fact is usually one a breakage grep will not surface. If you cannot prove it, the change is unreviewed; the maybes you listed do not change that.

Review Order

Top-down (spec → architecture → module → function → line) for first pass. Bottom-up (line → function → module) for detail pass. See references/review-order-and-checkpoints.md.

Severity Classification

Confidence Scoring

Security exception: a security-category finding below 80 confidence is NOT silently dropped. Surface it as an explicit open question in the review Summary (e.g., "Possible auth bypass at file:line — could not confirm exploit path"), not as a finding. The <80 floor drops everything else.

Parallel Review + Router Merge

When code-reviewer and failure-hunter run in parallel (BUILD workflow):

• code-reviewer (Assessment A): correctness, performance, spec compliance. Forms opinion WITHOUT seeing the hunter's scan.
• failure-hunter (Assessment B): silent failure scan using red-flags table. Does NOT see the reviewer's findings.
• Router-owned merge: after both complete, the router writes a merged findings summary into the workflow artifact before verifier handoff. Where both agree → high confidence. Where the hunter caught what the reviewer missed → keep. Where the hunter finding is a false positive → drop with reason. Contradictory verdicts: stricter verdict wins, logged in status_history.

Zero-Finding Halt

Zero findings on a non-trivial change → insufficient depth, not perfect code. Re-scan against heuristics and security checklist before reporting CLEAN.

Code Smells (Fowler Catalog)

Scan for these 16 named smells during review. Each is actionable — not a style preference. "Messy" is not actionable; "Mysterious Name" is. Each smell is a labelled heuristic and always a judgement call ("possible Feature Envy"), never a hard violation:

Repo standards override baseline: if the repo's documented conventions endorse something this baseline would flag, suppress the smell.

AI-Generated Anti-Patterns

Patterns commonly produced by AI code generation — flag with elevated priority:

• Over-eager memoization — useMemo/useCallback/React.memo wrapping everything without profiling evidence
• State duplication — same state stored in 2+ places, kept in sync manually (source of truth unclear)
• Sequential awaits — independent async calls awaited sequentially instead of Promise.all (latency multiplier)
• Over-fetching — fetching full objects (or more than the current view needs) when only one field is needed
• Premature abstraction / speculative generality — interface with single implementation "for future flexibility"
• Configurable when it should be constant — adding options/flags for flexibility nobody asked for
• Defensive coding for impossible states — null checks / over-engineered error handling for scenarios that can't happen (values typed non-nullable, internal code boundaries)
• Test mirrors implementation — test recomputes expected value using the same logic as the code (tautological test)
• Factory overkill — factory pattern for objects with no polymorphism
• Type assertions instead of type guards — as any/as Type instead of narrowing with a runtime check

Metric Honesty Rule

Never fabricate metrics. An LLM reading static source code cannot measure real-world LCP, INP, CLS, memory usage, or runtime performance.

• Can assess from code: algorithmic complexity (O(n) vs O(n²)), N+1 query patterns, obvious hot loops, missing indices
• Cannot assess from code: real-world latency, actual memory pressure, real INP/LCP/CLS values

State what you CAN verify from code. Tag anything else as "potential impact, not measured" — never invent numbers. Recommend specific tools (Lighthouse, profiler, benchmark suite) when runtime measurement is the real answer.