fix(telegram): advance reply anchor to latest message_id when batching stacked prompts - #43787
Closed
g0rdonL wants to merge 1 commit into
Closed
Conversation
…g stacked prompts When a user sends multiple messages quickly (stacked prompts), the text batching code in _enqueue_text_event merges them into a single event but never updated existing.message_id — it stayed pinned to the *first* message. The bot's reply then anchored to that first message instead of the most recent one, which looked like a wrong-reply bug. Fix: when merging a later event into an existing batch, update existing.message_id to the incoming event's message_id (guarded by truthiness so a missing id is not clobbered with None). Add a regression test that reproduces the exact scenario: two stacked prompts with distinct message_ids — asserts both texts are merged and the dispatched event carries the last message_id as the reply anchor.
Contributor
Code Review VerificationReviewed the diff (4 lines production + 35 lines test). The fix is correct and well-scoped:
No issues found. Clean fix with good test coverage. |
Contributor
|
Thanks for the focused regression test and narrow fix. The underlying behavior is still present on current main: Problems
Suggested changes
Automated hermes-sweeper review. |
12 tasks
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When a user sends multiple messages quickly ("stacked prompts"), the text batching code in
_enqueue_text_eventmerges them into a single event but never updatedexisting.message_id— it stayed pinned to the first message's ID. The bot's reply then anchored to that first message rather than the most recent one, which appeared to the user as the bot replying to the wrong message.The merge path at
telegram.py:Root Cause
_enqueue_text_eventwas designed for Telegram's 4096-char message splits (where consecutive chunks belong to the same logical message). For that case the originalmessage_idis fine. But the same code path runs for any two rapidly-sent messages, and in that case the reply anchor should track the last message so the reply threads to what the user most recently sent.Fix
One line: update
existing.message_idto the incoming event'smessage_idwhen merging, guarded by truthiness so a missing id is never clobbered withNone.Test
Added
test_stacked_prompts_reply_anchor_advances_to_last_messageto the existingtest_telegram_text_batching.pysuite. It reproduces the exact scenario — two events with distinctmessage_ids batched within the window — and asserts:message_idas the reply anchor.All 7 tests in the file pass.