fix(stella-pipeline): unbreak main — duplicated unbreak halves and baseline skew - #2009
Closed
macanderson wants to merge 3 commits into
Closed
fix(stella-pipeline): unbreak main — duplicated unbreak halves and baseline skew#2009macanderson wants to merge 3 commits into
macanderson wants to merge 3 commits into
Conversation
… of the same three fixes main compiles and its tests pass, but 'cargo clippy -p stella-pipeline --all-targets' reports five warnings, and the gate runs clippy at -D warnings — so stella-pipeline still cannot pass it. All five are the duplicate-unbreak shape: two PRs fixed the same three defects concurrently, and the merge kept both halves of each. - scope_stage.rs built a 'Spend' before the re-plan loop AND a fresh one inline per iteration. The inline reborrow is the correct half (a moved Spend could not be handed to the next attempt, and its comment says so), so the outer binding goes — it was unused and unnecessarily mut, two warnings from one line. - management_prompt/tests.rs listed ModelCallRole::Research twice in the same match arm; the second is an unreachable pattern. - verification_hardening.rs and its flip_halt_arming child BOTH defined PassingShell and shell_call_result, so the child shadowed the parent and left the parent's copies dead (three warnings, counting SHELL_TOOL). Kept the parent's: it is the documented version, it carries the SHELL_TOOL constant tying the fake's schema to the name flip_halt::command_of looks for, and the parent is the natural home if a second child ever needs them. The child reaches them through its existing 'use super::*'. No behavior change: one dead binding, one unreachable pattern, and one set of shadowed test doubles removed.
check-file-size failed on main for two god files nobody's PR touched: stella-core/src/driver.rs (2572 vs 2571) and stella-pipeline/src/pipeline/tests.rs (2537 vs 2536). Merged PRs grew each by exactly one line without regenerating the baseline — the baseline skew that is this repo's most frequent cause of a red main. Regenerated with 'make file-size-update' rather than hand-edited, per the baseline's own contract. The diff is net strongly tightening: two ceilings fall because the files genuinely shrank (bus.rs 2126 -> 1891 after the names.rs split, pipeline.rs 3451 -> 3181), and two rise by exactly one line each to match what already landed. Ceilings that fall are the point of the ratchet; the two that rise are recorded here rather than left as a gate nobody can pass.
![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.
|
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideCleans up duplicated unbreak changes in stella-pipeline that were causing clippy warnings and fixes the file-size baseline so File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
This was referenced Aug 7, 2026
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
…ngress (#1787) (#2012) > Supersedes #1982, which was auto-closed when the branch it was stacked on > went away. Same single commit, now rebased directly on `main`. > > Note: `main` currently fails `cargo clippy -p stella-pipeline` with five > pre-existing warnings unrelated to this change (fixed in #2009), so this > PR's clippy step inherits them until that lands. ## What & why The last unbounded ingress into the verdict prompt, and the item #1787 folds in at the end of its body: > Also worth folding in: the trusted evidence summary has no length bound > (`oracle_trace` grows per observation; the diff has a token budget, the > trusted zone does not) — `pipeline/evidence.rs` since the extraction. `verifier_evidence_summary` renders `oracle_trace` in full. That trace gains an observation per verification round, and the repair gate (#1479) keeps granting rounds for as long as a measured budget affords them — so the one channel that grows without limit was also the one channel with no ceiling. Every other input to that prompt is bounded: the diff has a token budget, recall frames have `bound_recalled_frames`, and `Verdict::reasoning` got its cap in #1932. Bounded to the newest 24 observations, with the drop **stated in-band**: ``` oracle_trace=[…76 earlier observation(s) omitted → candidate:pass → candidate:fail → …] ``` Three choices worth naming, because each has a wrong-looking alternative: - **Newest kept, oldest dropped.** The recent runs are what the verdict weighs; a trace clipped from the front would hand the verifier a history that stops before the evidence. - **Stated, not silent.** A trace that silently began mid-run reads as the whole run — the verifier would draw conclusions about a first observation that was not the first. - **Clipped where the value is constructed**, not at a downstream consumer. That is the same "structural, not by convention" rule #1932 applied to `reasoning`, and it is why the stored `LadderSnapshot` and `verdict_provenance` are deliberately untouched: the bound is on the *prompt ingress*, not on the record. 24 is sized far above a normal run (a baseline plus a handful of rounds), so the bound only ever bites a pathological loop. It is a named constant next to its rationale rather than a literal. ## The witness - [x] This PR includes a witness test `a_pathological_oracle_trace_is_clipped_with_the_drop_stated` — a 100-observation trace renders exactly 24 entries behind the `…76 earlier observation(s) omitted →` marker, and still ends on the newest observation. `an_ordinary_oracle_trace_renders_unchanged` is the other half, and the one that matters for regression: a 5-observation trace is asserted **byte-identical to `render_oracle_trace`**, main's own unbounded function, which is still present and still used for provenance. So every prompt the bound does not bite is unchanged to the byte — which also means no verdict-reuse digest (#1431) moves for an ordinary run. Honest note on "fails on main": these pin a bound that does not exist on `main`, so the failure there is that `bounded_oracle_trace` is not defined — the same shape as #1932's witnesses for the `reasoning` cap, and the shape any "add a missing ceiling" change has. The behavioural claim is carried by the second test, which compares against main's function directly rather than against a copied expectation. ## The gate - `cargo test -p stella-pipeline` — 585 + 5 + 2 + 5 + 6 + 4 = **607 passed, 0 failed** - `cargo clippy -p stella-pipeline --all-targets` — clean - `cargo fmt --check -p stella-pipeline` — clean - `scripts/check-file-size.sh` — OK, none grew (`evidence.rs` was extracted from `pipeline.rs` precisely so this kind of channel can be added without touching a god file, and that still holds) Full workspace left to CI. ## Nothing left behind `Refs #1787`, deliberately not `Closes` — this is the folded-in bound only. Item 1 (a provider-parity-aware structured verdict output path, invariant 8) remains open and is being approached from a different angle in #1964; item 2 shipped as #1932 and item 3 as #1951. Refs #1787 ## Summary by Sourcery Bound the oracle trace rendered in verifier evidence summaries and added tests to cover the new trusted-zone length cap. New Features: - Introduce a bounded oracle trace renderer for verifier prompts that limits the trusted evidence summary to the newest observations while indicating omissions in-band. Tests: - Add witness tests ensuring long oracle traces are clipped with an explicit omission marker and that ordinary short traces remain byte-identical to the unbounded renderer.
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.
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
maincompiles and its tests pass, but it cannot pass the gate:cargo clippy -p stella-pipeline --all-targetsreports five warnings and the gateruns clippy at
-D warnings, whilescripts/check-file-size.shfails on twogod files. Every PR against
maininherits both.No behavior change anywhere in this PR — a dead binding, an unreachable
pattern, one set of shadowed test doubles, and a regenerated baseline.
The five clippy warnings: two unbreak PRs each landed half of the same fix
Three defects were fixed concurrently by two different unbreak PRs, and each
merge kept both halves:
pipeline/scope_stage.rsSpendbuilt before the re-plan loop and a fresh one built inline per iterationmanagement_prompt/tests.rsModelCallRole::Researchlisted twice in the same match armtests/verification_hardening.rs+ itsflip_halt_armingchildPassingShellandshell_call_resultdefined in bothResolved by keeping one half of each, choosing on merit rather than on which
landed first:
scope_stage.rs— the inline per-iteration reborrow is the correcthalf, and its own comment says why: the loop re-plans after a rejected scope
card, so a moved
Spendcould not be handed to the next attempt. The outerlet mut spendwas never read — two warnings (unused_variables,unused_mut) from one line.management_prompt/tests.rs— the secondModelCallRole::Researchinthe arm is an
unreachable_pattern. Removed; the first, which carries theexplanatory comment, stays.
parent's copies dead (three warnings, counting the
SHELL_TOOLconstant).Kept the parent's. It is the documented version; it carries the
SHELL_TOOLconstant that ties the fake's advertised schema to the exactname
flip_halt::command_oflooks for — the coupling that stops the armingtest passing for the wrong reason — and the parent is the natural home if a
second child ever needs them. The child reaches them through its existing
use super::*, so its two tests are untouched and still run.The file-size guard: baseline skew
check-file-size.shfailed on two god files no PR in flight touched:stella-core/src/driver.rs(2572 vs 2571) andstella-pipeline/src/pipeline/tests.rs(2537 vs 2536). Merged PRs (#1979,#1962, #1964) grew each by exactly one line without regenerating the baseline.
Regenerated with
make file-size-update— never hand-edited, per thebaseline's own contract — and the resulting diff is net strongly tightening:
Two ceilings fall because those files genuinely shrank — which is the entire
point of the ratchet — and two rise by exactly one line to record what already
landed. Flagging the two rises explicitly rather than burying them: they are
the reviewable half of this hunk, and the alternative is a gate nobody can
pass.
The witness
None, and this is the case the template's "explain how you verified instead" is
for. The defect is that a lint and a guard fail; the evidence is that they
fail on
mainand pass here. There is no behavior to witness — nothingexecutable changes, and a test asserting "clippy is clean" is the guard itself.
The two #1793 witnesses that live in the affected test module are confirmed to
still run and pass under their new (unshadowed) doubles:
The gate
Run on this branch, rebased on
6c345532:cargo clippy -p stella-pipeline --all-targets— clean (onmain: 5 warnings = 5 errors under-D warnings)cargo clippy -p stella-cli -p stella-core -p stella-tui --all-targets— clean, confirmingstella-pipelinewas the only crate affectedcargo test -p stella-pipeline— 596 + 5 + 2 + 5 + 6 + 4 = 618 passed, 0 failed (unchanged frommain, as intended)cargo fmt --check -p stella-pipeline— cleanscripts/check-file-size.sh— OK, none grew (onmain: 2 failures)scripts/check-god-files.sh— OKscripts/check-left-behind.sh— OK, 0 grandfatheredscripts/check-gate-parity.sh— OK, 25 stepsFull workspace left to CI.
Nothing left behind
The root cause here — two sessions independently unbreaking the same red
main, then their fixes colliding — is the same class as #1976 (a merge thatdeletes another PR's test is invisible to CI), filed earlier today from the
previous round of this. The duplicate-unbreak half is not yet covered by that
issue; I will add it there rather than open a near-duplicate.
Summary by Sourcery
Resolve clippy and file-size gate failures in stella-pipeline so main can pass the gate again.
Bug Fixes:
Build: