feat: expose Discord channel topics to agent context#544
feat: expose Discord channel topics to agent context#544EZotoff wants to merge 4 commits intospacedriveapp:mainfrom
Conversation
|
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)
WalkthroughChannel topic metadata is extracted from Discord channel data, added as a standardized Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/api/channels.rs (1)
566-572:⚠️ Potential issue | 🟡 Minor
inspect_promptcurrently drops availablechannel_topiccontext.Line 571 hardcodes
None, so prompt inspection can differ from real channel runtime context when topic metadata exists.🔧 Proposed fix
Ok(Some(info)) => { let server_name = info .platform_meta .as_ref() .and_then(|meta| { meta.get("discord_guild_name") .or_else(|| meta.get("slack_workspace_id")) }) .and_then(|v| v.as_str()); + let channel_topic = info + .platform_meta + .as_ref() + .and_then(|meta| meta.get(crate::metadata_keys::CHANNEL_TOPIC)) + .and_then(|v| v.as_str()); prompt_engine .render_conversation_context( &info.platform, server_name, info.display_name.as_deref(), Some(&info.id), - None, + channel_topic, ) .ok() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/channels.rs` around lines 566 - 572, The call to render_conversation_context is passing None for the channel topic, which drops available topic metadata and causes inspect_prompt to diverge from runtime context; update the final argument in the render_conversation_context call to forward the actual channel topic (e.g., replace None with info.channel_topic.as_deref() or the appropriate field like info.topic.as_deref()) so inspect_prompt receives the same topic context as runtime.src/prompts/engine.rs (1)
271-277:⚠️ Potential issue | 🟠 MajorSignature change leaves a stale test callsite that will fail to compile.
Line 271-Line 277 adds a new required parameter, but
tests/context_dump.rs(Line 189-Line 191 in provided context) still uses the old 4-argument call. That test target will fail to compile until the extra argument is passed.Suggested fix (outside this file)
- let conversation_context = prompt_engine - .render_conversation_context("discord", Some("Test Server"), Some("#general"), None) - .ok(); + let conversation_context = prompt_engine + .render_conversation_context("discord", Some("Test Server"), Some("#general"), None, None) + .ok();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/prompts/engine.rs` around lines 271 - 277, The test callsite for render_conversation_context is still using the old 4-argument signature; update the test in tests/context_dump.rs (the call that invokes render_conversation_context) to pass the new fifth parameter channel_topic (e.g., Some("...") or None as appropriate) so the test compiles against the new function signature; ensure the value matches the other test inputs' intent.
🧹 Nitpick comments (2)
src/messaging/discord.rs (1)
939-946: Trim topic text before metadata insertion.Using
!topic.is_empty()lets whitespace-only topics pass through and produce noisy/blank context lines.♻️ Suggested refinement
- if let Some(ref topic) = guild_channel.topic { - if !topic.is_empty() { - metadata.insert( - crate::metadata_keys::CHANNEL_TOPIC.into(), - topic.clone().into(), - ); - } - } + if let Some(topic) = guild_channel + .topic + .as_deref() + .map(str::trim) + .filter(|topic| !topic.is_empty()) + { + metadata.insert( + crate::metadata_keys::CHANNEL_TOPIC.into(), + topic.to_string().into(), + ); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/messaging/discord.rs` around lines 939 - 946, The code inserts guild_channel.topic into metadata without trimming, allowing whitespace-only topics to pass; update the check and insertion so you trim the topic first (use topic.trim()) and skip insertion if the trimmed string is empty, and when inserting into metadata use the trimmed string (reference guild_channel.topic and crate::metadata_keys::CHANNEL_TOPIC to locate the logic that builds metadata).src/agent/channel.rs (1)
1300-1310: Consider extracting shared conversation-context metadata parsing into a helper.Both code paths now duplicate the same
server_name/channel_name/channel_topicextraction andrender_conversation_contextcall. Consolidating this reduces drift risk for future prompt-context changes.Also applies to: 1808-1818
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agent/channel.rs` around lines 1300 - 1310, Duplicate extraction of server_name, channel_name, channel_topic and the render_conversation_context call should be moved into a small helper to avoid drift; create a private helper (e.g., render_conversation_context_from_meta or build_and_render_conversation_context) that accepts the source/metadata (the same type as first), server_name, channel_name and self.conversation_id.as_deref(), reads metadata_keys::CHANNEL_TOPIC via metadata.get(...).and_then(|v| v.as_str()), calls prompt_engine.render_conversation_context(...) and returns the Result used to set self.conversation_context; replace both duplicated blocks (including the snippet using first.metadata and the other at lines ~1808-1818) to call this single helper and assign the result.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/api/channels.rs`:
- Around line 566-572: The call to render_conversation_context is passing None
for the channel topic, which drops available topic metadata and causes
inspect_prompt to diverge from runtime context; update the final argument in the
render_conversation_context call to forward the actual channel topic (e.g.,
replace None with info.channel_topic.as_deref() or the appropriate field like
info.topic.as_deref()) so inspect_prompt receives the same topic context as
runtime.
In `@src/prompts/engine.rs`:
- Around line 271-277: The test callsite for render_conversation_context is
still using the old 4-argument signature; update the test in
tests/context_dump.rs (the call that invokes render_conversation_context) to
pass the new fifth parameter channel_topic (e.g., Some("...") or None as
appropriate) so the test compiles against the new function signature; ensure the
value matches the other test inputs' intent.
---
Nitpick comments:
In `@src/agent/channel.rs`:
- Around line 1300-1310: Duplicate extraction of server_name, channel_name,
channel_topic and the render_conversation_context call should be moved into a
small helper to avoid drift; create a private helper (e.g.,
render_conversation_context_from_meta or build_and_render_conversation_context)
that accepts the source/metadata (the same type as first), server_name,
channel_name and self.conversation_id.as_deref(), reads
metadata_keys::CHANNEL_TOPIC via metadata.get(...).and_then(|v| v.as_str()),
calls prompt_engine.render_conversation_context(...) and returns the Result used
to set self.conversation_context; replace both duplicated blocks (including the
snippet using first.metadata and the other at lines ~1808-1818) to call this
single helper and assign the result.
In `@src/messaging/discord.rs`:
- Around line 939-946: The code inserts guild_channel.topic into metadata
without trimming, allowing whitespace-only topics to pass; update the check and
insertion so you trim the topic first (use topic.trim()) and skip insertion if
the trimmed string is empty, and when inserting into metadata use the trimmed
string (reference guild_channel.topic and crate::metadata_keys::CHANNEL_TOPIC to
locate the logic that builds metadata).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2142a213-f9d9-4e8d-b658-44f4df7a8c25
📒 Files selected for processing (6)
prompts/en/fragments/conversation_context.md.j2src/agent/channel.rssrc/api/channels.rssrc/lib.rssrc/messaging/discord.rssrc/prompts/engine.rs
There was a problem hiding this comment.
Pull request overview
This PR extends the agent’s rendered “Conversation Context” to include a Discord channel’s topic/description when available, so prompts carry more channel-specific context.
Changes:
- Adds a new standard metadata key
CHANNEL_TOPICand populates it from the Discord API when present. - Threads
channel_topicthroughPromptEngine::render_conversation_context()and its call sites. - Updates the conversation context prompt fragment to render a
Topic:line when a topic exists.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/lib.rs |
Adds metadata_keys::CHANNEL_TOPIC constant. |
src/messaging/discord.rs |
Extracts guild_channel.topic and inserts it into message metadata. |
src/prompts/engine.rs |
Extends render_conversation_context() to accept channel_topic and pass it to templates. |
src/agent/channel.rs |
Pulls CHANNEL_TOPIC from message metadata and passes it into prompt rendering. |
src/api/channels.rs |
Updates the render_conversation_context() call signature (currently passes None). |
prompts/en/fragments/conversation_context.md.j2 |
Conditionally renders Topic: {{ channel_topic }} in conversation context. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/messaging/discord.rs
Outdated
| if !topic.is_empty() { | ||
| metadata.insert( | ||
| crate::metadata_keys::CHANNEL_TOPIC.into(), | ||
| topic.clone().into(), |
There was a problem hiding this comment.
Inserting guild_channel.topic only checks !topic.is_empty(). A topic containing only whitespace (or with leading/trailing whitespace) will still be stored and later rendered as a meaningless/odd Topic: line. Consider using topic.trim() for the emptiness check and (optionally) storing a trimmed value to keep the prompt output clean and deterministic.
| if !topic.is_empty() { | |
| metadata.insert( | |
| crate::metadata_keys::CHANNEL_TOPIC.into(), | |
| topic.clone().into(), | |
| let trimmed_topic = topic.trim(); | |
| if !trimmed_topic.is_empty() { | |
| metadata.insert( | |
| crate::metadata_keys::CHANNEL_TOPIC.into(), | |
| trimmed_topic.to_owned().into(), |
| .render_conversation_context( | ||
| &info.platform, | ||
| server_name, | ||
| info.display_name.as_deref(), | ||
| Some(&info.id), | ||
| None, | ||
| ) |
There was a problem hiding this comment.
inspect_prompt now passes None for channel_topic, which means the debug/inspection output will never show a topic even for Discord channels where it exists. If the intention is to reflect the real runtime prompt, consider plumbing the topic from persisted platform_meta (or another available source) instead of hard-coding None here.
| /// Convenience method for rendering conversation context fragment. | ||
| pub fn render_conversation_context( | ||
| &self, | ||
| platform: &str, | ||
| server_name: Option<&str>, | ||
| channel_name: Option<&str>, | ||
| conversation_id: Option<&str>, | ||
| channel_topic: Option<&str>, | ||
| ) -> Result<String> { | ||
| self.render( | ||
| "fragments/conversation_context", | ||
| context! { | ||
| platform => platform, | ||
| server_name => server_name, | ||
| channel_name => channel_name, | ||
| conversation_id => conversation_id, | ||
| channel_topic => channel_topic, | ||
| }, | ||
| ) | ||
| } |
There was a problem hiding this comment.
render_conversation_context gained a new channel_topic parameter, but there are no unit tests covering the new rendering behavior. Since this module already has tests, consider adding a small test that asserts the topic line is rendered when Some("...") is passed and omitted when None is passed (also catching whitespace/newline formatting regressions).
Discord channels have topic descriptions that carry meaningful context for the agent. Currently only server name and channel name are passed through — the topic is discarded. Add CHANNEL_TOPIC metadata key, extract guild_channel.topic in the Discord adapter, thread it through render_conversation_context(), and render it in the conversation context template when present. When no topic is set, nothing additional is rendered. Zero breaking changes — purely additive.
EZotoff
force-pushed
the
feat/channel-topic-support
branch
from
8eaceae to
694b441
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
prompts/en/cortex.md.j2 (1)
69-69: Inconsistent casing in association type list.The list mixes PascalCase (
RelatedTo,CausedBy,PartOf) with lowercase (updates,contradicts). Consider using consistent casing across all association types to avoid confusing the LLM about the expected format.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@prompts/en/cortex.md.j2` at line 69, The association type list mixes PascalCase and lowercase which can confuse the LLM; locate the list containing "RelatedTo, updates, contradicts, CausedBy, PartOf" in prompts/en/cortex.md.j2 and make the casing consistent (e.g., change "updates" and "contradicts" to "Updates" and "Contradicts" to match PascalCase with the other entries), ensuring any examples or references elsewhere in the file are updated to the same casing.
🤖 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/lib.rs`:
- Around line 464-465: Remove the duplicate constant definition for
CHANNEL_TOPIC in src/lib.rs so the symbol is only defined once; locate both
occurrences of the pub const CHANNEL_TOPIC: &str = "channel_topic" (the one
around the 455–456 area and the one at 464–465) and delete one of them, keeping
the single canonical declaration used by the module.
In `@src/tools/file.rs`:
- Line 328: The call to self.context.resolve_writable_path(&args.path) uses a
missing FileContext method; either implement FileContext::resolve_writable_path
by delegating to the existing resolve_path and then applying write-specific
checks (e.g., forbid identity/memory paths and any other write restrictions per
guidelines) or change the call to use resolve_path if no extra write checks are
required; update references in file.rs (the code using resolve_writable_path)
and mirror the behavior/tests used by FileReadTool so write operations are
validated consistently.
- Line 221: The code calls a non-existent resolve_writable_path and also uses it
incorrectly in FileReadTool; add a new method FileContext::resolve_writable_path
that mirrors resolve_path but additionally checks and forbids writing to
protected paths (identity and memory) and returns the same Result<PathBuf,
Error> type, and update the read-only tool FileReadTool to call
self.context.resolve_path(&args.path)? instead of resolve_writable_path;
reference FileContext, resolve_path, resolve_writable_path, and FileReadTool
when making these changes.
---
Nitpick comments:
In `@prompts/en/cortex.md.j2`:
- Line 69: The association type list mixes PascalCase and lowercase which can
confuse the LLM; locate the list containing "RelatedTo, updates, contradicts,
CausedBy, PartOf" in prompts/en/cortex.md.j2 and make the casing consistent
(e.g., change "updates" and "contradicts" to "Updates" and "Contradicts" to
match PascalCase with the other entries), ensuring any examples or references
elsewhere in the file are updated to the same casing.
🪄 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: ef938336-d7bc-4559-a4fc-c4772c549e0c
📒 Files selected for processing (9)
prompts/en/cortex.md.j2prompts/en/fragments/conversation_context.md.j2src/agent/channel.rssrc/api/channels.rssrc/lib.rssrc/messaging/discord.rssrc/prompts/engine.rssrc/tools/file.rstests/context_dump.rs
✅ Files skipped from review due to trivial changes (2)
- prompts/en/fragments/conversation_context.md.j2
- src/agent/channel.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/prompts/engine.rs
src/tools/file.rs
Outdated
|
|
||
| async fn call(&self, args: Self::Args) -> Result<Self::Output, Self::Error> { | ||
| let path = self.context.resolve_path(&args.path)?; | ||
| let path = self.context.resolve_writable_path(&args.path)?; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that resolve_writable_path is not defined anywhere in FileContext
echo "=== Searching for resolve_writable_path method definition ==="
rg -n "fn resolve_writable_path" src/tools/file.rs
echo ""
echo "=== All methods defined on FileContext ==="
ast-grep --pattern 'impl FileContext {
$$$
fn $METHOD_NAME($$$) $$$
$$$
}'Repository: spacedriveapp/spacebot
Length of output: 171
🏁 Script executed:
#!/bin/bash
# Check actual content around line 221
echo "=== Content around line 221 ==="
sed -n '215,225p' src/tools/file.rs
echo ""
echo "=== Content around line 328 ==="
sed -n '320,335p' src/tools/file.rs
echo ""
echo "=== Search for all uses of resolve_writable_path in the file ==="
rg -n "resolve_writable_path" src/tools/file.rs
echo ""
echo "=== Search for resolve_writable_path in entire codebase ==="
rg -n "resolve_writable_path" .
echo ""
echo "=== Check FileContext struct and all its methods ==="
sed -n '37,100p' src/tools/file.rsRepository: spacedriveapp/spacebot
Length of output: 4200
resolve_writable_path method does not exist—this will cause a compilation error.
FileContext only defines resolve_path() (lines 37-91), but lines 221 and 328 call self.context.resolve_writable_path() which is undefined. This method needs to be implemented in FileContext to guard against writes to identity and memory paths per the coding guidelines.
Additionally, FileReadTool at line 221 should not use a "writable" path resolver for read operations—reads don't require write permissions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tools/file.rs` at line 221, The code calls a non-existent
resolve_writable_path and also uses it incorrectly in FileReadTool; add a new
method FileContext::resolve_writable_path that mirrors resolve_path but
additionally checks and forbids writing to protected paths (identity and memory)
and returns the same Result<PathBuf, Error> type, and update the read-only tool
FileReadTool to call self.context.resolve_path(&args.path)? instead of
resolve_writable_path; reference FileContext, resolve_path,
resolve_writable_path, and FileReadTool when making these changes.
Summary
Discord channels have topic descriptions that carry meaningful context for the agent. Currently only server name and channel name are passed through — the topic is discarded.
This PR threads the channel topic through to the agent's conversation context when available.
Changes
CHANNEL_TOPICmetadata key constant (src/lib.rs)guild_channel.topicfrom Discord API (src/messaging/discord.rs)channel_topicparameter torender_conversation_context()(src/prompts/engine.rs)src/agent/channel.rs,src/api/channels.rs)prompts/en/fragments/conversation_context.md.j2)Example
When no topic is set, nothing additional is rendered.
Testing