Tier 3: Opus Review (Deep Analysis)

PLANNING MODE ONLY: You are a REVIEWER. Do NOT write code, do NOT edit files, do NOT make commits. Your ONLY job is to verify and report findings.

Thorough architectural review. Catches what surface + logic tiers missed.

Completion Promise: <promise>OPUS_PASS</promise>

Checklist

Architectural Fit

• Implementation fits existing architecture
• No architectural violations (patterns, layers, boundaries)
• Integrates cleanly with existing code
• No tight coupling introduced

Standards Compliance

• Fail Fast followed (no silent fails)
• No hardcoded settings (config externalised)
• Modularity standards met
• LMCE principles applied (Lean, Mean, Clean, Effective)

Governance — Checkbox drift audit (two-tier cadence)

If the project uses a governance MCP server for checkboxes, fetch the issue body and parse the ## Proof of Work section — the canonical audit signal. Cross-reference entry order + evidence against the branch's git log --oneline.

Do NOT use timeline edited events — GitHub suppresses issue-author's own body edits from the timeline, so PATCH-event-based cadence audits false-negative honored invocations.

Classify every checked [x] box into one of two tiers:

• Tier A — Phase-deliverable checkboxes (items in Acceptance Criteria Phases 1..N that describe a concrete deliverable — a file edit, commit, function, section, table, example): STRICT interleaving required. Proof of Work entry order MUST interleave with git-log commit order within the same phase's commit window. Cluster-at-end (many entries appended after all phase commits) = FAIL.
• Tier B — Cross-cutting meta-checkboxes (Triple-Check Gate items, Engineering Requirements meta, Cleanup Requirements, Verification Loop self-gates, PREREQUISITE CHECKPOINT, Expected Behavior post-conditions): batched-at-end acceptable because these describe conditions observable ONLY after all phases complete.

Classification heuristic: if the checkbox text names a specific file / commit / section / function to build or change, it is Tier A. If it describes a cross-cutting condition requiring the WHOLE implementation to evaluate, it is Tier B. When in doubt, inspect the section: Acceptance Criteria sub-phases = Tier A; Triple-Check Gate / Engineering Requirements meta / Cleanup Requirements = Tier B.

• Flag Tier A batching against commit cadence as governance drift.
• Do NOT flag Tier B batching as drift. A Tier-B-only batch is acceptable and expected.
• If no governance MCP is available: inspect the branch git log and the issue body directly. Any phase-complete-but-still-unchecked Tier A box = drift.

Standards — Five Common Culprits (Architectural)

Deep analysis for the five recurring patterns across the entire implementation. Categories canonical in references/_common-culprits.md.

1. Duplicate Code (DRY / Modularity)

• No architectural duplication (same pattern implemented differently in multiple places)
• No cross-file copy-paste (same logic in src/ and tests/)
• Should there be a shared module for this?
• Does the implementation reuse existing utilities?

2. On-the-fly Calculations (Hardcoded Settings)

• No business formulas embedded in application code
• No calculation constants that vary by environment / strategy
• Are all formulas that could change externalised to config?
• Config-first architecture followed (YAML co-located with code)

3. Hardcoded Settings

• No settings that should be user-configurable
• No environment-specific values in code
• Would changing this value require a code change?
• All thresholds, limits, multipliers in config files

4. Obsolete / Dead Code (LMCE)

• No modules / classes that are never instantiated
• No API endpoints that are never called
• No database migrations for deleted tables
• If commented "for backwards compatibility" — is it actually needed?
• Implementation is Lean, Mean, Clean, Effective

5. Silent Fallbacks (Fail Fast)

• No cascading defaults that mask root cause
• No graceful degradation that hides broken dependencies
• If this fails, will the user / operator know immediately?
• Errors are unmistakable

6. Cross-Boundary Contract Audit — Architectural

Deep cross-system reasoning. Every value that crosses a boundary (code↔DB, code↔config, caller↔callee, backend↔frontend) must have its contract verified.

SQL / Schema Architecture

• All SQL in the diff audited against DB schema (migrations/) — column names, types, table existence verified
• No query references a column that was renamed, removed, or never added in any migration
• SQL literal values (strings in WHERE) cross-referenced against actual DB data — check for normalisation mismatches

Null Propagation Architecture

• Null propagation audit: identify all values that can be None (DB results, API responses, optional config) and trace to consumption points — all arithmetic / method-call / formatting sites guarded?
• Frontend data contract: for each API response field displayed in UI, verify null / undefined handled at display layer
• Type annotations match reality: if annotation says float, verify NO caller can pass None. If they can → annotation must be float | None and function must guard.

Config Architecture (Bidirectional)

• Bidirectional config audit: (1) code has no hardcoded values → config exists; (2) config has no orphaned keys → code consumes them
• Any config section added for a new feature: verify the feature's code reads EVERY key in that section

Lifecycle Wiring Architecture

• Lifecycle wiring audit: for each recovery / drain / replay function, map ALL lifecycle events where data could accumulate, confirm function wired to every one
• "Called at reconnect" does NOT imply "called at startup" — verify separately
• Scope completeness: enumerate every Acceptance Criteria checkbox from issue body — each must map to a specific file:line. Any without = NOT DONE.

Data Correction Architecture

• If bug produced bad DB state: fix includes BOTH (a) code fix for future AND (b) verified data repair for existing rows — confirm both exist and repair scope matches bug scope

7. Factual Claims Audit

• Enumerate all numeric assertions in documentation, issue body, and design rationale added by this implementation
• For each: does a verifiable source exist (benchmark, prior issue, measured observation, command output)?
• If no source: flag as unverified claim — must be sourced or removed before OPUS_PASS

Required when available: Code Graph Checks (Architectural Depth)

Documentation-only PR exemption: if the diff touches ONLY Markdown / YAML / JSON / TOML / shell, skip this section. Document [GRAPH: skipped — doc-only PR] in RESULT and return PASS.

Preconditions: a code-review-graph MCP server is available, graph fresh. If either fails, skip to fallback clause below.

• Dead code detection: graph query large_functions(min_lines=200) + manual orphan scan. For each candidate: graph query callers_of(<name>) returns empty in the same module ⇒ orphan. Subagent confirms whether call is via reflection / dynamic dispatch (not an orphan) or a true orphan (cleanup target).
• Impact scope validation: graph query impact(changed_files, max_depth=2) — enumerate all impacted modules; identify unexpected cross-boundary edges. Document each boundary: intended (defence-in-depth / architectural layering) vs emergent (refactor target).
• Large functions audit: confirm no function in the diff exceeded 200 lines. If any did, architectural red flag — require refactor.
• Full graph-tool discipline: any "no results" response from graph in an area with unsupported-language files (YAML, JSON, SQL, shell) is explicitly broadened to subagent exploration before drawing a negative conclusion; graph last_updated timestamp recorded in RESULT.

Fallback clause (retry-aware, evidence-required): first graph call fails → retry once. Second fails → RESULT MUST include the Explore-subagent's RESULT block demonstrating manual architectural audit (dead code + impact + large functions) was actually executed. Documenting only [graph unavailable] without subagent RESULT = silent skip = FAIL.

Overengineering Check

• Is there a simpler solution that works?
• No premature abstractions
• No unnecessary complexity
• JBGE (Just Barely Good Enough) applied

Pre-Merge Precondition Check (NOT the post-merge user review)

CRITICAL: This tier runs BEFORE merge. The items below are pre-merge preconditions. They are NOT the user review itself — that happens after merge.

• All Expected Behavior items verified
• All Acceptance Criteria items verified
• Engineering Requirements met
• Cleanup Requirements completed

Final Verification

• All commits pushed to remote
• Branch up to date with main (verified by reading git log origin/main..HEAD — do NOT run git fetch from this PLANNING-mode review)
• No merge conflicts (verified via git merge-tree or main-agent diff inspection)
• Ready for merge + post-merge user review

Bash Output Discipline

• If you ran any bash command producing > 200 lines: capture to a file, report only the path + verdict in RESULT. Do NOT paste the full output back to the main agent.

Pass Criteria

All checkboxes verified with evidence. Graph was available and not used for a structural architectural question = FAIL. If doc-only PR exempted via the short-circuit: PASS. Unavailable / stale / unsupported-language WITH subagent RESULT block showing manual architectural audit was performed: PASS. Architectural review without structural evidence (graph-backed OR subagent-backed with RESULT block) is incomplete and fails.

On Pass

Output: <promise>OPUS_PASS</promise>

On Fail

1. List architectural concern with specific file:line
2. Explain why it violates standards
3. Suggest a specific fix
4. Do NOT output the promise
5. Ralph loop restarts from Tier 1 after fixes