Skip to content

fix(telegram): restore reply context for replies to rich messages - #46569

Closed
GodsBoy wants to merge 1 commit into
NousResearch:mainfrom
GodsBoy:fix/telegram-rich-reply-context
Closed

fix(telegram): restore reply context for replies to rich messages#46569
GodsBoy wants to merge 1 commit into
NousResearch:mainfrom
GodsBoy:fix/telegram-rich-reply-context

Conversation

@GodsBoy

@GodsBoy GodsBoy commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

When telegram.extra.rich_messages: true is enabled (Bot API 10.1 sendRichMessage, added in #44829), final replies are sent with the agent text in a rich_message payload and no legacy text field. A user replying to one of those messages arrives with reply_to_message.text and .caption empty, so _build_message_event sets reply_to_text=None and gateway/run.py injects no [Replying to: "..."] pointer. The agent loses the context of what the user is replying to. This is most visible on replies to rich cron announcements (Cronjob Response: ...).

It worked before rich messages because the legacy sendMessage path always carried a real text field that round-tripped into reply_to_message.text.

This restores that context with a bounded outbound reply-context cache, at parity with legacy text replies. It does not disable rich messages and adds no new stale-task-revival surface beyond the existing legacy-reply behavior (reference-only semantics for quoted cron text remain the injection layer's responsibility, related: #26714).

Related Issue

Fixes #46568

Type of Change

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

Changes Made

  • gateway/platforms/telegram.py: add a bounded, TTL-expiring outbound reply-context cache (_outbound_context_cache, _remember_outbound_context, _prune_outbound_context, _lookup_outbound_context) keyed by (chat_id, message_id); register delivered content on both the rich (_try_send_rich) and legacy chunked (send) paths; fall back to the cached snippet in _build_message_event only when there is no native quote and no text/caption (precedence preserved, keeps the [Bug]: Telegram native partial quotes are expanded to the full replied-to message #22619 quote-narrowing behavior).
  • tests/gateway/test_telegram_rich_reply_context.py: new regression suite (cache helpers, send-path registration, inbound precedence, TTL/eviction, empty/failed-send guards, end-to-end rich cron-reply scenario).

The cache is in-memory only and bounded three independent ways: 1024 entries, 2000 chars per entry, and a 48h TTL. It stores only the bot's own outbound content and surfaces it only on an explicit user reply. Cron deliveries benefit automatically because live delivery already routes through adapter.send; no scheduler change was needed.

How to Test

  1. Set telegram.extra.rich_messages: true and run the bot.
  2. Have the bot send a rich message (e.g. a cron announcement, or any reply that renders via sendRichMessage).
  3. Reply to that message in Telegram.
  4. Before: the agent receives the reply with no [Replying to: "..."] context. After: the reply context is restored.

Automated:

  • scripts/run_tests.sh tests/gateway/test_telegram_rich_reply_context.py (21 tests).
  • scripts/run_tests.sh tests/gateway/test_telegram_rich_messages.py tests/gateway/test_telegram_reply_quote.py tests/gateway/test_telegram_reply_mode.py tests/gateway/test_reply_to_injection.py tests/gateway/test_telegram_thread_fallback.py (no regressions).
  • A RED/GREEN check confirmed the regression test fails on unfixed code (with the fallback neutralized) and passes with the fix.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(telegram): ...)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix (no unrelated commits)
  • I've run the suite via scripts/run_tests.sh (CI-parity) and all tests pass
  • I've added tests for my changes
  • I've tested on my platform: Ubuntu (Linux), via scripts/run_tests.sh and full CI (6 test shards green). Note: live end-to-end verification on a real Telegram bot was not run in my environment; the one edge that needs live confirmation is called out under Residual findings below.

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — N/A (no user-facing docs affected)
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A (cache bounds are internal constants by design, no new config key)
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact — pure stdlib (time, collections.OrderedDict), no platform-specific primitives
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A (no tool behavior change)

Screenshots / Logs

Backend gateway change, no UI. All CI checks green on the first run (6 test shards, both nix, both arch builds, e2e, check-attribution, ruff + ty diff).

Residual findings

Surfaced by an internal review pass; both are low severity and do not affect correctness.

  • [P3] Legacy-path cache entries are never read yet share the entry cap. Legacy text replies round-trip via reply_to_message.text, so cached legacy entries are never consulted on the fallback path, yet they occupy slots in the shared 1024-entry cache and can evict rich entries under sustained load before a reply arrives. Output stays correct; only effective retention degrades. Options: register only on the rich path, scope the cap per-chat, or document the tradeoff.
  • [P3, needs verification] Quoted reply to a rich message and the [Bug]: Telegram native partial quotes are expanded to the full replied-to message #22619 protection. If Telegram does not populate message.quote.text for native-quote replies to a sendRichMessage message, the cache fallback would inject the full cached body instead of the user's selected substring, regressing the [Bug]: Telegram native partial quotes are expanded to the full replied-to message #22619 quote-narrowing protection. Needs verification on live Telegram of whether quote.text round-trips for rich-message replies; if it does not, suppress full-body cache injection on quoted rich replies.

Advisory: the cache is in-memory only, so it is lost on gateway restart and not shared across processes (a reply after a restart, or on a different instance, falls back to no context). Reasonable for a first cut given the 48h TTL window.

When telegram.extra.rich_messages is enabled, final replies are sent via
sendRichMessage, which carries no legacy text field. A user replying to
such a message arrives with reply_to_message.text and .caption empty, so
the gateway loses reply context and injects no "[Replying to: ...]"
pointer. This was most visible on replies to rich cron announcements.

Add a bounded, TTL-expiring outbound reply-context cache on the Telegram
adapter, keyed by (chat_id, message_id) and populated on both the rich and
legacy send paths. When an inbound reply has no native quote and no
text/caption, _build_message_event falls back to the cached snippet. The
existing gateway injection logic then works unchanged.

This restores parity with legacy text replies, which already round-trip
through reply_to_message.text. The cache is in-memory only and bounded
three ways (entry count, per-entry chars, and a 48h TTL); it stores only
the bot's own outbound content and surfaces it only on an explicit user
reply, so it adds no stale-task-revival surface beyond the existing
legacy-reply behavior (see NousResearch#26714).
@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 Jun 15, 2026
@liuhao1024

Copy link
Copy Markdown
Contributor

Code Review — LGTM ✅

Scope: Telegram adapter outbound reply-context cache for rich message reply parity.

What was checked:

  • Cache bounded correctly: 1024-entry cap with LRU eviction, 2000-char snippet cap, 48h TTL
  • TTL uses time.monotonic() — immune to wall-clock adjustments
  • _outbound_context_cache() lazy-init handles object.__new__ construction path
  • _prune_outbound_context runs on every write (not on read) — read path only does per-entry TTL check
  • move_to_end(key) after value update ensures LRU ordering is correct
  • str(chat_id), str(message_id) normalization handles int-vs-str key mismatch
  • Both send paths (rich + legacy) register outbound context
  • Precedence chain in _build_message_event: native quote > real text/caption > cached snippet
  • 368 lines of tests covering: roundtrip, cap, TTL expiry, eviction, missing-id guard, send-path registration, inbound fallback, precedence, failure path, end-to-end cron parity

No issues found. The implementation is thorough and the test coverage is excellent.

@ModeoC

ModeoC commented Jun 15, 2026

Copy link
Copy Markdown

This seems like the right minimal fix for the rich-message regression.

One question: should cron deliveries eventually store a durable mapping from Telegram message_id to the cron run/session/output file?

The in-memory outbound cache handles the common case where a user replies soon after delivery, but cron messages are a bit special:

  • users may reply hours later
  • the gateway may restart between delivery and reply
  • cron output already has a durable session/output file

A small durable bridge like:

platform + chat_id + thread_id + message_id -> cron_job_id + cron_session_id + output_file + preview

would let the gateway inject a lightweight pointer on reply, and the agent could look up the full cron run only when needed.

Not blocking this PR. The cache fix is clean and restores parity for rich messages. Just wondering if cron deliveries deserve the durable mapping as a follow-up.

@GodsBoy

GodsBoy commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Closing this as implemented on main.

The rich-message reply-context regression is now covered by the mainline implementation using gateway/rich_sent_store.py, with send-time recording and inbound lookup for replies to Telegram sendRichMessage messages.

Verified:

  • origin/main contains the rich-message reply recovery path.
  • The Telegram rich-message regression tests pass locally.
  • Live Telegram check: replying to a rich cron response gave the agent the cron text context correctly.

The branch here is now redundant and dirty against current main, so closing this PR in favour of the mainline fix.

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 P2 Medium — degraded but workaround exists platform/telegram Telegram bot adapter type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Telegram rich cron announcements lose reply context because reply_to_message.text is empty for sendRichMessage replies

4 participants