fix(stella-pipeline,stella-cli): unbreak main — clippy arg count and a dangling boot.rs doc link - #1985
Merged
Merged
Conversation
…a dangling boot.rs doc link `main`'s required CI job has been red since #1951, and every open PR inherits it. Four distinct errors are involved; this change fixes the two that no open PR covers. - `cargo clippy -D warnings`: #1953 gave `plan_stage` a `research` argument, taking it to 8 (limit 7). `Spend<'_>` already groups the `budget`/`total` pair for seven sibling stage methods (`pipeline/stage_budget.rs`); `plan_stage` was the last one carrying them loose. Adopting it takes the count to 7 and *shrinks* `pipeline.rs`, which is closed to growth — an `#[allow]` would have been an expedient with no argument that the lint is wrong here. - `cargo doc -D warnings`: #1939 left `[`SkipReason::NoResumePoint`]` in boot.rs's module doc unresolvable. Qualified to the full path, the same remedy #1927 applied to the same file for the same reason. The other two errors — `flip_halt_arming.rs` referencing test helpers that #1945 never landed, and the `ModelCallRole::Research` match arm #1953 never added — are fixed by the open PR #1964, deliberately not duplicated here. `main` needs both changes: this one alone leaves `cargo test` red, and #1964 alone leaves clippy and rustdoc red. Verified: `cargo check -p stella-pipeline --all-targets` reports only the three #1964-owned errors, and `RUSTDOCFLAGS="-D warnings" cargo doc -p stella-cli --no-deps` exits 0. Refs #1953, #1939, #1964
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, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefactors the pipeline planning stage to use the existing Spend wrapper instead of separate budget/total arguments and updates call sites accordingly, and fixes a broken intra-doc link in the CLI daemon boot module by qualifying SkipReason with its full path. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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
…ft dangling A sixth break on `main`, from #1979 (#1667) and unrelated to the rest of this PR: `event.rs`'s `Compaction` variant documents its `rewrites` field with [`CompactionRewrite`], but that name is not in scope in `event.rs` — the field itself spells the fully-qualified `crate::CompactionRewrite`, and the type is only re-exported at the crate root from `compaction_rewrite.rs`. So `RUSTDOCFLAGS="-D warnings" cargo doc` fails the whole `fmt + clippy + test` job, on every open PR. The link now matches the path the field already uses — the same repair #1985 applied to `boot.rs`'s `SkipReason::NoResumePoint`, which is the second time this exact shape has broken `main` in a day. Caught only in CI, not locally, because `make guards-fast` runs no rustdoc and clippy does not check doc links: `doc-warnings` is a `make check`/`make gate` tier. Verified here with `make doc-warnings` over the whole workspace, which is now clean — worth doing directly, because each fix of this kind only lets rustdoc reach the next dangling link rather than proving there are none left.
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
… stacked behind one clippy error (#2000) ## What & why `main` was red at `e0fbbe02` on **five distinct breaks stacked behind one another**. CI's log showed only the first, because both compile-tier gates report one unit at a time: clippy stops at the first *hard* error in a crate, and `cargo doc --workspace` stops at the first crate that fails to document. That is why this is the sixth consecutive unbreak PR — each one can only reveal the next layer. Root cause tracked in #1986 (`ci.yml` does not run on a push to `main`); evidence from this session added there. ### 1. `pipeline/scope_stage.rs:34` — dead `spend` local ``` error: variable does not need to be mutable error: unused variable: `spend` ``` `plan_with_review` binds `let mut spend = Spend { budget, total };` and never reads it — the re-planning loop builds a fresh `Spend` by reborrowing on each iteration, which is the only construction the code uses. Dead since #1971, unmasked when #1985 cleared the `plan_stage` arg-count error above it. ### 2. `management_prompt/tests.rs` — `ModelCallRole::Research` listed twice `unreachable_patterns`. Kept the documented placement beside `Unknown`, whose comment explains why `Research` never reaches the chokepoint; dropped the copy appended after `Summarization`. ### 3. `verification_hardening.rs` — three items nothing constructs `dead_code` ×3 on `SHELL_TOOL`, `shell_call_result`, `PassingShell`. The child `flip_halt_arming` module defines its own, which shadow the parent's through `use super::*` — a glob import loses to a local definition silently, so this was never a name clash, just quietly unreachable code. The child's are the live pair *and* the newer one: a per-command `call_id: format!("call-shell-{command}")` that `FlipHalt` correlates on, versus the parent's fixed `"call-shell"` which cannot distinguish two shell calls. So the parent's stale copies go. Its `mod` doc claimed the child existed in order to reach the parent's fakes — the pre-split rationale, now false — and is rewritten to point at the child's own doc, where the anti-clobber reason for colocating them lives (#1997). **#2 and #3 are the same shape**: a merge landed the same addition twice. Neither side conflicts textually, so review saw nothing. ### 4. `stella-protocol/src/event.rs` — unresolved intra-doc link `AgentEvent::Compaction::rewrites` documents itself with `[`CompactionRewrite`]`, but `event.rs` never imports the type (the field spells it `crate::CompactionRewrite` inline), so `broken_intra_doc_links` failed `doc-warnings`. A *different gate step* from #1–#3, invisible while clippy was red. Fourth recurrence of the shape #1986 tracks. ### 5. `file-size` — two ceilings exceeded on `main` `driver.rs` at 2572/2571 and `pipeline/tests.rs` at 2537/2536. **Neither file is touched by this branch**; both were grown on `main` by merges that did not regenerate the baseline. That mechanism is #2004. Also regenerated `docs/wire/*` — the protocol types' doc comments *are* that contract, so break #4's fix mechanically changed the emitted `description`. ## About the two raised ceilings A raised ceiling is normally a defect, so this is stated plainly rather than buried: `make file-size-update` moved `driver.rs` and `pipeline/tests.rs` up by one line each, for growth **this branch did not author**, because the growth has already landed on `main` and reverting another PR's line is outside this task. The alternative was leaving the gate red. The same regeneration also **tightens** `pipeline.rs` from 3451 to 3181 — a 270-line shrink the baseline had not captured. This branch adds no lines to any god file. A maintainer who would rather see those two lines pushed into submodules should say so; that is their call, not mine. ## The witness - [x] No witness test. Four of the five are dead code, a duplicate match arm, and a doc link — no runtime behavior exists to witness, and the compiler is the oracle. The fifth is a generated baseline. Per CONTRIBUTING's carve-out for changes with no behavior delta, here is how it was verified instead: - `cargo clippy --workspace --all-targets -- -D warnings` — fails on `main` at break #1, exits 0 here. - `RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps` — fails on `main` at break #4, exits 0 here. - `cargo fmt --all --check` — exits 0. - `cargo test -p stella-pipeline` — 618 pass, 0 fail. That suite **did not compile at all** on `main` (break #3), so these tests were not running. - `scripts/check-file-size.sh` and `make wire-schema` — both exit 0. Exit codes were read from cargo directly, not through a pipe: `cargo … | tail` reports *tail's* status, which is always 0, and cargo colorizes `error` so a plain `^error` grep matches nothing. Both produced a false green in this session before being corrected. ## The gate - [x] `file-size` and `god-files` pass; baseline regenerated, never hand-edited. - [x] `wire-schema` passes; the diff is comment-only — no field added, removed, renamed or re-tagged, and no optional field made required, so the additive-only contract holds. - [x] No behavior change, no new flags, no new dependencies. ## Nothing left behind - **#2013** (filed) — sharing `CARGO_TARGET_DIR` between worktrees produces compile errors naming symbols that do not exist. Hit during this work: a phantom `E0004` for `AgentEvent::TurnParked`/`TurnWoken`, variants present in neither checkout, because a parallel job's build was linked in. Nearly caused a wrong "fix". - **#1986** — commented with the full five-layer breakdown as evidence for fixing the trigger rather than the instances. - **#2004** — owns the file-size baseline skew behind break #5. Refs #1972, #1986, #1997, #2004, #2013
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
…ngress (#1787) (#2002) ## Why this PR exists **#1787's fix is not in `main`.** PR #1982 carried it, but its base was the topic branch `unbreak-main-pipeline`, whose own PR (#1975) was **closed, not merged**. #1982 then merged into that dead branch, so the oracle-trace bound landed nowhere `main` can see, and nothing is carrying that branch forward. It also merged in a **broken** state. While the base was being reconciled with `main`, git's auto-merge of the two independently-written unbreaks concatenated both sides, leaving: - `struct PassingShell` and `fn shell_call_result` **defined twice** - `async fn a_revision_halts_at_the_step_where_the_tracked_test_flips` defined twice - a duplicate `ModelCallRole::Research` match arm (unreachable pattern) None of that compiles. `unbreak-main-pipeline` currently holds it; `main` is unaffected. This PR is the clean landing: **`main` plus `evidence.rs`, and nothing else.** ## What it does (#1787) Bounds the oracle trace at the verifier-prompt ingress. The trace grows once per verification round and the repair gate can keep granting rounds while a measured budget affords them — so unlike the diff, which rides under a token budget, this channel had **no ceiling at all**. - `MAX_ORACLE_TRACE_OBSERVATIONS = 24` — sized far above a normal run (baseline plus a handful of rounds) so the bound only bites a pathological loop. - `bounded_oracle_trace` keeps the **newest** observations and states the drop **in-band** (`…N earlier observation(s) omitted → …`), so the verifier reads "earlier observations exist" rather than a trace that silently starts mid-run. - The **stored snapshot keeps the full trace**; only the prompt ingress is clipped — the structural-bound rule from #1932. ## Witnesses - `a_pathological_oracle_trace_is_clipped_with_the_drop_stated` — a 100-observation trace renders clipped to the newest 24 with the omission counted in-band. - `an_ordinary_oracle_trace_renders_unchanged` — the bound does not touch a normal run, so this cannot ship as "always clip". Observations alternate pass/fail in the fixture so a clipped render is distinguishable from a repeated one. ## Verification - `cargo test -p stella-pipeline` — **585 pass**, 0 fail, including both witnesses above - `cargo fmt --check -p stella-pipeline` — clean - Diff vs `main` is exactly one file: `crates/stella-pipeline/src/pipeline/evidence.rs` (+74/−2) ## CI is red on `main`'s breaks, not this diff This branch is merged up to current `main`. Every failing step fails in a file this PR does not touch, and each already has a dedicated unbreak in flight: | Failing step | Where | Covered by | |---|---|---| | `check-file-size` | `scripts/file-size-baseline.txt` (parallel-merge skew) | **#2003**, **#2008** | | `cargo fmt --check` | not this crate's file | **#2005** | | clippy: unused `spend` / unused `mut` | `pipeline/scope_stage.rs:34` — a dead local `#1985` left behind | **#2000** | | rustdoc: unresolved `CompactionRewrite` | `stella-protocol` | **#2010** | The clippy one is worth naming precisely, since it is `stella-pipeline`: `main`'s `scope_stage.rs` binds `let mut spend = Spend { budget, total };` and then never uses it — the loop constructs a fresh `Spend` inline per iteration. `spend` occurs exactly once in the file. That is `main`'s dead local, untouched by this PR. No competing unbreak is included here on purpose — six are already open against `main`, and duplicating one is how `main` gets re-broken. ## Note on the dead branch `unbreak-main-pipeline` still holds the duplicate-definition breakage and the only copy of #1982's merge. It is not reachable from `main` and its PR is closed, so nothing needs to be reverted — but it should not be revived without first taking `main`'s copies of `flip_halt_arming.rs`, `management_prompt/tests.rs` and `scope_stage.rs`, which is what this PR does. Filed as #2001. Closes #1787
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 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.
What & why
main's requiredfmt + clippy + testjob has been red since #1951 (fiveconsecutive 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_stagehas 8 arguments (limit 7)crates/stella-pipeline/src/pipeline.rs. #1953 added aresearchparameter,pushing
plan_stageoverclippy::too_many_arguments.Spend<'_>(crates/stella-pipeline/src/pipeline/stage_budget.rs) alreadygroups the
budget/totalpair, and seven sibling stage methods take it;plan_stagewas the last one carrying the two loose. Adopting it takes thecount 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.rsis agrandfathered god file closed to growth, which this change shrinks.
2.
cargo doc -D warnings— unresolved link inboot.rscrates/stella-cli/src/daemon/boot.rs. #1939 left[`SkipReason::NoResumePoint`]in the module doc unresolvable(
no item namedSkipReasonin scope), which failsrustdoc::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.rsreferencesPassingShell/shell_call_result, testhelpers fix(stella-pipeline): arm FlipHalt on the authored-witness path and in revise turns (#1793) #1945 landed the test file without.
crates/stella-pipeline/src/management_prompt/tests.rs's exhaustive matchomits
ModelCallRole::Research, which feat(stella-pipeline): pre-plan research stage — triage names questions, parallel read-only sub-agents answer them (#1778) #1953 added.Duplicating them here is how two parallel unbreak PRs collide, so they are left
to #1964.
mainneeds both PRs. This one alone leavescargo testred; #1964 aloneleaves clippy and rustdoc red (verified: #1964's own CI still fails both, and
its clippy failure is the identical
plan_stageerror). Neither is sufficienton its own, and the merge order does not matter.
Because of that, this PR's own
cargo clippyandcargo teststeps willstay red until #1964 merges — clippy
--all-targetsreaches the missing testhelpers once the lib error is gone. That is expected, not a regression.
The witness
behavior change. Verified the artisanal way:
RUSTDOCFLAGS="-D warnings" cargo doc -p stella-cli --no-depsfails onmainand exits 0 with this change.cargo check -p stella-pipeline --all-targetsreports only the threefix(stella-pipeline,stella-cli): verdict preamble parsing, pipeline calibration, and a revision halted on an already-green command (#1787, #1595, #1793) #1964-owned errors; the
plan_stagearg-count error is gone and bothcall sites (
pipeline/scope_stage.rs,pipeline/tests/management_accounting.rs)compile.
cargo fmt --check -p stella-pipeline -p stella-cliexits 0.The gate
check-file-size.shandcheck-god-files.shpass;pipeline.rsshrinks,so no baseline change is needed.
Nothing left behind
boot.rs's module doc has now brokenmain's rustdoc twice intwo days by the same mechanism (fix(stella-cli): stop a parked run stalling the whole boot resume sweep (#1698) #1920 → fix(stella-cli): unbreak main — resolve the boot.rs doc link #1920 left dangling #1927, then fix(stella-cli): resume a crash that recorded an error at boot (#1696) #1939 → this PR), because
ci.ymldoes not run on a push tomain. Filed as a handoff.CandidateStateis hand-built at two sites, which is how fix(stella-pipeline,stella-protocol): record verifier degradation per candidate in a fan-out (#1787) #1951'sfield addition silently broke PR feat(stella-pipeline,stella-cli): resume re-enters the staged pipeline at the execute stage (#1671) #1962 at merge time. Noticed in the same
investigation.
Refs #1953, #1939, #1964