fix(gateway): flush fallback text before segment reset to prevent silent data loss#8417
Conversation
…ent data loss When streaming edits fail (flood control) and a tool boundary arrives, _reset_segment_state() clears the accumulated text without flushing it. This silently drops content the user never received. Flush pending fallback text before the reset, mirroring the existing _DONE handler pattern. Skip the flush for __no_edit__ platforms (Signal, webhook) where all text intentionally accumulates until stream end. Fixes NousResearch#8124
There was a problem hiding this comment.
Pull request overview
Fixes a gateway streaming edge case where buffered text in “fallback mode” could be silently dropped when a tool boundary (_NEW_SEGMENT) arrives before _DONE, by flushing pending fallback text prior to resetting segment state.
Changes:
- Flush pending fallback text on segment breaks (except
__no_edit__platforms) before calling_reset_segment_state(). - Update the existing segment-break test to assert the buffered fallback tail is delivered rather than dropped.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
gateway/stream_consumer.py |
Adds a guard to flush accumulated fallback text before segment reset to prevent mid-stream data loss on tool boundaries. |
tests/gateway/test_stream_consumer.py |
Updates regression coverage to verify fallback text is delivered across a tool boundary. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Flush any pending fallback text before resetting. | ||
| # Without this, accumulated text from a failed edit is | ||
| # silently dropped when a tool boundary arrives. (#8124) | ||
| # Skip for __no_edit__ platforms (Signal, webhook) where | ||
| # all text intentionally accumulates until _DONE. |
| if (self._fallback_final_send and self._accumulated | ||
| and self._message_id != "__no_edit__"): | ||
| await self._send_fallback_final(self._accumulated) | ||
| self._reset_segment_state(preserve_no_edit=True) |
|
Closing as superseded by #12414 (merged), which adopted @konsisumer's PR #11974 for the same bug. Your PR identified #8124 first (Apr 12, 6 days earlier) — both contributors credited in the salvage PR body. We went with #11974's implementation because its firing condition ( Thanks for the early identification and the clean diagnostic write-up. |
What does this PR do?
Fixes #8124 — streaming text silently dropped when a tool boundary arrives during fallback mode.
Problem
When a progressive edit fails (flood control), the stream consumer enters fallback mode: it sets
_fallback_final_send = Trueand continues accumulating text. At_DONE, the accumulated text is sent as a new message via_send_fallback_final().However, when a tool boundary (
_NEW_SEGMENT) arrives before_DONE,_reset_segment_state()clears_accumulated,_fallback_final_send, and_fallback_prefix— destroying the pending content. The user never receives the text that was buffered between the edit failure and the tool call.Before:
["Hello ▉", "Next segment"]— " world" silently droppedAfter:
["Hello ▉", "world", "Next segment"]— all text deliveredFix
Flush pending fallback text via
_send_fallback_final()before calling_reset_segment_state()on segment breaks. This mirrors the existing flush pattern in the_DONEhandler (line 241–242).The flush is skipped for
__no_edit__platforms (Signal, webhook withgithub_comment) where all text intentionally accumulates across boundaries and is sent once at stream end.Changes
gateway/stream_consumer.py: 3-line guard + flush before segment resettests/gateway/test_stream_consumer.py: updated existing test to verify content is delivered instead of droppedHow to Test
All 28 tests pass, including the updated regression test and the existing
__no_edit__path test (test_no_message_id_segment_breaks_do_not_resend).