Skip to content

fix(gateway): delete partial message after fallback send on flood control#17384

Closed
HuangYuChuh wants to merge 1 commit into
NousResearch:mainfrom
HuangYuChuh:fix/telegram-flood-duplicate
Closed

fix(gateway): delete partial message after fallback send on flood control#17384
HuangYuChuh wants to merge 1 commit into
NousResearch:mainfrom
HuangYuChuh:fix/telegram-flood-duplicate

Conversation

@HuangYuChuh

@HuangYuChuh HuangYuChuh commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

When Telegram flood control triggers 3+ consecutive edit failures during streaming, the stream consumer enters fallback mode and sends the complete response as a new message. This leaves the user seeing two messages: a frozen partial (often with cursor ▉) and the full duplicate.

After the fallback chunks are sent successfully, this PR deletes the original partial message so the user only sees one complete response.

Related Issue

Fixes #16668

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • gateway/stream_consumer.py: in _send_fallback_final(), save the stale message ID before the send loop, then delete it after all fallback chunks succeed. The delete is best-effort — if it fails (flood control still active, missing permissions, already deleted), the full answer is still delivered.

How to Test

  1. Configure Telegram bot with streaming enabled
  2. Send a message that produces a long response
  3. Simulate flood control (high-frequency edits in a busy group)
  4. Before fix: user sees [partial frozen message] + [complete duplicate]
  5. After fix: partial message is deleted, user sees only the complete response

Design Decisions

  • Best-effort delete: the delete itself may be rate-limited by Telegram. The comment in code acknowledges this. The fix improves the common case (delete succeeds) without making the worst case worse (delete fails → same behavior as before this PR).
  • except Exception: pass: matches the existing error handling pattern in this file (e.g., _try_strip_cursor). A failed delete should never crash the response delivery.
  • Scope: only the happy-path exit of _send_fallback_final() gets the delete. Early-return error paths (partial chunk failure) are left unchanged to avoid adding complexity to already-complex error handling.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix
  • Cross-platform: delete_message is defined on the base adapter; non-Telegram platforms that don't implement it return False (no crash)

@HuangYuChuh
fix(gateway): delete partial message after fallback send on flood con…
6f5dbf0 
…trol

When Telegram flood control triggers 3+ consecutive edit failures, the
stream consumer enters fallback mode and sends the complete response as
a new message. This leaves the user seeing two messages: a frozen
partial (with cursor) and the full duplicate.

After the fallback chunks are sent successfully, delete the original
partial message so the user only sees one complete response. The delete
is best-effort — if it fails (e.g. flood still active, missing
permissions), the full answer is still delivered.

Fixes NousResearch#16668
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/telegram Telegram bot adapter labels Apr 29, 2026
@teknium1 teknium1 mentioned this pull request May 10, 2026
Merged
teknium1 added a commit that referenced this pull request May 10, 2026
@teknium1
fix(gateway): align fallback delete with sibling style + add regressi…
ec1fad3 
…on tests

Follow-up to HuangYuChuh's #17384 cherry-pick:

- Use defensive getattr+logger.debug for delete_message lookup, mirroring
  the sibling _try_send_fresh_final cleanup pattern at L820+. Platforms
  that don't implement delete_message no longer raise AttributeError; the
  failure path now logs at debug for diagnosability instead of silently
  swallowing.
- Add three regression tests in tests/gateway/test_stream_consumer.py:
  - delete_message awaited on happy-path exit with stale id
  - delete_message NOT awaited when no fallback chunks reached the user
  - no crash on adapters that lack delete_message (spec-restricted mock)
@teknium1

teknium1 commented May 10, 2026

Copy link
Copy Markdown
Contributor

Merged via salvage PR #23416 (#23416). Your commit was cherry-picked onto current main with your authorship preserved. We added a follow-up commit aligning the delete with the sibling _try_send_fresh_final cleanup pattern (defensive getattr + logger.debug on failure) and three regression tests covering the happy-path delete, no-delete-on-failed-send, and adapter-without-delete_message paths. Thanks for the fix!

@teknium1 teknium1 closed this May 10, 2026
JZKK720 pushed a commit to JZKK720/hermes-agent that referenced this pull request May 11, 2026
@teknium1
fix(gateway): align fallback delete with sibling style + add regressi…
e7189c0 
…on tests

Follow-up to HuangYuChuh's NousResearch#17384 cherry-pick:

- Use defensive getattr+logger.debug for delete_message lookup, mirroring
  the sibling _try_send_fresh_final cleanup pattern at L820+. Platforms
  that don't implement delete_message no longer raise AttributeError; the
  failure path now logs at debug for diagnosability instead of silently
  swallowing.
- Add three regression tests in tests/gateway/test_stream_consumer.py:
  - delete_message awaited on happy-path exit with stale id
  - delete_message NOT awaited when no fallback chunks reached the user
  - no crash on adapters that lack delete_message (spec-restricted mock)
rmulligan pushed a commit to rmulligan/hermes-agent that referenced this pull request May 11, 2026
@teknium1
fix(gateway): align fallback delete with sibling style + add regressi…
ac0855e 
…on tests

Follow-up to HuangYuChuh's NousResearch#17384 cherry-pick:

- Use defensive getattr+logger.debug for delete_message lookup, mirroring
  the sibling _try_send_fresh_final cleanup pattern at L820+. Platforms
  that don't implement delete_message no longer raise AttributeError; the
  failure path now logs at debug for diagnosability instead of silently
  swallowing.
- Add three regression tests in tests/gateway/test_stream_consumer.py:
  - delete_message awaited on happy-path exit with stale id
  - delete_message NOT awaited when no fallback chunks reached the user
  - no crash on adapters that lack delete_message (spec-restricted mock)
JinyuID pushed a commit to JinyuID/hermes-agent that referenced this pull request May 11, 2026
@teknium1
fix(gateway): align fallback delete with sibling style + add regressi…
f0295b1 
…on tests

Follow-up to HuangYuChuh's NousResearch#17384 cherry-pick:

- Use defensive getattr+logger.debug for delete_message lookup, mirroring
  the sibling _try_send_fresh_final cleanup pattern at L820+. Platforms
  that don't implement delete_message no longer raise AttributeError; the
  failure path now logs at debug for diagnosability instead of silently
  swallowing.
- Add three regression tests in tests/gateway/test_stream_consumer.py:
  - delete_message awaited on happy-path exit with stale id
  - delete_message NOT awaited when no fallback chunks reached the user
  - no crash on adapters that lack delete_message (spec-restricted mock)
02356abc pushed a commit to 02356abc/hermes-agent that referenced this pull request May 14, 2026
@teknium1
fix(gateway): align fallback delete with sibling style + add regressi…
2da5e48 
…on tests

Follow-up to HuangYuChuh's NousResearch#17384 cherry-pick:

- Use defensive getattr+logger.debug for delete_message lookup, mirroring
  the sibling _try_send_fresh_final cleanup pattern at L820+. Platforms
  that don't implement delete_message no longer raise AttributeError; the
  failure path now logs at debug for diagnosability instead of silently
  swallowing.
- Add three regression tests in tests/gateway/test_stream_consumer.py:
  - delete_message awaited on happy-path exit with stale id
  - delete_message NOT awaited when no fallback chunks reached the user
  - no crash on adapters that lack delete_message (spec-restricted mock)
jsboige pushed a commit to jsboige/hermes-agent that referenced this pull request May 14, 2026
@teknium1
fix(gateway): align fallback delete with sibling style + add regressi…
3aa6bbc 
…on tests

Follow-up to HuangYuChuh's NousResearch#17384 cherry-pick:

- Use defensive getattr+logger.debug for delete_message lookup, mirroring
  the sibling _try_send_fresh_final cleanup pattern at L820+. Platforms
  that don't implement delete_message no longer raise AttributeError; the
  failure path now logs at debug for diagnosability instead of silently
  swallowing.
- Add three regression tests in tests/gateway/test_stream_consumer.py:
  - delete_message awaited on happy-path exit with stale id
  - delete_message NOT awaited when no fallback chunks reached the user
  - no crash on adapters that lack delete_message (spec-restricted mock)
AlexFoxD pushed a commit to AlexFoxD/hermes-agent that referenced this pull request May 21, 2026
@teknium1 @AlexFoxD
fix(gateway): align fallback delete with sibling style + add regressi…
0a1dd39 
…on tests

Follow-up to HuangYuChuh's NousResearch#17384 cherry-pick:

- Use defensive getattr+logger.debug for delete_message lookup, mirroring
  the sibling _try_send_fresh_final cleanup pattern at L820+. Platforms
  that don't implement delete_message no longer raise AttributeError; the
  failure path now logs at debug for diagnosability instead of silently
  swallowing.
- Add three regression tests in tests/gateway/test_stream_consumer.py:
  - delete_message awaited on happy-path exit with stale id
  - delete_message NOT awaited when no fallback chunks reached the user
  - no crash on adapters that lack delete_message (spec-restricted mock)
gweeteve pushed a commit to gweeteve/hermes-agent that referenced this pull request Jun 2, 2026
@teknium1
fix(gateway): align fallback delete with sibling style + add regressi…
67f0c6e 
…on tests

Follow-up to HuangYuChuh's NousResearch#17384 cherry-pick:

- Use defensive getattr+logger.debug for delete_message lookup, mirroring
  the sibling _try_send_fresh_final cleanup pattern at L820+. Platforms
  that don't implement delete_message no longer raise AttributeError; the
  failure path now logs at debug for diagnosability instead of silently
  swallowing.
- Add three regression tests in tests/gateway/test_stream_consumer.py:
  - delete_message awaited on happy-path exit with stale id
  - delete_message NOT awaited when no fallback chunks reached the user
  - no crash on adapters that lack delete_message (spec-restricted mock)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/telegram Telegram bot adapter type/bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Telegram streaming flood control can leave partial message and send duplicate final response

3 participants

@HuangYuChuh @teknium1 @alt-glitch