From 3b4c715e1c50cfa180934d37e8364b42b1bf158d Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 10 Jun 2026 13:32:13 -0700 Subject: [PATCH] fix(telegram): stripped-text fallbacks, re-finalize skip, and tail-only delete guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-ups on top of the two salvaged GodsBoy commits, all live-validated against the real Telegram Bot API: - _edit_overflow_split finalize fallbacks degrade to _strip_mdv2() clean text instead of putting raw **markdown** markers on screen (salvaged from PR #43463 minus its format-first sizing — live probes show Telegram's 4096 limit counts PARSED text, so MarkdownV2 escape inflation cannot cause MESSAGE_TOO_LONG and sizing against formatted wire length only causes premature splits and fragment messages). - Skip the redundant requires-finalize edit after a got_done edit that split-and-delivered (salvaged from PR #43463): re-finalizing re-splits the full text into the adopted continuation and duplicates chunks. - _send_fallback_final only deletes the stale partial message when the fallback re-sent the COMPLETE final text. When the prefix dedup sent only the missing tail, the partial IS the head of the answer; deleting it left users with only the second half of long responses (live- reproduced: flood-control during a long stream -> head deleted, ratio 0.54 of content visible). This is the third bug behind the 'Telegram cut messages' reports and was present on main and both PRs. --- gateway/platforms/telegram.py | 18 ++++- gateway/stream_consumer.py | 29 +++++-- tests/gateway/test_stream_consumer.py | 49 ++++++++++-- .../test_stream_consumer_fresh_final.py | 75 +++++++++++++++++++ 4 files changed, 156 insertions(+), 15 deletions(-) diff --git a/gateway/platforms/telegram.py b/gateway/platforms/telegram.py index 051a377dc06..5cba11d2ee9 100644 --- a/gateway/platforms/telegram.py +++ b/gateway/platforms/telegram.py @@ -2348,10 +2348,15 @@ class TelegramAdapter(BasePlatformAdapter): ) except Exception as fmt_err: if "not modified" not in str(fmt_err).lower(): + logger.warning( + "[%s] Overflow split: MarkdownV2 first-chunk edit " + "failed, falling back to plain text: %s", + self.name, fmt_err, + ) await self._bot.edit_message_text( chat_id=int(chat_id), message_id=int(message_id), - text=first_chunk, + text=_strip_mdv2(first_chunk), ) else: await self._bot.edit_message_text( @@ -2393,7 +2398,14 @@ class TelegramAdapter(BasePlatformAdapter): ) for use_markdown in (True, False) if finalize else (False,): try: - text = self.format_message(chunk) if use_markdown else chunk + if use_markdown: + text = self.format_message(chunk) + else: + # Plain attempt: on finalize the MarkdownV2 attempt + # failed, so degrade to clean stripped text, never + # the raw chunk (raw ** / ``` markers would render + # literally); streaming previews stay raw. + text = _strip_mdv2(chunk) if finalize else chunk sent_msg = await self._bot.send_message( chat_id=int(chat_id), text=text, @@ -2419,7 +2431,7 @@ class TelegramAdapter(BasePlatformAdapter): try: sent_msg = await self._bot.send_message( chat_id=int(chat_id), - text=chunk, + text=_strip_mdv2(chunk) if finalize else chunk, **retry_thread_kwargs, **self._link_preview_kwargs(), **self._notification_kwargs(metadata), diff --git a/gateway/stream_consumer.py b/gateway/stream_consumer.py index 2bd77d8dac6..53434da3c40 100644 --- a/gateway/stream_consumer.py +++ b/gateway/stream_consumer.py @@ -147,6 +147,9 @@ class GatewayStreamConsumer: self._edit_supported = True # Disabled when progressive edits are no longer usable self._last_edit_time = 0.0 self._last_sent_text = "" # Track last-sent text to skip redundant edits + # True when the most recent _send_or_edit split-and-delivered across + # continuation messages (the adapter adopted a new message id). + self._last_edit_overflowed = False self._fallback_final_send = False self._fallback_prefix = "" # True when fallback is sending only the missing tail after a partial @@ -586,14 +589,20 @@ class GatewayStreamConsumer: if self._accumulated: if self._fallback_final_send: await self._send_fallback_final(self._accumulated) - elif ( - current_update_visible - and not self._adapter_requires_finalize + elif current_update_visible and ( + not self._adapter_requires_finalize + or self._last_edit_overflowed ): # Mid-stream edit above already delivered the # final accumulated content. Skip the redundant - # final edit — but only for adapters that don't - # need an explicit finalize signal. + # final edit for adapters that don't need an + # explicit finalize signal, and for any adapter + # when that edit split-and-delivered across + # continuations: the split edit carried + # finalize=True itself, and re-finalizing with + # the full text would overflow-split again into + # the adopted continuation, duplicating chunks + # on screen. self._final_response_sent = True self._final_content_delivered = True elif self._message_id: @@ -882,7 +891,12 @@ class GatewayStreamConsumer: self._notify_new_message() # Remove the frozen partial message so the user only sees the - # complete fallback response. Best-effort — if the platform doesn't + # complete fallback response. ONLY safe when the fallback re-sent + # the FULL final text (continuation == final_text). When the + # prefix-based dedup above sent only the missing TAIL, the partial + # message IS the head of the answer — deleting it leaves the user + # with only the last part of the response (the "Gemini sent only + # the second half" symptom). Best-effort — if the platform doesn't # implement ``delete_message``, the delete fails (flood control still # active, bot lacks permission, message too old to delete), the # partial remains but at least the full answer was delivered. @@ -890,6 +904,7 @@ class GatewayStreamConsumer: stale_message_id and stale_message_id != last_message_id and not self._fallback_preserve_partial_messages + and continuation == final_text ): delete_fn = getattr(self.adapter, "delete_message", None) if delete_fn is not None: @@ -1228,6 +1243,7 @@ class GatewayStreamConsumer: return True # Failure already disabled drafts for this run; fall through to # the regular edit/send path below. + self._last_edit_overflowed = False try: if self._message_id is not None: if self._edit_supported: @@ -1284,6 +1300,7 @@ class GatewayStreamConsumer: and result.message_id and result.message_id != self._message_id ): + self._last_edit_overflowed = True self._message_id = str(result.message_id) self._message_created_ts = time.monotonic() self._last_sent_text = "" diff --git a/tests/gateway/test_stream_consumer.py b/tests/gateway/test_stream_consumer.py index 9a445532d0d..af012fb69a7 100644 --- a/tests/gateway/test_stream_consumer.py +++ b/tests/gateway/test_stream_consumer.py @@ -794,9 +794,11 @@ class TestSegmentBreakOnToolBoundary: ) @pytest.mark.asyncio - async def test_fallback_final_deletes_partial_after_chunks_succeed(self): - """After fallback chunks land, the frozen partial must be deleted so - the user sees only the complete response (#16668).""" + async def test_fallback_final_deletes_partial_after_full_resend(self): + """After fallback re-sends the COMPLETE response, the frozen partial + must be deleted so the user sees only the complete response (#16668). + Full resend happens when the visible prefix doesn't match the final + text (e.g. post-segment-break content, #10807).""" adapter = MagicMock() adapter.send = AsyncMock( return_value=SimpleNamespace(success=True, message_id="msg_new"), @@ -810,14 +812,49 @@ class TestSegmentBreakOnToolBoundary: config = StreamConsumerConfig(edit_interval=0.01, buffer_threshold=5) consumer = GatewayStreamConsumer(adapter, "chat_123", config) - # Seed the consumer as if it already edited a partial message that - # later got stuck (flood control etc.) — _message_id is the stale id. + # The stale partial shows pre-tool text that is NOT a prefix of the + # final response — fallback re-sends the complete final text. + consumer._message_id = "msg_partial" + consumer._last_sent_text = "Let me check that for you…" + + await consumer._send_fallback_final("Working on it. Done!") + + adapter.delete_message.assert_awaited_once_with("chat_123", "msg_partial") + assert consumer._final_response_sent is True + + @pytest.mark.asyncio + async def test_fallback_final_keeps_partial_after_tail_only_send(self): + """When the fallback sends only the missing TAIL (visible prefix + matches the final text), the partial message IS the head of the + answer — deleting it would leave the user with only the last part + of the response (the 'model sent only the second half' bug).""" + adapter = MagicMock() + adapter.send = AsyncMock( + return_value=SimpleNamespace(success=True, message_id="msg_new"), + ) + adapter.edit_message = AsyncMock( + return_value=SimpleNamespace(success=True), + ) + adapter.delete_message = AsyncMock(return_value=None) + adapter.MAX_MESSAGE_LENGTH = 4096 + + config = StreamConsumerConfig(edit_interval=0.01, buffer_threshold=5) + consumer = GatewayStreamConsumer(adapter, "chat_123", config) + + # Visible partial is a true prefix of the final response — the + # fallback dedup sends only the tail. consumer._message_id = "msg_partial" consumer._last_sent_text = "Working on i" await consumer._send_fallback_final("Working on it. Done!") - adapter.delete_message.assert_awaited_once_with("chat_123", "msg_partial") + # Tail was sent... + sent_contents = [ + c.kwargs.get("content", "") for c in adapter.send.call_args_list + ] + assert any("Done!" in s and "Working on i" not in s for s in sent_contents) + # ...and the head-bearing partial was NOT deleted. + adapter.delete_message.assert_not_awaited() assert consumer._final_response_sent is True @pytest.mark.asyncio diff --git a/tests/gateway/test_stream_consumer_fresh_final.py b/tests/gateway/test_stream_consumer_fresh_final.py index bf467781638..975c0ada590 100644 --- a/tests/gateway/test_stream_consumer_fresh_final.py +++ b/tests/gateway/test_stream_consumer_fresh_final.py @@ -466,6 +466,81 @@ class TestCancelledBestEffortDeliveryFinalizes: assert consumer.final_content_delivered is True +class TestGotDoneOverflowSplitNotRefinalized: + """A got_done finalize edit that split-and-delivered across continuation + messages must not be followed by the redundant requires-finalize edit. + + After a split, the consumer adopts the last continuation as the live + message and the redundant finalize edit re-submits the FULL accumulated + text against it; the adapter pre-flights that into another overflow + split, editing chunk 1 over the continuation and re-sending the rest, + so the user sees duplicated chunks. The finalize signal was already + carried by the split edit itself. + """ + + def _consumer(self, adapter): + # High interval/threshold so the only edit is the got_done finalize. + return GatewayStreamConsumer( + adapter=adapter, + chat_id="chat", + config=StreamConsumerConfig( + edit_interval=10.0, buffer_threshold=10_000, cursor=" ▉", + ), + ) + + @pytest.mark.asyncio + async def test_split_finalize_edit_is_not_refinalized(self): + adapter = _make_adapter() + adapter.REQUIRES_EDIT_FINALIZE = True + adapter.edit_message = AsyncMock(return_value=SimpleNamespace( + success=True, + message_id="cont_2", + continuation_message_ids=("cont_2",), + )) + consumer = self._consumer(adapter) + consumer.on_delta("oversize **markdown** final reply") + task = asyncio.create_task(consumer.run()) + await asyncio.sleep(0.05) # preview send lands; no interval edits + consumer.finish() + await task + + finalize_edits = [ + c for c in adapter.edit_message.call_args_list + if c.kwargs.get("finalize") + ] + assert len(finalize_edits) == 1, ( + "split finalize edit must not be re-finalized; the redundant " + "edit re-splits the full text into the adopted continuation " + "and duplicates chunks on screen" + ) + assert consumer.final_response_sent is True + assert consumer.final_content_delivered is True + + @pytest.mark.asyncio + async def test_non_split_finalize_edit_still_gets_explicit_refinalize(self): + """The narrow fix must not regress the requires-finalize contract: + a normal (non-split) got_done edit is still followed by the + explicit finalize edit (#25010 semantics unchanged).""" + adapter = _make_adapter() + adapter.REQUIRES_EDIT_FINALIZE = True + adapter.edit_message = AsyncMock(return_value=SimpleNamespace( + success=True, message_id="initial_preview", + )) + consumer = self._consumer(adapter) + consumer.on_delta("short final reply") + task = asyncio.create_task(consumer.run()) + await asyncio.sleep(0.05) + consumer.finish() + await task + + finalize_edits = [ + c for c in adapter.edit_message.call_args_list + if c.kwargs.get("finalize") + ] + assert len(finalize_edits) == 2 + assert consumer.final_response_sent is True + + class TestStreamConsumerConfigFreshFinalField: """The dataclass field must exist and default to 0 (disabled)."""