From 3454efcd98387b221c8fb44056c64eaa0a71c3b3 Mon Sep 17 00:00:00 2001 From: chengyongru Date: Mon, 20 Apr 2026 11:59:28 +0800 Subject: [PATCH] fix(telegram): address code review issues from cherry-pick merge - Fix critical plain-text fallback that was sending raw HTML tags to users: keep raw markdown available for the fallback path - Extract TELEGRAM_HTML_MAX_LEN (4096) constant to replace hardcoded magic number and document the difference from TELEGRAM_MAX_MESSAGE_LEN - Add fallback to _send_text for extra HTML chunks when HTML parse fails - Add missing @pytest.mark.asyncio decorator on test_send_delta_stream_end_html_expansion_does_not_overflow --- nanobot/channels/telegram.py | 32 +++++++++++++++++-------- tests/channels/test_telegram_channel.py | 1 + 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/nanobot/channels/telegram.py b/nanobot/channels/telegram.py index bfb7e70a9..ca0639bc1 100644 --- a/nanobot/channels/telegram.py +++ b/nanobot/channels/telegram.py @@ -26,6 +26,11 @@ from nanobot.security.network import validate_url_target from nanobot.utils.helpers import split_message TELEGRAM_MAX_MESSAGE_LEN = 4000 # Telegram message character limit +# Telegram's actual API limit is 4096; we split raw markdown at 4000 as a +# safety margin for mid-stream edits (plain text). For _stream_end, we +# convert to HTML first and then split at the true 4096-char boundary so +# the final rendered message never overflows. +TELEGRAM_HTML_MAX_LEN = 4096 TELEGRAM_REPLY_CONTEXT_MAX_LEN = TELEGRAM_MAX_MESSAGE_LEN # Max length for reply context in user message @@ -564,12 +569,13 @@ class TelegramChannel(BaseChannel): thread_kwargs = {} if message_thread_id := meta.get("message_thread_id"): thread_kwargs["message_thread_id"] = message_thread_id - html = _markdown_to_telegram_html(buf.text) - if len(html) <= 4096: + raw_text = buf.text + html = _markdown_to_telegram_html(raw_text) + if len(html) <= TELEGRAM_HTML_MAX_LEN: primary_html = html extra_html_chunks = [] else: - html_chunks = split_message(html, 4096) + html_chunks = split_message(html, TELEGRAM_HTML_MAX_LEN) primary_html = html_chunks[0] extra_html_chunks = html_chunks[1:] try: @@ -587,11 +593,13 @@ class TelegramChannel(BaseChannel): self._stream_bufs.pop(chat_id, None) return logger.debug("Final stream edit failed (HTML), trying plain: {}", e) + # Fall back to raw markdown (not HTML) so users don't see raw tags. + primary_plain = split_message(raw_text, TELEGRAM_MAX_MESSAGE_LEN)[0] if len(raw_text) > TELEGRAM_MAX_MESSAGE_LEN else raw_text try: await self._call_with_retry( self._app.bot.edit_message_text, chat_id=int_chat_id, message_id=buf.message_id, - text=primary_html, + text=primary_plain, ) except Exception as e2: if self._is_not_modified_error(e2): @@ -600,12 +608,16 @@ class TelegramChannel(BaseChannel): logger.warning("Final stream edit failed: {}", e2) raise # Let ChannelManager handle retry for extra_html_chunk in extra_html_chunks: - await self._call_with_retry( - self._app.bot.send_message, - chat_id=int_chat_id, text=extra_html_chunk, - parse_mode="HTML", - **thread_kwargs, - ) + try: + await self._call_with_retry( + self._app.bot.send_message, + chat_id=int_chat_id, text=extra_html_chunk, + parse_mode="HTML", + **thread_kwargs, + ) + except Exception: + # Fall back to _send_text which handles HTML→plain gracefully. + await self._send_text(int_chat_id, extra_html_chunk) self._stream_bufs.pop(chat_id, None) return diff --git a/tests/channels/test_telegram_channel.py b/tests/channels/test_telegram_channel.py index c5cc3cf4e..e02ca5318 100644 --- a/tests/channels/test_telegram_channel.py +++ b/tests/channels/test_telegram_channel.py @@ -497,6 +497,7 @@ async def test_send_delta_stream_end_splits_oversized_reply() -> None: assert "123" not in channel._stream_bufs +@pytest.mark.asyncio async def test_send_delta_stream_end_html_expansion_does_not_overflow() -> None: """Markdown that expands when converted to HTML is still split correctly.