feat(stella-serve,stella-tui): make a parked wait visible — count it in the turn tally, and run its clock on the deck (#2006, #2007) - #2040
Merged
Conversation
…is not read as a hang
`TallyFold::observe` dropped `AgentEvent::TurnParked`/`TurnWoken` into its
inert `_ => {}` arm — precisely the distinction the tally exists to make.
`TurnTally::stages` documents itself as the progress axis ("a turn whose
stages stopped advancing while a reverse request's wait climbs is wedged"),
but a parked turn stops advancing stages ON PURPOSE: it probes external state
on the engine's own clock, zero model calls, until the state changes or the
deadline expires. So a host reading the tally saw a stages-stall it could not
tell apart from a hang, and the one signal that explained it was on the
stream and discarded.
Four counters, all additive and zero-skipping so a record written before this
parses unchanged:
- `parked_spans` / `parked_spans_woken` — counted apart rather than assumed
equal, because the engine's park loop returns WITHOUT a wake when the turn
is cancelled or a soft stop is latched (`driver::waiting`). A span left open
means the turn ended inside the wait, which is a different diagnosis from
"waited and came back". `TurnTally::ended_parked` names it.
- `parked_polls` — the park's own progress axis, the direct analogue of
`stages` for a turn deliberately making no stage progress.
- `parked_deadline_secs` — the licence to sit still, and the only honest
duration this fold can produce: it counts, it never reads a clock.
`Metrics` gains `parked_spans_total` / `parked_deadline_secs_total` for the
same reason one layer up — a scrape that sees `model_calls_total` flat and
`turn_duration_ms_total` climbing otherwise has no way to tell a fleet
waiting on external state from one that is stuck.
Content-free by construction: `TurnParked.description` is tool-authored free
text and is never read, only the counts and the closed `WakeReason` token are
touched, and a new test pins that two parks differing only in their prose are
indistinguishable in the tally. The existing `payload_events_are_inert` test
is untouched.
`TurnTally` rides `ServeEvent`, not `ServerFrame`, so it reaches no
`docs/wire` artifact and no schema regen is required.
Refs #1857, #1471
Closes #2006
…k/classify.rs `deck.rs` sat at 1492 of the 1500-line guard, and #2007's parked-wait clock needs about fourteen lines in it. The sanctioned move is to extract a coherent cluster rather than raise a ceiling, exactly as `prompt_queue.rs` was split out of this same file for the same reason. The cluster is the file's "Event → derived attributes" section: four pure functions over `&AgentEvent` that hold no state — `event_intensity`, `status_from_event`, `trace_of` and `snip`. They were private, called only from within `deck.rs`, so the cut needs no visibility change beyond the `pub(super)` a child module requires. A pure move: the function bodies are byte-identical, `deck.rs` re-imports the four names so every call site is unchanged, and no test changed. deck.rs drops to 1207 lines. Refs #2007
…ng down, not just the budget #1994 shipped the chip; this is the clock. A park can last up to its deadline (30 minutes is an ordinary `deadline_secs`) and the engine emits *nothing* for the whole span — it sleeps, replays a read-only probe, and loops. So the deck drew `⏳ parked until CI for branch main settles · every 30s, up to 1800s` and then sat motionless for half an hour. The row stated the BUDGET and never the ELAPSED, so a park ten seconds old and one twenty-nine minutes into its deadline read identically, and a genuinely wedged engine read like both. Option 1 of the three the issue weighed, split so that no clock is ever read inside a fold: - `SessionModel::parked: Option<OpenPark>` — the pure *what*, set by `TurnParked` and cleared by `TurnWoken`. A turn ending closes an open span too, because `driver::waiting` returns WITHOUT a wake when the turn is cancelled or soft-stopped; a retryable error is mid-flight and leaves it alone, the same reading the proof rail and plan take of that event. L-T1 is untouched: `replay(&log) == replay(&log)` still holds, and `a_parked_wait_folds_into_typed_entries_not_narration` is unchanged. - `deck::AgentEntry::parked_since_ms` — the *when*, stamped from the deck's injected `now_ms` exactly as `turn_started_ms` is. Nothing clears it; `AgentEntry::live_park` gates on the pure fold instead, so a leftover stamp is inert rather than a resurrected chip (there is a test for precisely that). - `render_hud` grows the chip: `⏳ parked 4:12 / 30:00 · CI for branch main settles`. It takes elapsed as a plain number, so it reads no clock and a golden frame can pin it. The countdown lands on the stat box rather than in the transcript on purpose: a transcript is a log of things that happened, and the ⏳ row already written to scrollback has to keep reading as history after the wake — not freeze holding a counter that stopped. The settled park still reads correctly in scrollback, unchanged. Deck goldens are undisturbed: a session with no open park renders byte-for-byte as before, which is also asserted directly. `deck.rs` was at 1492 of the 1500-line guard, so its pure event classifiers moved to `deck/classify.rs` first (previous commit) rather than the ceiling moving. `views/session.rs` is at its own exact ceiling, so its one call site changed in place, one line for one line. Refs #1857, #1471 Closes #2007
Contributor
There was a problem hiding this comment.
Sorry @macanderson, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
Reviewer's GuideImplements parked-wait observability across stella-serve and stella-tui: TurnTally and Metrics now count parked spans and deadlines so parked turns are distinguishable from wedged ones, and the TUI deck gains live parked-wait state plus a HUD clock that advances on the deck’s own clock without reading time inside folds; deck classifiers were extracted to a helper module to stay under file-size guards. Sequence diagram for parked-wait events through serve tally and metricssequenceDiagram
participant Engine as Engine
participant ServeObserver as TallyFold
participant TurnTally as TurnTally
participant Metrics as Metrics
Engine->>ServeObserver: observe(AgentEvent::TurnParked)
ServeObserver->>TurnTally: increment parked_spans
ServeObserver->>TurnTally: add deadline_secs to parked_deadline_secs
Engine->>ServeObserver: observe(AgentEvent::TurnWoken)
ServeObserver->>TurnTally: increment parked_spans_woken
ServeObserver->>TurnTally: add polls_used to parked_polls
Engine->>Metrics: observe(ServeEvent::TurnSettled { tally })
Metrics->>Metrics: fetch_add(parked_spans_total, tally.parked_spans)
Metrics->>Metrics: fetch_add(parked_deadline_secs_total, tally.parked_deadline_secs)
Metrics->>Metrics: snapshot()
Metrics-->>Engine: Snapshot { parked_spans_total, parked_deadline_secs_total }
Sequence diagram for parked-wait state into TUI HUD clocksequenceDiagram
participant Engine as Engine
participant Session as SessionModel
participant Deck as AgentEntry
participant View as views::session::render
participant HUD as render_hud
Engine->>Session: apply_event(AgentEvent::TurnParked)
Session->>Session: parked = Some(OpenPark { .. })
Engine->>Session: apply_event(AgentEvent::TurnWoken)
Session->>Session: parked = None
loop each frame
View->>Deck: WorkspaceModel::apply_event(..., now_ms)
Note over Deck: on TurnParked
Deck->>Deck: parked_since_ms = Some(now_ms)
View->>Deck: live_park(model.now_ms)
Deck-->>View: Option<(&OpenPark, elapsed_ms)>
View->>HUD: render_hud(&Hud, parked, area, buf)
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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
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
Both remaining consumers from #1857 that still cannot see a parked wait. They are one logical change — "make a park visible to the surfaces that read a stall" — approached from the two ends the #1857 sweep left open, and the issues themselves note they are worth sequencing together.
#2006 —
stella-serve: the turn tally could not tell a parked turn from a wedged one.TallyFold::observedroppedAgentEvent::TurnParked/TurnWokeninto its inert_ => {}arm — precisely the distinction the tally exists to make.TurnTally::stagesdocuments itself as the progress axis ("a turn whosestagesstopped advancing while a reverse request's wait climbs is wedged"), but a parked turn stops advancing stages on purpose: it probes external state on the engine's own clock, zero model calls, until the state changes or the deadline expires. A host reading the tally saw a stages-stall it could not tell apart from a hang, and the one signal that explained it was on the stream and discarded.Four counters, additive and zero-skipping so a record written before this parses unchanged:
parked_spans/parked_spans_wokendriver::waitingreturns without a wake when the turn is cancelled or a soft stop is latched, so a turn can settle mid-span.TurnTally::ended_parked()names that case — "ended inside the wait" is a different diagnosis from "waited and came back".parked_pollsstagesfor a turn deliberately making no stage progress.parked_deadline_secsMetricsgainsparked_spans_total/parked_deadline_secs_totalfor the same reason one layer up — a scrape seeingmodel_calls_totalflat andturn_duration_ms_totalclimbing otherwise cannot tell a fleet waiting on external state from one that is stuck. Fixing the record and leaving the scrape blind would have closed half the gap.#2007 —
stella-tui: the deck showed the park but not the clock.#1994 shipped the chip; this is the clock. A park lasts up to its deadline (30 minutes is an ordinary
deadline_secs) and the engine emits nothing for the whole span. So the deck drew⏳ parked until CI for branch main settles · every 30s, up to 1800sand sat motionless for half an hour: the row stated the budget and never the elapsed, so a park ten seconds old and one twenty-nine minutes in read identically — and a genuinely wedged engine read like both.Option 1 of the three the issue weighed, split so no clock is ever read inside a fold:
SessionModel::parked: Option<OpenPark>— the pure what. Set byTurnParked, cleared byTurnWoken; a turn ending closes an open span too, because a cancelled or soft-stopped park never gets its wake. A retryableErroris mid-flight and leaves it alone — the same reading the proof rail and the plan already take of that event.deck::AgentEntry::parked_since_ms— the when, stamped from the deck's injectednow_msexactly asturn_started_msis. Nothing clears it;AgentEntry::live_parkgates on the pure fold instead, so a leftover stamp is inert rather than a resurrected chip.render_hudgrows the chip:⏳ parked 4:12 / 30:00 · CI for branch main settles. Elapsed arrives as a plain number, so the renderer reads no clock and a golden frame can pin it.The countdown lands on the stat box rather than in the transcript deliberately: a transcript is a log of things that happened, and the ⏳ row already in scrollback must keep reading as history after the wake, not freeze holding a counter that stopped.
Closes #2006
Closes #2007
Refs #1857, #1471
The witness
main, passes here)Three, one per claim:
stella-serve—observe::tally::tests::a_parked_turn_is_distinguishable_from_a_wedged_onefolds[Stage, TurnParked, TurnWoken]against[Stage]alone and asserts the two tallies differ whilestagesis equal. Onmainthe two are byte-identical, which is the defect. (a_turn_that_settles_mid_park_leaves_the_span_openandthe_park_description_never_reaches_the_tallycover the open-span and content-free halves.)stella-tui—deck::tests::the_park_clock_advances_on_the_decks_own_clock_with_no_further_eventsparks a lane, then advancesnow_msalone with no further input of any kind, and asserts elapsed moved. That is the feature stated exactly: onmainthere is nothing there to count.stella-tui—render::tests::the_hud_counts_an_open_park_up_against_its_deadlinerenders the same park at two moments and asserts the rows differ and readparked 0:10 / 30:00.Both features are new fields, so the witnesses do not compile against
mainrather than failing an assertion — the usual shape for an additive-field feature; the assertions are written so they would fail (not merely not-build) against a version of the field that was never populated.The gate
cargo fmt --checkcargo clippy -p stella-serve -p stella-tui --all-targets -- -D warningscargo test -p stella-serve(168 pass) andcargo test -p stella-tui(814 lib + every integration suite)make guards-fast— all 20 toolchain-free guards, includingfile-size,god-files,module-reachabilityandgate-parityCloses #Nappears both above and as a commit trailerNot run locally:
cargo test --workspace/cargo clippy --workspace. A Terminal-Bench 2.1 match is running on this machine and a workspace build would perturb the measurement it exists to produce. The exposure isstella-cli, the only crate downstream of the two touched here: both changes are purely additive (new struct fields, one newpub(crate)fn parameter whose sole call site is updated in place), and there is no exhaustiveSessionModel/TurnTally/AgentEntryliteral anywhere in the workspace — verified by grep. CI is the check on that.Nothing left behind
One thing worth naming rather than filing: #1858 (
RemoteToolExecutorcannot forward parked-wait requests across the wire) is still open, and until it lands a served turn cannot park at all — so #2006's counters are correct-and-unexercised on the serve path today. That is the observability half of the same story arriving first, as #2006 itself describes, not an omission here.Ground-rule check
stella-core(untouched); no new depsTurnTally's four counters are exercised with non-zero values inevents_round_trip_byte_for_byte, because theirskip_serializing_ifmeans a zeroed tally would have proven nothing about themAnything reviewers should know?
deck.rswas split first, in its own commit. It sat at 1492 of the 1500-line guard and #2007 needs ~14 lines in it. Rather than move a ceiling, its four pure event classifiers (event_intensity,status_from_event,trace_of,snip) moved todeck/classify.rs— the same extractionprompt_queue.rstook out of this same file, for the same reason. A pure move: bodies byte-identical,deck.rsre-imports the names so every call site is unchanged, no test touched, 1492 → 1207. Reviewing that commit alone should be quick.views/session.rsis at its exact ceiling (1611/1611), which is why its call site changed one line for one line rather than growing. That constraint shaped the design —AgentEntry::live_parkexists partly so the whole join fits in a single call.Deck goldens are undisturbed and that is asserted, not assumed: a session with no open park adds no spans, so it renders byte-for-byte as before.
the_park_clock_rolls_past_an_hour_and_is_absent_when_not_parkedpins the absent case directly. NoBLESS=1was run and no snapshot changed.Exemplar followed for the clock discipline:
deck::AgentEntry::turn_started_msandfleet_dashboard'snow: Instantparameter — both inject the clock rather than reading one inside a fold, which is what keepsSessionModel::replay(&log) == SessionModel::replay(&log)(L-T1) true. Option 2 from #2007 (a per-probeTurnParkProbewire event) was rejected as the issue suggested: it buys precision the user does not need, adds stream volume proportional to park length, and taxes replay.Summary by Sourcery
Expose parked waits as first-class state in both the server observability path and the TUI deck, so deliberate parks are visible and distinguishable from wedged turns.
New Features:
Enhancements:
Tests: