Repository navigation
octos-app: a turn completes on the kernel's durable text, not on what streamed - #92
Conversation
BH3GEI
left a comment
There was a problem hiding this comment.
Thanks for this — the measured diagnosis (ephemeral deltas dropped + the direct-sent legacy terminal overtaking the ledger-forwarded durable row) and the hold-at-terminal shape are the right call. Before the concerns, what I verified as solid:
- The CI gate passes locally:
cd app && cargo clippy -p octos-app -p octos-app-store -p octos-app-transport -p octos-app-render --all-targets --no-deps -- -D warnings. cargo test -p octos-app --lib: 191 passed / 7 failed on this branch, and the same 7 fail identically at the merge-base (97e76cd) — the corpus/exemplar drift is pre-existing, as stated.- The
split_l0_blocksrewrite: I traced the scanner over empty input, EOF-truncated fences, CRLF, adjacent/repeated fences, backticks inside string literals, and prose mentions; it matches the prompt contract (app/app/src/lib.rs:159), can't loop, and never slices a non-boundary. One cosmetic follow-up: the streaming display gatedefer_unclosed_card_fence(app/app/src/lib.rs:2293-2310) still uses substringcontains("```"), so mid-stream it can disagree with the line-based extractor on exotic shapes (e.g. aText("```")literal in partially-streamed ledger source renders as a growing code block instead of the placeholder). Transient and self-healing once the close line arrives, but worth aligning eventually.
The blocking concern is that attribution of the durable row is not tight enough — three manifestations of one root cause: an assistant row that merely exists for the session/turn is treated as THE final durable text of the held turn. I reproduced two of them with tests driving the agent with the wire shapes the pinned kernel actually produces.
Kernel facts these rest on (submodule octos @ deb433e9, crates/octos-cli/src/api/ui_protocol_transport.rs):
handle_session_hydratestamps everyHydratedMessagewithturn_id: None("Message struct does not carry typed turn_id today", ~:25201) — so the turn-tagged lookup incomplete_from_hydratenever matches on the real wire; every hydrate-resolved hold rides the "taking the newest" fallback.- A turn with empty final text persists no assistant row at all:
final_assistant_message_for_responsereturnsNoneonresponse.content.is_empty()(:38683) andis_metadata_only_assistant_row(:5103) suppresses the envelope — whiletry_emit_terminalstill sendsturn/completed. - Every non-empty assistant row of a turn (intermediate/carrier rows and the final row alike) is emitted as an
AssistantPersistedenvelope carrying the same turn thread id (message_commit_observer, :5155-5177); theassistant_segment_idthat distinguishes final from iteration is discarded atapp/app/src/backend/octos_ui.rs:851.
(1) An empty final reply completes on the previous turn's text — reproduced. Hold → hydrate reply (rows untagged, no row for this turn, one prior assistant row) → the fallback takes the newest row → [TextAuthoritative { text: "PREVIOUS TURN CARD" }, TurnComplete]. Since lib.rs prefers authoritative over the streamed text at turn end (app/app/src/lib.rs:10362-10381), the chat re-renders the previous answer as this turn's reply. This is exactly the case the 8 s expiry is documented to cover, but the hydrate reply preempts it. (complete_from_hydrate, app/app/src/backend/octos_ui.rs:364-410.)
(2) A late row for an already-retired turn poisons the next turn — reproduced. With the forwarder backlogged (this PR's own premise), a hold resolved by its hydrate reply retires the turn while the row envelopes are still queued behind it. The next same-session prompt can be in flight immediately — the auto-repair turn fires inside the TurnComplete handler (app/app/src/lib.rs:10642-10667). When turn 1's row envelope finally lands, the session fallback at app/app/src/backend/octos_ui.rs:858-868 routes it to that in-flight prompt: [TextAuthoritative { prompt_id: …0002, text: "TURN ONE CARD" }], and persisted_seen gains turn 2 — so turn 2's terminal then completes without waiting for turn 2's own row, on turn 1's text.
(3) Same root, code-verified: a turn with more than one assistant row (router narration, then the card) can have its hold completed by the carrier envelope — authoritative_arrived (app/app/src/backend/octos_ui.rs:340-357) completes on any row of the turn since the segment id is discarded — which re-creates the truncated-reply outcome this PR fixes, on multi-row turns.
A secondary, independent issue: hold_hydrates only decrements when a SessionHydrated event arrives (app/app/src/backend/octos_ui.rs:554-567). If the reply never materializes as that event — the kernel answering an RPC error instead, or the transport dropping it (try_emit on a full event channel; the post() of HydrateSession is best-effort) — the session's counter leaks, and expiry doesn't clear it either (:414-436). The session's next genuine session/hydrate (a sidebar resume) is then consumed by the stale count and swallowed, so the resumed chat never loads. One resume lost per leak, then it self-heals.
On the tests: the hydrated() fixture (app/app/src/backend/octos_ui.rs:1327-1341) tags rows with turn_id values the pinned kernel never sends, so the primary path the new tests exercise is dead on the real wire — every production hold logs "taking the newest" and rides the fallback whose failure modes are (1)-(3). Similarly, several new tests drive PayloadV2::TurnTerminal, which a stdio connection never receives (stdio_defaults leaves projection_envelope_v2 off and the app transport never negotiates v2). Replacing the hydrate fixture with untagged rows — as the kernel produces — would have caught (1) directly.
For direction: the kernel already carries the distinction the app needs — assistant_segment_id marks the final segment, and hydrate rows could be correlated by message_id with the live envelope. Even a conservative guard would close (1) and most of (2): only take the newest-row fallback when the held turn actually streamed some text (a turn with nothing streamed has nothing to salvage; completing on the empty stream, as the 8 s path already does, is the honest outcome), and don't route rows whose turn ids name retired turns through the session fallback. Kernel-side turn tagging of hydrate rows, which you already list as follow-up, is the clean long-term answer — at that point the tagged lookup stops being dead code.
The overall shape (hold at terminal + hydrate stand-in + expiry) is worth keeping; it's the attribution that needs one more tightening pass before this lands.
78a1a8d to
cb82a76
Compare
… streamed With the kernel pointed at DeepSeek V4 Flash (a thinking model, ~200 deltas/s) the hosted AppCard drew its weather card as a code block, cut off mid-line, while the same flow rendered with a slow local Qwen. Measured on the OnePlus (session 6aa89116-00000001): the kernel's ledger holds all 1840 text deltas (7194 chars, a clean ```runl0 block) but the app finalized on 897 of them (3251 chars, unclosed fence -> "still streaming" -> shown as code), and it never saw the durable assistant row before turn/completed. Reproduced on the desktop against a mock replaying that exact stream: "stream/persisted MISMATCH streamed=1937 persisted=7194". Why. `message/delta` is ephemeral: octos drops it when its stdio writer queue (1024 frames) is full (`WsConnection::try_enqueue` -> BackpressureDrop, debug-level only). The durable `assistant_persisted` row rides the ledger forwarder, which lags under the same backpressure, while the legacy `turn/completed` is direct-sent and overtakes it - the kernel's own comment in `send_notification_lifecycle` names this ordering as "the real first-turn soak failure" and fixes it only for v2-negotiated clients. The reasoning text itself never reached the message: `reasoning_delta` was already bridged to the thinking strip. Thinking merely made the model fast. Fix, backend/octos_ui.rs: a successful terminal (legacy or v2) finalizes only once the turn's durable text is in hand. Otherwise the turn is held, `session/hydrate` is requested, and the turn completes on whichever lands first - the live `assistant_persisted` row or the hydrate reply's assistant row for that turn - bridged as `TextAuthoritative` right before `TurnComplete`, which lib.rs already prefers over the streamed accumulation. A hold expires after 8 s onto the streamed text (an empty reply, a kernel without the v2 lane), so nothing hangs. The hydrate replies a hold requested are consumed by it even when the live row resolved the hold first; otherwise they would replay the session over the chat. The persisted row is routed by the envelope's turn id, falling back to the session as before. Terminal metrics are recorded once per turn. Also, app/l0_card.rs: `split_l0_blocks` treated the first "```runl0" substring as the opener and the next "```" substring as the close. Fences are lines now: the opener is a line that is ```runl0 (Markdown's three leading spaces allowed), the close is the next line that is exactly ```. DeepSeek's visible reasoning names the fence in prose and drafts the card inside a bare ``` block; substring scanning would have made a ledger out of the model's sentence had that text ever reached the message. Tests: the held-terminal paths (live row, hydrate reply, expiry, swallowed late reply, ordered lane) and the fence scanner against the recorded reasoning-plus-card shape. Verified on the desktop against the recorded stream and on the phone against DeepSeek. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
cb82a76 to
bca585e
Compare
What broke
With the kernel pointed at DeepSeek V4 Flash (a thinking model that streams at ~200 deltas/s) the hosted AppCard drew its weather card as a code block, cut off mid-line; the same flow rendered correctly with a slow local Qwen.
Measured on the OnePlus (session
6aa89116-00000001):assistant_final= 7194 chars, a clean```runl0block;assistant_reasoningseparate (3065 chars, no fence at all this run)assistant_deltaenvelopes = the full 7194 charsaichat UI turn complete content_chars=3251— 897 of 1840 deltas, unclosed fence → "still streaming" → shown as code; nostream/persisted MISMATCHline, so the durableassistant_persistedrow had not arrived beforeturn/completedReproduced on the desktop against a mock OpenAI server replaying that exact delta stream at speed:
stream/persisted MISMATCH — streamed=1937 persisted=7194 chars; using persisted(there the row happened to arrive in time).Root cause
message/deltais ephemeral: the kernel drops it when its stdio writer queue (1024 frames) is full —WsConnection::try_enqueue→BackpressureDrop, logged at debug level only (ui_protocol_transport.rs).assistant_persistedrow rides the ledger's live forwarder, which lags under the same backpressure (and only logsLagged, it does not replay), while the legacyturn/completedis direct-sent and overtakes it. The kernel's own comment insend_notification_lifecyclecalls this ordering "the real first-turn soak failure" and fixes it only forprojection.envelope.v2-negotiated clients.reasoning_deltawas already bridged to the thinking strip). Thinking only made the model fast enough to trigger the drop; a stray```runl0in the reasoning was not the cause — butsplit_l0_blockswould have mis-parsed it had it ever reached the message.Fix
backend/octos_ui.rs— a successful terminal (legacy or v2) finalizes a turn only once its durable text is in hand. Otherwise the turn is held,session/hydrateis requested, and the turn completes on whichever lands first: the liveassistant_persistedrow, or the hydrate reply's assistant row for that turn. Either is bridged asTextAuthoritativeright beforeTurnComplete, whichlib.rsalready prefers over the streamed accumulation. A hold expires after 8 s onto the streamed text (empty reply / kernel without the v2 lane), so nothing hangs. Hydrate replies a hold requested are consumed by it even when the live row resolved the hold first (otherwise they'd replay the session over the chat). The persisted row is routed by the envelope's turn id, falling back to the session as before. Terminal metrics are recorded once per turn.app/l0_card.rs—split_l0_blocksis line-based: the opener is a line that is```runl0(≤3 leading spaces), the close is the next line that is exactly```. A mid-line mention or a bare```block is prose.Tests
cargo test -p octos-app --lib: 191 passed; the 7 remaining failures (capability_bridge,exemplar_drift,l0_theme_axes, twol0_eval,nav_is_generated_from_an_l0_spec,the_language_reference_lists_every_admitted_theme) fail identically onmainwithout this change (corpus/exemplar drift, unrelated).New: held-terminal paths (live row, hydrate reply, expiry, swallowed late reply, ordered lane, error terminal) and the fence scanner against the recorded reasoning-plus-card shape.
Device
Phone run against DeepSeek V4 Flash, OctoSense pin bump OctoSense-org/OctoSense#25, which has the screenshots:
content_charsassistant_finalL0 card acceptedBoth turns logged
legacy turn/completed … arrived before its persisted row; holding completion. The persisted row then replaced the partial stream.Not changed (kernel)
No kernel change: the drop is within the protocol's contract (spec § 9 "ephemeral frames may be dropped"; "client must rehydrate via cursor"). Worth a kernel follow-up: route the legacy
turn/completedonto the ordered ledger lane for stdio clients too (whatsend_notification_lifecycledoes for v2 clients), and/or negotiateprojection.envelope.v2from this app. Thinking cannot be switched off per turn from this app:ReasoningEffortLevelhas no "off", anddeepseek-flashmatches none of the kernel's DeepSeekReasoningStylehints, so the request carries no reasoning control and DeepSeek's default (thinking on) applies.🤖 Generated with Claude Code