feat(telegram): isolate forum topic threads into separate conversations#326
feat(telegram): isolate forum topic threads into separate conversations#326toanalien wants to merge 5 commits intospacedriveapp:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds docs for multiple Telegram bot instances and binding-level routing; implements Telegram forum topic support by extracting and propagating an optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
src/messaging/telegram.rs
Outdated
| .metadata | ||
| .get("telegram_thread_id") | ||
| .and_then(|v| v.as_i64()) | ||
| .map(|v| v as i32)?; |
There was a problem hiding this comment.
Nice addition. Small robustness tweak: as i32 will truncate on overflow; I'd rather try_from and ignore unexpected values.
| .map(|v| v as i32)?; | |
| let id = message | |
| .metadata | |
| .get("telegram_thread_id") | |
| .and_then(|v| v.as_i64()) | |
| .and_then(|v| i32::try_from(v).ok()) | |
| .filter(|&v| v > 0)?; |
| Spacebot reads messages from all topics the bot can see. Each topic maintains its own conversation context, so the bot won't mix up discussions between topics. | ||
|
|
||
| <Callout type="info"> | ||
| Telegram doesn't support creating named threads via the Bot API. Spacebot replies within the same topic where the message was sent, keeping replies in context. |
There was a problem hiding this comment.
Nit: Telegram does expose forum-topic APIs (e.g. createForumTopic). If the intent is "Spacebot doesn't create topics", maybe rephrase.
| Telegram doesn't support creating named threads via the Bot API. Spacebot replies within the same topic where the message was sent, keeping replies in context. | |
| Telegram supports forum-topic APIs in the Bot API, but Spacebot doesn’t create/manage topics yet. Spacebot replies within the same topic where the message was sent, keeping replies in context. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/content/docs/(messaging)/telegram-setup.mdx (1)
215-226: Optional: add one line clarifying topic-mode logconversation_idformat.A brief note that topic messages can appear as
telegram:<chat_id>:<thread_id>in logs would reduce confusion when users are trying to extract chat IDs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/docs/`(messaging)/telegram-setup.mdx around lines 215 - 226, Add a single clarifying sentence under the "Threads (Group Topics)" section (near the "Spacebot reads messages from all topics..." paragraph) stating the log/conversation_id format for topic-mode messages so users can extract chat/thread IDs; specifically note that topic messages may appear as conversation_id values like telegram:<chat_id>:<thread_id> in logs and tooling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/messaging/telegram.rs`:
- Around line 103-108: The metadata extraction uses unchecked casts (v as i32)
for "telegram_thread_id" and "telegram_message_id" which can silently wrap;
replace those .map(|v| v as i32) conversions with a checked conversion like
.and_then(|v| i32::try_from(v).ok()) so overflow yields None instead of
incorrect values; update the chains that build ThreadId(MessageId(...)) and any
similar use in the same file (e.g., where telegram_message_id is converted) to
use i32::try_from and preserve the surrounding Option handling.
- Around line 365-373: Rename the abbreviated local bindings to full names
across the file: change any pattern like "if let Some(tid) = thread_id" to "if
let Some(thread_id_value) = thread_id" and rename any short request builders
bound as "req" to "request" (e.g., where you build request with
self.bot.send_audio(...), self.bot.send_message(...), etc.). Update all matching
occurrences listed (the blocks around the shown diff and the other line ranges
mentioned) so the new binding names are used consistently (e.g., call
message_thread_id(thread_id_value) and continue using "request" for the
builder), adjusting any subsequent uses of the old identifiers to the new names
to preserve compilation.
---
Nitpick comments:
In `@docs/content/docs/`(messaging)/telegram-setup.mdx:
- Around line 215-226: Add a single clarifying sentence under the "Threads
(Group Topics)" section (near the "Spacebot reads messages from all topics..."
paragraph) stating the log/conversation_id format for topic-mode messages so
users can extract chat/thread IDs; specifically note that topic messages may
appear as conversation_id values like telegram:<chat_id>:<thread_id> in logs and
tooling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e6f87da5-938d-4a72-82ea-c94737c8633f
📒 Files selected for processing (2)
docs/content/docs/(messaging)/telegram-setup.mdxsrc/messaging/telegram.rs
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/content/docs/(messaging)/telegram-setup.mdx (1)
228-228:⚠️ Potential issue | 🟡 MinorLine 228: Rephrase Bot API capability statement to avoid factual mismatch.
Telegram Bot API does expose forum-topic methods; this should say Spacebot currently does not create/manage topics, rather than Telegram not supporting it.
Proposed wording
-Telegram doesn't support creating named threads via the Bot API. Spacebot replies within the same topic where the message was sent, keeping replies in context. +Telegram supports forum-topic APIs in the Bot API, but Spacebot doesn’t create or manage topics yet. Spacebot replies within the same topic where the message was sent, keeping replies in context.Does the Telegram Bot API currently include forum-topic management methods (for example createForumTopic / editForumTopic / closeForumTopic)?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/docs/`(messaging)/telegram-setup.mdx at line 228, Replace the sentence "Telegram doesn't support creating named threads via the Bot API." with a factual rephrase stating that Spacebot itself does not create or manage forum topics (e.g., "Spacebot currently does not create or manage forum topics; it replies within the same topic where the message was sent, keeping replies in context."), and optionally note that Telegram's Bot API does include forum-topic methods (such as createForumTopic/editForumTopic/closeForumTopic) if you want to acknowledge the platform capability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/content/docs/`(messaging)/telegram-setup.mdx:
- Line 228: Replace the sentence "Telegram doesn't support creating named
threads via the Bot API." with a factual rephrase stating that Spacebot itself
does not create or manage forum topics (e.g., "Spacebot currently does not
create or manage forum topics; it replies within the same topic where the
message was sent, keeping replies in context."), and optionally note that
Telegram's Bot API does include forum-topic methods (such as
createForumTopic/editForumTopic/closeForumTopic) if you want to acknowledge the
platform capability.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 08471342-2553-4447-ac81-dc5b1b554b89
📒 Files selected for processing (2)
docs/content/docs/(messaging)/telegram-setup.mdxsrc/messaging/telegram.rs
29a485e to
b1a3bf8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/content/docs/`(messaging)/telegram-setup.mdx:
- Around line 239-240: Update the "Bot doesn't see topic messages"
troubleshooting entry to instruct users to not only disable privacy mode in
BotFather but also remove and re-add the bot to the group so the new privacy
setting takes effect; specifically modify the table row that mentions "privacy
mode" (the "Bot doesn't see topic messages" entry) to add a short step saying
"disable privacy mode in BotFather, then remove and re-add the bot to the group"
and ensure Topics are enabled in the group settings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9fdb772c-ec70-4a5a-9c17-d0dfc9b115b1
📒 Files selected for processing (1)
docs/content/docs/(messaging)/telegram-setup.mdx
|
Hello @jamiepine, I just fixed code for check, clippy, format and test. Help me review and approve. Thank you |
- Add `thread_id: Option<ThreadId>` field to `ActiveStream` struct
- Extract thread ID via `extract_thread_id()` helper for topic messages
- Scope `conversation_id` to thread: `telegram:{chat_id}:{thread_id}`
- Store `telegram_thread_id` in metadata when `is_topic_message`
- Pass thread ID to all outbound send functions: `send_formatted`,
`send_plain_text`, `send_poll`, `send_audio`, `send_document`,
`send_chat_action`
- Route typing indicator and stream placeholder to correct forum topic
- Add "Multiple Telegram Instances" section with UI and TOML config examples - Add "Threads (Group Topics)" section explaining forum topic isolation behavior - Extend troubleshooting table with 2 new rows covering thread-related issues
- Replace v as i32 with i32::try_from(v).ok() for safe overflow handling in extract_message_id and extract_thread_id - Rename req → request in StreamStart and send_chat_action paths - Add conversation_id format note to docs (telegram:<chat_id>:<thread_id>)
Prefix unused ActiveStream.thread_id with underscore to suppress dead_code warning under -Dwarnings. Reformat method chains to satisfy rustfmt. Correct docs claim about Bot API topic support.
d6a177e to
47334b5
Compare
Summary