fix(stella-pipeline,scripts): unbreak main — two unbreak PRs fixed the same three things, plus the file-size ratchet blocking every PR - #2008
Merged
Conversation
![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.
|
4 tasks
Contributor
Reviewer's GuideThis PR resolves duplicate fixes that landed in parallel for stella-pipeline and tests, restores clippy cleanliness, and unblocks the file-size ratchet by deduplicating shell test doubles, fixing an over-broad pattern match, removing an unused hoisted variable, and regenerating the file-size baseline to reflect current main. Sequence diagram for flip_halt_arming test doubles and FlipHalt::observesequenceDiagram
actor TestRunner
participant flip_halt_arming
participant PassingShell
participant FlipHalt
TestRunner->>flip_halt_arming: flip_halt_arming
flip_halt_arming->>PassingShell: shell_call_result
PassingShell-->>flip_halt_arming: shell_output_with_exit_code_0
flip_halt_arming->>FlipHalt: observe
FlipHalt-->>flip_halt_arming: flip_halt_latched
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
…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.
… 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.
macanderson
force-pushed
the
unbreak-duplicate-research-arm
branch
from
August 7, 2026 03:09
49eecd0 to
234cfe3
Compare
A fifth break on `main`, landed by #1994 (6c34553) minutes ago and unrelated to everything else in this PR: `crates/stella-protocol/src/event/tests.rs` ends without the blank line after `mod tag_table;` that rustfmt emits, so `cargo fmt --all --check` fails and takes the `fmt + clippy + test` job with it — on every open PR, not just this one. Applied with `cargo fmt --all`; the whole diff is one newline. Folded in here rather than filed because this PR already exists to make `main` build, and a one-newline fix in its own PR would spend more review attention than it costs to read.
Owner
Author
|
Rebased onto
Also worth noting from the rebase: the file-size ratchet now passes without the two +1 ceiling raises being load-bearing any more — main's own baseline caught up in the interim. The regeneration commit stays because it also closes 270 lines of stale headroom on
|
Owner
Author
|
Correction to my previous comment — I got the file-size half wrong. Main's baseline has not caught up:
The rest of that comment holds: the fifth break (#1994's missing trailing newline) is real and fixed here, and |
…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.
Owner
Author
|
CI caught a sixth break, also not mine — and my local verification had a real gap that let it through.
Fixed by pointing the link at the path the field already uses — the same repair #1985 applied to My gap: I had been verifying with Full |
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.
Owner
Author
|
Full local verification, every tier CI runs — after the gap I described above, I stopped relying on the fast rungs:
That is Six breaks now fixed here, none of them originating in this branch:
1–3 are the #1985/#1995 collision; 4–6 are independent and each arrived from a different merge. |
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.
Why main is red
mainat e0fbbe0 failscargo clippy -p stella-pipeline --all-targets -- -D warningsandfile 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:
ModelCallRole::Researchtwice in one or-pattern (unreachable_patterns)management_prompt/tests.rslet mut spendand a per-iteration reborrow (unused_variables+unused_mut)pipeline/scope_stage.rsPassingShell/shell_call_resultrestored into both the parent and its child (dead_code×3)tests/verification_hardening.rsscripts/file-size-baseline.txtThe judgment calls
(2) — kept #1985's per-iteration reborrow, not the hoisted binding. Not arbitrary:
plan_with_reviewloops, replanning after a rejected scope card, and only aSpendreborrowed 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_TOOLas a const distinct fromWRITING_TOOL, and say why the trailing[exit code: 0]marker is load-bearing — without itFlipHalt::observenever latches and the arming test passes for no reason. Those are the ones kept. They live inflip_halt_armingwith 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'smoddoc 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:
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.rsfrom 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 viamake file-size-update, never hand-edited.Verification
cargo clippy -p stella-pipeline --all-targets -- -D warnings— cleancargo test -p stella-pipeline --lib— 596 passed, 0 failed, both FlipHalt never arms on the authored-witness path, and revise turns pass None even for configured commands #1793 witnesses among themmake guards-fast— green,file-sizeandgod-filesincludedRelated
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:
Enhancements:
Build: