Skip to content

fix(telegram): preserve reply context across message batches - #65526

Open
the3asic wants to merge 1 commit into
NousResearch:mainfrom
the3asic:upstream/telegram-reply-batching-20260716T084720Z
Open

fix(telegram): preserve reply context across message batches#65526
the3asic wants to merge 1 commit into
NousResearch:mainfrom
the3asic:upstream/telegram-reply-batching-20260716T084720Z

Conversation

@the3asic

Copy link
Copy Markdown
Contributor

What does this PR do?

Telegram short-message batching currently groups by chat/thread but does not compare the full reply context. Consecutive messages aimed at different reply targets can therefore be merged under the first message's quote/author metadata.

This change:

  • compares all reply fields that propagate across a batch: message ID, quoted text, author ID/name, and whether the reply targets the bot;
  • immediately flushes the current batch when incoming reply context differs;
  • keeps same-target reply batching and ordinary non-reply batching unchanged;
  • preserves reply context for Telegram client-split long messages whose continuation chunk lacks reply metadata.

Verification

  • Tested on Ubuntu 26.04 / Linux x86_64
  • 129 Telegram and shared text-batching regression tests passed
  • ruff check plugins/platforms/telegram/adapter.py tests/gateway/test_telegram_text_batching.py
  • python -m py_compile plugins/platforms/telegram/adapter.py tests/gateway/test_telegram_text_batching.py
  • git diff --check

Tests use synthetic Telegram events and perform no live message delivery.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have 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 16, 2026

@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 a real Telegram batching issue: current main appends same-session text without checking reply context (plugins/platforms/telegram/adapter.py:8179-8193).

Problems

  • The proposed immediate flush uses a detached asyncio.create_task(self.handle_message(existing)) (plugins/platforms/telegram/adapter.py:8002). That task is absent from _pending_text_batch_tasks, so it bypasses the disconnect cancellation and delayed-delivery guard introduced in 8ad15ff7d; handle_message itself has no disconnected-state check (gateway/platforms/base.py:4676-4699).
  • Queue-mode text debounce remains a sibling aggregation path: it merges same-sender text without reply-context comparison and retains old quote/author fields while changing only the anchor (gateway/platforms/base.py:4357-4369).

Suggested changes

  • Flush the incompatible batch through a tracked, guarded helper and add a disconnect-race regression test.
  • Define and test reply-context behavior for the queue-mode debounce path, or split that work into a focused follow-up.

Automated hermes-sweeper review.

"[Telegram] Flushing text batch %s before incompatible reply context",
key,
)
asyncio.create_task(self.handle_message(existing))

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 detached task is not retained in _pending_text_batch_tasks, so disconnect() cannot cancel it and it bypasses _should_drop_delayed_delivery(). Please dispatch via a tracked/guarded flush helper and add a disconnect-race test; current teardown explicitly protects delayed batches from stale delivery.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have platform/telegram Telegram bot adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants