fix(stella-pipeline,stella-protocol,repo): unbreak main — five breaks stacked behind one clippy error - #2000
Merged
Merged
Conversation
…lan_with_review
`plan_with_review` binds `let mut spend = Spend { budget, total };` and never
reads it: the re-planning loop builds a fresh `Spend` by reborrowing
`budget`/`total` on every iteration, which is the only construction the code
actually uses. The binding is a leftover from #1971 and is dead.
It was invisible until now because clippy stops at the first hard error in a
crate. #1985 removed the `plan_stage` arg-count error immediately above it,
which let the linter reach line 34 and fail `main`'s required job on
`unused_mut` + `unused_variables` under `-D warnings`.
Refs #1972, #1986
![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
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRemoves an unused mutable Spend binding in stella-pipeline’s scope_stage so that clippy with -D warnings passes on main, without changing runtime behavior. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
… the test targets Both are the same shape: a merge landed the same addition twice, and clippy could not see either until the lib-level errors above them were cleared. management_prompt/tests.rs listed `ModelCallRole::Research` twice in one match arm (`unreachable_patterns`). Kept the documented placement beside `Unknown`, whose comment explains why Research is never dispatched through the chokepoint; dropped the copy appended after `Summarization`. verification_hardening.rs defined `SHELL_TOOL`, `shell_call_result` and `PassingShell` that nothing constructs (`dead_code`): the child `flip_halt_arming` module defines its own, which shadow the parent's through `use super::*`. The child's are the live pair and the newer one — a per-command `call_id` the halt correlates on, and a `1 passed` body — so the parent's stale copies go. Its `mod` doc claimed the child existed to reach the parent's fakes, which was the pre-split rationale and is now false; rewritten to point at the child's own doc, where the anti-clobber reason for colocating them lives. Refs #1972, #1986, #1997
…tra-doc link `AgentEvent::Compaction::rewrites` documents itself with [`CompactionRewrite`], but `event.rs` never imports the type — the field spells it `crate::CompactionRewrite` inline — so rustdoc cannot resolve the bare name and `broken_intra_doc_links` fails the doc-warnings gate under -D warnings. Written as a linked path rather than a bare qualified one so the rendered text stays `CompactionRewrite`, matching how compaction_rewrite.rs's own module doc links back to `AgentEvent::Compaction`. This is the fourth recurrence of the private/out-of-scope intra-doc shape that #1986 tracks: the doc gate reports one crate at a time, so each unbreak PR can only ever surface the next one. Refs #1986
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
… merged past A fourth break on `main` at e0fbbe0, independent of the three duplicate-fix collisions in the parent commit and of anything either unbreak PR wrote: two grandfathered files sit one line over their recorded ceiling. crates/stella-core/src/driver.rs 2572 vs 2571 (+1) crates/stella-pipeline/src/pipeline/tests.rs 2537 vs 2536 (+1) Both are `main`'s own state — this branch touches neither file — from PRs (#1979, #1962) that grew them without regenerating the baseline in the same commit. It fails `file size ratchet` on every open PR, mine and #2000 alike, so nothing can land until someone absorbs it. Regenerated with `make file-size-update`, never hand-edited. **Saying the unflattering half out loud, per CLAUDE.md.** Two ceilings go UP by one line each. A raised ceiling to turn a gate green is normally a defect against the PR that raises it — the difference here is that these lines are already merged and shipping, so the choice is not "grow or don't" but "record what merged, or leave `main` red for everyone". I am recording it and naming it rather than letting it pass as routine. The two lines are somebody's to reclaim; neither is mine to judge irreducible. The same regeneration also TIGHTENS `crates/stella-pipeline/src/pipeline.rs` from 3451 to 3181 — 270 lines of headroom that had gone stale and is now closed off, which is the ratchet working as intended and more than offsets the two. `make guards-fast`: green. `cargo test -p stella-pipeline --lib`: 596 passed, 0 failed, both #1793 witnesses among them.
Owner
Author
|
Heads up — we built the same unbreak in parallel (mine: #TBD, branch Your Both are already Add that one commit and yours is complete — happy to close mine. Only one of these should land; merging both is how the collision we are fixing happened in the first place. |
Owner
Author
|
Mine is #2008, for reference — same three fixes, plus the |
Two gate steps were red for reasons the compile tiers cannot see. wire-schema: docs/wire is generated from the protocol types, and a type's doc comment IS part of that contract — the previous commit's intra-doc fix changed the emitted `description` text. Regenerated with scripts/export-agentevent-schema.sh. The diff is comment-only: no field added, removed, renamed or re-tagged, and no optional field made required, so the additive-only contract is intact. file-size: two grandfathered files sit one line over their recorded ceiling on main already — driver.rs at 2572/2571 and pipeline/tests.rs at 2537/2536 — neither of which this branch touches. Parallel merges grew them without regenerating, which is the baseline skew that keeps main red. Regenerated with make file-size-update rather than hand-edited. Called out explicitly because a raised ceiling is normally a defect: those two +1s are other PRs' growth, not this branch's, and this branch adds no lines to any god file. The same regeneration TIGHTENS pipeline.rs from 3451 to 3181, recording a 270-line shrink the baseline had not yet captured. Refs #1972, #1986
This was referenced Aug 7, 2026
Owner
Author
|
Independent verification from a parallel session — this PR is the one that unbreaks rustdoc, and I can confirm it works. I hit the same Verified on a local tree of
So the four open unbreak PRs — this one, #2003 (baseline skew), #2005 (fmt newline) — are jointly sufficient; main is green on none of them individually. One caution when this lands. Merging this branch conflicts with main in Also: merging this and #2003 in either order leaves |
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
… merged past A fourth break on `main` at e0fbbe0, independent of the three duplicate-fix collisions in the parent commit and of anything either unbreak PR wrote: two grandfathered files sit one line over their recorded ceiling. crates/stella-core/src/driver.rs 2572 vs 2571 (+1) crates/stella-pipeline/src/pipeline/tests.rs 2537 vs 2536 (+1) Both are `main`'s own state — this branch touches neither file — from PRs (#1979, #1962) that grew them without regenerating the baseline in the same commit. It fails `file size ratchet` on every open PR, mine and #2000 alike, so nothing can land until someone absorbs it. Regenerated with `make file-size-update`, never hand-edited. **Saying the unflattering half out loud, per CLAUDE.md.** Two ceilings go UP by one line each. A raised ceiling to turn a gate green is normally a defect against the PR that raises it — the difference here is that these lines are already merged and shipping, so the choice is not "grow or don't" but "record what merged, or leave `main` red for everyone". I am recording it and naming it rather than letting it pass as routine. The two lines are somebody's to reclaim; neither is mine to judge irreducible. The same regeneration also TIGHTENS `crates/stella-pipeline/src/pipeline.rs` from 3451 to 3181 — 270 lines of headroom that had gone stale and is now closed off, which is the ratchet working as intended and more than offsets the two. `make guards-fast`: green. `cargo test -p stella-pipeline --lib`: 596 passed, 0 failed, both #1793 witnesses among them.
8 tasks
This was referenced Aug 7, 2026
macanderson
changed the title
spend local clippy could not see until #1985
Owner
Author
|
Deadlock found — see #2015. Verified against #2015 carries the hunks from this PR plus the others so the set can land in one merge. The substantive work is yours — I collected it rather than re-deriving it, and said so in that PR. If you would rather sequence these individually, the equivalent fix is to add the other half to one of these branches; either route works, but a lone merge of any single one will stay red. All five gates verified green on the combined tree: workspace clippy |
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
…eir fixes (#2015) ## The problem `main` at 6c34553 fails **five** independent gates. Four fixes already exist across #2000, #2003 and #2005 — but **two of those PRs are red on exactly the gate the other one repairs**, so none of them can merge: - **#2003** regenerates `scripts/file-size-baseline.txt` and touches nothing else → fails `wire-schema`, because main's `docs/wire/` is stale against its own types. - **#2005** regenerates `docs/wire/` and the park consumers → fails `file-size`, because the baseline skew is #2003's fix. That is a deadlock, and it is why main has stayed red while three unbreak PRs sat open. This branch carries both halves plus #2000's repairs so the set can land in one merge. ## The five gates | Gate | Break | Fix from | |---|---|---| | `lint` | a dead `spend` local | #2000 | | `doc-warnings` | `[`CompactionRewrite`]` resolves only via the crate-level re-export | #2000 | | `format-check` | missing trailing newline in `event/tests.rs` | #2005 | | `file-size` | baseline skew (`driver.rs` +1, `pipeline/tests.rs` +1) | #2003 | | `wire-schema` | `docs/wire/` stale against #1994's `TurnParked`/`TurnWoken` | #2005 | The rustdoc one is worth a note: **layered masking**, the shape #1965 records. Rustdoc stops at the first crate that fails to document, so #1970 had to repair `stella-cli` before `stella-protocol` underneath it became visible at all. Anyone fixing one layer and re-running would reasonably have believed they were done. ## Authorship The substantive hunks are **from #2000, #2003 and #2005** — collected here, not re-derived, so their authors keep the credit. Close those three as superseded if this lands, or close this one if they can be sequenced another way; the point is that they cannot each be green independently. I also opened #2010 for the rustdoc break before finding #2000 already covered it, and closed it as a duplicate. ## Verification Run against this exact tree, each gate with the command the Makefile uses: | Check | Result | |---|---| | `cargo clippy --workspace --all-targets -- -D warnings` | clean | | `RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps` | clean | | `cargo fmt --all -- --check` | clean | | `scripts/check-file-size.sh` | OK — none grew | | `make wire-schema` | OK — `docs/wire/` matches the types | | `cargo test -p stella-protocol -p stella-pipeline -p stella-tui -p stella-core` | pass | Each was also confirmed **failing** on `origin/main` beforehand, so this is a demonstrated repair rather than an assumed one. One caveat, stated rather than buried: `stella-tui`'s `run_deck_paints_folds_resizes_and_restores_under_a_real_pty` failed once in the batch run and passed in isolation (20s, against an 81s timeout) — a real-PTY timing flake under concurrent build load, not a regression. The baseline was regenerated with `make file-size-update`, never hand-edited — a hand-merged baseline is what produced the current skew. ## Note for reviewers No witness test: every hunk restores an existing gate to green rather than changing behavior. The reproduction table above is the evidence, and each gate flips from fail to pass across this diff. ## Summary by Sourcery Unstick main by combining previously separate fixes so all gates pass together, including lint, doc warnings, formatting, file-size checks, and wire-schema consistency. Bug Fixes: - Repair lint break by removing the unused `Spend` local from the pipeline scope stage. - Fix rustdoc warnings by correcting the `CompactionRewrite` intra-crate link to the crate-level re-export and mirroring it in generated wire docs. - Restore format-check to green by adding the missing trailing newline in `event/tests.rs`. - Update `scripts/file-size-baseline.txt` to reflect current binary sizes so file-size checks match the regenerated code and tests. - Bring `docs/wire` schemas back in sync with protocol types and serveframe definitions, including the `CompactionRewrite` documentation changes. - Ensure observatory transcripts correctly capture and render `turn_parked` and `turn_woken` events so journal gaps and wake reasons are visible. - Adjust the management prompt tests to match the current set of model roles and avoid stale expectations. - Update the fleet dashboard UI to properly represent parked turns as a distinct state instead of misclassifying them as blocked. Enhancements: - Extend the observatory database query and journal rendering to include parked and woken turn events, with operator-facing descriptions and timing details. - Add UI support in the fleet dashboard for displaying parked turns and holding their state across park/wake so operators can distinguish deliberate waits from stalls. - Clarify the flip-halt arming test module layout and move doubles into the child module to protect against silent deletion on parent rewrites. Tests: - Tidy verification hardening tests around flip-halt arming by delegating doubles into the child module and simplifying the parent’s documentation. - Align management prompt tests with the current role handling to keep the test suite reflecting real behavior. - Confirm wire-format tests and tag tables remain unchanged while restoring formatting and wire-schema consistency. Chores: - Regenerate wire schema artifacts and file-size baselines using the project’s existing tooling so all gates share a consistent view of the repository state.
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
… restart (#1992) ## What Adds a `/reload` deck command, and makes a SETTINGS-tab save take effect in the running session instead of waiting for a restart. `Config::reload_from_disk` re-reads the settings scope chain (user + project, managed ceiling folded in) and re-applies everything `load_with_settings` derives from it — engine posture, tool policy, authority, and the recap/trace/reward/worktree switches — to the live `Config`. Provider/model/credential resolution is deliberately **not** re-run: it needs the full startup chain (interactive prompt included), and swapping provider mid-session is a much larger step than a config refresh. `/model` and the SETTINGS tab remain the seam for that. ## The interesting part: a reload cannot happen mid-turn The first cut threaded `&mut Config` down to the deck's overlay handlers and reloaded inline. That does not compile, and the borrow checker was right on the substance: the deck's in-turn recv site sits in the same `select!` as the turn coroutine, which holds `&Config` and is actively reading the very fields a reload rewrites (tool policy, authority, engine posture). Reloading there tears config out from under a running turn. So the handlers no longer reload. They report `stale`, and the caller re-derives at a safe boundary — the discipline `/budget` already follows with `pending_budget`: - **idle site** — reload immediately; the next prompt sees it. - **in-turn site** — park it, and apply after the turn ends, right beside the parked `/budget` cap. The delay is invisible in the UI: `engine_config_inbound` and `tool_policy_inbound` both re-read the scope chain from disk already, so the panels show what the files say regardless. Only *subsequent turns* depend on the live `Config`. Exemplar for the shape: this is the same "park the mutation, apply it at the safe boundary" pattern `pending_budget` uses a few lines above, which in turn mirrors AGENTS.md invariant #6 ("budget aborts at safe boundaries only"). ## Witness test `config::tests::reload_from_disk_reapplies_the_settings_scope_chain` — writes `{"enable_recap": "on", "tools": {"bash": "off"}}` to the user scope *after* the `Config` is built, calls `reload_from_disk`, and asserts both the recap toggle and the `bash` switch flipped. Verified the artisanal way: with `reload_from_disk`'s body replaced by `Ok(())`, the test fails (`reload must re-derive the recap toggle from the scope chain on disk`); with the real body it passes. It redirects the user scope through the thread-local paths seam (`paths::test_user_home`, #1139) rather than `$HOME` — no env mutation, no `unsafe`, no cross-thread race. Worth noting for anyone writing a similar test: `UserPaths::test_default()` keeps the developer's **real** home (`..Self::from_environment()`), so an earlier draft of this test was silently reading my own `~/.stella/settings.json`. ## File-size guard `command_deck.rs` is a god file closed to growth, so none of this landed in it. The SETTINGS overlay handlers and the `/reload` body moved out to `command_deck/settings_io.rs` (the `skills.rs` / `authoring.rs` pattern), and `reload_from_disk` lives in `config/reload.rs` rather than pushing `config.rs` (1498 on main) over the ceiling. Net effect: `command_deck.rs` **shrinks** 4621 → 4566, which is the single line the regenerated baseline carries. ## Review feedback: a failed reload was not all-or-nothing The Vercel review bot caught a real defect, now fixed. `reload_from_disk` assigned six `self` fields before `settings.reward_policy()?` — the only fallible step downstream of the load — could fail. That falsified an invariant this PR itself documents on `apply_pending_reload`: *"A failed reload leaves the session on its previous (still coherent) values."* The failure mode is worse than a torn write because it is **silent**. Both callers tell the user the reload failed and the previous values were kept, while the next turn actually runs under a hybrid posture — tool policy re-derived from disk, authority and reward weights from session start — that no scope chain ever produced. The repair is a derive-then-commit split: every fallible call now runs into a local before `self` is touched, and the commit block is infallible, so `?` can only fire while `self` is still pristine. A phase comment states the rule, so a future fallible getter lands above the commit block instead of rediscovering the hazard. `apply_pending_reload`'s doc now names where its coherence claim is actually guaranteed, rather than assuming it. **Second witness** — `config::tests::a_failed_reload_leaves_every_field_untouched` writes a well-formed `settings.json` whose `verifier_weight: 2.0` outranks the deterministic weight (`reward_policy()` refuses by name rather than clamping), then asserts the recap toggle and the `bash` switch are unmoved. Checked the artisanal way: against the old interleaved body it fails on the first assertion (`a failed reload must not leave the recap toggle applied`); against the split it passes. Both reload witnesses now share a `reload_fixture` helper, so the redirected user home and the all-defaults `Config` are built once. ## Not in this PR - `main` is red on two gates this branch does not touch, and **four** unbreak PRs are already open for them, so I deliberately did not add a fifth: - **file-size ratchet** — `stella-core/src/driver.rs` (2572 vs a ceiling of 2571) and `stella-pipeline/src/pipeline/tests.rs` (2537 vs 2536) are over the baseline on `origin/main` itself, the parallel-merge skew. Covered by #2003, #2008, #2009. - **clippy** — a dead `spend` local in `stella-pipeline/src/pipeline/scope_stage.rs`. Covered by #2000. Both are inherited: `cargo clippy -p stella-cli --all-targets -- -D warnings` reports zero findings in a `stella-cli` file, and `check-file-size` names only the two files above, neither of them this PR's. - This PR's earlier CI red was a stale base: the run tested a merge against `43402ae4`, where `stella-pipeline`'s tests did not compile (`PassingShell`/`shell_call_result` missing, `ModelCallRole::Research` uncovered). `main` has since repaired all three; the branch is merged up to `6c345532`. - An open TOOLS panel keeps a stale render after `/reload` (and after `/model`, pre-existing) — filed as #1990 with the suggested `DeckCommand` approach, because an accurate row list needs the MCP-inclusive live stack that `run_deck_command` does not hold. ## Verification - `cargo test -p stella-cli` — 1463 + 12 integration targets, all passed, 0 failed. - `cargo clippy -p stella-cli --all-targets -- -D warnings` — zero findings in `stella-cli`; the only errors are `stella-pipeline`'s pre-existing dead `spend` local (#2000). - `cargo fmt -p stella-cli -- --check` — clean. - `check-god-files`, `check-left-behind` — OK. `check-file-size` fails only on the two inherited files named above. - Both reload witnesses re-run against the pre-fix body to confirm each one genuinely flips fail → pass. Refs #1990 ## Summary by Sourcery Add live settings reload support, including a /reload deck command and automatic application of SETTINGS tab changes without restarting. New Features: - Introduce a /reload deck command that re-reads settings from disk and reapplies them to the running session. - Allow SETTINGS tab saves for engine configuration and tool switches to take effect in the current session via deferred reloads at safe boundaries. Enhancements: - Refactor SETTINGS overlay I/O handlers into a new command_deck::settings_io module to keep command_deck.rs within size limits. - Add Config::reload_from_disk as a focused mutation API for reapplying the settings scope chain to an existing configuration. Documentation: - Document the new /reload command in the chat command reference, clarifying its effect and relationship to SETTINGS and model changes. Tests: - Add a config reload test verifying that post-construction settings edits are reapplied to enable recap and disable tools as specified on disk.
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
mainwas red ate0fbbe02on five distinct breaks stacked behind oneanother. 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 --workspacestops 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.ymldoes not run on a push tomain); evidence from this session added there.1.
pipeline/scope_stage.rs:34— deadspendlocalplan_with_reviewbindslet mut spend = Spend { budget, total };and neverreads it — the re-planning loop builds a fresh
Spendby reborrowing on eachiteration, which is the only construction the code uses. Dead since #1971,
unmasked when #1985 cleared the
plan_stagearg-count error above it.2.
management_prompt/tests.rs—ModelCallRole::Researchlisted twiceunreachable_patterns. Kept the documented placement besideUnknown, whosecomment explains why
Researchnever reaches the chokepoint; dropped the copyappended after
Summarization.3.
verification_hardening.rs— three items nothing constructsdead_code×3 onSHELL_TOOL,shell_call_result,PassingShell. The childflip_halt_armingmodule defines its own, which shadow the parent's throughuse super::*— a glob import loses to a local definition silently, so this wasnever 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}")thatFlipHaltcorrelates on, versusthe parent's fixed
"call-shell"which cannot distinguish two shell calls. Sothe parent's stale copies go. Its
moddoc claimed the child existed in orderto 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 linkAgentEvent::Compaction::rewritesdocuments itself with[CompactionRewrite],but
event.rsnever imports the type (the field spells itcrate::CompactionRewriteinline), so
broken_intra_doc_linksfaileddoc-warnings. A different gatestep from #1–#3, invisible while clippy was red. Fourth recurrence of the shape
#1986 tracks.
5.
file-size— two ceilings exceeded onmaindriver.rsat 2572/2571 andpipeline/tests.rsat 2537/2536. Neither file istouched by this branch; both were grown on
mainby merges that did notregenerate the baseline. That mechanism is #2004.
Also regenerated
docs/wire/*— the protocol types' doc comments are thatcontract, 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-updatemoveddriver.rsandpipeline/tests.rsup byone line each, for growth this branch did not author, because the growth has
already landed on
mainand reverting another PR's line is outside this task.The alternative was leaving the gate red.
The same regeneration also tightens
pipeline.rsfrom 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
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 onmainatbreak feat(brand): Stella logo assets + README logo #1, exits 0 here.
RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps— fails onmainat break sync(stella): Gemini/Vertex/Bedrock providers, custom tools, skills from monorepo #4, exits 0 here.cargo fmt --all --check— exits 0.cargo test -p stella-pipeline— 618 pass, 0 fail. That suite did notcompile at all on
main(break feat: port Phase 3/4/5 crates (context/fleet/graph/mcp/media/pipeline/tui) from the monorepo #3), so these tests were not running.scripts/check-file-size.shandmake wire-schema— both exit 0.Exit codes were read from cargo directly, not through a pipe:
cargo … | tailreports tail's status, which is always 0, and cargo colorizes
errorso aplain
^errorgrep matches nothing. Both produced a false green in thissession before being corrected.
The gate
file-sizeandgod-filespass; baseline regenerated, never hand-edited.wire-schemapasses; 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.
Nothing left behind
CARGO_TARGET_DIRbetween worktrees producescompile errors naming symbols that do not exist. Hit during this work: a
phantom
E0004forAgentEvent::TurnParked/TurnWoken, variants present inneither checkout, because a parallel job's build was linked in. Nearly caused
a wrong "fix".
fixing the trigger rather than the instances.
Refs #1972, #1986, #1997, #2004, #2013