Skip to content

feat(code-review): in-process run-prefix segment runner + subprocess A/B parity oracle (PLN-1229 Phase 1) - #173

Merged
wongk merged 3 commits into
mainfrom
feat/code-review-run-prefix-runner
Jul 2, 2026
Merged

feat(code-review): in-process run-prefix segment runner + subprocess A/B parity oracle (PLN-1229 Phase 1)#173
wongk merged 3 commits into
mainfrom
feat/code-review-run-prefix-runner

Conversation

@wongk

@wongk wongk commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

PLN-1229 Phase 1. Collapses the deterministic review prefix (stages setupcache-check) from one orchestrator turn per stage into a single resumable process, and lands the subprocess A/B parity oracle that guarantees the batching is behavior-preserving. Zero orchestrator changesstart.md is untouched; the runner is validated purely by the parity test. Route + partition (Segment 3) fold into the runner in Phase 2.

Phase 0 (the golden harness + fixtures) merged in #172; this builds the runner on top of it.

What's here

Production (code_review_helpers.py)

  • cmd_run_prefix — reads run_plan.json + setup.json, walks stages from --resume-from, resolves each stage's <ANGLE_BRACKET> tokens from prior-stage artifacts, dispatches the cmd_* in-process with the stage's stdout redirect, and honors on_failure + validation gates. Pauses at genuine decision points, emitting a status JSON:
    • hygiene_exit (Gate A), needs_singleton (a PLN-725 prepare stage that needs an agent → resume at the sibling consolidate), ready_for_route (Phase-1 boundary before partition), error (a stage aborted → orchestrator falls back to the per-stage walk).
    • Resumable: each segment is a fresh process, so the depends_on completed set is reconstructed from artifacts on disk.
  • _execute_stage_inprocess mirrors the start.md Walker Contract steps 1-5; a failed continue_with_coverage_gap stage emits a canonical agent-failure system finding.
  • Registered run-prefix in config/cli.json (46th subparser).

Parity oracle (prefix_golden_harness.py + test_prefix_golden.py)

  • A-side: one subprocess per stage (python3 code_review_helpers.py <stage>, real > file redirects) — reproduces what start.md does today. B-side: python3 code_review_helpers.py run-prefix per segment. A and B implement the walk wrapper independently, so a shared wrapper bug can't hide.
  • Asserts byte-identical normalized artifacts through cache-check across all 7 P0-B fixtures, plus a pause-sequence test pinning the resumable 3-segment contract. A failing gh stub shadows PATH so subprocess stages take the deterministic no-PR branch; env is snapshotted inside the hermetic HOME redirect (so the cache never resolves to the real ~/.claude).

Contract tests (test_code_review_helpers.py, 31 tests) — token resolution, completed-set reconstruction, singleton needs_agent detection, on_failure abort/continue incl. coverage-gap emission, and cmd_run_prefix error/boundary returns.

DocsSCHEMA.md §7b documents the run-prefix result contract.

Testing

  • Full code-review suite: 1299 passed, 3 skipped
  • uv run ruff check . clean · uv run pyright 0 errors (repo-wide)
  • A/B parity + pause-sequence: 14 tests green (deterministic across repeated runs; no real-cache pollution)

Version: code-review 3.1.1 → 3.2.0 (MINOR — additive subcommand).

🤖 Generated with Claude Code

wongk and others added 3 commits July 2, 2026 09:22
…A/B parity oracle (PLN-1229 Phase 1)

Collapse the deterministic prefix (stages 01→cache_check) from one
orchestrator turn per stage into a single resumable process.

Production (code_review_helpers.py):
- cmd_run_prefix: reads run_plan.json + setup.json, resolves each stage's
  <ANGLE_BRACKET> tokens from prior-stage artifacts, dispatches cmd_* in-process
  with the stage's stdout redirect, honors on_failure + validation gates, and
  pauses at the genuine decision points — Gate A (hygiene_only), a PLN-725
  singleton needs_agent, or the Phase-1 boundary (stage_17_partition →
  ready_for_route). Resumable: the depends_on completed set is reconstructed
  from artifacts on disk, so each --resume-from segment is a fresh process.
- _execute_stage_inprocess mirrors the start.md Walker Contract steps 1-5;
  continue_with_coverage_gap emits a canonical agent-failure system finding.
- Registered run-prefix in config/cli.json (46th subparser).

Parity oracle (prefix_golden_harness.py + test_prefix_golden.py):
- Subprocess A-side (one python3 helpers.py <stage> per stage, real > redirect)
  vs B-side (python3 helpers.py run-prefix per segment). A and B implement the
  walk wrapper independently, so a shared wrapper bug cannot hide. Byte-equal
  (normalized) artifacts through cache_check across all 7 P0-B fixtures, plus a
  pause-sequence test pinning the resumable 3-segment contract.
- A failing gh stub shadows PATH so subprocess stages take the deterministic
  no-PR branch; env is snapshotted inside the hermetic HOME redirect.

P0-C contract tests (test_code_review_helpers.py, 31 tests): token resolution,
completed-set reconstruction, singleton needs_agent detection, on_failure
abort/continue incl. coverage-gap emission, and cmd_run_prefix error/boundary
returns.

SCHEMA.md §7b documents the run-prefix result contract. Zero orchestrator
changes (start.md untouched) — validated purely by the parity oracle. Route +
partition (Segment 3) fold in at Phase 2.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… Phase 1)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…stage

Addresses review of PR #173 (all MEDIUM, non-blocking):
- Capture each in-process stage's stderr and fold its tail into the returned
  status message, so a failing stage's own diagnostic is attributed to that
  stage now that one process spans many stages (was: bare process stderr with
  no stage attribution). Written back to the real stderr so live visibility is
  unchanged.
- Thread that diagnostic into _emit_prefix_stage_failure_finding: the
  continue_with_coverage_gap agent-failure finding now carries the actual
  rc/exception/stderr in its explanation instead of a generic string.
- Log the full traceback on an unexpected stage crash (the broad except no
  longer discards it), so a Phase-1 crash is diagnosable without reproduction.
- SCHEMA.md §7b: document that ready_for_route with resume_stage=null means the
  walk completed with no partition stage (skip Gate B), distinct from the
  normal partition boundary.

The docstring subparser-count finding was already correct (46). Adds a
regression test asserting the stderr diagnostic reaches the finding explanation.
Full code-review suite 1300 passed / 3 skipped; ruff + pyright clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@wongk
wongk merged commit 215dc1f into main Jul 2, 2026
5 checks passed
@wongk
wongk deleted the feat/code-review-run-prefix-runner branch July 2, 2026 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant