feat(stella-pipeline): pre-plan research stage — triage names questions, parallel read-only sub-agents answer them (#1778) - #1953
Merged
Conversation
…ns, parallel read-only sub-agents answer them Docs half: the canonical stage order in AGENTS.md and the website gains the conditional research stage between recall and plan. Closes #1778
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @macanderson, your pull request is larger than the review limit of 150000 diff characters
Contributor
Reviewer's GuideIntroduces a new pre-plan research stage in the inference pipeline that turns optional triage-specified research questions into parallel read-only sub-agents, feeds their bounded findings into the planner prompt, and threads a new StageKind::Research / ModelCallRole::Research through the engine, wire contracts, CLI, TUI, and tests, while splitting oversized pipeline/protocol files into dedicated modules to stay within size limits. Sequence diagram for the new pre-plan research stagesequenceDiagram
participant Pipeline
participant TriageStage as triage_stage
participant ResearchStage as research_stage
participant Engine
participant Planner
Pipeline->>TriageStage: triage(goal, budget, total_cost)
TriageStage->>TriageStage: parse_triage_response(text)
TriageStage->>TriageStage: parse_research_questions(text)
TriageStage-->>Pipeline: (TaskAssessment, research_questions)
Pipeline->>ResearchStage: research_stage(goal, research_questions, budget, total_cost)
alt questions non_empty
ResearchStage->>Engine: run_sub_agent_with_sender(SubAgentSpec)
Engine-->>ResearchStage: SubAgentOutcome
ResearchStage->>ResearchStage: bound_research_findings(findings)
ResearchStage-->>Pipeline: Vec<ResearchFinding>
else questions empty or failure
ResearchStage-->>Pipeline: Vec<ResearchFinding> (empty)
end
Pipeline->>Planner: plan_with_review(goal, recall_frames, research_findings, budget, total_cost)
Planner->>Planner: build_planner_prompt(goal, recall, research, repo_structure, revision)
Planner-->>Pipeline: PlannedScope
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
This was referenced Aug 7, 2026
Merged
Owner
Author
|
Merge-order note (parallel-PR sweep): this PR conflicts with its sibling from the same batch — #1953 and #1962 both extract submodules out of |
Contributor
There was a problem hiding this comment.
Sorry @macanderson, your pull request is larger than the review limit of 150000 diff characters
7 tasks
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
`cargo clippy -p stella-pipeline --all-targets -- -D warnings` fails on b5ab7f8. A compile error in the lib masks the test target entirely, so this reads as one failure and is really four, from two different merges. From #1953 (the #1778 research stage): 1. `management_prompt/tests.rs` — `ModelCallRole::Research` is a new variant and `management_system_block`'s match is exhaustive on purpose (E0004). Research rides the sub-agent primitive, so its system prompt travels on the `SubAgentSpec`, never through `metered_raw_call`: it joins the never-dispatched arm, and `ALL_ROLES` grows to 15. 2. `pipeline.rs` — the new `research` parameter pushed `plan_stage` to 8 arguments, one over clippy's cap. Bundled `budget`/`total` into the `Spend` struct every stage downstream of the fan-out already takes, rather than `#[allow]`-ing the lint. From #1951, which rewrote `tests/verification_hardening.rs` wholesale and dropped three items #1945 had added to it hours earlier — a same-seam clobber, in a file #1951's own subject (per-candidate verifier degradation) never needed to touch: 3. `PassingShell` and `shell_call_result` went with it, leaving the child module `flip_halt_arming.rs` referencing two helpers that exist nowhere in the tree (E0425 ×2). Restored to their original home, which the child reaches through `use super::*`. 4. `a_revision_halts_at_the_step_where_the_tracked_test_flips` went too — the configured-command **witness for #1793**. Deleting it did not fail any gate, because the crate stopped compiling for reason 3 first: #1793 has been shipping with half its witness silently gone. Restored verbatim. Both #1793 witnesses now run and pass. Neither is vacuous: each asserts a scripted-prompt count, so a `PassingShell` that omitted the `[exit code: 0]` marker `flip_halt::exit_status` parses would leave the halt unarmed, the revision would consume the steps scripted beyond the flip, and the count would be wrong. `cargo test -p stella-pipeline`: 605 passed, 0 failed. `cargo clippy -p stella-pipeline --all-targets -- -D warnings`: clean.
macanderson
pushed a commit
that referenced
this pull request
Aug 7, 2026
…build main landed its own #1793 fix (#1945) while this branch was open, arriving at the same two changes: `witness_on_demand` arms `state.flip_halt`, and revise turns receive it instead of `None`. Main's implementation is taken wholesale; this branch's duplicate is dropped. One refinement survives on top, as `flip_halt::for_revision`. Main gates the revision on `FlipHalt::unfired`, which refuses a latch that already fired — the halt the EXECUTE turn stopped on. It cannot cover the other way a revision opens on an already-green command, because that halt has never fired: `witness_on_demand` arms a FRESH `FlipHalt` after execution, and by then the witness it names has usually already flipped (it is written to pass once the work is done). A verifier can still reject such a candidate — a lint regression, a refuted verdict — and the revision would inherit an unfired halt on a command that is green before it starts, ending at the first step boundary where the model re-ran that test. So the oracle decides: only `FlipState::Failing` means the flip is still ahead of the turn. `for_revision` composes with `unfired` rather than replacing it. The research stage (#1778, landed on main since) built its engine without the pipeline's attachments, so it inherited neither the pause gate nor the calibration this branch adds — the exact "seeded N-1 of N engines" shape `attach` exists to prevent. It now goes through `attach` like the other four, which also fixes a pre-existing bug: a paused run's research sub-agents did not park. ## Unbreaking the pipeline test build `origin/main` at 43402ae does not compile its own test suite. Verified in a clean worktree at that commit, three errors, none of them from this branch: * `pipeline/tests/verification_hardening/flip_halt_arming.rs` (added by #1945) references `PassingShell` and `shell_call_result`, which exist nowhere in the crate — the fixtures were never committed. * `management_prompt/tests.rs`'s deliberately-exhaustive match does not cover `ModelCallRole::Research`, added later by #1953. Both are fixed here because this branch cannot be verified, let alone merged, on a base whose tests do not build. The two fixtures are reconstructed from what the test's own comments require, and the semantics are pinned by the test passing for the stated reason: `shell_call_result` puts the command under the `command` key `command_of` reads, and `PassingShell` emits the trailing `[exit code: 0]` marker `exit_status` parses — without either, the halt could never latch and `an_authored_witness_arms_the_revision_flip_halt` would have been green for no reason. `Research` joins the "never dispatched through the management chokepoint" arm because its calls are sub-agents run through `Engine::run_sub_agent_with_sender`, not `metered_raw_call`. `cargo test -p stella-pipeline`: 588 unit + 22 integration, 0 failures, including main's own arming witness. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 7, 2026
macanderson
pushed a commit
that referenced
this pull request
Aug 7, 2026
…a private intra-doc link Both reproduce on origin/main at 43402ae and are unrelated to this branch's own changes; see #1972 for the full account. * `plan_stage` reached 8 arguments when #1953 added `research`, one past clippy's limit, so `make lint` fails workspace-wide. Fixed by taking the `Spend` the pipeline already uses for exactly this pair rather than `budget` and `total` loose — the established idiom here, and it removes a line at each of the three sites instead of adding an #[allow]. * `daemon/boot.rs`'s module doc intra-doc-links `SkipReason::NoResumePoint`, which is `pub(super)` and therefore absent from a non-private rustdoc build, failing `cargo doc -D warnings`. Demoted to a code span, matching how #1965 fixed the sibling case in stella-store. Refs #1972 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 7, 2026
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
…nd, incl. a silently deleted #1793 witness (#1971) ## What `main` at b5ab7f8 did not build: `cargo clippy -p stella-pipeline --all-targets -- -D warnings` failed. A compile error in the lib masks the test target, so it read as one failure and was really **four**, from two different merges. ### From #1953 (the #1778 research stage) 1. **`management_prompt/tests.rs` (E0004)** — `ModelCallRole::Research` is a new variant and `management_system_block`'s match is exhaustive by design. `Research` rides the sub-agent primitive, so its system prompt travels on the `SubAgentSpec`, never through `metered_raw_call` — it joins the never-dispatched arm, and `ALL_ROLES` grows to 15. 2. **`pipeline.rs` (`too_many_arguments`)** — the new `research` parameter pushed `plan_stage` to 8 args, one over the cap. Bundled `budget`/`total` into the `Spend` struct every stage downstream of the fan-out already takes, rather than `#[allow]`-ing the lint. `pipeline.rs` is a god file, so the doc comment is written to land the file back at **exactly** its 3462 ceiling — no growth. ### From #1951 — a same-seam clobber #1951 rewrote `tests/verification_hardening.rs` wholesale and dropped three items #1945 had added to it hours earlier, in a file its own subject (per-candidate verifier degradation) never needed to touch: 3. **`PassingShell` + `shell_call_result` (E0425 ×2)** — the child module `flip_halt_arming.rs` was left referencing two doubles that exist nowhere in the tree. 4. **`a_revision_halts_at_the_step_where_the_tracked_test_flips`** — the configured-command **witness for #1793**. Deleting it failed no gate, because the crate had already stopped compiling for reason 3. **#1793 has been shipping with half its witness silently gone.** Restored verbatim from eddf970. ## The structural fix, not just the restore Restoring the witness took the parent file to 1557 lines, which `file-size` rejects outright — and the baseline takes no new entries. So this splits rather than exempts, and the split is the one the content was asking for: **both** #1793 witnesses and the two doubles they share now live in `verification_hardening/flip_halt_arming.rs`, the module already named for the concern. Parent drops to 1434. That the two witnesses were ever in separate files is what let the clobber happen quietly. With the cluster in one file, the same wholesale rewrite is a merge conflict instead of a silent deletion — the module doc records why. ## Witness Not a pure refactor: items 3 and 4 restore two witness tests, and neither passes vacuously. Each asserts a scripted-prompt count, so a `PassingShell` that omitted the `[exit code: 0]` marker `flip_halt::exit_status` parses would leave the halt unarmed, the revision would consume the steps scripted beyond the flip, and the count would be wrong. ``` test ...flip_halt_arming::a_revision_halts_at_the_step_where_the_tracked_test_flips ... ok test ...flip_halt_arming::an_authored_witness_arms_the_revision_flip_halt ... ok ``` - `cargo test -p stella-pipeline` — **605 passed, 0 failed** - `cargo clippy -p stella-pipeline --all-targets -- -D warnings` — clean - `make guards-fast` — green, `file-size` and `god-files` included ## Overlap with #1965 This branch originally also retired the stale `crates/stella-protocol/src/event.rs` baseline entry (1454 lines against a recorded 2965 — a fifth red gate on main). **#1965 landed the same fix while this was in flight**, so that work was dropped here in favour of theirs on rebase; the remaining commit is only the split. No duplicate baseline edit.
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
…alibration, and a revision halted on an already-green command (#1787, #1595, #1793) (#1964) > **Correction to my comment above:** the tracking issue for `main`'s broken test build is **#1972**, not #1970 — I cited the number before the issue was filed. Nothing else in that comment changes. ## What & why Three defects picked off the backlog for Terminal-Bench impact, one per metric. **#1793 landed on `main` independently while this was open (#1945), so this PR now defers to that implementation** — see §2. What remains is described below. ### #1787 — solve rate: a verifier that thinks before it answers parsed as no answer `parse_verifier_response` consulted only the first non-empty line. A reply opening with `Here is my assessment:` carried no verdict token there and returned `None` — and `None` is the signal for `heuristic_fallback`, which passes only on an observed flip or green touched tests and **otherwise FAILS**. So this did not degrade one verdict; it converted *every* verdict from a preambling model into a heuristic that mostly refuses. Reasoning models preamble by construction, which makes this a whole model family the verifier could not grade with. Two positions are now authoritative, tried in order: the reply's **first** non-empty line, then its **last**. Deliberately two, and not a scan of the body — intermediate prose is where a verifier *discusses* failing tests, and reading it would reintroduce the misread the token set was narrowed to prevent. The head still wins outright, so a reply that leads with its verdict parses exactly as before whatever its closing line says. This is the narrow half of the issue's item 1. The forced structured-output path needs a provider-parity posture per invariant 8 and is deliberately **not** here; #1787 stays open for it and for item 3. ### #1595 — cost: the default `stella run` estimated worst Five of `stella-cli`'s seven assembly sites seeded a `CalibrationMap` and handed it to the engine. The two that did not were `run_pipeline_one_shot` — which *is* `stella run` — and fleet workers, because `Pipeline` had no way to accept one. The cost is larger than a lost seed: with no map at all the correction is **inert for the whole run**, so the drift those turns measured was never applied to them either. `Pipeline` gains `with_calibration`, borrowed rather than owned because `CalibrationMap` is deliberately not `Clone` (an owned field would also cost `PipelineConfig` its `Clone`, which many call sites rely on). It attaches to **every** engine the pipeline builds through one `attach` helper — because seeding N-1 of N engines is exactly the shape of the bug being fixed, one level down. That turned out to be load-bearing within this PR: the review bot caught that the **research stage** (#1778, landed on `main` since I wrote `attach`) built its engine outside it. Fixed, and it also fixes a pre-existing bug unrelated to calibration — **a paused run's research sub-agents did not park**, because that engine never received the turn gate either. ### #1793 — a revision halted on an already-green command `main` fixed #1793 first (#1945), arriving at the same two changes this branch had. Main's implementation is taken wholesale and the duplicate dropped. One refinement survives, as `flip_halt::for_revision`, composed **on top of** `FlipHalt::unfired` rather than replacing it. `unfired` refuses a latch that already fired — the halt the *execute* turn stopped on. It cannot cover the other way a revision opens on an already-green command, because that halt has never fired: `witness_on_demand` arms a **fresh** `FlipHalt` after execution, and by then the witness it names has usually already flipped — the ordinary case, since a witness is written to pass once the work is done. A verifier can still reject such a candidate (a lint regression, a refuted verdict), and the revision then inherits an unfired halt on a command that is green before it starts, ending at the first step boundary where the model re-runs that test — before addressing anything the verifier objected to. So the oracle decides: only `FlipState::Failing` means the flip is still *ahead* of the turn. Same rule `run_candidate` already applies when it refuses to arm on a green configured baseline. Closes #1595 Refs #1787 Refs #1793 Refs #1972 ## The witness - [x] This PR includes a witness test (fails on `main`, passes here) | Witness | Covers | |---|---| | `a_verifier_that_preambles_is_read_at_its_conclusion` | a preamble before PASS/FAIL is no longer an unparseable reply | | `intermediate_prose_never_decides_a_verdict` | the widening's safety property: body prose never decides, and a reply stating no verdict is still `None` | | `the_pipeline_path_sizes_its_budget_with_the_callers_calibration` | a lent map reaches the engines | | `a_pipeline_lent_no_calibration_reports_the_identity_factor` | an unlent one invents nothing | | `a_revision_on_a_green_or_flaky_command_is_never_halted` | the `FlipState::Failing` guard — the half `unfired` cannot cover | | `a_revision_inherits_the_halt_while_the_tracked_command_is_still_red` | and it still arms when the flip is genuinely ahead | The two #1787 witnesses were checked the artisanal way: `git stash push -- crates/stella-pipeline/src/verify.rs` (source only, tests left in place) → both fail; restored → both pass. The calibration witnesses read the factor off each **engine** step's `StepManifest`, filtered to `call_seq == 0`. The pipeline's management roles ride the same step at 1, 2, … and carry the identity factor whatever the engines were lent — a raw management call has no transcript to compact — so counting them would make the test assert something untrue of the seam. ## The gate - [x] `cargo fmt --check` - [x] `cargo clippy --workspace --all-targets -- -D warnings` - [x] `cargo test --workspace` - [x] Docs updated where behavior changed (`pipeline/attachments.rs` module doc; `for_revision`'s rule stated where the semantics live; `stella-parity`'s `calibration.drift` row rewritten) - [x] `Closes #N` appears both here and as a commit trailer ## Nothing left behind - [x] Filed: #1948, #1949, #1950, #1972 - **#1972** — `main` cannot compile `stella-pipeline`'s test suite, independent of this branch. Fixed here because this PR could not be verified on it; filed separately so unbreaking `main` is tracked either way. Details in the comment above. - **#1948** — `Verdict::reasoning` is bounded twice in *different units* (chars at construction, bytes when forwarded); a multi-byte reply gets cut to a third of the budget with two truncation markers. - **#1949** — the arming half of #1793 has no end-to-end witness beyond `main`'s own; `turn.halt_on_goal_met` stays `ShippedUnwitnessed`. - **#1950** — nothing proves `run_pipeline_one_shot` and the fleet worker do the *lending*. Delete either `.with_calibration(..)` and the workspace suite stays green — which is how the original gap survived. `calibration.drift` stays `ShippedUnwitnessed` with its `missing` field rewritten to say so. ## Ground-rule check - [x] No I/O added to `stella-core`; no new dependencies - [x] No new outbound network calls - [x] No new cross-boundary types ## Anything reviewers should know? **Three commits here are not mine to claim as feature work** — they unbreak the base. `main` at `43402ae` fails `cargo test -p stella-pipeline --no-run` with three errors (#1945's test fixtures were never committed; #1953's new `ModelCallRole` doesn't satisfy #1941's deliberately-exhaustive match), and `cargo doc -D warnings` fails on a private intra-doc link in `daemon/boot.rs` that is byte-identical on `main`. Reconstructing #1945's fixtures is the part worth a reviewer's eye: the semantics are pinned by its witness passing *for the stated reason* (the `command` key `command_of` reads; the `[exit code: 0]` marker `exit_status` parses), but if that author intended something different, this is where to say so. **The file-size baseline moves, and mostly down.** `agent.rs` +3 and `fleet_cmd.rs` +3 are the call-site wiring — the seed and the lend, which need the store, the config and the pipeline all in scope. `pipeline/tests.rs` +1 is a `mod` declaration. Against those, **`pipeline.rs` ratchets down 11 lines**: `with_turn_gate` moved into the new `pipeline/attachments.rs` beside `with_calibration`, and the repeated gate-attachment blocks collapsed into `attach`. **Not measured on the bench.** These are picked for Terminal-Bench impact and argued from the code and the issues, not from a before/after run — no provider credentials were available in the environment this was written in, so no benchmark run was performed. The cost and wall-clock claims are mechanism-level; #1770/#1289 are where a real measurement would land. --------- Co-authored-by: Claude <noreply@anthropic.com>
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
…a dangling boot.rs doc link (#1985) ## What & why `main`'s required `fmt + clippy + test` job has been **red since #1951** (five consecutive merges), so every open PR inherits a red gate. Four distinct errors are involved. This PR fixes the two that **no open PR covers**. ### 1. `cargo clippy -D warnings` — `plan_stage` has 8 arguments (limit 7) `crates/stella-pipeline/src/pipeline.rs`. #1953 added a `research` parameter, pushing `plan_stage` over `clippy::too_many_arguments`. `Spend<'_>` (`crates/stella-pipeline/src/pipeline/stage_budget.rs`) already groups the `budget`/`total` pair, and **seven** sibling stage methods take it; `plan_stage` was the last one carrying the two loose. Adopting it takes the count to 7. This is the right fix rather than `#[allow(clippy::too_many_arguments)]` because there is no argument that the lint is wrong *here* — the grouping type the lint is asking for already exists — and because `pipeline.rs` is a grandfathered god file closed to growth, which this change **shrinks**. ### 2. `cargo doc -D warnings` — unresolved link in `boot.rs` `crates/stella-cli/src/daemon/boot.rs`. #1939 left ``[`SkipReason::NoResumePoint`]`` in the module doc unresolvable (`no item named `SkipReason` in scope`), which fails `rustdoc::broken_intra_doc_links`. Qualified to the full path — the same remedy #1927 applied to *this same file* for *this same reason* after #1920. That recurrence is filed as #1986. ## What this PR deliberately does NOT fix The other two errors belong to the open PR **#1964**: - `flip_halt_arming.rs` references `PassingShell` / `shell_call_result`, test helpers #1945 landed the test file without. - `crates/stella-pipeline/src/management_prompt/tests.rs`'s exhaustive match omits `ModelCallRole::Research`, which #1953 added. Duplicating them here is how two parallel unbreak PRs collide, so they are left to #1964. **`main` needs both PRs.** This one alone leaves `cargo test` red; #1964 alone leaves clippy and rustdoc red (verified: #1964's own CI still fails both, and its clippy failure is the identical `plan_stage` error). Neither is sufficient on its own, and the merge order does not matter. Because of that, **this PR's own `cargo clippy` and `cargo test` steps will stay red until #1964 merges** — clippy `--all-targets` reaches the missing test helpers once the lib error is gone. That is expected, not a regression. ## The witness - [x] No witness test needed — a lint fix and a doc-link fix, neither a behavior change. Verified the artisanal way: - `RUSTDOCFLAGS="-D warnings" cargo doc -p stella-cli --no-deps` fails on `main` and **exits 0** with this change. - `cargo check -p stella-pipeline --all-targets` reports **only** the three #1964-owned errors; the `plan_stage` arg-count error is gone and both call sites (`pipeline/scope_stage.rs`, `pipeline/tests/management_accounting.rs`) compile. - `cargo fmt --check -p stella-pipeline -p stella-cli` exits 0. ## The gate - [x] `check-file-size.sh` and `check-god-files.sh` pass; `pipeline.rs` shrinks, so no baseline change is needed. - [x] No behavior change, no new flags, no docs pages affected. - [x] No new dependencies. ## Nothing left behind - **#1986** — `boot.rs`'s module doc has now broken `main`'s rustdoc twice in two days by the same mechanism (#1920 → #1927, then #1939 → this PR), because `ci.yml` does not run on a push to `main`. Filed as a handoff. - **#1974** — `CandidateState` is hand-built at two sites, which is how #1951's field addition silently broke PR #1962 at merge time. Noticed in the same investigation. Refs #1953, #1939, #1964
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
…hree things and the merge kept both #1985 and #1971/#1995 independently repaired the breaks #1953 left, converged on the same designs, and landed within minutes of each other. Git merged the two additively rather than conflicting, so `main` at e0fbbe0 carries each fix twice and fails `cargo clippy -p stella-pipeline --all-targets -- -D warnings` three ways: 1. `management_prompt/tests.rs` — `ModelCallRole::Research` appears twice in the same or-pattern (`unreachable_patterns`). Kept one. 2. `pipeline/scope_stage.rs` — both PRs bundled `plan_stage`'s budget+total into `Spend`, but the call site kept #1985's per-iteration reborrow AND the other's hoisted `let mut spend`, now unused (`unused_variables` + `unused_mut`). Kept #1985's: the loop replans after a rejected scope card, and only a reborrow per attempt survives that. 3. `tests/verification_hardening.rs` — both restored `PassingShell` and `shell_call_result` after #1951 deleted them, one into this file and one into its `flip_halt_arming` child, leaving the parent's pair dead (`dead_code` ×3, counting `SHELL_TOOL`). For (3) the two copies were not equivalent, so this is not an arbitrary pick: #1985's are better documented — they name `SHELL_TOOL` as a const distinct from `WRITING_TOOL` and say why the `[exit code: 0]` marker is load-bearing (without it the halt never latches and the arming test passes for no reason). Those are the ones kept. They move to the child, which is where both #1793 witnesses now live, because co-location is what makes the next wholesale rewrite of the parent a merge conflict instead of the silent deletion that started this (#1997). The parent's now-stale `mod` doc is corrected in place rather than left describing a layout that no longer holds. `cargo clippy -p stella-pipeline --all-targets -- -D warnings`: clean.
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
…e same three things, plus the file-size ratchet blocking every PR (#2008) >⚠️ **Overlaps #2000 — merge exactly one of these, never both.** We built the same unbreak in parallel and reached the *identical* resolution on all three collisions. This PR additionally fixes a fourth break (the file-size ratchet) that is currently failing #2000's checks and every other open PR. If #2000 picks up that one commit, close this; otherwise close #2000. Merging both is how the collision being fixed here happened. ## Why main is red `main` at e0fbbe0 fails `cargo clippy -p stella-pipeline --all-targets -- -D warnings` **and** `file size ratchet`. Two unbreak PRs (#1985, and #1971 via #1995) independently repaired the breaks #1953 left, **converged on the same designs**, and landed minutes apart. Git merged them additively rather than conflicting, so main now carries each fix twice: | # | Break | Where | |---|---|---| | 1 | `ModelCallRole::Research` twice in one or-pattern (`unreachable_patterns`) | `management_prompt/tests.rs` | | 2 | Both a hoisted `let mut spend` **and** a per-iteration reborrow (`unused_variables` + `unused_mut`) | `pipeline/scope_stage.rs` | | 3 | `PassingShell`/`shell_call_result` restored into *both* the parent and its child (`dead_code` ×3) | `tests/verification_hardening.rs` | | 4 | Two grandfathered files one line over their ceiling | `scripts/file-size-baseline.txt` | ## The judgment calls **(2) — kept #1985's per-iteration reborrow, not the hoisted binding.** Not arbitrary: `plan_with_review` loops, replanning after a rejected scope card, and only a `Spend` reborrowed per attempt survives that. The hoisted version would have been moved on the first iteration. **(3) — kept #1985's doubles, in the child.** The two copies were *not* equivalent. #1985's are better documented: they name `SHELL_TOOL` as a const distinct from `WRITING_TOOL`, and say why the trailing `[exit code: 0]` marker is load-bearing — without it `FlipHalt::observe` never latches and the arming test passes for no reason. Those are the ones kept. They live in `flip_halt_arming` with both #1793 witnesses, because co-location is what turns the next wholesale rewrite of the parent into a merge conflict instead of the silent deletion that started this (#1997). The parent's `mod` doc is corrected in place rather than left describing a layout that no longer holds. **(4) — recording growth that already merged, and saying so.** Two ceilings go **up** by one line each: ``` crates/stella-core/src/driver.rs 2571 → 2572 crates/stella-pipeline/src/pipeline/tests.rs 2536 → 2537 ``` Per CLAUDE.md, a raised ceiling to turn a gate green is normally a defect against the PR that raises it, so this is flagged rather than slipped through. The difference: **this branch touches neither file.** Both grew on main via #1979 and #1962, which did not regenerate the baseline in the same commit. The choice is therefore not "grow or don't" but "record what already merged, or leave main red for everyone". The two lines are somebody's to reclaim; neither is mine to judge irreducible. The same regeneration **tightens** `pipeline.rs` from 3451 to 3181 — 270 lines of stale headroom now closed off, which is the ratchet working as intended and more than offsets the two. Regenerated via `make file-size-update`, never hand-edited. ## Verification - `cargo clippy -p stella-pipeline --all-targets -- -D warnings` — clean - `cargo test -p stella-pipeline --lib` — **596 passed, 0 failed**, both #1793 witnesses among them - `make guards-fast` — green, `file-size` and `god-files` included ## Related - #1997 — why a deleted test failed no gate in the first place - #1985, #1995, #2000 — the colliding unbreaks ## Summary by Sourcery Unbreaks main by reconciling overlapping clippy and test fixes in stella-pipeline, consolidating flip-halt arming test doubles, and updating the file-size baseline so guards and ratchet checks pass again. Bug Fixes: - Resolve unreachable pattern warning in management_prompt tests by removing the duplicate ModelCallRole::Research arm - Fix clippy unused variable warnings in scope_stage by relying on per-iteration Spend reborrows - Restore and colocate shell tooling doubles for flip halt arming tests so dead-code warnings are cleared while preserving #1793 coverage Enhancements: - Clarify documentation and structure of flip halt arming tests by moving shared shell doubles into the child module and updating the parent module description Build: - Regenerate file-size baseline to reflect recent growth in driver.rs and pipeline tests while tightening the pipeline.rs ceiling so file-size ratchet gates pass again
This was referenced Aug 9, 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
1 participant
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.
Closes #1778. Refs #1768, #1776 (the first two slices: triage evidence, model-side sibling-spawn concurrency).
What this builds
The decisions with the highest downstream leverage — what the plan names — were taken at the point of least evidence: nothing between triage and execute could read a file. This adds the pre-plan research stage from the issue's validated design sketch:
TRIAGE_INSTRUCTIONSgains an optionalRESEARCH:line (multionly, up to 4|-separated self-contained questions);triage::parse_research_questionsparses it withparse_flag-style tolerance. Absent line /none/ any class below the resolvedMultiStep⇒ no questions ⇒ the stage is skipped byte-for-byte (L-E2 untouched). Gating on the resolved class keeps the deterministic floor authoritative.pipeline/research_stage.rs). Exemplars followed exactly: dispatch isEngine::run_sub_agent_with_sender+SubAgentSpec(thestella-core::goal.rsverifier's shape — so the pause gate, soft stop, and read-only enforcement come from the primitive, not reimplementation), money iscandidate_fanout::FanOutBudget(pre-dispatch gate,headroom / widthcarve, settle in completion order, index-order cost summing).write_access: false, depth 1, per-child receipt slots. A per-child latency ceiling (research_latency_ceiling, default 45s, inside the future so the sub-agent drop guard still settles spend) guarantees research can never wedge a turn; a timed-out child'sFinishedbracket is emitted by the stage so the event stream stays balanced.research::bound_research_findings: 4k chars/finding, 12k total, honest drop markers), rendered as## Research findingsinbuild_planner_prompt— never mixed into recall frames (different provenance). Instruction blocks stay&'static str(Management calls (triage/verdict/guidance) have no cacheable prefix — and cannot have one until the adapters carry cache-control #1434).ModelCallRole::Research+StageKind::Research(wire schema regenerated, round-trip pinned);stage_rankplaces Research between ContextRecall and Plan; every exhaustive consumer match (TUI, diag bridge, wire-contract samples) updated.God-file accounting (all three were at their exact ceilings)
pipeline.rs3573 → 3466: the triage stage moved out topipeline/triage_stage.rs(thewitness_stage/scope_stagepattern) to make room for the ~20 lines of wiring.pipeline/tests.rs2573 → 2533: the inlineverification_honestymodule moved to its own file.event.rs2965 → 1549: its inline test module moved toevent/tests.rs(theregistry.rsprecedent, refactor(tools): move registry.rs's test module out of the file it tests (3791 → 2015 lines) #1122).scripts/file-size-baseline.txtregenerated viamake file-size-update(retightens all three).Definition of done → tests
triage::tests::research_line_parses_pipe_separated_questions_in_order(+ cap, tolerance, absence)SubAgentbrackets betweenStage::TriageandStage::Planpipeline::tests::research::triage_questions_fan_out_as_sub_agents_between_triage_and_planScriptedProvider::shapes())pipeline::tests::research::the_planner_prompt_carries_the_bounded_findings_sectionpipeline::tests::research::a_failed_research_round_degrades_to_the_no_research_prompt(byte-equal planner prompts) +no_questions_means_no_stage_no_sub_agents_no_sectionWitness property: the two feature tests reference
StageKind::Researchand theRESEARCH:protocol, neither of which exists onmain— the test file does not compile there, so the feature is genuinely absent on the old code.Decisions the issue left open
MultiSteponly (notSingleTask), as the issue suggested — the cheap classes never pay for the stage.Role::Plan: the findings exist for the planner, so they ride the planner's model choice and overrides; a dedicated router role would be a config-surface change this slice doesn't need.inference-pipeline.mdxnow includes research.Verification
make guards+doc-warnings+doc-links+command-docsgreen;cargo test --workspacegreen; clippy-D warningsgreen across all targets; wire schema regenerated andmake wire-schemagreen; file-size + god-files guards green with the retightened baseline. No new dependencies.Known caveat (stated, not hidden)
A child cancelled by the latency ceiling has its in-flight provider call dropped mid-request; its already-settled spend is folded back through the sub-agent drop guard, but the final in-flight call's usage is unobservable (the same window every cancellation has — the raw-call path records
UsageIncompletefor this; an engine-turn analog does not exist yet).Summary by Sourcery
Add a pre-plan research stage that fans out triage-named questions to parallel read-only sub-agents and feeds their bounded findings into the planner, while refactoring triage and wire/test modules to keep god-files within size limits.
New Features:
Enhancements:
Documentation:
Tests: