fix(gateway): delete the streaming preview the consumer abandoned once the final lands - #105200
AlexxRussell wants to merge 1 commit into
Conversation
SummaryDeletes the streaming preview the consumer abandoned once the gateway-sent final lands: Findings
Verified
VerdictLooks good. Three independent guards with the failure modes tested individually. |
|
Note on how this relates to #105340, since the two came out of the same incident and could look overlapping. #105340 fixes the cause. A flood-refused preview edit ignored the #105200 fixes the leftover. On the one path the consumer does not own, where the gateway sends the final itself, nothing deleted the frozen preview, so it stayed above the real answer showing raw MarkdownV2 and the streaming cursor. They are complementary rather than alternatives, and neither depends on the other. #105340 makes the abandoned-preview path much rarer without removing it: a penalty above the cap, an adapter that reports no Two files appear in both, Happy to combine them into one PR if that is easier to review, or to rebase either onto a newer main on request. |
e320f82 to
829b486
Compare
829b486 to
c2d1079
Compare
c2d1079 to
6d5e879
Compare
|
Rebased onto current main ( Three things changed in the rebase. Two of them are load-bearing, so they are 1. 2. The truncation flag moved to where the truncation happens. Upstream 3. Guard order. The change itself is unchanged: the previews the consumer abandoned are deleted
|
6d5e879 to
6a2bf8e
Compare
…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.
6a2bf8e to
369379d
Compare
A streaming preview that the consumer abandons is left in the chat when the gateway sends the final reply itself, so the reader sees a truncated message with raw MarkdownV2 markers and the streaming cursor sitting above the complete answer.
What happens
The stream consumer deletes the previews it replaces.
_delete_previewshas four call sites and every one of them is inside the consumer, on a path where the consumer itself sends the replacement: the fresh-final path ingateway/stream_consumer_transport.py, and the partial, empty and silence-marker paths ingateway/stream_consumer_fallback.py.There is a fifth case the consumer does not own. When its edits stop working entirely, control returns to
gateway/run_turn.pyand the gateway sends the final instead. That branch, theelif _sc is not None:arm of_run_agent_mark_streamed_delivery, is labelled a duplicate-risk diagnostic and only logs. Nothing cleans up behind it, so the abandoned preview stays.Observed
On a 1 GB deployment running the Telegram adapter, 6 September 2026:
The four preview edits asked for a 9 second wait, which exceeds the adapter's 5 second inline cap, so each failed closed rather than sleeping. The preview froze mid-sentence. Six seconds later the complete 2888 character reply arrived as a separate message. Nothing was lost, but the frozen preview remained, and because a partial preview is rendered without MarkdownV2 its asterisks and heading markers were literal.
This pattern appears 6 times in that deployment's logs since 25 May, 4 of them in the last 8 days as traffic grew. It is cosmetic, never a lost reply.
_delete_previewsalready anticipates the neighbouring case: its docstring notes that "the same flood window that broke the finalize edit can reject this delete too, leaving the preview bubble next to the fresh final (#71047 Problem B)". This is the same class of problem on the one path that had no cleanup at all.The change
Three files, one small piece each.
gateway/stream_consumer_transport.py: two public methods on the transport mixin,abandoned_preview_ids()anddelete_abandoned_previews(), so the gateway never reaches into consumer privates. The ids are the current segment's only. A tool boundary finalizes the segment before it and resets the per-segment state, but the turn-wide preview set keeps those ids; handing that set over would delete delivered preambles whose text is absent from the final answer. And only a single frozen bubble of a non-split segment is handed over. Adapters cap a long final by message count or length and still report success (Discord keeps the first N-1 chunks and replaces the rest with a notice, WeCom and others cut at their limit, none of them marks the result). One bubble holds at most one platform message of the reply's prefix, which any successful final send covers; a split chain or several bubbles could hold more than a capped final delivered, so those stay. The delete goes through the existing_delete_previewswithretry_on_false=True, because Telegram reports a refused delete by returning False and the flood window that stranded the preview can refuse the delete too.gateway/platforms/base.py:_fire_post_delivery_callbacktakesdeliveredand stamps it on the session's interrupt event before popping the callback, the same way the run generation already travels on that event. The value is a newtext_deliveredflag, kept apart from the aggregatedelivery_succeeded: it is set only by the final text send itself, or by a TTS caption that carried the complete text. A voice reply can land its audio while the text send is refused, and that must not read as "text delivered". Nor may a truncated fallback: after a formatting failure_send_with_retrysends the first 3500 characters as plain text and returns that success, soSendResultgains atruncatedflag the fallback sets when it cut the text, and the recorder ignores such results. The hook itself stays unconditional: goal continuation and the background-review release depend on it firing either way.One ordering change on the pending-drain path. A follow-up queued mid-turn is drained by a new task that shares the session's interrupt event and re-stamps its run generation as soon as it starts. Popped from
finallyafter the hand-off, the callback could be the next turn's, fired with this turn's outcome while the next final is still in flight. The hook is therefore fired before_spawn_drain_taskon that branch, andfinallyskips it. Everywhere else the timing is unchanged.gateway/run_turn.py:_run_agent_schedule_abandoned_preview_cleanup, a sibling of_run_agent_schedule_bubble_cleanupusing the same post-delivery registration, called as the last statement of thatelifarm.Three guards keep this from ever removing the reader's only copy of the answer:
A consumer without the transport mixin, an adapter without the post-delivery hook, an empty stale set, a failed registration and a refused delete are all no-ops.
Tests
tests/gateway/test_abandoned_preview_cleanup.py, 24 cases. 18 fail without the change and the 6 pure guard tests pass there as no-ops.GatewayStreamConsumer: after a segment break,abandoned_preview_ids()returns the new segment's bubble only; a split chain and a multi-bubble segment return nothing; the__no_edit__sentinel is never included._process_message_backgroundwith aBasePlatformAdaptersubclass and a mocked_send_with_retry: success deletes, a refused send keeps the preview._send_with_retry: a formatting failure whose plain-text fallback truncates a long reply keeps the preview, and a complete fallback deletes it.Scope: this is the abandoned-preview path only. It does not change when a preview is created, when suppression fires, or the adapter's flood handling. Adapters that cap a send could set the new
SendResult.truncatedthemselves in a follow-up, which would let the cleanup cover split chains too; this PR does not depend on that. One known cosmetic edge remains: Slack's rich-blocks final clips a heading over 150 characters in the rendered message while the fallback text keeps it, so deleting a preview that showed the raw heading loses that tail from the rendered view. The consumer's own fresh-final path has the same property, so it is left alone here.