feat(discord): tag forwarded messages with channel/thread/DM context#1555
Merged
asheshgoplani merged 3 commits intoJul 12, 2026
Merged
Conversation
The Discord bridge forwarded message text to the conductor with no indication of where it came from. Slack messages already carry a `[from:... (id)] [channel:#... (id)]` prefix (see resolve_slack_channel); Discord did not, so the conductor could not tell which channel, thread, or DM a request originated in. Add build_discord_context_tag() and prepend it in on_message, mirroring the Slack convention. Discord additionally emits a `[thread:#name (id) in #parent]` variant for messages sent inside a thread — Slack has no inline equivalent because it routes threads via thread_ts instead. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013GN4ktNBa9zcKUXvV6HPZu
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis change adds Discord message context tagging in the bridge, prepends the tag before forwarding messages to the conductor, and adds tests for the tag format and fallback cases. ChangesDiscord Context Tagging
Sequence Diagram(s)sequenceDiagram
participant DiscordMessage
participant conductor_bridge
participant ConductorSession
DiscordMessage->>conductor_bridge: on_message(message)
conductor_bridge->>conductor_bridge: build_discord_context_tag(message)
conductor_bridge->>ConductorSession: cleaned_msg with context tag
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/session/conductor_bridge.py (1)
2245-2269: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo test coverage for the new helper.
Slack's equivalent tagging logic has dedicated unit tests (
internal/session/test_slack_message_enrichment.py), butbuild_discord_context_taghas no analogous tests covering the channel/thread/DM branches, fallback name handling, or the final tag format.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/session/conductor_bridge.py` around lines 2245 - 2269, Add unit tests for build_discord_context_tag in conductor_bridge to cover the DM, thread, and normal channel branches, plus fallback author/channel naming when display_name, name, or parent are missing. Mirror the style of the existing Slack tests by asserting the exact final tag format for each case, including thread parent formatting and the combined [from:...] [channel:...]/[thread:...]/[dm] output.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/session/conductor_bridge.py`:
- Around line 2245-2269: The thread/DM handling in build_discord_context_tag is
currently unreachable because on_message filters out anything whose channel id
does not match bot.target_channel_id before this helper runs. Update the message
gating in on_message so it also allows child thread channels under the target
channel and any intended DM path, then keep build_discord_context_tag as the
place that formats private/thread/channel tags using message.channel and its
type.
---
Nitpick comments:
In `@internal/session/conductor_bridge.py`:
- Around line 2245-2269: Add unit tests for build_discord_context_tag in
conductor_bridge to cover the DM, thread, and normal channel branches, plus
fallback author/channel naming when display_name, name, or parent are missing.
Mirror the style of the existing Slack tests by asserting the exact final tag
format for each case, including thread parent formatting and the combined
[from:...] [channel:...]/[thread:...]/[dm] output.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0751d61c-2fb7-4505-b3da-c84cf8014d48
📒 Files selected for processing (1)
internal/session/conductor_bridge.py
…ches Addresses CodeRabbit review on asheshgoplani#1555: the Slack tagging logic has test_slack_message_enrichment.py but build_discord_context_tag had none. Mirrors the Slack test style (a faithful mirror of the closure) and asserts the exact tag format for regular channels, public/private threads, threads with an unresolvable parent, DMs, and the author/ channel name fallbacks. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013GN4ktNBa9zcKUXvV6HPZu
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/session/test_discord_message_enrichment.py (1)
39-49: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider covering the
news_threadbranch.
thread_typesincludesnews_thread, but no test exercises it. Minor coverage gap, same shape as the existing public/private thread tests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/session/test_discord_message_enrichment.py` around lines 39 - 49, Add a test case that exercises the `news_thread` branch in `test_discord_message_enrichment.py` so `thread_types` is covered for all thread variants. Reuse the same pattern as the existing public/private thread coverage by creating a `discord.ChannelType.news_thread` channel with a parent, then assert the generated `chan_tag` matches the thread format in the enrichment logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/session/test_discord_message_enrichment.py`:
- Around line 39-49: Add a test case that exercises the `news_thread` branch in
`test_discord_message_enrichment.py` so `thread_types` is covered for all thread
variants. Reuse the same pattern as the existing public/private thread coverage
by creating a `discord.ChannelType.news_thread` channel with a parent, then
assert the generated `chan_tag` matches the thread format in the enrichment
logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: cf451648-6392-4e2c-befe-5d0b887d97c4
📒 Files selected for processing (1)
internal/session/test_discord_message_enrichment.py
Owner
|
🤖 autopilot: no CI checks found for this PR. Autopilot cannot assess safety without CI results — please trigger CI and ensure all checks pass before this is eligible for merge. Generated by Claude Code |
Addresses CodeRabbit nitpick on asheshgoplani#1555: thread_types includes news_thread but no test exercised it. Adds an announcement-thread case matching the public/private thread coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013GN4ktNBa9zcKUXvV6HPZu
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.
What
The Discord bridge forwarded a user's message text to the conductor with no indication of where it came from. The Slack path already enriches messages with a
[from:… (id)] [channel:#… (id)]prefix (seeresolve_slack_channel); Discord did not — so a conductor answering a Discord request couldn't tell which channel, thread, or DM it originated in, and couldn't reference it back.This adds
build_discord_context_tag()and prepends it inon_message, mirroring the Slack convention so a conductor sees a consistent shape from either platform.Message examples
Given user
alicesendingdeploy the thing, the conductor now receives:[from:alice (987654321)] [channel:#general (456)] deploy the thing[from:alice (987654321)] [thread:#deploy-thread (789) in #general] deploy the thing[from:alice (987654321)] [thread:#secret (791) in #ops] deploy the thing[from:alice (987654321)] [dm] deploy the thingCompared with the existing Slack output for the same message:
The
from/channel/dmtags match Slack exactly. Discord additionally emits a[thread:#name (id) in #parent]variant — Slack has no inline equivalent because it routes threads out-of-band viathread_tsrather than tagging them in the message body. Parent falls back to?if it can't be resoldisplay_name→name→?`.Summary by CodeRabbit
Summary
New Features
Tests