fix(gateway): delete the frozen streamed bubble once the ledger redelivers the refused final - #106370
AlexxRussell wants to merge 2 commits into
Conversation
eac682d to
577c492
Compare
alt-glitch
added
type/bug
PR #106370 — fix(gateway): delete the frozen streamed bubble once the ledger redelivers the refused finalSummary: Large change across Findings (all Non-blocking):
Verdict: Careful lifecycle work; guards read correctly. Safe to merge. |
577c492 to
40a125e
Compare
|
Rebased onto current main ( No conflicts of its own: everything upstream moved underneath it was resolved in One upstream test needed retargeting, and it is the part of this rebase most The one dash in this diff is upstream's own
|
40a125e to
8420dc6
Compare
|
Rebased onto current main (together with #105200, which this stacks on). One conflict is worth pointing out because it only shows up as context in the diff: main now releases the turn marker inside |
…e the final lands The stream consumer deletes the previews it replaces, but only on paths where it sends the replacement itself. When its edits stop working entirely (a Telegram flood window longer than the 5s inline cap) control returns to run_turn.py and the gateway sends the final instead. That branch only logged, so the frozen preview stayed in the chat above the complete reply, showing raw MarkdownV2 markers and the streaming cursor. The cleanup registers the consumer's own preview deletion through the post-delivery callback, with three guards so it can never remove the reader's only copy of the answer: - skipped when the stream did deliver the content (those previews are the reply); - a new public seam on the transport mixin that hands over only the single frozen bubble of a non-split segment: finalized earlier segments survive a tool boundary, and one bubble holds at most one platform message of the reply's prefix, which any successful final send covers even when an adapter capped a long reply by message count or length and still reported success; - gated on a new delivery stamp base.py writes on the session event before firing the hook. The stamp means the COMPLETE final text reached the user: its own send, or a TTS caption carrying the whole text. Bare audio does not count, and neither does the plain-text fallback when it truncated the reply (new SendResult.truncated). On the pending-drain path the hook now fires before the hand-off, so a finishing turn can no longer pop the next turn's callback with its own outcome. The post-delivery hook itself stays unconditional for its lifecycle consumers. tests/gateway/test_abandoned_preview_cleanup.py: 24 cases, 18 fail on unmodified main.
…ivers the refused final The abandoned-preview cleanup (parent commit) reached one branch and one outcome: the consumer gave up and the gateway's own final landed inline. Two sibling cases kept leaving the frozen bubble in the chat, seen on Telegram on 8 and 9 Sep 2026: * Stale finalize. The consumer's finalize edit had landed a stale snapshot, so run_turn tried a reconciliation edit; a flood window refused it and the normal final send put the complete reply below the frozen bubble. That branch registered no cleanup at all. * Ledger redelivery. The gateway's final was refused too (a 102 s and a 269 s penalty, both over the 60 s inline cap), the ledger redelivered it minutes later with the flood marker, and the registered cleanup had already stood down because the post-delivery stamp said the text did not land. The redelivery runs outside the turn, so nothing fired it. Now the stale-finalize branch registers the same cleanup when its edit is refused, and a cleanup that stands down on a refused send hands its delete to the ledger row carrying that final. The row id comes from the turn's own final send: _send_final_text returns it and the post-delivery hook stamps it on the session event beside the delivery stamp, then clears it (in finally, so a cancelled callback cannot leave it behind), so neither a queued-lane send inside the handler nor an earlier turn on the shared event can leave a row behind for a turn whose own final never went out. The runner keeps a small process-local registry keyed by obligation id (24 h cutoff enforced on registration and by a timer, cap 64, oldest first), and _redeliver_claimed_obligations fires the registered follow-ups once the redelivered send has landed untruncated. A row this process already redelivered runs a late follow-up at once (the reconnect sweep redelivers inside the refused send's own finalization). A follow-up fires only for its own row; with no row nothing will replace the bubble and it stays; a redelivery after a restart finds an empty registry. Tests: 27 in tests/gateway/test_redelivered_final_preview_cleanup.py; 13 of the original 21 fail on the parent commit, the rest pass there as no-op guards. The parent's 24 still pass.
8420dc6 to
f2a6e8b
Compare
This branch has not been deployed
3 participants
Stacked on #105200 (its commit is the first one here; this PR is the second commit only). It closes the two cases that one does not reach.
What the reader sees
Telegram, a long reply streamed by editing one preview bubble. Twice in two days (8 Sep 12:05 UTC, 9 Sep 07:26 UTC) the chat ended up with a frozen partial bubble above the complete reply:
_run_agent_mark_streamed_deliverytried the reconciliation edit; a flood window refused it (Stale-finalize reconciliation edit failed ... (flood_control:269.0)), the normal final send delivered the complete reply below, and that branch registers no cleanup at all.retry after 102sand269s, both over the 60 s inline cap), so_finalize_delivery_obligationmarked the row failed and the flood timer redelivered it minutes later with the flood marker (Redelivered recovered final response ... attempt 1). The cleanup fix(gateway): delete the streaming preview the consumer abandoned once the final lands #105200 registers had already fired from the post-delivery hook with_hermes_final_delivered=Falseand stood down, correctly: at that moment the frozen bubble was the reader's only copy. Nothing fires it after the redelivery, because the redelivery runs outside the turn.So the reader keeps a truncated copy with raw MarkdownV2 markers and the cursor, and directly under it a complete copy prefixed with "part of it may already have arrived above".
The change
run_turn.py: the stale-finalize branch schedules the same cleanup when its reconciliation edit is refused (only in the arm that actually attempted an edit; the split-chain and no-editable-message arms are untouched). The callback no longer simply stands down on a refused send: if the turn's own final was ledgered, it hands the delete to that row's redelivery.platforms/base.py: the row id comes from the turn's own final send and nowhere else._send_final_text(normal lane) now returns it, through a private_send_final_ledgeredthat the publicsend_final_ledgeredwraps unchanged, and_process_message_backgroundpasses it to_fire_post_delivery_callback, which stamps it on the session event (_hermes_final_obligation_id) right before the callback fires and clears it afterwards. A queued-lanesend_final_ledgeredinside the handler (an earlier message's reply), a turn whose final was suppressed or never produced, and a stale value from an earlier turn on the shared event therefore all read as "no row".run_startup.py: a small process-local registry on the runner,_register_redelivery_followup/_fire_redelivery_followups, keyed by obligation id (24 h cutoff to match the ledger's stale window, enforced on registration and by a timer armed with each new row; cap 64, oldest first)._redeliver_claimed_obligationsfires the row's follow-ups aftermark_delivered, only when the redelivered send was not truncated. Rows this process already redelivered are remembered, so a follow-up that registers late (the reconnect sweep redelivers inside the refused send's own finalization) runs at once instead of waiting forever.Guards
Tests
tests/gateway/test_redelivered_final_preview_cleanup.py, 27 cases. 13 of the original 21 fail on the parent commit (stale-finalize registration, deferral, own-row-only firing, registry bounds, the redelivery loop's landed / refused / truncated outcomes, the base.py stamp lifecycle including a stale value from an earlier turn, and an end-to-end run on the real test-isolated ledger: refused final, row failed, redelivery, delete). The rest pass there as no-op guards. The six added after review cover the queued-lane-inside-the-handler ordering, a cancelled hook still clearing the stamp, the public bracket's unchanged return, a follow-up registering after its row already landed, and expiry without another registration. #105200's 24 still pass. The delivery ledger, restart-resume, post-delivery-callback, queued-final-ledger and platform base suites are unchanged.Relation to #105340
#105340 stops the refused preview edits from escalating the flood penalty, which is why the gateway's final ran into a 102 s and a 269 s window at all. This PR cleans up after a refusal that still happens. Independent of each other, both wanted.
Follow-up, deliberately not here
Once the redelivery can prove that no partial copy remains above it,
FLOOD_MARKER's "part of it may already have arrived above" can become evidence-based instead of unconditional. Separate change.