docs: Phase 2 design — local whole-codebase review

[smartwatermelon]

Summary

• Design doc for replacing all remote AI review (Claude CI + Sentry/Seer) with local whole-codebase review
• New --mode=codebase in run-review.sh gives agents full tool access (Read, Grep, Glob) at pre-push
• Non-blocking findings auto-filed as GitHub issues via shared library extracted from pre-merge-review.sh
• Based on analysis of what Seer catches across repos that local review currently misses

Test plan

• Design review and approval
• Implementation follows in subsequent PRs

🤖 Generated with Claude Code

[claude]

PR Review: Phase 2 Design — Local Whole-Codebase Review

Scope: Documentation-only addition of docs/plans/2026-04-02-phase2-local-whole-codebase-review.md. No code, hooks, or scripts are changed in this PR.

---

Summary

This design doc is well-structured and internally consistent. The motivation is clearly grounded in observed evidence (the table of Seer-caught bug classes), and the implementation plan follows a safe, step-by-step approach with validation before cutover. No code regressions are possible from this change.

---

Observations (non-blocking, ordered by maintenance impact)

1. Step 1 validation claim is circular without a test harness reference
docs/plans/2026-04-02-phase2-local-whole-codebase-review.md (Implementation plan, Step 1): "Verify pre-merge behavior unchanged" — this implies a test or replay mechanism exists for pre-merge-review.sh, but none is referenced. If the shared library extraction (Step 1) is validated only by manual observation, a silent behavioral change could pass through. Consider linking to or defining the validation method before implementation begins.

2. Parallelism section omits failure handling semantics
Lines in the "Parallelism" section describe both modes running in parallel and "Both must pass for push to proceed," but the plan doesn't address what happens when one finishes and BLOCKs before the other completes — should the second be cancelled, or always run to completion to collect all findings? Clarifying this will affect how run-review.sh and the pre-push hook are wired together.

3. "Ralph burndown" is undefined for future readers
The Risks table references "Ralph burndown" as the mitigation for non-blocking issue spam. This term is not explained anywhere in the doc. Adding a one-line note (e.g., "Ralph: the automated issue-triage agent") would make the doc self-contained.

4. --mode=full-diff described as existing but not yet documented in this plan's scope
The pipeline diagram shows run-review.sh --mode=full-diff as pre-existing, but the parent plan (2026-03-25-infrastructure-consolidation-design.md) is referenced without confirming that mode already exists in the live infrastructure. If that mode is also aspirational or partially implemented, Step 3's parallel wiring could arrive before its dependency. Worth a note clarifying current implementation status.

5. Minor: success criterion 5 cost estimate uses a range with no baseline
"~$0.001-0.005/PR" is a rough estimate with no source or comparison point. Not blocking, but future-Andrew will find it useful to know what the current CI cost is to measure against.

---

Security / Reliability

No concerns. This is a design document with no executable content.

VERDICT: PASS