Skip to content

fix(telegram): per-chat send cooldown + skip progress fallback on flood control - #66722

Open
profikid wants to merge 1 commit into
NousResearch:mainfrom
profikid:fix/telegram-flood-cooldown
Open

fix(telegram): per-chat send cooldown + skip progress fallback on flood control#66722
profikid wants to merge 1 commit into
NousResearch:mainfrom
profikid:fix/telegram-flood-cooldown

Conversation

@profikid

Copy link
Copy Markdown

Telegram flood-control penalties escalated to multi-thousand-second back-offs because a single user turn routinely fanned out 3-5 sends across independent code paths that were not coordinated with each other.

Root cause

A single user turn triggers sends from several independent code paths:

  • Status callbacks fired via safe_schedule_threadsafe (no per-chat serialization)
  • Progress bubbles from the progress-queue consumer
  • Streaming previews from the stream consumer
  • The final answer
  • Photo batches

Each path had its own per-send retry-after handling, but the cumulative burst crossed Telegram's ~1 msg/sec/chat limit. The penalty then kept escalating because the progress path's flood-control fallback issued a fresh adapter.send() during the penalty window — exactly the burst pattern that triggered the penalty in the first place.

Fix 1 — per-chat send cooldown in the telegram adapter

plugins/platforms/telegram/adapter.py:

  • New _send_cooldown_until dict keyed by chat_id with a default minimum gap of 1.1s (send_cooldown_seconds)
  • Configurable via platforms.telegram.extra.send_cooldown_seconds and send_cooldown_max_wait_seconds
  • When the wait exceeds the ceiling, send() returns a retryable flood_control-style error instead of blocking the chat path for two hours
  • Cooldown is stamped on both the legacy send loop AND the rich fast path so they cannot race each other

Fix 2 — skip the progress fallback on flood control

gateway/run.py:

  • The progress handler used to fall back to a fresh adapter.send() when the edit hit flood control. That send re-triggers the penalty. With the fix, the tick is dropped, progress_lines and progress_msg_id are kept intact, and the next tick retries the edit once the per-chat cooldown (Fix 1) releases the chat
  • Non-flood permanent failures (message deleted, permission revoked) still fall back to a fresh send, since those are local failures and not Telegram-side rate limits

Tests

  • 5 new tests in tests/gateway/test_telegram_send_cooldown.py:
    • test_first_send_stamps_cooldown_for_same_chat — successful send records a cooldown timestamp
    • test_second_send_within_window_blocks — second send within the gap waits for the gate
    • test_second_send_after_window_passes_immediately — no sleep when the cooldown already expired
    • test_independent_cooldowns_per_chat — chat A's cooldown does not block chat B
    • test_oversized_wait_returns_retryable_error — multi-thousand-second penalty yields a retryable error instead of blocking the chat path
  • 1 new test in tests/gateway/test_run_progress_interrupt.py:
    • test_progress_flood_control_does_not_trigger_fallback_send — pins the gateway-side fix: at most one progress-bubble send (the initial bubble) is allowed; additional sends triggered by flood-control edit failures are regressed on
  • All 28 tests in the affected telegram + run-progress suite pass; no regression

…od control

Telegram flood-control penalties escalated to multi-thousand-second
back-offs ('Retry in 7000+ seconds') because a single user turn
routinely fanned out 3-5 sends across independent code paths that
were not coordinated with each other:

  - status callbacks fired via safe_schedule_threadsafe (no per-chat
    serialization)
  - progress bubbles from the progress-queue consumer
  - streaming previews from the stream consumer
  - the final answer
  - photo batches

Even though each path had its own per-send retry-after handling, the
cumulative burst crossed Telegram's ~1 msg/sec/chat limit and the
penalty kept escalating because the progress path's flood fallback
(see gateway/run.py) issued a fresh adapter.send() during the
penalty window \u2014 the exact burst pattern that triggered the
penalty in the first place.

Fix 1: per-chat send cooldown in the telegram adapter
  - New _send_cooldown_until dict keyed by chat_id with a
    default minimum gap of 1.1s (send_cooldown_seconds)
  - Configurable via platforms.telegram.extra.send_cooldown_seconds
    and send_cooldown_max_wait_seconds; defaults tuned for
    ~1 msg/sec/chat with a 5s ceiling on the per-send wait
  - When the wait exceeds the ceiling, send() returns a
    retryable flood_control-style error instead of blocking the
    chat path for two hours
  - Cooldown is stamped on both the legacy send loop AND the rich
    fast path so they cannot race each other

Fix 2: skip the progress fallback on flood control
  - The progress handler in gateway/run.py used to fall back to a
    fresh adapter.send() when the edit hit flood control. That
    send re-triggers the penalty. With the fix, the tick is
    dropped, progress_lines and progress_msg_id are kept
    intact, and the next tick retries the edit once the
    per-chat cooldown (Fix 1) releases the chat
  - Non-flood permanent failures (message deleted, permission
    revoked) still fall back to a fresh send, since those are
    local failures and not Telegram-side rate limits

Tests:
  - 5 new tests in test_telegram_send_cooldown.py pin the gate's
    behaviour (cooldown stamp, second-send blocking, per-chat
    independence, post-window pass-through, oversized-wait error)
  - 1 new test in test_run_progress_interrupt.py
    (test_progress_flood_control_does_not_trigger_fallback_send)
    pins the gateway-side fix
  - All 28 tests in the affected telegram + run-progress suite
    pass; no regression
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins platform/telegram Telegram bot adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 18, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #50965, #55869, and #53865. This combines a per-chat adapter cooldown with the gateway progress-fallback path, so it is competing/superset work rather than a duplicate.

@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 isolating the progress fallback; the current-main premise is real: gateway/run.py:18537-18552 sends a new progress message immediately after a flood-controlled edit.

Problems

  • The new map is not a per-chat serializer. The rich path awaits _try_send_rich() before its new timestamp write (plugins/platforms/telegram/adapter.py:4045 in the PR), and callers sleeping for one deadline wake together without reserving the next slot.
  • The bounded-wait claim is disconnected from live RetryAfter: current send() still directly sleeps the server value at plugins/platforms/telegram/adapter.py:4189-4203; the PR never records that value in its new map.
  • Native media bypasses send() (send_media_group at plugins/platforms/telegram/adapter.py:6537, send_photo at :6797), so photo traffic is not coordinated.
  • The overflow progress-edit path still converts a failed edit to a fresh send via gateway/run.py:18423-18428 then :18569-18576.

Suggested changes

  • Use a per-chat reservation/lock before every await, propagate real RetryAfter deadlines with structured rate-limit metadata, cover media paths, and test concurrent sends plus overflow-edit flood control.

Automated hermes-sweeper review.

# next non-rich send (or another rich one) honours
# the per-chat gate. Without this, the rich path
# would bypass the limiter entirely and a
# rich→markdown sequence would race.

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 timestamp is written only after _try_send_rich() has awaited the Bot API. Concurrent rich sends can all pass the empty cooldown check first; callers that sleep for an existing deadline also wake together because no next slot is reserved before the await. Use a per-chat reservation/lock and add a concurrent-send regression.

@@ -4011,6 +4106,17 @@ async def send(
_TimedOut = None # type: ignore[assignment,misc]

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 records only the fixed minimum gap. The unchanged RetryAfter handler below still awaits the server-provided value directly, and never updates this map, so a real 7000-second penalty bypasses send_cooldown_max_wait_seconds. Record the server deadline or return a structured rate-limit result there, with a RetryAfter-based test.

@webtecnica

Copy link
Copy Markdown
Contributor

Verification on current main (2026-08-01) — fix works, needs rebase + one small change

I ported this PR's changes onto current upstream/main (cb2311fe2) and ran the test suite. Thanks for isolating the progress fallback — the issue premise is confirmed on current main: gateway/run.py (~L3898) still issues a fresh adapter.send() right after a flood-controlled edit, which is exactly the burst that keeps the penalty pinned at multi-thousand-second back-offs.

What I verified

  • Ported cleanly (conceptually) — the diff does not apply verbatim anymore (git apply --check fails on gateway/run.py and plugins/platforms/telegram/adapter.py because main has moved), so the PR needs a rebase before it can land. I applied the same changes manually on current main; no semantic drift found.
  • 5/5 cooldown tests pass on current main (tests/test_telegram_send_cooldown.py — see note below on placement).
  • 4/4 run-progress tests pass including the new test_progress_flood_control_does_not_trigger_fallback_send.
  • RED-GREEN confirmed: with the gateway/run.py change reverted, the new regression test fails with assert 2 <= 1 (the handler issues a 2nd progress-bubble send after the flood edit failure); with the fix it passes with exactly 1 send (the initial bubble). The test genuinely pins the bug.

One small change needed to land cleanly (per issue #76494)

tests/gateway/test_telegram_send_cooldown.py triggers a pre-existing test-isolation landmine: tests/gateway/conftest.py installs a process-wide telegram MagicMock into sys.modules at import time, so any new file under tests/gateway/ collected before tests/test_telegram_polling_progress_ptb.py breaks it with TypeError: object MagicMock can't be used in 'await' expression. Moving the new test file to top-level tests/ (where the real-PTB tests already live) avoids it. I verified the suite passes with the file at tests/test_telegram_send_cooldown.py.

Observations (not blockers)

Summary

The PR implements exactly what issue #76494 asks for and resolves the escalation when ported to current main. Needs: (1) rebase onto current main, (2) move tests/gateway/test_telegram_send_cooldown.pytests/test_telegram_send_cooldown.py. Happy to help with the rebase or the test relocation if useful.

@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

One PR, #66722, addresses the reported Telegram penalty escalation by suppressing a fresh progress-message send after a flood-controlled edit and adding a per-chat send cooldown. Its diff targets the immediate fallback-send cause, but the cooldown implementation does not fully serialize concurrent senders or propagate Telegram RetryAfter deadlines across all outbound paths.

Related pull requests

  • fix(telegram): per-chat send cooldown + skip progress fallback on flood control #66722 related — (+529/-12) — n/a: The diff skips the fresh progress fallback send on detected flood control and adds cooldown tests, but its timestamp map does not reserve slots before awaits; contributor review also identifies uncoordinated native-media paths, live RetryAfter handling, and another overflow fallback path. The visible keep_open review recommends salvaging the progress-fallback isolation, while current-main verification at cb2311f confirms the premise and tests but says the PR requires a rebase and a small correction.

Suggested consolidation

Keep #66722 open for author action: rebase onto current main and revise the cooldown into a per-chat reservation/lock that records live RetryAfter deadlines and covers native media and the remaining overflow fallback path. This preserves the verified progress-fallback fix while explicitly addressing the blocking technical concerns in the contributor keep_open review.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins P2 Medium — degraded but workaround exists platform/telegram Telegram bot adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform 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.

5 participants