Skip to content

Commit 78ceb52

Browse files
wongkclaude
andauthored
feat(code-review): fold Gate B route + partition into run-prefix (PLN-1229 Phase 2) (#174)
* feat(code-review): fold Gate B route + partition into run-prefix (PLN-1229 Phase 2) Extend cmd_run_prefix to run the WHOLE deterministic prefix in one process, through Gate B route + stage_17_partition + derive-spawn-spec, stopping at the reviewer fleet. - After stage_19_cache_check the runner invokes `route` itself (writing spawn.json.route), caches fast_path / max_bha_agents on the context, and on fast-path deletes the cached-BHA replay artifact. - stage_17_partition is skipped entirely in fast-path (no partitions.json / patches_p<N>.txt); otherwise its args are augmented with --loc-budget 500 --max-files 25 --max-bha-agents <N>, swapping --diff-data to uncached_diff_data.json when a cache dir is active. - The terminal return is now `ready_for_reviewers`, carrying fast_path, max_bha_agents, and cache_status_message so the orchestrator prints the routing + cache notices without re-reading spawn.json. A route failure surfaces as error with failed_stage="route". Parity oracle: the subprocess A-side gains a subprocess `route` runner and the same partition skip/augmentation, and both sides now run through to the reviewer fleet — so the A/B byte-equality test covers the full prefix (incl. partitions.json / spawn.json) across the fast-path and partitioned branches on all seven fixtures. Adds route/partition contract tests; SCHEMA.md §7b documents the ready_for_reviewers result and its fast_path / max_bha_agents / cache_status_message fields. Zero orchestrator changes (start.md untouched). Full code-review suite 1305 passed / 3 skipped; ruff + pyright clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(code-review): changelog + README for run-prefix route/partition folding (PLN-1229 Phase 2) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(code-review): resumable route-failure error + unconditional Gate B route (PLN-1229 Phase 2) Addresses review of PR #174: - Route-failure error was non-resumable: it set failed_stage="route" (not a real plan stage) and resume_stage=stage_17_partition (which would skip route on retry). Anchor both on stage_19_cache_check so a per-stage fallback re-runs cache-check → route → partition, restoring the resume_stage == failed_stage == real-stage-id invariant every other error path holds. Adds a test that stubs route to fail and asserts the contract. - run-prefix gated the Gate B route call on `status != "skipped"`, diverging from start.md's Walker Contract (route reads the diff, not the cache result, so it fires unconditionally after cache-check) and from the parity oracle's A-side, which runs route unconditionally. Removed the guard so both sides — and the walker contract — agree; the A/B byte-equality oracle stays green. - SCHEMA.md §7b now states exactly which result fields (fast_path / max_bha_agents / cache_status_message) are present per next_action, and that they are omitted (not null) elsewhere, so consumers use .get(). Full code-review suite 1306 passed / 3 skipped; ruff + pyright clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 215dc1f commit 78ceb52

8 files changed

Lines changed: 382 additions & 105 deletions

File tree

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,12 @@ All notable changes to the claude-plugins project will be documented in this fil
44

55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). Entries are listed newest-first; each plugin section is treated as released when merged to `main`.
66

7+
### code-review v3.3.0
8+
9+
#### Changed
10+
- `run-prefix` now runs the **entire** deterministic review prefix in one process, folding in the Gate B model-routing (`route`) and file partitioning that previously sat outside the runner. After the cache check it computes the routing decision itself (writing `spawn.json.route`), then — unless the fast path is selected — partitions the changed files (applying the reviewer-budget caps and, when a cache directory is active, restricting partitions to the files that missed the cache). In fast-path mode partitioning is skipped entirely and the cached Bug-Hunter-A replay artifact is removed. The terminal result is now `ready_for_reviewers`, which carries the `fast_path` decision, the Bug-Hunter-A agent cap, and the cache status message so the orchestrator can print the routing and cache notices without re-reading `spawn.json`. Routing runs unconditionally after the cache check (matching the walker contract, since it reads the diff rather than the cache result), and a routing failure is surfaced as an `error` result anchored on the cache-check stage so a per-stage fallback re-runs routing rather than skipping it. Documented in `SCHEMA.md`, which now also specifies exactly which result fields are present per outcome.
11+
- The subprocess A/B parity oracle now walks the whole prefix through partitioning and spawn-spec derivation on both sides, so its byte-identical-artifact guarantee covers the fast-path and partitioned branches (including `partitions.json` and `spawn.json`) across all seven fixtures.
12+
713
### code-review v3.2.0
814

915
#### Added

plugins/code-review/.claude-plugin/plugin.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "code-review",
33
"description": "Code review plugin",
4-
"version": "3.2.0",
4+
"version": "3.3.0",
55
"author": {
66
"name": "ClosedLoop",
77
"email": "support@closedloop.ai"

plugins/code-review/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ The helper script is a multi-subcommand Python CLI. The orchestrator invokes it
231231
| `finalize-result` | Consolidates validated findings + coverage state + verdict into the canonical `review_result.json` envelope; deep-merges `<cr_dir>/telemetry.json` into the canonical `telemetry` block and populates `telemetry.cache_hit_rate["bha"]` from `cache_result.json` (PLN-719 Phase 7/9) |
232232
| `arbitrate-budget` | Applies the canonical reviewer cap policy; emits coverage gaps for required reviewers that overflow (PLN-719) |
233233
| `prepare-run` | Emits a declarative `run_plan.json` describing the 30-stage pipeline (PLN-719) |
234-
| `run-prefix` | Runs the deterministic prefix (setup→cache-check) in one process, resolving tokens and honoring gates/`on_failure`; pauses at the hygiene-only exit, a singleton needing an agent, or the route/partition boundary, emitting a status JSON and resuming from a given stage (PLN-1229) |
234+
| `run-prefix` | Runs the whole deterministic prefix (setup through Gate B route + partition + spawn-spec derivation) in one process, resolving tokens and honoring gates/`on_failure`; pauses at the hygiene-only exit or a singleton needing an agent, otherwise returns `ready_for_reviewers` with the fast-path/cache-status decision — emitting a status JSON and resuming from a given stage (PLN-1229) |
235235

236236
## GitHub CI Mode
237237

plugins/code-review/SCHEMA.md

Lines changed: 48 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -495,51 +495,63 @@ Stages from plans 01/03/05/06 are present in `run_plan.json` but marked
495495

496496
## 7b. `run-prefix` result contract (PLN-1229)
497497

498-
`run-prefix` runs the deterministic prefix (stages 01→`cache_check`) in ONE
499-
process instead of one orchestrator turn per stage. It reads `run_plan.json` +
500-
`setup.json` from `--cr-dir`, walks from `--resume-from` (default: the first
501-
plan stage), and stops at the next genuine decision point — emitting a status
502-
JSON (to stdout, or `--output <path>`) that tells the orchestrator what to do
503-
next. The runner is **resumable**: after handling a pause the orchestrator
504-
re-invokes `run-prefix --resume-from <resume_stage>`. Because each segment is a
505-
fresh process, the `depends_on` `completed` set is reconstructed from artifacts
506-
on disk (a prior stage counts as done iff its literal `expected_outputs` exist).
498+
`run-prefix` runs the **entire** deterministic prefix (stages 01 through Gate B
499+
`route` + `partition` + `derive-spawn-spec`) in ONE process instead of one
500+
orchestrator turn per stage. It reads `run_plan.json` + `setup.json` from
501+
`--cr-dir`, walks from `--resume-from` (default: the first plan stage), and stops
502+
at the next genuine decision point — emitting a status JSON (to stdout, or
503+
`--output <path>`) that tells the orchestrator what to do next. The runner is
504+
**resumable**: after handling a pause the orchestrator re-invokes
505+
`run-prefix --resume-from <resume_stage>`. Because each segment is a fresh
506+
process, the `depends_on` `completed` set is reconstructed from artifacts on disk
507+
(a prior stage counts as done iff its literal `expected_outputs` exist).
507508

508509
**Result fields:**
509510

510-
| Field | Type | Meaning |
511-
| --------------- | --------------- | ----------------------------------------------------------------------- |
512-
| `next_action` | string (enum) | The pause reason — authoritative (read this, not the exit code). |
513-
| `resume_stage` | string \| null | The stage id to pass as `--resume-from` on the next invocation. |
514-
| `singleton` | string \| null | `"extract_signals"` \| `"coverage_critic"` when `needs_singleton`. |
515-
| `failed_stage` | string \| null | The aborting stage id when `next_action == "error"`. |
516-
| `ran_stages` | string[] | Stage ids executed (or `continue`-failed) this segment, in order. |
517-
| `message` | string \| null | Short diagnostic on `error`, else null. |
511+
| Field | Type | Meaning |
512+
| ---------------------- | --------------- | ----------------------------------------------------------------------- |
513+
| `next_action` | string (enum) | The pause reason — authoritative (read this, not the exit code). |
514+
| `resume_stage` | string \| null | The stage id to pass as `--resume-from` on the next invocation. |
515+
| `singleton` | string \| null | `"extract_signals"` \| `"coverage_critic"` when `needs_singleton`. |
516+
| `failed_stage` | string \| null | The aborting stage id when `next_action == "error"`. |
517+
| `ran_stages` | string[] | Stage ids executed (or `continue`-failed) this segment, in order. |
518+
| `message` | string \| null | Short diagnostic on `error`, else null. |
519+
| `fast_path` | bool | Gate B routing decision. Present **only** on `ready_for_reviewers`. |
520+
| `max_bha_agents` | int \| null | Gate B Bug-Hunter-A agent cap. Present **only** on `ready_for_reviewers`.|
521+
| `cache_status_message` | string \| null | `cache_result.json.status_message` to print. Present on `ready_for_reviewers` and `hygiene_exit`. |
522+
523+
`next_action`, `resume_stage`, `singleton`, `failed_stage`, `ran_stages`, and
524+
`message` are present on every result. The three Gate-B fields above are
525+
**omitted entirely** (not set to null) on the results that don't carry them —
526+
`fast_path` / `max_bha_agents` appear only on `ready_for_reviewers`, and
527+
`cache_status_message` only on `ready_for_reviewers` / `hygiene_exit`. Read them
528+
with `.get()`, not direct indexing.
518529

519530
**`next_action` values:**
520531

521-
| Value | Fires at | Orchestrator does next |
522-
| ------------------- | ------------------------------------- | ---------------------------------------------------------------------------- |
523-
| `needs_singleton` | `stage_11` / `stage_15` `needs_agent` | Spawn the `singleton` agent, write its output, re-invoke from `resume_stage`. |
524-
| `hygiene_exit` | Gate A (`hygiene_only` after hygiene) | Present hygiene findings and stop (no verdict/footer). |
525-
| `ready_for_route` | reaching `stage_17_partition` | Run Gate B (`route`) + partition + the rest of the walk. |
526-
| `error` | a stage aborted / a gate failed | Fall back to the per-stage walk from `failed_stage`; partials are preserved. |
527-
528-
`ready_for_route` distinguishes its two cases by `resume_stage`: a non-null
529-
`resume_stage` (`stage_17_partition`) is the normal boundary — run Gate B +
530-
partition from there. A **null** `resume_stage` means the walk reached the end
531-
of the plan without a partition stage (e.g. a depth tier that filters partition
532-
out); there is nothing left to route, so the orchestrator skips Gate B and
533-
partition and proceeds directly to the reviewer fleet.
532+
| Value | Fires at | Orchestrator does next |
533+
| --------------------- | ------------------------------------- | ---------------------------------------------------------------------------- |
534+
| `needs_singleton` | `stage_11` / `stage_15` `needs_agent` | Spawn the `singleton` agent, write its output, re-invoke from `resume_stage`. |
535+
| `hygiene_exit` | Gate A (`hygiene_only` after hygiene) | Print `cache_status_message`, present hygiene findings, stop (no verdict). |
536+
| `ready_for_reviewers` | the whole deterministic prefix is done | Print `cache_status_message` + the `fast_path` notice; spawn the reviewer fleet (`stage_20`). |
537+
| `error` | a stage aborted / a gate failed | Fall back to the per-stage walk from `failed_stage`; partials are preserved. |
538+
539+
On `ready_for_reviewers` the runner has already run Gate B `route` (writing
540+
`spawn.json.route`) and — unless `fast_path``stage_17_partition` (with the
541+
`--loc-budget 500 --max-files 25 --max-bha-agents <N>` augmentation, and the
542+
`uncached_diff_data.json` swap when a cache dir is active). In `fast_path` mode
543+
partition is skipped (no `partitions.json` / `patches_p<N>.txt`) and any cached
544+
BHA replay artifact is deleted. The `fast_path` / `max_bha_agents` /
545+
`cache_status_message` fields let the orchestrator print the routing + cache
546+
notices without re-reading `spawn.json`.
534547

535548
The exit code is `0` for every well-formed result (including `error`) — the
536-
`next_action` field is the contract. Route + partition (Segment 3) fold into the
537-
runner in Phase 2, at which point `ready_for_route` becomes `ready_for_reviewers`
538-
(carrying `fast_path` + `cache_status_message`). The `on_failure` policy of each
539-
stage is honored exactly as the Walker Contract prescribes: `abort``error`;
540-
`continue` → proceed; `continue_with_coverage_gap` → proceed after writing an
549+
`next_action` field is the contract. The `on_failure` policy of each stage is
550+
honored exactly as the Walker Contract prescribes: `abort``error`; `continue`
551+
→ proceed; `continue_with_coverage_gap` → proceed after writing an
541552
`agent-failure` system finding to `agent_<stage>-failed.json` (collected by
542-
`collect-findings`).
553+
`collect-findings`). A `route` failure is surfaced as `error` with
554+
`failed_stage: "route"`.
543555

544556
---
545557

0 commit comments

Comments
 (0)