fix(repo,stella-pipeline,stella-protocol): unbreak main — file-size skew, a dead binding, and a broken doc link - #2003
Conversation
…arallel-merge skew `make gate`'s `file-size` step is red on `main` itself, and has been since #1991 merged, so every open PR inherits the failure: crates/stella-core/src/driver.rs grew to 2572 lines, over its baseline ceiling of 2571 (+1) crates/stella-pipeline/src/pipeline/tests.rs grew to 2537 lines, over its baseline ceiling of 2536 (+1) Neither PR did anything wrong. #1979 added three lines to `driver.rs` and #1962 added a line to `pipeline/tests.rs`, each regenerating the baseline on top of a `main` that did not yet carry the other's growth. Both merges are individually correct; their composition is not. This is the repository's most common cause of a red `main`, and the reason the baseline is generated rather than edited: a hand-patch of the two visible numbers fixes the symptom and leaves the next skew just as invisible. Regenerated with `make file-size-update` on top of current `main` — the whole file, not the failing lines — which is what makes the result reproducible by anyone re-running the command rather than a set of numbers someone chose. The diff moves in both directions, and the tightenings are the larger half: crates/stella-core/src/bus.rs 2126 -> 1891 (-235) crates/stella-pipeline/src/pipeline.rs 3451 -> 3181 (-270) crates/stella-core/src/driver.rs 2571 -> 2572 (+1) crates/stella-pipeline/src/pipeline/tests.rs 2536 -> 2537 (+1) The two reductions are #1994's `bus/names.rs` split and the `pipeline.rs` extraction landing in the ledger, which the same skew had been hiding — the ratchet had been holding two ceilings 505 lines looser than the tree needed. The two raises are +1 apiece against code that is already on `main` behind a review, which is exactly the irreducible case the escape hatch documents. No file dropped below the 1500-line limit, so the god-file tables in AGENTS.md and the crate READMEs are untouched; `check-god-files.sh` confirms all three copies still agree. Verified: scripts/check-file-size.sh OK - 1129 files, 30 grandfathered, none grew scripts/check-god-files.sh OK - 22 god files across 7 crates The same guard fails on a clean branch off `origin/main` with no other change applied, which is how this was isolated from PR #1992's own diff.
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRegenerates the file-size baseline artifact from the current main branch to resolve a skew caused by two parallel merges, tightening some ceilings and slightly raising others so that the check-file-size guard passes again without altering any source code. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Verified from a parallel session: this PR is one of four independently-red gates on main at 6c34553, and all four must land. The others are #2000 (rustdoc + clippy) and #2005 (fmt newline). On a local tree of Ordering caution, since this PR owns the baseline. Merging this alongside #2000 leaves |
…ding and an unresolved rustdoc link
`main` fails `cargo clippy -D warnings` and `cargo doc -D warnings`, both
independently of the file-size skew this branch already fixes. Neither is
reachable from the ratchet, so the branch had to carry all three or land green
on a tree that is still red.
`scope_stage.rs:34` built a `Spend` that nothing ever read:
error: variable does not need to be mutable
error: unused variable: `spend`
--> crates/stella-pipeline/src/pipeline/scope_stage.rs:34:13
It is the binding's only occurrence in the file. The loop below it deliberately
constructs a *fresh* reborrow per iteration — its own comment says why ("a moved
`Spend` could not be handed to the next attempt") — so the outer binding is
leftover from that refactor, not a value the loop shadows. Deleted rather than
underscore-prefixed: `_spend` would keep a dead constructor and read as
intentional.
`event.rs:545` linked an item that is not in the `event` module's scope:
error: unresolved link to `CompactionRewrite`
--> crates/stella-protocol/src/event.rs:545:53
The type is re-exported at the crate root (`lib.rs:75`) and the field two lines
below already spells it `crate::CompactionRewrite`; only the doc link was
unqualified. Qualified it to match, which keeps the link pointing at the same
item the field names.
Both are one-line changes to already-reviewed code. No behavior changes: the
deleted binding had no reader, and a doc link is not code.
…, not by rewriting the published description
The previous commit fixed `cargo doc -D warnings` by qualifying the link as
[`crate::CompactionRewrite`]. That turned `docs/wire` red instead:
check-wire-schema: FAIL — docs/wire/agentevent.schema.json is stale.
check-wire-schema: FAIL — docs/wire/agentevent.d.ts is stale.
check-wire-schema: FAIL — docs/wire/serveframe.schema.json is stale.
check-wire-schema: FAIL — docs/wire/serveframe.d.ts is stale.
Doc comments on wire types are not commentary — they are exported into all four
generated artifacts as the field's `description`. The `$ref` never moved, so the
shape was identical and only the prose differed, but that prose ships: a
TypeScript consumer reading `agentevent.d.ts` would have been handed a `crate::`
path, which is Rust syntax that names nothing on their side.
So fix the scope rather than the text. `CompactionRewrite` is publicly
re-exported (`lib.rs:75`) from a public module (`lib.rs:54`), and this file
already imports its siblings exactly this way — `crate::context_event::…`,
`crate::subagent_event::…`, `crate::tool::…`. Importing it puts the item in
scope, so the unqualified link resolves, and the field drops its inline path to
match every other field in the enum.
The doc comment is now byte-identical to `main` — it does not appear in this
change's diff at all — so no generated artifact moves and `docs/wire` needs no
regeneration. The `$ref` is derived from the type's schema name and never from
the use-site path spelling, so the field edit is invisible to the exporters too.
|
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 |
…d match arm, and the shell doubles #1971 left orphaned Three more `main` failures, all of which the earlier lib-level errors were hiding: `cargo clippy --all-targets` stops at the first crate that fails, so `stella-pipeline`'s *lib test* target was never checked until the `spend` fix let the lib compile. Each is in a file this branch had not otherwise touched. `cargo fmt --check` — `event/tests.rs` lost its trailing newline in #1994's split: Diff in crates/stella-protocol/src/event/tests.rs:1487: mod tag_table; + Fixed by `cargo fmt --all`; that one byte is the entire formatting diff. `unreachable pattern` — `management_prompt/tests.rs` names `ModelCallRole::Research` twice in the same or-pattern: error: unreachable pattern --> crates/stella-pipeline/src/management_prompt/tests.rs:98:11 90 | | ModelCallRole::Research <- matches all the relevant values 98 | | ModelCallRole::Research <- no value can reach this Dropped the trailing one. The surviving arm is the one the comment directly above the match names ("`Research` (#1778) rides the sub-agent primitive"), and it sits in the position that comment describes; the copy after `Summarization` is a merge appending a role that was already there. `dead_code` ×3 — `SHELL_TOOL`, `shell_call_result` and `PassingShell` in `verification_hardening.rs` are unreachable, because `flip_halt_arming.rs` defines its own `PassingShell` and `shell_call_result` and an explicit item shadows a `use super::*` glob. Nothing errored when #1971 restored the deleted witness with its own doubles; the parent's pair just went quietly dead. Deleted the parent's three rather than the child's two, because the child's module doc is the normative statement and it argues for exactly this layout: They were split apart once already, and the parent's next wholesale rewrite deleted the configured-command witness and both doubles without failing a gate [...] Keeping the cluster in one file is what makes that clobber a merge conflict instead of a silent deletion. The child is already self-contained — its doubles spell the tool name `"bash"` inline and never read the parent's `SHELL_TOOL` — so removing the parent's copy takes nothing away and restores the one-cluster-one-file invariant that #1971 and #1997 exist to protect. Retargeted the parent's `mod` doc, which still claimed the child reaches the shell fakes through this file.
…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.
…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
… 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.
… the observatory journal and fleet row (#1857 follow-up) (#2005) Follow-up to #1994 (which merged while this was being written). Two independent things, the first urgent. ## 1. Unbreak main — `cargo fmt --check` is red #1994 left `crates/stella-protocol/src/event/tests.rs` without a trailing newline, so `cargo fmt --check` fails on `main` and **every open PR inherits the red**. One character. My break, and worth recording how it escaped: the `mod tag_table;` declaration was appended by a heredoc *after* that PR's rustfmt pass, so the local gate had already run over the file in its earlier shape and reported green. ## 2. Three wildcard consumers where the park silently regressed Adding `TurnParked`/`TurnWoken` broke five crates one at a time — the compiler caught those. It could not catch the consumers matching `AgentEvent` behind a wildcard, and because the park used to arrive as `Text`, all three silently got **worse** when it became its own variant: - **`stella-observatory` `execution_journal`** selects an `event_type` allowlist. `'text'` was on it; `'turn_parked'`/`'turn_woken'` were not — so the park **disappeared entirely** from the transcript an operator opens to ask why an execution took so long. It is the one event that explains a wall-clock gap containing no other events. - Added to the allowlist, plus `journal_entry` arms carrying the payload (a row saying a park happened but not what was awaited or for how long answers nothing), plus a frontend arm in `assets/index.html` — without one it fell through to the `"answer"` branch and drew a blank row. - **`stella-tui` fleet dashboard** froze `row.action` on the last tool, so a worker deliberately waiting read as one stuck mid-tool. New `LastAction::Parked` rather than reusing `Blocked`: `Blocked` means *a human must act*, and a park needs nobody — mislabelling it would send an operator hunting an approval prompt that does not exist. `TurnWoken` deliberately holds the park rather than clearing to `Idle`, since the next tool or message repaints a beat later and clearing would only flicker. ## Deliberately not done - **`stella-serve`'s `TallyFold`** (`observe/tally.rs`) drops both variants into `_ => {}`. Not a regression (the park was already dropped as `Text`), but its own doc says a turn whose stages stop advancing is *wedged* — and park/wake are exactly the signal distinguishing a deliberate wait from a hang. Filed rather than bundled, since it means adding a field to `TurnTally`. - **`TENDENCY_EVENT_TYPES`** excludes park/wake. Judged correct, not accidental: every other member is a defect or corrective signal (retry, loop, budget denial, fallback); a park is normal intended operation. ## Note on the `file-size` gate `main` is *also* red on `file-size` — `driver.rs` and `pipeline/tests.rs` are each +1 over ceiling from a parallel-merge skew. **Neither file is touched here**, and #2003 is already the unbreak for it, so this PR deliberately does not duplicate that baseline regeneration. Refs #1857 ## Summary by Sourcery Unbreak the main branch formatting gate and update observability surfaces so parked and woken spans are correctly included and represented in operator-facing views. Bug Fixes: - Restore display of parked and woken spans in the observatory execution journal and fleet dashboard so operators can see and understand deliberate waits. - Fix the main branch formatting gate by restoring the trailing newline in stella-protocol event tests. Enhancements: - Extend observatory journal payload handling and frontend rendering to show detailed context for turn_parked and turn_woken events. - Add a distinct Parked last-action state in the TUI fleet dashboard to accurately represent engine-side waits without mislabeling them as human-blocked.
`scripts/file-size-baseline.txt` is one shared cell every growing PR must write, and three times running (#1761, #1782, #2003) two PRs that each wrote it CORRECTLY composed into a red `main`. Each regenerates the whole baseline against a snapshot of `main` that does not yet carry the other's growth, so each records a stale ceiling for a file it never touched. The merge is textually clean — the two sides edit different lines — and the result is a ceiling one line below an actual. `main` then stays red and every subsequent PR inherits a failure it did not cause; #1992 sat blocked on exactly this with a tree byte-identical to `main`. Note what did not happen in that composition: the file never grew. Its ceiling moved down underneath it. A guard with one tree to look at cannot tell those apart, so it now asks the per-change question against the base: fail only when current > max(ceiling, size at base) Both halves of the `max` earn their place. `ceiling` alone is the old check and fails the innocent PR. `size at base` alone would pass a file already over its ceiling that THIS change grows further — turning inherited drift into a standing licence to bloat, which is worse than the red main being fixed. Taking the larger fails real growth and is silent about inherited violations. A ceiling raised deliberately via `--update` still passes exactly as before, since the regenerated ceiling equals the current size. The base is the pair check-deleted-tests.sh already uses, for the same reason: on a `pull_request` the checkout is `refs/pull/N/merge`, so HEAD^1 is the base branch tip and the question becomes "does this MERGE grow a god file?". ci.yml already sets the `fetch-depth: 2` that needs. Locally it is the merge base with origin/main; on a linear push it is HEAD^1. When no base resolves — shallow clone, root commit, no origin — the guard falls back to the old whole-tree check, because an unresolvable base must make the ratchet stricter, never weaker. Drift is still reported, on stderr and in the summary line, so the baseline debt stays visible instead of merely being tolerated. It just no longer fails the next PR to walk past it. Witness: scripts/test-file-size.sh gains five hermetic cases that build the skew from two commits in a throwaway repo. B1 (a ceiling lowered under an untouched file) and B2 (drift inherited by an unrelated change) both FAIL on the old guard and pass now. B3, B4 and B5 pass on both, which is the point of them — they constrain the fix rather than being satisfied by it, and B4 in particular pins the case a naive "already over, so ignore it" rule would wave through. Closes #2004
…) (#2267) ## The bug `scripts/file-size-baseline.txt` is one shared cell that every growing PR must write, and **three times running** (#1761, #1782, #2003) two PRs that each wrote it *correctly* composed into a red `main`. Each regenerates the whole baseline against a snapshot of `main` that does not yet carry the other's growth, so each records a stale ceiling for a file it never touched. The merge is textually clean — the two sides edit different lines — and the result is a ceiling one line below an actual. `main` then stays red and every subsequent PR inherits a failure it did not cause; #1992 sat blocked on exactly this with a tree byte-identical to `main`. The detail that names the fix: **in that composition the file never grew.** Its ceiling moved *down* underneath it. A guard with one tree to look at cannot tell those two apart, because "is this tree consistent with this baseline snapshot?" is a whole-tree question and the thing worth preventing is a per-change one. ## The fix The ratchet now judges the change: ``` fail only when current > max(ceiling, size at base) ``` Both halves of the `max` earn their place, and that is the whole design: - **`ceiling` alone** is the old check, and it fails the innocent PR above. - **`size at base` alone** would pass a file that is already over its ceiling and that *this* change grows further — turning inherited drift into a standing licence to bloat. That is strictly worse than the red `main` being fixed here. Taking the larger fails a change that genuinely grows a god file past what it inherited, and stays silent when the violation arrived from somewhere else. A ceiling raised deliberately via `--update` passes exactly as before, since the regenerated ceiling equals the current size — that path is untouched. **Where the base comes from** is the same pair `scripts/check-deleted-tests.sh` already uses, for the same reason: on a `pull_request` the checkout is `refs/pull/N/merge`, so `HEAD^1` is the base branch tip and the question becomes *"does this merge grow a god file?"* — the question a required check should answer. `ci.yml` already sets the `fetch-depth: 2` that needs, for that guard. Locally it is the merge base with `origin/main`; on a linear push it is `HEAD^1`. **When no base resolves** — shallow clone, root commit, no origin — the guard falls back to the old whole-tree check. That direction is deliberate and is pinned by a test: an unresolvable base must make the ratchet *stricter*, never weaker. Drift is still reported, on stderr and in the summary line, so the baseline debt stays visible rather than merely tolerated. It just no longer fails the next PR to walk past it. ## Witness tests `scripts/test-file-size.sh` gains six hermetic cases that rebuild the skew from two commits in a throwaway repo — no network, no reliance on this repository's real history. Verified the artisanal way, by running the new suite against `origin/main`'s guard: ``` $ git show origin/main:scripts/check-file-size.sh > scripts/check-file-size.sh $ ./scripts/test-file-size.sh FAIL B1 a ceiling lowered under an untouched file is not this change's failure FAIL B2 inherited drift does not fail a change that touched something else ok B3 a change that genuinely grows a god file still fails ok B4 growth on top of inherited drift still fails ok B5 an unresolvable base falls back to the strict whole-tree check FAIL B6 a refs/pull/N/merge checkout finds its base branch tip unaided passed 10, failed 3 ``` **B1, B2, B6 are the witnesses** — they fail on the old code and pass on the new. **B3, B4, B5 pass on both, which is the point of them**: they constrain the fix rather than being satisfied by it. B4 is the one I would ask a reviewer to look at hardest — it is the case a naive *"already over at the base, so ignore it"* rule would wave through, and it is why the rule takes the `max` rather than either term alone. B6 exists because B1–B4 all pass the base in by hand, which left the rung production actually depends on as the one rung nothing exercised. With the fix in place: **13 passed, 0 failed** (the 7 pre-existing language-coverage cases are untouched and still green). ## Verification ``` ./scripts/test-file-size.sh 13 passed, 0 failed make guards-fast exit 0 shellcheck scripts/check-file-size.sh scripts/test-file-size.sh clean ./scripts/check-file-size.sh OK — 1187 files … (none grew by this change) ./scripts/check-god-files.sh OK — 22 god files, named identically in AGENTS.md and every crate README ``` `make gate`'s Rust tiers are unaffected — this change is shell and markdown only. ## Definition of done, from the issue - [x] Two PRs growing *different* god files, each regenerating against a `main` lacking the other's growth, compose green — B1/B2, and B6 in the real merge shape. - [x] A PR that genuinely grows a god file past its ceiling still fails, message unchanged — B3, asserting the exact original string. - [x] A hermetic case covering the skew, built like the issue's repro. - [x] AGENTS.md § "God files" updated to state what the ratchet now judges. - [x] `check-god-files.sh` still green; guards green. ## Scope I deliberately did not take The same shared-cell shape could in principle red an innocent PR through the `OBSOLETE` and `STALE` branches (a sibling retires a baseline entry you did not). That has never been observed, and widening the change-relative rule to those paths without a witness would be speculative — the three recorded occurrences are all the `GREW` branch. Noted here rather than silently left. Also worth stating plainly, since two issues were recently written on the opposite premise: **splitting a god file buys structure, not slack.** `--update` retightens every ceiling to its file's current size, so a freshly split file sits at zero headroom again — see my comment on this issue. This PR is what removes the tax; the split does not. Closes #2004 --- ## Follow-up found in this PR's own CI log The first green run exposed something no red check would have: **`.github/workflows/file-size.yml` — the cheap, requirable status context for this very guard — checked out at the default depth of 1**, under a comment stating full history was unnecessary because the guard "reads the working tree and the committed baseline, never a diff against the base." That was true until this PR and is now false. At depth 1 the merge commit's parents are not in the clone, so the base cannot resolve and the guard falls back to the strict whole-tree check. The job stays green and reports the ratchet exactly as before — so the fix would have been **silently absent from the one job whose purpose is to report this guard cheaply enough to be required.** `ci.yml`'s guards job was already at `fetch-depth: 2` and unaffected; this workflow is now too, the same trade it already makes for `check-deleted-tests.sh`. To stop that class of silent non-application recurring, the guard now names its mode in the summary line: ``` check-file-size: OK — 1187 … (none grew by this change). Judged against 20e47d9. check-file-size: OK — 1187 … (none grew by this change). No base resolved — strict whole-tree check. ``` The change-relative rule is silent by nature — it only shows itself when something drifts — so without that line a too-shallow checkout is indistinguishable from a clean run in every log it will ever print. Now it is one glance.
…, so a single stage can be ablated (closes #2381) (#2462) ## What Makes **who performs each pipeline responsibility** and **whether it runs at all** configuration instead of a literal at every call site — the `stella_pipeline::roster` module. #2381 asked for a per-stage off-switch so triage and the verifier can be ablated independently; this delivers that as one mechanism rather than a bespoke flag, so the same key also reassigns a responsibility to a different agent. ```json { "agent_engine_config": { "responsibilities": { "triage": { "enabled": false }, // ablate a stage "witness_author": { "agent": "triage" }, // reassign it "verdict": { "agent": "worker" } // self-grade, and be told so } } } ``` Reachable from `settings.json`, `stella.toml` (`[agents.responsibilities.*]`), and an ArenaBench match TOML (`[contestant.engine.responsibilities.*]`). ## Why this shape **`ModelCallRole` is the responsibility vocabulary — no fourth enum was added.** Every paid call in the crate already names a `ModelCallRole` (what the call is *for*) beside a `Role` (who *serves* it): `WitnessAuthor` beside `Role::Verifier`, `Triage` beside `Role::Triage`. That pair *is* the binding this PR makes configurable. Introducing a new "responsibility" enum beside `Role`, `ModelCallRole` and `StageKind` would have added a fourth lookalike id to the six AGENTS.md already keeps a glossary for. `ModelCallRole` is also total by construction (`ALL` is macro-derived and compiler-checked) and is the unit the paid-call ledger attributes spend to, so a roster row and a usage line spell the same job the same way. **Adding a responsibility is a compile error until someone owns it.** `roster::default_agent` is an exhaustive match over `ModelCallRole`; a new variant fails it with `E0004` until its default binding is declared. That is the whole maintenance contract — modelled on `model_call_roles!` in `stella-protocol`, which binds `ALL` to the enum the same way. **Stage *order* is deliberately not configurable, and I want that reviewed as a decision rather than an omission.** The ordering is not a workflow, it is a proof protocol: the witness is authored in a pristine snapshot of the pre-execution tree by a model that is not the worker, and its fail→pass flip is credited only when the worker never touched the witness files. A config file that could move authoring after the diff would still *report* a flip — it would simply have stopped meaning anything. `replay::stage_transition_legal` encodes the same ordering for recorded streams, and an operator-authored graph would make replay validation undecidable rather than merely stricter. So: the set of responsibilities and their order are code; assignment and enablement are configuration. **Repairs follow their principal rather than carrying a row.** `plan_repair` and `witness_repair` re-attempt the same call, so they are issued to whichever agent produced the output being repaired. Naming one in config is a named error pointing at the row the operator meant (`RosterError::FollowsPrincipal`), not a silently ignored key — every row in the roster steers something. ## Behaviour - **Defaults are unchanged.** `Roster::default()` reproduces the bindings each call site hard-coded, pinned by `roster::tests::the_default_roster_is_the_pipeline_that_shipped` and by the existing 662-test pipeline suite passing untouched. An ArenaBench arm that declares nothing emits no `responsibilities` key, so **posture digests for existing arms do not move**. - **An ablated stage emits no `StageKind` frame** — requirement 2. A reader of `stella-events.jsonl` sees the ablation instead of inferring it from a call that never happened. - **A disabled verdict does not become a claimed-done** — requirement 3. The deterministic ladder still runs (it is not the verifier), and the turn routes to the *abstention* rung (`ProofStep::VerificationUnavailable`), not the degradation rung: nothing degraded, the operator removed the stage. The resulting `passed: true` means "nothing failed this", never "something proved it". - **Self-grading is legal and never silent.** Binding `verdict` to the worker is reported as a lost independence. Binding `witness_author` to the worker disables authoring rather than producing a false proof — that filter is unconditional and not a roster question. - **An unhonourable roster refuses before spend** (`PipelineError::InvalidRoster`), reporting every problem at once. The refusal lives in `Pipeline::run` rather than in the CLI, so every host — serve, fleet, deck — gets it; `Roster::validate` is total because key-level rejections are recorded on the roster itself. ### One semantic fix the witness test surfaced Ablating triage initially also removed **verification**, because the fallback was the deterministic floor and the floor's cheapest class (`SimpleLookup`) skips the ladder. An operator ablating one stage would have silently ablated two and attributed both effects to triage. `triage_ablated` now clamps at `SingleTask`: the *fast path* is what disappears with triage, while the floor's own upward evidence survives it. An unresolvable triage keeps the plain floor — an ablation is not an outage, which is the theme running through the whole change. ## Witness `crates/stella-pipeline/src/pipeline/tests/roster_ablation.rs`, exactly as #2381 specified: one prompt with triage disabled asserts no `StageKind::Triage` frame while `Execute` and `Verify` still appear. It fails on `main` because there is no key to disable triage with, so the frame is emitted on every run. Three siblings cover the verdict ablation, the untouched default, and the before-spend refusal. Plus 16 unit tests in `roster/tests.rs` and 10 in `arenabench/tests/test_responsibilities.py`. ## Baseline change: none, after the main merge **This PR no longer touches `scripts/file-size-baseline.txt`.** It originally raised `pipeline/tests.rs` 2537 → 2538 for one `mod roster_ablation;` line, because that file sat exactly at its ceiling. Merging `main` removed the need: `main` split `pipeline/tests.rs` down to **2475** lines, so the `+1` is no longer earned and the entry was reverted to `main`'s value in d638ff1. The baseline diff against `main` is now empty — which is the outcome worth having, since that file is the one shared cell behind the parallel-merge-skew incidents (#1761, #1782, #2003). `pipeline.rs` itself **shrank** 3165 → 3161. ## The main merge, and the repair it needed `main` was merged in 4073d9e. Git reported no conflict on three stage signatures and was wrong to: `main` renamed `PipelineBudgetAbort` → `PipelineStageAbort` while this branch independently edited the same signatures, so the merge kept this side's text and left three references to a type that no longer exists. Each side was internally consistent, which is exactly why the rename/edit collision merged silently. It broke `cargo check --workspace` and the release smoke build; the `E0277` "size cannot be known" errors in `pipeline.rs` were cascade from the unresolved error type, not separate faults. d638ff1 repairs it — three signatures in `triage_stage.rs` and `verifier_stage.rs`, plus the baseline revert above. ## Exemplars `ModelCallRole::ALL`'s `model_call_roles!` macro (this workspace) for the compiler-enforced totality argument; `rustfmt`/`cargo`'s pattern of a pure decision module over owned data with the I/O seam beside it, matching this crate's existing `triage.rs` / `triage_stage.rs` split. ## Left behind (filed, not skipped) - #2456 — the agent vocabulary is still closed at four names, so a *genuinely new* agent is still a code change. The seam is built for it (`AgentId` is a string newtype, resolution funnels through one function); opening it needs `AgentEngineAgents` to become a map, which is 235 `EngineAgentKind` references across 16 CLI files and orthogonal to the ablation controls #2374 is blocked on. - #2457 — `ContextRecall` and `ScopeReview` issue no model call, so they have no roster row and cannot be ablated. Two of #2374's fourteen features need that; the issue states the design question rather than prejudging it. - #2458 — witness authoring is now decided in two places (`witness_writer` AND the roster), ANDed additively. The concrete bug hiding in the duplication is that the resume frame carries only one of them. Closes #2381 Refs #2374 ## Summary by Sourcery Introduce a configurable responsibility roster for the Stella pipeline, allowing per-stage enablement and agent reassignment while preserving the default pipeline behaviour and stage ordering. New Features: - Add a responsibility roster in stella-pipeline to configure who performs each pipeline responsibility and whether individual stages (e.g., triage, verdict) run at all. - Expose responsibility configuration via CLI settings, TOML, JSON engine config, and ArenaBench match templates, including posture reporting for ablations and self-grading. - Support responsibility-aware routing in pipeline stages so triage, research, planning, witness authoring, guidance, and verdict can respect roster enablement and agent assignment. Bug Fixes: - Ensure triage ablation no longer implicitly removes verification by clamping the deterministic floor so verification still runs without triage. Enhancements: - Centralize responsibility-to-agent bindings in a dedicated roster module with validation, independence-loss detection, and compile-time defaults matching the shipped pipeline. - Wire the pipeline to validate responsibility rosters before spend and to emit explicit posture warnings for ablated stages and loss of verifier independence. - Refactor pipeline stage routing (triage, research, planning, witness authoring, guidance, verdict) to go through the roster seam rather than hard-coded role resolution. Documentation: - Document the responsibility roster and single-stage ablation semantics in the inference pipeline docs, including configuration examples and rationale for non-configurable stage ordering. Tests: - Add unit and integration tests for roster behaviour, single-stage ablation, invalid roster refusal, and ArenaBench responsibility configuration and posture emission. - Update file size baseline and pipeline tests to cover the new roster module and roster-based ablation behaviour.
What & why
mainis red on three independent gate steps, so every open PR inheritsfailures it did not cause. This PR is the smallest reviewable change that turns
all three green. None of them is reachable from the others, which is why they are
here together rather than in three PRs — landing one still leaves
mainred.file size ratchetcargo clippy -D warningsspendbindingcargo doc -D warnings1.
file size ratchet— a parallel-merge baseline skewNeither contributing PR did anything wrong. #1979 added three lines to
driver.rsand #1962 added a line topipeline/tests.rs, and each regeneratedscripts/file-size-baseline.txton top of amainthat did not yet carry theother's growth. Both were green on their own merge commits; the composition is
red. This is the repository's most common cause of a red
main, and it is whythe baseline is generated rather than edited — hand-patching the two visible
numbers fixes today's symptom and leaves the next skew just as invisible.
Regenerated with
make file-size-updateon top of currentmain— the wholefile, not the failing lines — so the result is reproducible by re-running the
command rather than a set of numbers someone picked. The diff moves in both
directions, and the tightenings are the larger half:
crates/stella-core/src/bus.rscrates/stella-pipeline/src/pipeline.rscrates/stella-core/src/driver.rscrates/stella-pipeline/src/pipeline/tests.rsThe two reductions are #1994's
bus/names.rssplit and thepipeline.rsextraction finally reaching the ledger — work the same skew had been hiding. The
ratchet had been holding those two ceilings 505 lines looser than the tree
actually needs, which is the direction that quietly readmits bloat.
The two raises are +1 apiece against code already on
mainbehind a review —precisely the irreducible case the escape hatch is documented for (AGENTS.md §
"God files"). No new baseline entry, and no file grandfathered that was not
already. No entry dropped below the 1500-line limit either, so the god-file
tables in
AGENTS.mdand the crate READMEs are untouched.2.
cargo clippy -D warnings— a dead bindinglet mut spend = Spend { budget, total };is the binding's only occurrencein the file. The loop immediately below deliberately constructs a fresh
reborrow on each iteration, and its own comment says why:
So the outer binding is leftover from that refactor, not a value the loop
shadows. Deleted rather than underscore-prefixed —
_spendwould keep a deadconstructor alive and read as deliberate to the next person.
3.
cargo doc -D warnings— an unqualified intra-doc linkThe type is re-exported at the crate root (
lib.rs:75) but is not in theeventmodule's scope. The field two lines below the doc comment already spells it
crate::CompactionRewrite; only the link was unqualified. Qualified it to match,so the link and the field now name the same item by the same path.
The witness
Not applicable, and deliberately so — this is a build-artifact regeneration plus
two one-line corrections to already-reviewed code, with no behavior change: the
deleted binding had no reader, and a doc link is not code. The gate steps are
the test, and all three flip fail → pass.
The ratchet flip was verified locally on a branch cut straight from
origin/mainwith no other change applied:That same probe branch is how the ratchet failure was isolated from PR #1992's
diff — #1992 is byte-identical to
mainfor both the baseline and both failingfiles, so the red was
main's, not its.The clippy and rustdoc fixes were verified in CI, not locally, and that was a
deliberate choice. Three Terminal-Bench runs are live on this machine right now,
including a Stella-vs-Claude-Code head-to-head. A full workspace clippy + rustdoc
compile would have contended for CPU with a measured benchmark and skewed its
wall-clock numbers. Per CLAUDE.md's "measure honestly" rule, a slower verification
path is the correct trade against corrupting a benchmark this project reports in
public. The cheap guards above compile nothing, which is why they were safe to run.
Ground-rule check
stella-core#[allow]used to silence either lint — both were real defectsNothing left behind
#2004 — the ratchet has no defense against this skew, and this is its third
occurrence (#1761, #1782, now this). Two PRs can each regenerate the baseline
correctly against different snapshots of
mainand compose into a red tree;nothing detects it until the next push pays for it. The proposal is to make the
guard judge the change rather than the tree — a file already over its ceiling
at the merge base must not fail a PR that did not grow it. AGENTS.md already
rejects the shared-cell design for
GATE_STEPScounts (#1883) for exactly thisreason; the baseline has the same shape and never got the same treatment.
This PR deliberately does not attempt that fix.
mainis red right now, andan unbreak should be the smallest reviewable thing that turns it green.
Related to #1986 —
ci.ymldoes not run on a push tomain, which is why allthree of these survived on
mainrather than being caught at merge time.Anything reviewers should know?
This unblocks #1992, the only other open PR, which is
MERGEABLE/BLOCKEDsolely on these checks. Its own merge conflict is already resolved (
origin/mainis an ancestor of its head); GitHub had simply not recomputed the stale
CONFLICTINGflag. It will needmainmerged in after this lands.