fix(telegram): format final replies in bound topics - #43463
Conversation
…V2 survives the split
|
Related: competing fix for #43441 alongside #43458 and #43470. This PR has the broadest scope — format-before-chunk in |
|
Reviewed the full diff across
The test suite is comprehensive: 8 new tests covering cancel-path finalization, overflow split formatting, chunk indicator escaping, inflation window routing, and the non-refinalize guard. No issues found. |
|
Consolidating the comparison since the triager flagged #43458 and #43470 as competing fixes. All three PRs agree on the core chunk-ordering change in
On the narrow PRs themselves: #43458's tests cover the split sizing and we converge on mechanics. #43470 ships no tests, and its indicator re-escape hardcodes the total ( This PR carries 19 regression tests (each demonstrated to fail before its fix), the before/after screenshots, and the documented interaction with #42766's recovery work. Happy to rebase or split if smaller increments are preferred. |
austinpickett
left a comment
There was a problem hiding this comment.
Approve: fix(telegram): format final replies in bound topics
Canonical fix — subsumes #43458. This PR is the comprehensive solution and should merge; #43458 covers only a subset of this diff.
Core correctness ✅
The bug: _edit_overflow_split chunked raw text at the 4096 UTF-16 limit, then formatted each chunk individually. MarkdownV2 escaping inflates content by ~4–8%, so a chunk that was at 4096 raw bytes would exceed 4096 after formatting. Telegram rejected it; the fallback sent raw Markdown markers to the user.
The fix correctly mirrors send(): format the entire content once, then chunk the already-formatted text, so every chunk is guaranteed within the limit.
What's complete beyond #43458
-
Inflation-window fix in
edit_message— raw text under 4096 whose formatted version exceeds it now correctly routes to the split path instead of falling through to the in-place plain-text fallback (which would show raw Markdown). -
_escape_chunk_indicator— the inline regex that escapes the(1/2)suffix is extracted as a named helper (also used insend()), eliminating duplicate logic. -
stream_consumer._last_edit_overflowed— prevents the stream consumer from issuing a redundant second finalize after an overflow split that already carriedfinalize=True, which would overflow-split again into the adopted continuation and duplicate chunks on screen. -
Cancellation path —
_send_or_editin theCancelledErrorhandler now passesfinalize=True, is_turn_final=False, so Telegram applies final formatting on early cancellation instead of leaving a raw streaming preview on screen. -
_strip_mdv2on all plain fallbacks — all three use-markdown=False branches (first_chunk, continuation, no-anchor retry) fall back to clean stripped text rather than the raw pre-formatted chunk.
Edge cases reviewed ✅
if not chunks:— #43463 changed the defensive fallback fromlen(chunks) <= 1tonot chunks. Correct:truncate_messagealways produces ≥ 1 chunk from non-empty input, so the old<= 1guard was accidentally suppressing valid single-chunk splits (when the finalize=True format already produced exactly one chunk). The new guard is tighter.- Double-format on inflation-window route — the comment correctly documents that
formattedprecomputed inedit_messageis NOT forwarded to_edit_overflow_split; the split function does its own pass. Acceptable: this rare path formats twice; the result is identical. - Non-finalize path — streaming previews correctly stay plain (no
parse_mode), confirmed bytest_non_finalize_overflow_keeps_plain_chunks.
Test coverage ✅
8 new tests across 4 files: format-before-chunk assertion, per-chunk size enforcement, topic/thread-id preservation, chunk-indicator escaping, MarkdownV2 fallback to stripped text (continuation, first-chunk edit, no-anchor retry), inflation-window routing, non-finalize independence.
Minor nit (non-blocking)
The log warning added on first-chunk MarkdownV2 failure uses self.name — fine, just confirm TelegramAdapter always has a non-None name by the time _edit_overflow_split is called (it does, as it's set in __init__). No issue.
Approved. ✅
…ly delete guard 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.
|
Merged in part via PR #43761 — your cancel-path commit was cherry-picked onto current main with your authorship preserved in git log (da81851), and your stripped-text fallbacks + re-finalize skip landed in the follow-up commit with credit. What shipped from this PR:
What was dropped, with evidence: the format-first sizing ( Your residual-findings list (entity-boundary splits in Thanks for the rigorous work — the TDD discipline and the honest residual-findings section made this salvage straightforward. |
…ly delete guard 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.
…ly delete guard 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 NousResearch#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 NousResearch#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. (cherry picked from commit 3b4c715)
…ly delete guard 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 NousResearch#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 NousResearch#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.
…ly delete guard 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 NousResearch#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 NousResearch#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.
…ly delete guard 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 NousResearch#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 NousResearch#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.
…ly delete guard 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 NousResearch#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 NousResearch#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.
…ly delete guard 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 NousResearch#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 NousResearch#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.
…ly delete guard 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 NousResearch#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 NousResearch#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.
…ly delete guard 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 NousResearch#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 NousResearch#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.
What does this PR do?
Final assistant replies in Telegram bound/forum topics could arrive showing raw Markdown markers literally (
**bold**, backticks, triple backtick fences as plain text), while the exact same content pasted manually into the same topic rendered fine. The same turns sometimes co-occurred with the overflow clipping tracked in #42765.Two delivery paths caused it, and both are fixed:
_edit_overflow_splitchunked the raw text at the full 4096 UTF-16 limit and ranformat_messageper chunk afterwards. MarkdownV2 escaping inflates real-world text by roughly 4 to 8 percent, so every formatted chunk exceeded the limit, Telegram rejected each attempt with MESSAGE_TOO_LONG, and the fallbacks delivered the raw chunk with no parse mode.send()formats first and then chunks, so normal sends never hit this. The finalize split now mirrorssend(): format the whole reply once, chunk the formatted text, re-escape the chunk indicators, and degrade per chunk to clean stripped text (never the raw chunk).edit_messagealso pre-flights finalize edits against the formatted length, so content in the inflation window (raw under 4096, formatted over it) splits properly instead of losing its formatting to the in-place plain fallback.finalize=False, which is plain by design on Telegram. The whole final reply stayed a raw streaming preview while the success flags suppressed the gateway's formatted re-send. The best-effort delivery now usesfinalize=True, is_turn_final=False(the latter keeps the fresh-final path from claiming the flags, per the Discord: tool-using responses (api_calls≥2) silently dropped — noSending responselog afterresponse ready#29346 semantics).While covering the new pre-flight, review surfaced that a got_done finalize edit which split across continuations was followed by the redundant requires-finalize edit, which re-split the full text into the adopted continuation and duplicated chunks on screen. The redundant edit is now skipped only when the first one split-and-delivered, so the explicit finalize contract for unchanged text (#25010) is untouched.
Related Issue
Fixes #43441
Related: #42766 and #42765 (overflow clipping, a distinct problem in the same continuation loop; see note below), #42443 (MarkdownV2 escaping inside code blocks), #42421 (same bug class for progress message edits, merged).
Note for #42766: its consumer-side recovery matches delivered chunks against the raw accumulated text. With this change, finalize chunks are formatted before sizing, so whichever PR merges second needs to rework that prefix match (raw content offsets, or matching against formatted text).
SendResultsemantics are unchanged here.Type of Change
Changes Made
gateway/platforms/telegram.py: finalize overflow splits format first and chunk the formatted text; shared_escape_chunk_indicatorhelper used bysend()and the split path; all split fallbacks (first chunk edit, continuations, reply-not-found retry) degrade via_strip_mdv2instead of sending the raw chunk;edit_messagefinalize pre-flight sizes against the formatted length; non-finalize streaming previews are byte-for-byte unchanged.gateway/stream_consumer.py: cancellation best-effort delivery usesfinalize=True, is_turn_final=False; a finalize edit that adopted continuation messages is not re-finalized (prevents chunk duplication).tests/gateway/test_telegram_format.py: 13 new tests (formatted split delivery, topic metadata preservation, escaped chunk indicators, stripped fallbacks, inflation window routing, unchanged plain previews,_escape_chunk_indicatorunit tests).tests/gateway/test_stream_consumer_fresh_final.py: 6 new tests (cancel path finalize semantics incl. fresh-final-enabled config and failure path, split-not-refinalized regression pair)..github/pr-screenshots/telegram-bound-topic-markdown/: evidence screenshots.How to Test
scripts/run_tests.sh tests/gateway/test_telegram_format.py tests/gateway/test_stream_consumer_fresh_final.py tests/gateway/test_stream_consumer.py tests/gateway/test_stream_consumer_thread_routing.py tests/gateway/test_telegram_send_draft_format.pyOutput:
5 files, 242 tests passed, 0 failed.finalize=False, and the re-finalize test fails with two finalize edits.tests/gateway/run is green on this host except one pre-existingtest_agent_cachemtime memoization failure, reproduced identically on a cleanorigin/mainworktree (environment dependent, unrelated to this diff).Checklist
Code
fix(scope):,feat(scope):, etc.)scripts/run_tests.sh, see How to Test for the one pre-existing host-specific failure also present on main)Documentation & Housekeeping
cli-config.yaml.exampleif I added/changed config keys, or N/A (no config changes)CONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows, or N/A (no architecture change)Screenshots / Logs
Bot-sent final reply in a bound topic before the fix, raw markers visible:
The same content pasted manually by the user into the same topic, rendering correctly (what bot replies look like after the fix):
Residual Review Findings
Known follow-ups noted during review, intentionally out of scope to keep this diff minimal and avoid colliding with #42766:
gateway/stream_consumer.py:659: a second cancellation arriving during the best-effort finalize escapes the inner exception guard, so the success flags stay unset and the gateway fallback can duplicate a delivery that actually landed. Pre-existing handler structure, slightly widened by the finalize path doing more work; a fix needs a design decision (shielding with a small budget, or catching BaseException).gateway/platforms/telegram.py:2347: chunking already formatted text can split a MarkdownV2 entity (links especially) across a chunk boundary, degrading the affected chunks to clean stripped plain text. Same exposure exists insend()today; the proper fix is an entity balance guard intruncate_messagenext to its existing backtick parity guard.gateway/platforms/telegram.py:2455: the reply-not-found retry sends stripped plain text even though the failure was the reply anchor, not parsing; it could retry the formatted chunk first.gateway/stream_consumer.py:594: a got_done delivery that goes through a first send (no prior streaming message) in the inflation band can still double-split, becausesend()does not report continuation ids; needs an additiveSendResultchange.Post-Deploy Monitoring & Validation
Overflow split,MarkdownV2 edit failed, falling back to plain text,Overflow split: MarkdownV2 first-chunk edit failed,Overflow split: stopped at.