feat(code-review): in-process run-prefix segment runner + subprocess A/B parity oracle (PLN-1229 Phase 1) - #173
Merged
Merged
Conversation
…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>
This was referenced Jul 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PLN-1229 Phase 1. Collapses the deterministic review prefix (stages
setup→cache-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 changes —start.mdis 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— readsrun_plan.json+setup.json, walks stages from--resume-from, resolves each stage's<ANGLE_BRACKET>tokens from prior-stage artifacts, dispatches thecmd_*in-process with the stage's stdout redirect, and honorson_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).depends_oncompletedset is reconstructed from artifacts on disk._execute_stage_inprocessmirrors thestart.mdWalker Contract steps 1-5; a failedcontinue_with_coverage_gapstage emits a canonicalagent-failuresystem finding.run-prefixinconfig/cli.json(46th subparser).Parity oracle (
prefix_golden_harness.py+test_prefix_golden.py)python3 code_review_helpers.py <stage>, real> fileredirects) — reproduces whatstart.mddoes today. B-side:python3 code_review_helpers.py run-prefixper segment. A and B implement the walk wrapper independently, so a shared wrapper bug can't hide.cache-checkacross all 7 P0-B fixtures, plus a pause-sequence test pinning the resumable 3-segment contract. A failingghstub shadowsPATHso subprocess stages take the deterministic no-PR branch; env is snapshotted inside the hermeticHOMEredirect (so the cache never resolves to the real~/.claude).Contract tests (
test_code_review_helpers.py, 31 tests) — token resolution,completed-set reconstruction, singletonneeds_agentdetection,on_failureabort/continue incl. coverage-gap emission, andcmd_run_prefixerror/boundary returns.Docs —
SCHEMA.md§7b documents therun-prefixresult contract.Testing
uv run ruff check .clean ·uv run pyright0 errors (repo-wide)Version: code-review 3.1.1 → 3.2.0 (MINOR — additive subcommand).
🤖 Generated with Claude Code