-
Notifications
You must be signed in to change notification settings - Fork 237
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
Changes from all commits
147b2d7
f208b6c
2ffcd45
cf928e8
d2563d8
f5a9683
3cdcaa4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ import { | |
LgChatLeafygreenChatProvider, | ||
LgChatMessage, | ||
LgChatMessageFeed, | ||
LgChatMessageActions, | ||
LgChatInputBar, | ||
spacing, | ||
css, | ||
|
@@ -16,11 +17,13 @@ import { | |
palette, | ||
useDarkMode, | ||
} from '@mongodb-js/compass-components'; | ||
import { useTelemetry } from '@mongodb-js/compass-telemetry/provider'; | ||
|
||
const { ChatWindow } = LgChatChatWindow; | ||
const { LeafyGreenChatProvider, Variant } = LgChatLeafygreenChatProvider; | ||
const { Message } = LgChatMessage; | ||
const { MessageFeed } = LgChatMessageFeed; | ||
const { MessageActions } = LgChatMessageActions; | ||
const { InputBar } = LgChatInputBar; | ||
|
||
interface AssistantChatProps { | ||
|
@@ -105,9 +108,15 @@ const errorBannerWrapperStyles = css({ | |
export const AssistantChat: React.FunctionComponent<AssistantChatProps> = ({ | ||
chat, | ||
}) => { | ||
const track = useTelemetry(); | ||
const darkMode = useDarkMode(); | ||
const { messages, sendMessage, status, error, clearError } = useChat({ | ||
chat, | ||
onError: (error) => { | ||
track('Assistant Response Failed', () => ({ | ||
error_name: error.name, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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 Otherwise this is just:
which is probably fine, but not super helpful either. (This is probably more a limitation of vercel's ai libraries that anything.) |
||
})); | ||
}, | ||
}); | ||
|
||
// Transform AI SDK messages to LeafyGreen chat format | ||
|
@@ -127,10 +136,43 @@ export const AssistantChat: React.FunctionComponent<AssistantChatProps> = ({ | |
(messageBody: string) => { | ||
const trimmedMessageBody = messageBody.trim(); | ||
if (trimmedMessageBody) { | ||
track('Assistant Prompt Submitted', { | ||
user_input_length: trimmedMessageBody.length, | ||
}); | ||
void sendMessage({ text: trimmedMessageBody }); | ||
} | ||
}, | ||
[sendMessage] | ||
[sendMessage, track] | ||
); | ||
|
||
const handleFeedback = useCallback( | ||
( | ||
event, | ||
state: | ||
| { | ||
feedback: string; | ||
rating: string; | ||
} | ||
| { | ||
rating: string; | ||
} | ||
| undefined | ||
) => { | ||
if (!state) { | ||
return; | ||
} | ||
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 commentThe 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 |
||
rating === 'liked' ? 'positive' : 'negative'; | ||
|
||
track('Assistant Feedback Submitted', { | ||
feedback, | ||
text: textFeedback, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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. |
||
request_id: null, | ||
}); | ||
}, | ||
[track] | ||
); | ||
|
||
return ( | ||
|
@@ -155,7 +197,14 @@ export const AssistantChat: React.FunctionComponent<AssistantChatProps> = ({ | |
sourceType="markdown" | ||
{...messageFields} | ||
data-testid={`assistant-message-${messageFields.id}`} | ||
/> | ||
> | ||
{messageFields.isSender === false && ( | ||
<MessageActions | ||
onRatingChange={handleFeedback} | ||
onSubmitFeedback={handleFeedback} | ||
/> | ||
)} | ||
</Message> | ||
))} | ||
{status === 'submitted' && ( | ||
<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.
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