Skip to content

fix(telegram): route forum-topic replies to correct thread#512

Merged
penso merged 2 commits intomainfrom
fix/forum-topic-reply-routing
Mar 28, 2026
Merged

fix(telegram): route forum-topic replies to correct thread#512
penso merged 2 commits intomainfrom
fix/forum-topic-reply-routing

Conversation

@penso
Copy link
Copy Markdown
Collaborator

@penso penso commented Mar 28, 2026

Summary

  • Use target.outbound_to() (which encodes chat_id:thread_id) instead of target.chat_id in all outbound calls (send_text, send_html, send_media, send_text_silent, send_text_with_suffix, send_stream, send_location) so forum-topic replies land in the correct Telegram thread
  • Add thread_id to ChannelReplyTargetKey so stream-dedup distinguishes topics within the same supergroup — prevents silently skipping text delivery for topic B when topic A's stream completes first

Fixes #442

Validation

Completed

  • cargo check -p moltis-chat — compiles clean
  • cargo +nightly-2025-11-30 fmt --all -- --check — no formatting issues
  • cargo clippy -p moltis-chat --all-targets -- -D warnings — no warnings
  • cargo test -p moltis-chat — all 151 tests pass
  • cargo test -p moltis-channels -- outbound_to — 2 tests pass
  • cargo test -p moltis-telegram -- topic — 5 tests pass

Remaining

  • Manual QA: send messages from two different forum topics in a Telegram supergroup, verify replies land in the correct topic thread
  • Manual QA: verify streaming (edit-in-place) works per-topic — concurrent streams in two topics should not interfere
  • Manual QA: verify cron delivery to topic targets still works
  • ./scripts/local-validate.sh

Manual QA

  1. Create a Telegram supergroup with forum topics enabled
  2. Send a message in Topic A → verify the bot reply appears in Topic A (not top-level)
  3. Send a message in Topic B while Topic A is still streaming → verify Topic B gets its own reply
  4. Check that voice replies, screenshots, documents, and location pins all route to the correct topic

🤖 Generated with Claude Code

Outbound calls in deliver_channel_replies_to_targets and related
functions passed target.chat_id (the raw numeric chat ID) instead of
target.outbound_to() (which encodes chat_id:thread_id for forum
topics). This caused all replies in a Telegram supergroup to land in
the top-level chat regardless of which topic originated the message.

Additionally, ChannelReplyTargetKey was missing the thread_id field,
causing stream-dedup to treat all topics in the same supergroup as
equivalent — if topic A's stream completed first, topic B's text
delivery would be incorrectly skipped.

Fixes #442

Entire-Checkpoint: 38dddac7b136
@codspeed-hq
Copy link
Copy Markdown
Contributor

codspeed-hq bot commented Mar 28, 2026

Merging this PR will degrade performance by 30.21%

❌ 1 regressed benchmark
✅ 38 untouched benchmarks
⏩ 5 skipped benchmarks1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
env_substitution 10.1 µs 14.5 µs -30.21%

Comparing fix/forum-topic-reply-routing (a009f19) with main (277e7db)2

Open in CodSpeed

Footnotes

  1. 5 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on main (35ae27f) during the generation of this report, so 277e7db was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 28, 2026

Greptile Summary

This PR fixes Telegram forum-topic replies being routed to the wrong thread by replacing all raw target.chat_id arguments in outbound delivery calls with target.outbound_to(), which encodes "chat_id:thread_id" for forum-topic targets. It also adds thread_id to ChannelReplyTargetKey so stream-dedup correctly treats concurrent streams in different topics of the same supergroup as independent — preventing Topic B's text delivery from being silently dropped when Topic A's stream completes first.

Key changes:

  • All outbound send methods (send_text, send_html, send_media, send_text_silent, send_text_with_suffix, send_stream, send_location) now use target.outbound_to().into_owned() instead of target.chat_id
  • ChannelReplyTargetKey gains a thread_id: Option<String> field, closing a deduplication gap that was most impactful when message_id is None (e.g. cron-style deliveries where two forum-topic targets in the same supergroup had identical keys)
  • All warn!/debug! log sites in the changed functions now include thread_id for improved observability

Confidence Score: 5/5

Safe to merge — changes are systematic, compile-verified, and cover all outbound call sites consistently.

All outbound delivery paths are correctly updated to use outbound_to(), the dedup key fix is structurally sound (adding thread_id to the Hash/Eq-derived struct), and CI checks confirm clean compilation, zero clippy warnings, and 151 passing tests. The only remaining finding is a P2 suggestion to add a regression test for the cross-topic dedup scenario.

No files require special attention — crates/chat/src/lib.rs is the sole changed file and the modifications are consistent throughout.

Important Files Changed

Filename Overview
crates/chat/src/lib.rs Systematically switches all outbound delivery calls from target.chat_id to target.outbound_to(), and adds thread_id to ChannelReplyTargetKey — both changes are correct and address the forum-topic routing bug. No P1 issues found.

Sequence Diagram

sequenceDiagram
    participant UA as User (Topic A)
    participant UB as User (Topic B)
    participant Dispatcher as ChannelStreamDispatcher
    participant OutboundA as Outbound (chat:thread_a)
    participant OutboundB as Outbound (chat:thread_b)
    participant Delivery as deliver_channel_replies

    UA->>Dispatcher: message → push target {chat, thread_a}
    UB->>Dispatcher: message → push target {chat, thread_b}
    Dispatcher->>OutboundA: send_stream(outbound_to = "chat:thread_a")
    Dispatcher->>OutboundB: send_stream(outbound_to = "chat:thread_b")
    OutboundA-->>Dispatcher: Ok → insert key{chat, thread_a} into completed
    OutboundB-->>Dispatcher: Ok → insert key{chat, thread_b} into completed
    Dispatcher->>Delivery: completed_target_keys = {key_a, key_b}
    Delivery->>Delivery: key_a ∈ streamed → skip text for Topic A ✓
    Delivery->>Delivery: key_b ∈ streamed → skip text for Topic B ✓
    Note over Delivery: Before fix: thread_id absent → key_a == key_b
    Note over Delivery: Topic B incorrectly skipped on cron/no-message_id paths
Loading

Reviews (2): Last reviewed commit: "fix(telegram): restore raw chat_id in lo..." | Re-trigger Greptile

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 28, 2026

Codecov Report

❌ Patch coverage is 19.64286% with 45 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/chat/src/lib.rs 19.64% 45 Missing ⚠️

📢 Thoughts on this report? Let us know!

Address PR review: chat_for_log in stream dispatcher was changed to
the composite outbound address — revert to raw chat_id so structured
log consumers are not surprised. Add thread_id field to all warn!/
debug! macros across delivery helpers so forum-topic failures can be
correlated with the specific topic that failed.

Entire-Checkpoint: 9302e4358187
@penso penso merged commit e77c74d into main Mar 28, 2026
13 of 23 checks passed
@penso penso deleted the fix/forum-topic-reply-routing branch March 28, 2026 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant