Tier 2: Sonnet Review (Logic Checks)

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.

Medium-depth validation. Catches the issues that surface review misses.

Completion Promise: <promise>SONNET_PASS</promise>

Checklist

Evidence Quality

• Evidence proves completion (not just claims)
• Evidence matches actual work done
• Evidence is verifiable (file paths, commits, outputs)
• No "completed" checkboxes without evidence
• Any quantified claim (counts, ratios, durations, capacities) in documentation or issue body: source identified (command, reference, or calculation). "Seems reasonable" is not a source.
• No numbers copied from other documents without re-verification against current state

Scope Alignment

• PREREQUISITE: Read the full Issue body line-by-line BEFORE this section. Don't skim. Don't trust prior summaries. The Issue body is the source of truth for scope.
• Implementation matches Issue scope exactly
• No scope drift (adding unrequested features)
• No scope shortfall (missing requested features)
• All phases completed per Acceptance Criteria

Governance — Checkbox coverage gate

• If the project uses a governance MCP server for checkboxes (e.g. github-checkbox): run get_issue_checkboxes on the issue and cross-reference against phase completions. Any phase-complete-but-checkbox-unchecked state = FAIL — narrative-only progress is not a substitute.
• If no governance MCP is available: verify issue-body checkboxes are closed with explicit evidence lines (file:line / commit hash / command output).

Fail Fast Policy

• No silent fallbacks (errors fail loudly)
• No fake defaults (missing required config = error, not default)
• No swallowed exceptions
• Error messages are actionable

Observability

• Every decision boundary has a structured log (key=value or JSON, not free-text prose)
• Every state transition logs before / after values and trigger
• Every external call (DB / API / file / subprocess) logs inputs, duration, outcome
• Quantifiable behaviour has metrics (counters, durations, success / failure ratios)
• State changes touching production data / money / user-visible behaviour have an append-only audit trail (actor + timestamp + reason)
• No print() as logging strategy (use a structured logger)
• No empty except: blocks, bare pass on exception, silent return None on error, or continue on unexpected state
• Logs are searchable (consistent field names, no log-and-pray)

Code Reuse

• Searched the codebase before creating new modules (Glob / Grep / Explore) — evidence required: name 2-3 grep patterns or Glob queries actually run, with results count
• No duplicate modules created
• References existing modules where applicable

Codebase Hygiene

• No dead code (unreachable, never-called functions)
• No obsolete code (deprecated, superseded by new implementation)
• No orphaned code (from failed / rescoped approaches)
• No commented-out "old" code blocks
• No unused imports / dependencies
• No leftover temp / WIP patterns

Standards — Five Common Culprits (Logic)

Trace code paths for the five recurring patterns. Category names canonical in references/_common-culprits.md.

1. Duplicate Code (DRY / Modularity)

• No same calculation logic in multiple files
• No repeated parsing / formatting patterns
• Could this be extracted to a shared utility?

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

• No inline math that depends on business rules (e.g. price * 1.1 for markup)
• No date / time calculations with hardcoded offsets
• Should this formula be in a config file?

3. Hardcoded Settings

• No multipliers, percentages, thresholds in code
• No retry counts, timeout values, buffer sizes
• Is there a co-located YAML config this should be in?

4. Obsolete / Dead Code (LMCE)

• No functions that are never called (trace the call graph)
• No imports that are never used
• No feature flags for completed / removed features

5. Silent Fallbacks (Fail Fast)

• No .get(key, {}) chains that hide missing data
• No try / except: pass or catch(e) { } empty handlers
• No or default that masks configuration errors
• Does the error path fail loudly with an actionable message?

6. Cross-Boundary Contracts — Trace-Level

Logic-depth checks requiring cross-file tracing. These catch the bugs that syntactic scans miss.

SQL / Schema Contracts

• For every new / modified SQL query: list columns in WHERE / SELECT, open the migration for that table, confirm all columns exist
• Trace SQL query return values — if a query returns None / empty, verify the WHERE literal values match actual DB data (e.g. side='SELL' not side='SLD' if DB normalises on insert)
• Any query driving control flow (if result: / if not result:) has its column values cross-checked against DB schema

Null / None Propagation

• For functions with nullable parameters (Optional[T] or T | None): trace every call site — are callers guaranteed non-None, or must the function guard internally?
• Any function called at 2+ locations with a nullable input: verify the guard exists IN the function, not assumed from callers
• Frontend null-safety: trace API fields that can be null through to display / formatting — every .toFixed() on a nullable must have a null check

Config Wiring (Bidirectional)

• For every new YAML / config key: verify it is read by application code (grep key name → config.get('key') or config['key'])
• Config keys with zero code references = dead config = LMCE violation. Remove or wire.

Lifecycle Wiring

• For each recovery / drain / replay function: list ALL lifecycle entry points (startup, reconnect, restart) and verify it is called at EACH one
• "Called at reconnect" does NOT imply "called at startup" — verify separately

Data Correction Completeness

• For bugs that corrupt / mis-set data: verify a repair step exists covering ALL existing affected rows, not just future ones
• Run a count query to confirm zero remaining bad rows after repair

Cross-Function Behavioural Contracts

• For every try / except in the diff: open the wrapped function and confirm it has a reachable raise path. If it returns sentinel values (None, False), the try / except is dead code.
• Hot paths (fill handlers, order processors): count DB round-trips — flag any path querying the same row more than once

Bash Output Discipline

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

Sample Invocation Gate (workflow validation)

Unit + smoke tests are necessary but NOT sufficient for pipeline / backtest / orchestration / CLI-wiring / cross-module function-arg propagation / persistent-state-write changes.

• Scope check: does this change touch any of the above? If yes → the next checkbox is mandatory. If no → document the scope-skip reason here.
• If in-scope: evidence of a REAL-CLI sample invocation exists — either a log file path, or an Issue comment with exit code + DB row-count + downstream-consumer verification. Exit code 0 alone is NOT sufficient.
• If in-scope: any mock used in the fix's tests uses explicit call_args.kwargs["<key>"] == <expected> assertions — NOT a **kwargs-swallowing mock that would pass regardless of whether the code actually propagated the arg.

Required when available: Code Graph Checks

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

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

• For each modified function: graph query callers_of(<function_name>) → list all call sites; verify each handles the changed signature / behaviour.
• For each modified function: graph query callees_of(<function_name>) → confirm no newly-called functions have incompatible contracts (null-safety, config access).
• graph query search(pattern) for duplicate implementations: search for new calculation / parsing / schema-handling logic — confirm it doesn't already exist.
• MCP-access discrimination: every subagent RESULT block discussing graph queries starts with mcp_graph_available: yes|no. Yes + no graph-query evidence = FAIL (lazy fallback). No + grep fallback = PASS.

Fallback clause (retry-aware, evidence-required): if first graph call fails, retry once. If second fails, the RESULT block MUST include the Explore-subagent's RESULT block demonstrating manual call-graph audit was actually executed. Documenting only [graph unavailable] without subagent RESULT = silent skip = FAIL.

Pass Criteria

All checkboxes verified with evidence (graph-backed + source spot-check, OR documented fallback + subagent RESULT block). If graph was available and not used for a structural question: FAIL. If doc-only PR exempted via the short-circuit above: PASS. If unavailable / stale / unsupported-language WITH subagent-backed fallback evidence: PASS.

On Pass

Output: <promise>SONNET_PASS</promise>

On Fail

1. List failed items with evidence of failure.
2. Specify which Fail Fast / Registry violation was found.
3. Do NOT output the promise.
4. Ralph loop restarts from Tier 1.