-
Notifications
You must be signed in to change notification settings - Fork 233
chore(compass-assistant): encourage more search_content calls #7391
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR modifies the Compass Assistant to encourage more frequent usage of the 'search_content' tool and fixes duplicate source titles in the UI. The changes move from conditional to unconditional search_content tool usage and implement deduplication for source links.
- Updated prompts to always call the 'search_content' tool instead of only for technical questions
- Added deduplication logic to prevent showing duplicate source titles in the UI
- Added comprehensive test coverage for the duplicate source handling
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
packages/compass-assistant/src/prompts.ts | Simplified search_content tool usage instruction to always call it |
packages/compass-assistant/src/components/assistant-chat.tsx | Added deduplication logic for source titles using Set tracking |
packages/compass-assistant/src/components/assistant-chat.spec.tsx | Added test case to verify duplicate source title handling |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
const seenTitles = new Set<string>(); | ||
const sources = []; | ||
for (const part of parts) { | ||
if (part.type === 'source-url') { | ||
const title = part.title || 'Documentation Link'; | ||
if (!seenTitles.has(title)) { | ||
seenTitles.add(title); | ||
sources.push({ | ||
children: title, | ||
href: part.url, | ||
variant: 'Docs', | ||
}); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The deduplication logic only considers titles but ignores URLs. This means different URLs with the same title will be filtered out, potentially hiding useful distinct resources. Consider using a combination of title and URL for deduplication, or use the first URL encountered for each title.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that's the point :)
Doesn't seem like this always gets called anyhow so doesn't seem like there's much harm.