fix(telegram): prevent retryable draft flood from cascading to edit path by elphamale · Pull Request #54337 · NousResearch/hermes-agent

elphamale · 2026-06-28T16:21:36Z

Stacked on #53865. This branch is based on fix/telegram-draft-flood-cascade; once that PR merges the diff will reduce to the single commit shown.

Fixes two edge cases from szafranski's review of #53865 (issue #54275).

Edge case 1 — retryable draft flood falls through to edit/send

When send_draft() returns success=False, retryable=True (RetryAfter > 5 s), _send_draft_frame previously returned False, causing _send_or_edit to fall through to the regular edit-message path — re-creating the edit cascade that draft streaming is meant to prevent.

Fix: return True (skip frame silently, stay in draft mode). The frame is simply not rendered; the next tick will try again.

Edge case 2 — `sendRichMessageDraft` RetryAfter burns a legacy `sendMessageDraft` call

_try_send_rich_draft returned bool, so a flood-control exception mapped to False and send_draft immediately fell through to sendMessageDraft in the same rate-limit bucket — wasting the call.

Fix: change return type to Optional[SendResult]:

SendResult(success=True) — frame landed
SendResult(success=False, retryable=True, retry_after=N) — flood control; caller returns immediately, no legacy call
None — capability/transient error; caller falls through to legacy (correct)

Test plan

test_stream_consumer_draft.py — test_retryable_failure_does_not_cascade_to_edit_send: verifies _send_draft_frame returns True and never calls adapter.send/adapter.edit_message
test_telegram_send_draft_format.py — test_rich_draft_retry_after_returns_retryable_not_legacy_fallback: verifies flood control on rich draft returns retryable without calling send_message_draft; test_rich_draft_capability_error_falls_back_to_legacy: verifies capability errors still fall through correctly
All 41 draft-related tests pass

🤖 Generated with Claude Code

…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>

Two edge cases from szafranski's review of NousResearch#53865 (issue NousResearch#54275): 1. Retryable draft flood falls through to edit/send: when send_draft() returns success=False + retryable=True (RetryAfter > 5s), _send_draft_frame returned False, causing _send_or_edit to fall through to the regular edit-message path — re-creating the edit cascade the draft transport is meant to prevent. Fix: return True (skip frame silently, stay in draft mode). 2. sendRichMessageDraft RetryAfter burns legacy sendMessageDraft: the bool return from _try_send_rich_draft mapped all failures to False, so a flood exception caused immediate fallback to sendMessageDraft in the same rate-limit bucket. Fix: return Optional[SendResult] — flood control returns a retryable result (no legacy fallback); capability errors and transient failures return None (legacy fallback is correct). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

teknium1

Thanks for tracing both draft-flood paths. Current main still has the two fallthroughs described in #54275, so the narrow retryable-result direction is useful.

Problems

gateway/stream_consumer.py:1134-1141 returns handled for a retryable draft failure but does not retain or enforce retry_after; the next stream tick can immediately call send_draft() again. The legacy long-flood return at plugins/platforms/telegram/adapter.py:3510 also drops retry_after.
plugins/platforms/telegram/adapter.py:3472-3481 and 3519-3527 convert all rejected/non-Markdown draft failures to success. That masks hard/capability errors, matching the broader concern explicitly recorded in #54275 rather than its requested narrow rate-limit contract.
The new regression test calls _send_draft_frame() directly at tests/gateway/test_stream_consumer_draft.py:297; it does not exercise _send_or_edit() or prove the proposed cooldown behavior.

Suggested changes

Store a draft cooldown deadline from retry_after, skip frames until it expires, and test the real stream path.
Keep hard failures distinguishable; scope suppression to the RetryAfter case.

Automated hermes-sweeper review.

teknium1 · 2026-07-15T07:03:08Z

+                # edit/send path, which would re-create the edit cascade the
+                # draft transport is meant to prevent.
+                logger.debug("send_draft retryable failure (not counted): %s", error)
+                return True


This treats the frame as handled but never records or observes result.retry_after, so the next normal stream tick calls send_draft() again during the server's flood window. Please retain a cooldown deadline and skip draft attempts until it expires; add a stream-loop test that proves no repeated draft calls occur during that interval.

teknium1 · 2026-07-15T07:03:08Z

                    self.name, chat_id, draft_id, e,
                )
-                return SendResult(success=False, error=str(e))
+                return SendResult(success=True, message_id=None)


This turns every hard non-Markdown failure into success, including unsupported/capability failures. That conflicts with the narrower contract noted in #54275: rate limits should cool down, while hard failures remain distinguishable and use the established fallback behavior. Please scope suppression to the RetryAfter case.

elphamale · 2026-07-15T19:44:11Z

Closing as superseded by #53865.

This branch's own contribution (commit 2d51db4fb) is two fixes for szafranski's edge cases from #54275:

_send_draft_frame returning True instead of False on a retryable failure, so _send_or_edit doesn't fall through to the edit path.
_try_send_rich_draft returning Optional[SendResult] instead of bool, so a flood-control RetryAfter doesn't burn a legacy sendMessageDraft call in the same rate-limit window.

Both are now covered on #53865, which has since been rebased and extended with a proper cooldown mechanism (_draft_cooldown_until / _last_draft_retryable in gateway/stream_consumer.py) that persists the server's retry_after and skips frames until it expires — a strictly more complete fix than this PR's skip-once approach. adapter.py's _try_send_rich_draft there already returns Tuple[bool, Optional[float]] and propagates retry_after into the SendResult, matching what this PR's edge-case-2 fix does.

Re: this PR's sweeper review — point 1 (persist/enforce retry_after) is resolved by the above. Point 2 (adapter code converting rejected/non-Markdown draft failures to success=True, masking hard/capability errors) is a real observation but applies equally to the current #53865 code, since the same suppress-as-success pattern lives there now — that's better tracked against #53865 directly rather than kept open here. Point 3 (test coverage gap) is moot once this branch is closed.

elphamale and others added 5 commits June 28, 2026 02:46

teknium1 reviewed Jul 15, 2026

View reviewed changes

teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 15, 2026

elphamale closed this Jul 15, 2026

elphamale mentioned this pull request Jul 15, 2026

fix(telegram): prevent draft-failure cascade that causes 4-minute flood-control silence #53865

Open

Navigation Menu

fix(telegram): prevent retryable draft flood from cascading to edit path - #54337

fix(telegram): prevent retryable draft flood from cascading to edit path#54337
elphamale wants to merge 5 commits into
NousResearch:mainfrom
elphamale:fix/draft-flood-edge-cases-v2

elphamale commented Jun 28, 2026 •

edited

Loading

Uh oh!

teknium1 left a comment

Uh oh!

teknium1 Jul 15, 2026

Uh oh!

teknium1 Jul 15, 2026

Uh oh!

elphamale commented Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

elphamale commented Jun 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Edge case 1 — retryable draft flood falls through to edit/send

Edge case 2 — sendRichMessageDraft RetryAfter burns a legacy sendMessageDraft call

Test plan

Uh oh!

teknium1 left a comment

Choose a reason for hiding this comment

Problems

Suggested changes

Uh oh!

teknium1 Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

teknium1 Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

elphamale commented Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

elphamale commented Jun 28, 2026 •

edited

Loading

Edge case 2 — `sendRichMessageDraft` RetryAfter burns a legacy `sendMessageDraft` call