-
Notifications
You must be signed in to change notification settings - Fork 162
feat (AI): Feedback system #8614
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
base: main
Are you sure you want to change the base?
Conversation
AdityaHegde
left a comment
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.
Leaving comments for initial round of review.
| // Log the error but still return a response | ||
| s.logger.Warn("failed to analyze feedback", zap.String("session_id", s.id), zap.Error(err)) | ||
| return &UserFeedbackResult{ | ||
| Response: "Thanks for your feedback. I apologize that my response didn't meet your expectations. I'll use this to improve.", |
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.
nit: should we not add the error here?
| this.conversationQuery = createQuery(queryOptionsStore, queryClient); | ||
|
|
||
| // Hydrate feedback state when conversation data loads | ||
| this.conversationQuery.subscribe((query) => { |
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.
nit: need to handle unsub to avoid leaks
| private async startStreaming(request: { | ||
| prompt?: string; | ||
| context?: RuntimeServiceCompleteBody; | ||
| userFeedbackContext?: { |
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.
Why not use V1UserFeedbackContext?
| DeveloperAgentContext developer_agent_context = 12; | ||
| // Optional feedback context. If provided, the router_agent calls the user_feedback tool, | ||
| // which records feedback and, for negative feedback, attributes the cause. | ||
| UserFeedbackContext user_feedback_context = 13; |
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.
nit: missing param in CompleteRequest and its handling in chat.go
| return nil, fmt.Errorf("agent %q not found", args.Agent) | ||
| } | ||
| // User feedback | ||
| case args.UserFeedbackArgs != nil: |
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.
Whats wrong with using Agent arg instead? If there is a legitimate reason please add a comment somewhere.
| ...context, | ||
| prompt: request.prompt, | ||
| // Don't send agent for feedback - let router handle user_feedback directly | ||
| agent: request.userFeedbackContext ? undefined : this.agent, |
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.
Same here, agent is for routing directly. So we could just send ToolName.FeedbackAgent
This PR adds thumbs-up/thumbs-down buttons to the AI chat. Users can submit positive or negative feedback. Negative feedback is analyzed by the LLM to attribute the issue to one of:
rill: the AI made an error or misunderstood the questionproject: the project is missing data or has incomplete documentationuser: the question was ambiguous or outside the AI's scopeFor project and user attributions, users receive a suggested action to help them get better results.
Implements this tech design. Closes APP-555.
Checklist: