feat(telegram): add forum topic/thread support#366
Open
lutr0 wants to merge 2 commits intospacedriveapp:mainfrom
Open
feat(telegram): add forum topic/thread support#366lutr0 wants to merge 2 commits intospacedriveapp:mainfrom
lutr0 wants to merge 2 commits intospacedriveapp:mainfrom
Conversation
Telegram supergroups with forum mode enabled use message_thread_id to route messages to specific topics. Without this, all bot replies land in the General topic regardless of where the user sent their message. Changes: - Extract thread_id from inbound Telegram messages (including reply chains) - Encode thread_id in conversation_id as telegram:<chat>:thread:<id> - Pass thread_id through all outbound send paths (text, files, polls, streaming placeholders, ephemeral, scheduled) - Store telegram_message_thread_id and telegram_is_topic_message in message metadata for downstream consumption - Recover thread_id from conversation_id when metadata is unavailable (e.g. retrigger/branch relay paths) This ensures each forum topic gets its own conversation context and replies stay in the correct topic automatically, including for any new topics created in the future.
Contributor
WalkthroughExtracts Telegram thread context (ThreadId) from inbound messages and threads it through outbound operations. Conversation ID creation, metadata, and all send paths (text, formatted, files, polls, replies, streaming edits) now accept and propagate an optional ThreadId. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
| message | ||
| .metadata | ||
| .get("telegram_message_thread_id") | ||
| .and_then(|v| v.as_i64()) |
Contributor
There was a problem hiding this comment.
v as i32 will silently truncate on overflow. A checked conversion avoids routing replies to the wrong topic if metadata is malformed.
Suggested change
| .and_then(|v| v.as_i64()) | |
| .and_then(|v| v.as_i64()) | |
| .and_then(|v| i32::try_from(v).ok()) | |
| .map(|v| ThreadId(MessageId(v))) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/messaging/telegram.rs (1)
371-433: Thread propagation for file/audio sending looks correct.Minor style note:
tidis used as an abbreviation ofthread_idthroughout these blocks. Per coding guidelines, consider using the full name for consistency, though the scope is very limited in each usage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/messaging/telegram.rs` around lines 371 - 433, The local pattern variable `tid` used in the message threading checks should be the full name `thread_id` for clarity and consistency; replace occurrences like `if let Some(tid) = thread_id { request = request.message_thread_id(tid); }` with `if let Some(thread_id) = thread_id { request = request.message_thread_id(thread_id); }` (do this for both the send_audio and send_document branches where message_thread_id(...) is set) so the code consistently uses the full identifier while still passing the value to the message_thread_id(...) call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/messaging/telegram.rs`:
- Around line 371-433: The local pattern variable `tid` used in the message
threading checks should be the full name `thread_id` for clarity and
consistency; replace occurrences like `if let Some(tid) = thread_id { request =
request.message_thread_id(tid); }` with `if let Some(thread_id) = thread_id {
request = request.message_thread_id(thread_id); }` (do this for both the
send_audio and send_document branches where message_thread_id(...) is set) so
the code consistently uses the full identifier while still passing the value to
the message_thread_id(...) call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dc941f4a-7d92-4cd7-931f-b9a05a02251f
📒 Files selected for processing (1)
src/messaging/telegram.rs
Comment on lines
+59
to
+65
| fn effective_thread_id_for_message(message: &Message) -> Option<ThreadId> { | ||
| message.thread_id.or_else(|| { | ||
| message | ||
| .reply_to_message() | ||
| .and_then(effective_thread_id_for_message) | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
This recursion is probably fine in practice, but an iterative walk is simpler and avoids any surprises with deeply nested reply_to_message chains.
Suggested change
| fn effective_thread_id_for_message(message: &Message) -> Option<ThreadId> { | |
| message.thread_id.or_else(|| { | |
| message | |
| .reply_to_message() | |
| .and_then(effective_thread_id_for_message) | |
| }) | |
| } | |
| fn effective_thread_id_for_message(message: &Message) -> Option<ThreadId> { | |
| let mut current = Some(message); | |
| while let Some(message) = current { | |
| if let Some(thread_id) = message.thread_id { | |
| return Some(thread_id); | |
| } | |
| current = message.reply_to_message(); | |
| } | |
| None | |
| } |
Comment on lines
+67
to
+73
| fn thread_id_from_conversation_id(conversation_id: &str) -> Option<ThreadId> { | ||
| let marker = ":thread:"; | ||
| let idx = conversation_id.rfind(marker)?; | ||
| let raw = &conversation_id[idx + marker.len()..]; | ||
| let parsed = raw.parse::<i32>().ok()?; | ||
| Some(ThreadId(MessageId(parsed))) | ||
| } |
Contributor
There was a problem hiding this comment.
Small robustness tweak: parse only the thread ID segment so this keeps working if the conversation id ever grows extra suffix segments.
Suggested change
| fn thread_id_from_conversation_id(conversation_id: &str) -> Option<ThreadId> { | |
| let marker = ":thread:"; | |
| let idx = conversation_id.rfind(marker)?; | |
| let raw = &conversation_id[idx + marker.len()..]; | |
| let parsed = raw.parse::<i32>().ok()?; | |
| Some(ThreadId(MessageId(parsed))) | |
| } | |
| fn thread_id_from_conversation_id(conversation_id: &str) -> Option<ThreadId> { | |
| let marker = ":thread:"; | |
| let idx = conversation_id.rfind(marker)?; | |
| let raw = conversation_id[idx + marker.len()..].split(':').next()?; | |
| let parsed = raw.parse::<i32>().ok()?; | |
| Some(ThreadId(MessageId(parsed))) | |
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/messaging/telegram.rs (1)
376-378: Consider using full variable name per coding guidelines.The abbreviation
tidis used throughout forthread_id. While the scope is narrow, the coding guidelines prefer unabbreviated names.As per coding guidelines: "Don't abbreviate variable names. Use
queuenotq,messagenotmsg."♻️ Example change pattern
-if let Some(tid) = thread_id { - request = request.message_thread_id(tid); +if let Some(thread_id) = thread_id { + request = request.message_thread_id(thread_id); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/messaging/telegram.rs` around lines 376 - 378, Replace the abbreviated local `tid` with an unabbreviated name to follow naming guidelines: change the pattern `if let Some(tid) = thread_id { request = request.message_thread_id(tid); }` to use a full name (e.g., `if let Some(thread_id_value) = thread_id { request = request.message_thread_id(thread_id_value); }` or shadow using `if let Some(thread_id) = thread_id { request = request.message_thread_id(thread_id); }`) in the same block that calls `request.message_thread_id(...)` in src/messaging/telegram.rs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/messaging/telegram.rs`:
- Around line 376-378: Replace the abbreviated local `tid` with an unabbreviated
name to follow naming guidelines: change the pattern `if let Some(tid) =
thread_id { request = request.message_thread_id(tid); }` to use a full name
(e.g., `if let Some(thread_id_value) = thread_id { request =
request.message_thread_id(thread_id_value); }` or shadow using `if let
Some(thread_id) = thread_id { request = request.message_thread_id(thread_id);
}`) in the same block that calls `request.message_thread_id(...)` in
src/messaging/telegram.rs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 06a128c0-3ee9-41eb-a001-768a26e7b306
📒 Files selected for processing (1)
src/messaging/telegram.rs
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
2 participants
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.
Summary
Telegram supergroups with forum mode enabled use
message_thread_idto route messages to specific topics. Without this, all bot replies land in the General topic regardless of where the user sent their message.Changes
thread_idfrom inbound Telegram messages (including reply chains)conversation_idastelegram:<chat>:thread:<id>thread_idthrough all outbound send paths (text, files, polls, streaming placeholders, ephemeral, scheduled)telegram_message_thread_idandtelegram_is_topic_messagein message metadata for downstream consumptionthread_idfromconversation_idwhen metadata is unavailable (e.g. retrigger/branch relay paths)Behavior
Testing
Tested with a Telegram supergroup with forum mode enabled, multiple topics, voice messages, file sends, polls, and streaming responses.
Note
Auto-generated summary: Changes to
src/messaging/telegram.rsthat add forum topic/thread support for Telegram. The implementation extracts thread IDs from inbound messages (recursively through reply chains), encodes them in conversation IDs for routing, and passes them to all outbound send operations (formatted text, polls, file uploads, streaming). Helper functions handle both direct thread ID extraction and recovery from conversation IDs. All messaging paths (audio, documents, streaming placeholders, ephemeral, scheduled) now preserve thread context. Message metadata now stores the thread ID and topic flag for downstream consumption.Written by Tembo for commit 21359c6. This will update automatically on new commits.