Skip to content

fix(telegram): prevent rapid-fire edits and streaming suppression after tool calls - #54331

Open
elphamale wants to merge 13 commits into
NousResearch:mainfrom
elphamale:fix/telegram-streaming-formatting
Open

fix(telegram): prevent rapid-fire edits and streaming suppression after tool calls#54331
elphamale wants to merge 13 commits into
NousResearch:mainfrom
elphamale:fix/telegram-streaming-formatting

Conversation

@elphamale

@elphamale elphamale commented Jun 28, 2026

Copy link
Copy Markdown

Summary

Fixes three bugs that caused Telegram streaming to stall mid-response or deliver plain-text markdown during streaming.

  • buffer_threshold bypassed edit_interval — the OR condition len(accumulated) >= threshold fired every ~60ms for fast LLMs, triggering 100 s+ Telegram flood-control penalties and entering fallback mode (user sees no streaming until full answer arrives). Fix: require elapsed >= min(edit_interval, 1.0) on the buffer-threshold branch so rapid LLM output cannot fire edits faster than the platform's per-chat rate limit.

  • should_hold_streaming_for_rich suppressed all streaming after tool calls — when the LLM's final response started with a table and no message was on screen yet (_message_id is None), the hold blocked every streaming tick until got_done. Fix: return False — MarkdownV2 streaming handles partial tables gracefully (incomplete rows pass as raw text; complete tables render via _wrap_markdown_tables), so suppression causes worse UX than partial rendering — particularly for tool-call responses where the user would see nothing at all until the full answer arrived.

  • Streaming ticks sent plain textedit_message(finalize=False) had no parse_mode, so MarkdownV2 markup rendered as raw **bold** characters during streaming. Fix: try MarkdownV2 first via format_message(), fall back to plain text only on BadRequest; non-BadRequest errors (flood, network) propagate to the outer handler as before.

Also adds finalize=True to the _send_fallback_final edit-in-place (introduced in #53865) so stale partial messages are replaced with fully formatted content rather than plain text.

Test plan

  • tests/gateway/test_stream_consumer_rich_hold.py — finalize=True in fallback, buffer-threshold 1 s floor, rich-hold mechanism (9 tests)
  • tests/gateway/test_telegram_streaming_tick.py — MarkdownV2 streaming tick, BadRequest plain-text fallback, flood-error propagation, should_hold_streaming_for_rich always-False (9 tests)
  • Manually verified in group chat: streaming no longer stalls at table headers or after tool calls

Update: rebased onto main, fixed the buffer_threshold floor properly, dropped rich-hold (addresses hermes-sweeper review)

This branch was 275 commits behind origin/main (it shared its first four commits with #53865's original branch, before that PR was rebased). Rebased directly onto origin/main — the earlier "stacked on #53865" framing no longer applies literally; the two PRs share duplicate underlying commits rather than one being a git-level stack on the other, since #53865 was rebased independently. The rebase itself cleanly resolved the same _try_strip_cursor conflict already worked out in #53865 (origin/main's 04898631c/4aa499ff9 had independently converged on the same "leave message intact" behavior).

buffer_threshold floor — corrected further. sweeper found that min(edit_interval, 1.0) doesn't actually provide the stated one-second floor for a short configured edit_interval (e.g. 0.1s): min(0.1, 1.0) = 0.1, so the floor silently collapses back to the short interval exactly when it should be protecting against it. Fixed with a fixed 1.0 constant — not max(edit_interval, 1.0), which was my first instinct but turns out to break the other documented behavior this branch's own test (test_buffer_waits_1s_before_firing) already encodes: a long configured edit_interval (e.g. 10s) should still let a full buffer flush early at the 1-second mark instead of waiting the whole interval — that's the entire point of this early-fire branch. max() would force it to wait the full 10s instead. A bare 1.0 fixes the short-interval bug while leaving the long-interval behavior byte-for-byte unchanged (min(10.0, 1.0) == 1.0 == the new constant).

One honest caveat: for configured intervals below 1 second specifically, this fix has no independently observable effect on stream timing — the interval-based should_edit branch immediately above it already fires at the same or an earlier elapsed time with a strictly weaker bar (any accumulated text, vs. buffer_threshold characters), so it dominates regardless of what the buffer_threshold branch's own floor is set to. The fix is still correct and worth making — it's the difference between the code actually satisfying its own documented "at least 1 second regardless of edit_interval" contract instead of only claiming to, and it removes a latent inconsistency that could matter if the interval-branch's behavior ever changes independently — but it isn't testable via stream-timing assertions in that regime, so no test claims to observe it there.

Also fixed test_buffer_waits_1s_before_firing itself, which had a pre-existing bug unrelated to sweeper's finding: it asserted on the very first-ever flush, which always fires immediately regardless of any floor logic (_last_edit_time starts at 0.0, so "elapsed since last edit" is enormous on the first tick — correct UX, first content should appear ASAP, but not what this test meant to check). Confirmed via git stash that this test failed identically on the code as originally written, before any of my changes. Rewrote it to check the floor against a second flush, after an initial one establishes a realistic _last_edit_time baseline.

Dropped the rich-hold mechanism. sweeper flagged should_hold_streaming_for_rich as dead extension surface: Telegram (the only adapter) always returns False, by explicit design, so the consumer's _rich_hold state, the hold-check block, and three dedicated test classes had no live production behavior at all. Removed the hook, the consumer-side state, Telegram's always-False stub, and the tests (TestRichHold, test_rich_hold_initialised_false, TestShouldHoldStreamingForRich). Renamed test_stream_consumer_rich_hold.py — which no longer has anything to do with rich-hold — to test_stream_consumer_fallback_finalize_and_buffer_floor.py. Reintroduce the hook if a concrete adapter actually needs and enables it.

Confirmed test_telegram_streaming_tick.py::test_streaming_tick_flood_propagates is a separate, pre-existing failure (already flagged as an undiagnosed design question in this PR's own comment history) unrelated to either fix, and the usual 9 telegram test-order-pollution failures elsewhere in the suite are unrelated (same ones confirmed against a clean origin/main in earlier PRs this session).

🤖 Generated with Claude Code

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery platform/telegram Telegram bot adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages P2 Medium — degraded but workaround exists labels Jun 28, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: Telegram streaming/draft-flood cluster. Stacked on #53865 (fix/telegram-draft-flood-cascade); test-coverage follow-ups for the same edge cases live in issue #54275 and PR #54286. Once #53865 merges, this reduces to the single streaming-formatting commit. Reviewers should sequence after #53865.

@elphamale
elphamale force-pushed the fix/telegram-streaming-formatting branch from d6ebcde to 974d40e Compare July 9, 2026 09:41
@elphamale

elphamale commented Jul 9, 2026

Copy link
Copy Markdown
Author

Rebased onto current origin/main — the branch was stale (created/abandoned 2026-06-28) and main had independently gained a0a3c716fc (dedup saturated mid-stream overflow previews) touching the same edit_message call site this PR rewrites for MarkdownV2 streaming ticks. Resolved by keeping main's saturation-dedup cache write and layering this PR's MarkdownV2-first + BadRequest-fallback formatting across all three success exits of that block.

Also found and fixed one thing already latent in this PR before the rebase (confirmed pre-existing on the original branch, not introduced by the rebase): should_hold_streaming_for_rich was checked with a truthy test in stream_consumer.py; a bare MagicMock() test adapter auto-vivifies that attribute as a callable returning a truthy Mock, silently activating rich-hold in ~20 unrelated tests using unspec'd mock adapters (mostly in test_stream_consumer.py). Production is unaffected (the real adapter always returns False there) — switched to a strict is True check, which fixed those ~20.

Also updated tests/gateway/test_telegram_format.py::test_non_final_edit_uses_plain_text_without_markdown, which asserted the old plain-text streaming-tick behavior this PR intentionally replaces — split into two tests covering the MarkdownV2-success and BadRequest-fallback paths.

Full tests/gateway/ suite: 19 failed/9003 passed — exactly the 13-failure established repo-wide baseline plus 6 failures already present in this branch's own new test files before this rebase (confirmed on the original, pre-rebase branch tip):

  • 3x test_stream_consumer.py::TestSegmentBreakOnToolBoundary::test_fallback_final_*
  • 2x test_stream_consumer_rich_hold.py (the is True fix above didn't touch these — they have their own separate issue, not yet diagnosed)
  • 1x test_telegram_streaming_tick.py::test_streaming_tick_flood_propagates — looks like a genuine design question (should a streaming-tick flood retry inline or fail fast?) rather than an obvious bug

None of the 6 are regressions from this rebase — flagging rather than fixing since diagnosing them wasn't part of this rebase's scope.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing two behaviors that are still present on current main: gateway/stream_consumer.py:638 lets buffer_threshold bypass the edit interval, and plugins/platforms/telegram/adapter.py:4002-4007 sends non-final ticks without MarkdownV2 formatting.

Problems

  • gateway/stream_consumer.py:656 uses min(self._current_edit_interval, 1.0). A configured edit_interval=0.1 still permits threshold-driven updates every 0.1 seconds, so this does not provide the stated one-second protection.
  • The new rich-hold extension has no live consumer: plugins/platforms/telegram/adapter.py:1502 always returns False, while the consumer only holds when the hook returns True (gateway/stream_consumer.py:675-684). This is dead extension surface and dedicated test coverage without production behavior.

Suggested changes

  • Define the rate-limit behavior explicitly for Telegram and make the threshold guard satisfy that contract for sub-second configured intervals.
  • Drop the rich-hold hook/state/tests unless a concrete adapter needs and enables it.
  • Salvage narrowly against the later fallback recovery changes already on main (04898631c, 4aa499ff9).

Automated hermes-sweeper review.

Comment thread gateway/stream_consumer.py Outdated
# faster than platforms' per-chat edit rate limits.
or (
len(self._accumulated) >= self.cfg.buffer_threshold
and elapsed >= min(self._current_edit_interval, 1.0)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

min() does not establish a one-second floor: with edit_interval=0.1, this condition still fires every 0.1 seconds once the threshold is reached. Please scope the Telegram limit explicitly or use a condition that actually satisfies the documented cadence contract.

Comment thread plugins/platforms/telegram/adapter.py Outdated
particularly after tool calls where the response starts with tables and
the user would see nothing until the full answer arrives.
"""
return False

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This always returns False, so the new generic should_hold_streaming_for_rich machinery cannot affect production Telegram behavior. Please remove the dead hook/state/tests unless a concrete adapter needs to opt into it.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 15, 2026
@elphamale
elphamale force-pushed the fix/telegram-streaming-formatting branch from d9d4e8e to ea9b045 Compare July 15, 2026 17:58
@elphamale

Copy link
Copy Markdown
Author

Fixed both, pushed ea9b045a2 (also rebased this branch onto origin/main — it was 275 commits behind, sharing early history with #53865's pre-rebase branch).

buffer_threshold floor: confirmed — min(edit_interval, 1.0) collapses back to the raw interval whenever it's under a second, silently dropping the floor. My first fix was max(edit_interval, 1.0), but that breaks this branch's own test_buffer_waits_1s_before_firing, which validates that a long configured interval (10s) should still let a full buffer flush early at the 1s mark rather than waiting the whole interval — the entire point of this branch. Went with a bare 1.0 constant instead: fixes the short-interval bug you flagged, leaves the long-interval behavior unchanged (min(10.0, 1.0) and the new constant are both 1.0).

One thing I want to flag rather than quietly gloss over: for configured intervals below 1 second, this fix has no independently observable effect on stream timing — the plain interval-based should_edit check right above it already fires at the same or earlier elapsed time with a weaker bar (any accumulated text vs. buffer_threshold characters), so it dominates regardless of what this branch's floor is set to. The fix is still correct — it's the difference between the code actually satisfying its own documented contract vs. only claiming to, and it matters if that other branch's behavior ever changes independently — but I didn't want to claim a timing-based regression test proves something it can't actually observe. Separately, found and fixed a real, pre-existing bug in test_buffer_waits_1s_before_firing itself (asserted on the very first-ever flush, which always fires immediately regardless of any floor — confirmed via git stash this failed identically before any of my changes).

Rich-hold: removed entirely — hook, consumer state, Telegram's always-False stub, and the three dead-code test classes. Renamed the now-misleadingly-named test file.

@elphamale
elphamale force-pushed the fix/telegram-streaming-formatting branch from ea9b045 to dc6d5bf Compare July 16, 2026 07:50
@elphamale

Copy link
Copy Markdown
Author

Rebased onto the current tip of #53865 (fix/telegram-draft-flood-cascade), which this branch had drifted apart from since neither has merged yet (today's PR-conflict-drift audit flagged this pair).

All 7 of this PR's own commits replayed cleanly on top of #53865's sendMessageDraft-failure-handling fixes. Conflict resolution, commit by commit:

Also found and fixed a pre-existing bug in this PR's own test suite, unrelated to the rebase: test_telegram_streaming_tick.py::test_streaming_tick_flood_propagates used retry_after=2, which actually falls into edit_message()'s long-standing (pre-dating both PRs) ≤5s inline-retry-then-succeed path — so success=True is the correct, existing behavior, not the failure the test asserted. Confirmed this fails identically on the original pre-rebase tip in isolation. Fixed by moving the propagation-case test to a >5s wait (where the code genuinely returns failure without retrying) and adding a companion test for the short-wait retry-and-succeed case.

Full sweep after the fix: test_telegram_streaming_tick.py, test_telegram_send_draft_format.py, test_stream_consumer_fallback_finalize_and_buffer_floor.py, test_stream_consumer_draft.py, test_telegram_send_draft_flood_control.py, test_stream_consumer.py — 181/181 passing. check-windows-footguns.py --diff origin/main clean. Force-pushed with a verified lease (remote was still at the pre-rebase tip).

@teknium1 teknium1 added the area/streaming Streaming responses: gateway delivery, provider wire label Jul 19, 2026

@GottZ GottZ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was generated by AI during triage.

Summary

Three PRs address or reference Telegram streaming truncation and flood-control behavior: #4650 restores final-send fallback after failed edits, #7683 adds merged adaptive backoff and reliable continuation delivery, and #54331 extends the current draft/edit paths with cooldown handling, threshold pacing, finalized fallback edits, and MarkdownV2 streaming ticks.

Related pull requests

  • #4650 [closed] related — (+9/-5) — superseded: This directly fixes the original truncation path by marking a failed progressive edit as not delivered and returning immediately on long Telegram flood waits, but it remains relevant as the minimal reference implementation superseded by #4727.
  • #7683 [merged] related — (+117/-25) — merged reference implementation: This merged the broader edit-stream reliability fix—adaptive flood backoff, overflow preservation, cursor cleanup, fallback-send retry, and a safer default interval—covering the same failed-edit truncation cause more comprehensively than #4650.
  • #54331 related — (+1486/-90) — keep open for narrow salvage: The diff adds useful current-path fixes for MarkdownV2 streaming ticks, draft RetryAfter cooldowns, and finalized edit-in-place fallback, but it also carries a large stacked draft-flood cluster and its fixed 1-second threshold guard does not stop the independent interval branch from emitting every 0.1 seconds when configured that way. Consistent with the contributor keep_open review on #54331, it should be narrowed and the Telegram pacing contract completed before merge.

Duplicates

#4650 and the merged #7683 substantially overlap on failed-edit flood-control truncation, with #7683 providing the broader implementation; #54331 overlaps that reliability area but also contains distinct draft-transport and streaming-formatting changes.

Suggested consolidation

Merge #54331 only after narrowing it to behavior still missing after #7683 and its #53865 base, and after enforcing the stated Telegram pacing contract across both threshold- and interval-triggered ticks; this follows the keep_open review on #54331 rather than overriding it. #4650 can remain closed as superseded, while merged #7683 should remain the reference implementation rather than be treated as a closure candidate.

Cross-PR triage: Reviewed 3 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 102 kB of PR diffs, 10 kB of issue/PR text, 9 kB of discussion (6 comments), 1 verify verdict. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

@elphamale
elphamale force-pushed the fix/telegram-streaming-formatting branch from dc6d5bf to 1ca1d27 Compare July 30, 2026 09:32
@elphamale
elphamale force-pushed the fix/telegram-streaming-formatting branch from 1ca1d27 to 82df3af Compare August 13, 2026 07:11
elphamale and others added 8 commits August 20, 2026 18:02
…od-control silence

A single sendMessageDraft failure permanently disabled draft streaming for the
entire response, forcing every subsequent token through editMessageText.  On a
long response (~200 tokens) this exhausted Telegram's per-chat edit quota and
triggered a 280 s RetryAfter on both the stream consumer and the base gateway
send, causing 4+ minutes of silence with a stuck ▉ cursor.

Three targeted fixes:

1. Failure threshold (_MAX_DRAFT_FAILURES = 3): transient draft errors no
   longer permanently disable draft streaming on the first occurrence; only
   three consecutive non-retryable failures trigger the edit fallback.

2. Flood-control retryable signal: sendMessageDraft RetryAfter ≤5 s is handled
   inline (sleep + retry); RetryAfter >5 s returns retryable=True so the caller
   does not count it against the disable threshold.

3. Cursor-strip delete fallback: when the cursor-removing edit is itself
   flood-controlled, try delete_message (different Bot API quota) so the stuck ▉
   disappears before the full response arrives.

Also adds TestDraftFallbackOnFailure and TestTryStripCursor to
test_stream_consumer_draft.py, and a new
test_telegram_send_draft_flood_control.py (13 tests total).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…view

Address gaps identified in code review:

- TestDraftFallbackOnFailure: add boundary test for exactly 2 failures (must
  NOT disable), tighten >=  to == on the 3-failure assertion, restore
  integration test (test_draft_failure_falls_back_and_delivers_final_via_send)
  verifying the fallback path delivers the final message, with an explanatory
  note on why _use_draft_streaming stays True in run-loop context

- TestTryStripCursor: add __no_edit__ sentinel noop test, add negative case for
  _is_flood_error (non-flood error must not trigger delete), add
  test_delete_message_raises_is_swallowed for the except Exception: pass branch

- test_telegram_send_draft_flood_control.py: fix fragile module-wide asyncio
  patch (replace entire asyncio mock with targeted asyncio.sleep patch), add
  5.01s boundary test confirming the long-wait path, remove dead RetryAfter
  stub that was never used by any test

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…cade

Root cause: when sendMessageDraft fails (typing action expired, unsupported
client, transient error), the stream consumer fell back to editMessageText for
every remaining token — 200+ rapid edits on a long response exhaust Telegram's
per-chat quota, causing a 280s RetryAfter and 4+ minutes of silence.

Fix: any sendMessageDraft failure that is not a long flood-control wait now
returns success=True, message_id=None (frame silently dropped) instead of a
failure result. The consumer stays in draft mode throughout and never switches
to the edit path. The final response is delivered by the base gateway send()
at finalize — one call, no quota-burning intermediate edits.

This mirrors the pattern in PR NousResearch#51886 (guest chats): returning success=True for
structurally-unsupported frames keeps the consumer on the draft path.

Cases that now return success=True (suppress):
- ok=False from the Bot API (typing action expired, client too old, etc.)
- Any non-MarkdownV2 exception (network hiccup, DRAFT_ID_INVALID, etc.)
- Short flood-control (≤5s) sleep+retry that still fails after sleep

Only long flood-control waits (>5s) still return retryable=True so the
consumer can log them without counting against _draft_failures.

_MAX_DRAFT_FAILURES is kept as a safety net but is now effectively unreachable
via the adapter (no failures propagate as success=False).

Also revert the earlier _try_strip_cursor delete-fallback: a partial message
with a stuck cursor is better UX than deleting it and leaving the user with
nothing for the flood-control wait duration.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…allback

When _send_fallback_final fires and the complete text fits in one message,
try editMessageText on the existing partial first.  No new message, no
deletion, no extra rate-limit cost.  Falls back to the original send+delete
path if the edit fails (flood control, message too old, etc.).

Also removes three tests that became vacuous or redundant after prior fixes:
- test_two_consecutive_failures_do_not_disable_drafts (range covered by 1 and 3)
- test_missing_delete_message_does_not_raise (_try_strip_cursor no longer deletes)
- test_non_flood_edit_failure_skips_delete (no deletion logic exists to skip)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…eal send/edit

szafranski's review (echoed by hermes-sweeper) identified two edge cases
where a long RetryAfter on a draft frame still cascades into the exact
flood-control storm this PR set out to prevent:

- _send_draft_frame's retryable branch correctly skipped counting the
  failure, but _send_or_edit treated any False return as "drafts already
  gave up, fall through to adapter.send()/edit_message()" — so a long
  flood-control wait immediately spent another call (a real message send)
  in the same window instead of waiting it out.
- TelegramAdapter._try_send_rich_draft collapsed every exception,
  including RetryAfter, to a bare False, so send_draft() always fell
  through to the legacy sendMessageDraft endpoint for the same frame —
  burning a second call in the same flood-control bucket.

Fixes:
- _try_send_rich_draft now returns (success, retry_after); a RetryAfter
  propagates as SendResult(retryable=True, retry_after=...) directly,
  without touching the legacy endpoint. The legacy sendMessageDraft path's
  own long-wait branch now also sets retry_after (previously only
  embedded in the error string, which _send_draft_frame never parsed).
- _send_draft_frame stores that retry_after as a cooldown deadline
  (_draft_cooldown_until) and skips the API call outright while still
  cooling down, rather than retrying every tick.
- _send_or_edit only falls through to the real send/edit path when the
  miss was NOT a retryable cooldown (_last_draft_retryable) — ordinary
  (non-retryable) draft misses still fall through immediately exactly as
  before, preserving the existing get-a-real-message-fast behavior for
  actual rejections.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012CvMiTppKDazXfakuoLgQK
Rebasing onto origin/main (this branch was 2372 commits behind) surfaced
a real interaction between this PR's own "edit stale partial in-place"
commit and three pre-existing NousResearch#10807/NousResearch#16668 regression tests in
TestSegmentBreakOnToolBoundary: those tests asserted delivery happened
specifically via adapter.send() (plus a delete_message() call for the
full-resend case), which the edit-in-place path correctly bypasses when
the final text fits in one message — same outcome (correct content
visible, no duplicate/stale message), different mechanism.

Updated the three tests to accept delivery via edit_message() as well as
send(), added a dedicated test forcing the edit to fail so the
send+delete fallback path still gets direct coverage, and confirmed all
9 gateway/test_stream_consumer.py + test_stream_consumer_draft.py +
telegram draft/rich/flood-control/final-delivery suites pass (252 tests).
Also verified the 9 pre-existing telegram test-order-pollution failures
in the wider suite are identical on a clean origin/main checkout with
none of this branch's commits — unrelated to this PR.

Addresses hermes-sweeper's third point on this PR: "the fallback-final
implementation has moved materially since this branch... reconcile with
current main before salvage." The rebase itself resolved the textual
_try_strip_cursor conflict with 0489863/4aa499ff9 (origin/main had
already converged on the same "leave message intact" behavior this PR's
first commit introduces); this commit resolves the remaining behavioral
interaction the rebase surfaced.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012CvMiTppKDazXfakuoLgQK
send_draft()'s legacy sendMessageDraft path returned success=True for
every non-flood-control failure (ok=False, expired typing action,
unsupported chat, network hiccup, retry-after-retry failures) to avoid
recreating the edit cascade. That's broader than necessary: it also
silently defeated _MAX_DRAFT_FAILURES, since a real/permanent failure
(bot blocked, chat gone) never counted toward the 3-strikes fallback
to edit-based delivery.

Narrow the suppression to flood control only (retryable=True with
retry_after) — the one case that actually needs special handling to
avoid burning another call in the same rate-limit window. Every other
failure now returns a normal success=False miss, so it participates
in the existing consecutive-failure fallback like any other adapter.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012CvMiTppKDazXfakuoLgQK
…od-control silence

A single sendMessageDraft failure permanently disabled draft streaming for the
entire response, forcing every subsequent token through editMessageText.  On a
long response (~200 tokens) this exhausted Telegram's per-chat edit quota and
triggered a 280 s RetryAfter on both the stream consumer and the base gateway
send, causing 4+ minutes of silence with a stuck ▉ cursor.

Three targeted fixes:

1. Failure threshold (_MAX_DRAFT_FAILURES = 3): transient draft errors no
   longer permanently disable draft streaming on the first occurrence; only
   three consecutive non-retryable failures trigger the edit fallback.

2. Flood-control retryable signal: sendMessageDraft RetryAfter ≤5 s is handled
   inline (sleep + retry); RetryAfter >5 s returns retryable=True so the caller
   does not count it against the disable threshold.

3. Cursor-strip delete fallback: when the cursor-removing edit is itself
   flood-controlled, try delete_message (different Bot API quota) so the stuck ▉
   disappears before the full response arrives.

Also adds TestDraftFallbackOnFailure and TestTryStripCursor to
test_stream_consumer_draft.py, and a new
test_telegram_send_draft_flood_control.py (13 tests total).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
elphamale and others added 5 commits August 20, 2026 18:04
…od-control silence

A single sendMessageDraft failure permanently disabled draft streaming for the
entire response, forcing every subsequent token through editMessageText.  On a
long response (~200 tokens) this exhausted Telegram's per-chat edit quota and
triggered a 280 s RetryAfter on both the stream consumer and the base gateway
send, causing 4+ minutes of silence with a stuck ▉ cursor.

Three targeted fixes:

1. Failure threshold (_MAX_DRAFT_FAILURES = 3): transient draft errors no
   longer permanently disable draft streaming on the first occurrence; only
   three consecutive non-retryable failures trigger the edit fallback.

2. Flood-control retryable signal: sendMessageDraft RetryAfter ≤5 s is handled
   inline (sleep + retry); RetryAfter >5 s returns retryable=True so the caller
   does not count it against the disable threshold.

3. Cursor-strip delete fallback: when the cursor-removing edit is itself
   flood-controlled, try delete_message (different Bot API quota) so the stuck ▉
   disappears before the full response arrives.

Also adds TestDraftFallbackOnFailure and TestTryStripCursor to
test_stream_consumer_draft.py, and a new
test_telegram_send_draft_flood_control.py (13 tests total).

Co-Authored-By: Claude <noreply@anthropic.com>
…er tool calls

Three bugs caused Telegram streaming to stall or deliver plain text:

1. buffer_threshold bypassed edit_interval — the OR condition
   `len(accumulated) >= threshold` fired every ~60ms for fast LLMs,
   triggering Telegram's flood-control (100s+ penalty) and entering
   fallback mode. Fix: gate buffer_threshold on `elapsed >= min(interval, 1.0)`.

2. should_hold_streaming_for_rich suppressed all streaming after tool
   calls — when the LLM response started with a table and no message was
   on screen yet, the hold blocked every streaming tick until got_done,
   so the user saw nothing until the full answer arrived. Fix: return False
   (MarkdownV2 streaming handles partial tables gracefully).

3. finalize=False streaming ticks sent plain text — MarkdownV2 markup
   rendered as raw characters during streaming. Fix: edit_message with
   finalize=False now tries MarkdownV2 first and falls back to plain text
   only on BadRequest.

Also adds finalize=True to the _send_fallback_final edit-in-place so
stale partial messages are replaced with formatted content.

Co-Authored-By: Claude <noreply@anthropic.com>
…ion; update streaming-tick markdown test

should_hold_streaming_for_rich was checked with a truthy test; a bare
MagicMock() test adapter auto-vivifies that attribute as a callable
returning a truthy Mock, silently activating rich-hold (and suppressing
the first send) in ~20 unrelated tests using unspec'd mock adapters.
Production is unaffected — TelegramAdapter.should_hold_streaming_for_rich
always returns a real False — but switch to a strict 'is True' check so
duck-typed/mocked adapters without a real implementation are not
misread as opting in.

Also updates test_telegram_format.py's
test_non_final_edit_uses_plain_text_without_markdown, which asserted the
pre-existing plain-text streaming-tick behavior this PR intentionally
replaces with MarkdownV2-first + BadRequest fallback. Split into two
tests covering the MarkdownV2-success and BadRequest-fallback paths.

Co-Authored-By: Claude <noreply@anthropic.com>
…hook

hermes-sweeper review of this PR raised two points:

1. gateway/stream_consumer.py's buffer_threshold early-fire branch used
   min(self._current_edit_interval, 1.0) as its "at least 1 second"
   guard. For a configured edit_interval below 1.0, min() collapses
   straight back to that short interval, silently dropping the floor
   exactly when it mattered (a fast LLM with edit_interval=0.1 blows
   through the stated one-second protection).

   Fixed with a fixed 1.0 constant, not max(): a configured long interval
   (e.g. 10s) must still let a full buffer flush early at the 1s mark
   instead of waiting the whole interval — that's the entire point of
   this early-fire branch, and this repo's own
   test_buffer_waits_1s_before_firing already encodes exactly that
   expectation. max(interval, 1.0) would have fixed the short-interval
   case but broken the long-interval one; a bare 1.0 fixes the reported
   bug while leaving the long-interval behavior byte-for-byte unchanged
   (min(10.0, 1.0) == 1.0 == the new constant).

   Note: for configured intervals below 1 second specifically, this fix
   has no independently observable effect on stream timing — the
   interval-based should_edit branch right above it already fires at
   the same (or an earlier) elapsed time with a strictly weaker bar (any
   accumulated text vs. buffer_threshold characters), so it dominates
   regardless of the buffer_threshold branch's own floor. The fix is
   still correct and worth making — it's the difference between the code
   actually satisfying its own documented "at least 1 second regardless
   of edit_interval" contract instead of only claiming to — but it isn't
   independently testable in that regime, so no new test asserts timing
   behavior for the sub-second case specifically. Fixed
   test_buffer_waits_1s_before_firing itself instead: it asserted on the
   very first-ever flush, which always fires immediately regardless of
   any of this (_last_edit_time starts at 0.0, so elapsed-since-last-edit
   is enormous on the first tick) — confirmed via git stash that this
   test failed identically before any of these changes. Rewrote it to
   check the floor against a *second* flush, after an initial one
   establishes a realistic _last_edit_time baseline.

2. The rich-hold mechanism (adapter.should_hold_streaming_for_rich,
   consumer._rich_hold state, the hold-check block in the run loop) had
   no live consumer: Telegram — the only adapter — always returned
   False, by explicit design (partial tables render fine via
   _wrap_markdown_tables, so suppressing streaming would be worse UX).
   Dead extension surface with dedicated tests but no production
   behavior. Removed the hook, the consumer-side state/logic, Telegram's
   always-False stub, and the three dedicated tests (TestRichHold,
   test_rich_hold_initialised_false, TestShouldHoldStreamingForRich).
   Reintroduce if a concrete adapter actually needs and enables it.
   Renamed test_stream_consumer_rich_hold.py, which no longer has
   anything to do with rich-hold, to
   test_stream_consumer_fallback_finalize_and_buffer_floor.py.

Confirmed tests/gateway/test_telegram_streaming_tick.py::
test_streaming_tick_flood_propagates is a separate, pre-existing failure
(already flagged as an undiagnosed design question in this PR's own
history, unrelated to either fix above) and the usual 9 telegram
test-order-pollution failures are unrelated (same as confirmed on a
clean origin/main in prior PRs this session).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012CvMiTppKDazXfakuoLgQK
…tick test

test_streaming_tick_flood_propagates used retry_after=2, which falls into
edit_message()'s long-standing (pre-dating this branch) <=5s inline-retry
path — the mocked retry succeeds, so success=True is the correct outcome,
not the failure the test asserted. Bump the propagation case to a wait
>5s (where the code genuinely returns failure without retrying) and add
a companion test covering the short-wait retry-then-succeed path.
@elphamale
elphamale force-pushed the fix/telegram-streaming-formatting branch from 82df3af to 81d368d Compare August 20, 2026 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/streaming Streaming responses: gateway delivery, provider wire comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/telegram Telegram bot adapter sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants