-
Notifications
You must be signed in to change notification settings - Fork 234
chore(compass-assistant): add feedback submission and telemetry COMPASS-9608 #7243
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
* @category Gen AI | ||
*/ | ||
type AssistantPromptSubmittedEvent = CommonEvent<{ | ||
name: 'Assistant Prompt Submitted'; |
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.
I originally wanted to combine these with "AI Prompt...
equivalent but I think they diverge enough to be separate. Open to ideas though
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.
also open to ideas on what other props to include
} | ||
const { rating } = state; | ||
const textFeedback = 'feedback' in state ? state.feedback : undefined; | ||
const feedback: 'positive' | 'negative' = |
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.
keeping it this way to align with our NLQ AI Prompt feedback wording
chat, | ||
onError: (error) => { | ||
track('Assistant Response Failed', () => ({ | ||
error_name: error.name, |
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.
I think error name should be safe from any sensitive info right?
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.
In my experience I don't think there are error names. The errors I saw from inside ai sdk were from new Error('something')
. But I suppose some third party dep might be doing more?
Otherwise this is just:
> (new Error('')).name
'Error'
which is probably fine, but not super helpful either.
(This is probably more a limitation of vercel's ai libraries that anything.)
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 adds telemetry tracking and feedback buttons to the MongoDB Compass assistant feature. The changes enable tracking of user interactions with the assistant and collecting user feedback on responses.
- Adds four new telemetry events for tracking assistant usage, prompts, feedback, and errors
- Implements feedback buttons on assistant messages to collect user ratings and text feedback
- Tracks entry point usage when assistant is opened from explain plan or connection error flows
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
packages/compass-telemetry/src/telemetry-events.ts | Defines new telemetry event types for assistant interactions |
packages/compass-assistant/src/compass-assistant-provider.tsx | Adds telemetry tracking for assistant entry points |
packages/compass-assistant/src/assistant-chat.tsx | Implements feedback buttons and tracks user prompts and errors |
packages/compass-assistant/src/assistant-chat.spec.tsx | Adds comprehensive tests for feedback functionality |
packages/compass-assistant/package.json | Adds compass-telemetry dependency |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
73669f9
to
f5a9683
Compare
|
||
track('Assistant Feedback Submitted', { | ||
feedback, | ||
text: textFeedback, |
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.
Maybe if this is a dev build we can just safely include the prompt and the message?
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.
We can probably also do that in some follow-up. Let's get this merged.
import type { AssistantMessage } from './compass-assistant-provider'; | ||
|
||
describe('AssistantChat', function () { | ||
let originalScrollTo: typeof Element.prototype.scrollTo; |
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.
driveby?
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.
yeah this was meant to be removed earlier just ended up staying
Adds telemetry events and feedback buttons to the assistant
