fix: unbreak main's fmt gate, and stop the parked span vanishing from the observatory journal and fleet row (#1857 follow-up) - #2005
Conversation
…om the operator journal and the fleet row Three consumers matched AgentEvent behind a wildcard, so the compiler could not flag them — and because the park used to arrive as Text, all three silently got WORSE when it became its own variant: - stella-observatory's 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. Added to the allowlist, with journal_entry arms carrying the payload (a row saying a park happened but not what was awaited answers nothing) and a frontend arm — without one it fell through to 'answer' and drew a blank row. - stella-tui's 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. Refs #1857
…event/tests.rs #1994 left crates/stella-protocol/src/event/tests.rs without a final newline, so cargo fmt --check fails on main and every open PR inherits the red. My break: the mod declaration was appended by a heredoc after the rustfmt pass in that PR, so the local gate had already run over the file in its earlier shape. Refs #1857
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. 1 Skipped Deployment
|
Reviewer's GuideFixes main’s formatting gate by restoring a trailing newline, and makes parked/woken turn events visible and semantically correct in the observatory journal and TUI fleet dashboard. Flow diagram for handling TurnParked and TurnWoken events in observatory and TUIflowchart TD
AgentEvent_TurnParked["AgentEvent::TurnParked"]
AgentEvent_TurnWoken["AgentEvent::TurnWoken"]
subgraph Observatory_journal
DB_query["execution_journal SQL event_type IN (... 'turn_parked', 'turn_woken')"]
journal_entry_fn["journal_entry adds payload fields"]
html_render["index.html renders turn_parked / turn_woken rows"]
end
subgraph TUI_fleet_dashboard
LastAction_enum["LastAction adds Parked(String)"]
TaskRow_action["TaskRow action formatting for Parked"]
end
AgentEvent_TurnParked --> DB_query
AgentEvent_TurnWoken --> DB_query
DB_query --> journal_entry_fn --> html_render
AgentEvent_TurnParked --> LastAction_enum
LastAction_enum --> TaskRow_action
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
CI red here is entirely inherited from
Files this PR touches: The fmt row is the one that matters for sequencing: it is my break from #1994, no other open PR carries the fix, and it is red on One methodology note worth recording, since it is why I nearly missed this: I first checked clippy with |
|
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 |
|
Superseded by #2015 — this can be closed. #2015 now contains every commit from this branch, byte-for-byte (it picked them up from
…and it additionally carries the pipeline clippy fixes and a So #2015 is the strictly-larger, correct one. Landing both risks the duplicate-unbreak collision this repo has already hit twice; landing #2015 alone loses nothing from here. Leaving the close to a human rather than doing it myself, since I cannot tell from outside whether anything else is sequenced behind this branch. The one thing that must not get dropped in the shuffle: the fmt fix is the only repair for a break I introduced in #1994, and |
…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
Follow-up to #1994 (which merged while this was being written). Two independent things, the first urgent.
1. Unbreak main —
cargo fmt --checkis red#1994 left
crates/stella-protocol/src/event/tests.rswithout a trailing newline, socargo fmt --checkfails onmainand 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/TurnWokenbroke five crates one at a time — the compiler caught those. It could not catch the consumers matchingAgentEventbehind a wildcard, and because the park used to arrive asText, all three silently got worse when it became its own variant:stella-observatoryexecution_journalselects anevent_typeallowlist.'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.journal_entryarms carrying the payload (a row saying a park happened but not what was awaited or for how long answers nothing), plus a frontend arm inassets/index.html— without one it fell through to the"answer"branch and drew a blank row.stella-tuifleet dashboard frozerow.actionon the last tool, so a worker deliberately waiting read as one stuck mid-tool. NewLastAction::Parkedrather than reusingBlocked:Blockedmeans a human must act, and a park needs nobody — mislabelling it would send an operator hunting an approval prompt that does not exist.TurnWokendeliberately holds the park rather than clearing toIdle, since the next tool or message repaints a beat later and clearing would only flicker.Deliberately not done
stella-serve'sTallyFold(observe/tally.rs) drops both variants into_ => {}. Not a regression (the park was already dropped asText), 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 toTurnTally.TENDENCY_EVENT_TYPESexcludes 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-sizegatemainis also red onfile-size—driver.rsandpipeline/tests.rsare 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:
Enhancements: