feat: Add Langfuse user feedback for ChatAgent#24
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Code Review
This pull request integrates Langfuse for tracing and user feedback. It introduces a LangfuseTracingPlugin on the backend to attach trace IDs to events and updates the frontend to capture these IDs and submit feedback scores. Feedback focuses on optimizing LangfuseWeb instantiation to avoid per-message overhead, using stable IDs for feedback scores to prevent duplicate entries in Langfuse, and ensuring trace IDs are captured across all branches of the streaming response logic.
This PR implements the following changes: - In Python ADK, adds `LangfuseTracingPlugin` to stamp every emitted event with `langfuse_trace_id` in its `customMetadata`. - Registers the plugin in `adk/cofacts_ai/agent.py` by wrapping `ai_writer` into an `App`. - Updates `src/lib/adk.ts` to include `langfuseTraceId` in `ChatMessage` interface. - Updates `src/lib/chatCache.ts` to apply the metadata from SSE stream to `ChatMessage.langfuseTraceId`. - Integrates `LangfuseWeb` SDK into `src/components/AgentMessage.tsx` to handle thumb up/down feedbacks, emitting `langfuse.score()` calls. - Adds `VITE_LANGFUSE_PUBLIC_KEY` and `VITE_LANGFUSE_BASE_URL` to `.env` and `.env.example`. Co-authored-by: MrOrz <108608+MrOrz@users.noreply.github.com>
Moves the initialization of the LangfuseWeb client outside of the `AgentMessage` React component to ensure it's instantiated only once at the module level. Co-authored-by: MrOrz <108608+MrOrz@users.noreply.github.com>
2c5563b to
dbb5230
Compare
- Add VITE_LANGFUSE_PUBLIC_KEY and VITE_LANGFUSE_BASE_URL to Dockerfile as build arguments. - Pass secrets.LANGFUSE_PUBLIC_KEY and hardcoded base URL to docker build in deploy workflow. - Update .env.example with correct base URL. - Update README.md to instruct users on configuring the frontend .env file. Co-authored-by: Antigravity <antigravity@google.com>
The ADK AgentLoader expects an App instance to be named 'app' in the agent module. Co-authored-by: Antigravity <antigravity@google.com>
…LEAN Co-authored-by: Antigravity <antigravity@google.com>
- Move fullText calculation into the onClick handler - Remove unused useMemo import - Fix langfuseTraceId check to use undefined check Co-authored-by: Antigravity <antigravity@google.com>
- Extract user feedback thumbs up/down into an independent FeedbackButtons component. - Move rendering logic to ChatArea to display feedback buttons only once per langfuseTraceId. - Filter out function responses in chatCache to maintain correct message sequence for feedback rendering. - Remove redundant copy buttons and Langfuse dependencies from AgentMessage. Co-authored-by: Antigravity <antigravity@gemini.google.com>
- Add showAvatar prop to AgentMessage to conditionally show header - Calculate showAvatar in ChatArea based on previous message author - Reduce space-y in AgentMessage content - Remove default space-y in ChatArea messages container Co-authored-by: Antigravity <antigravity@google.com>
… ID persistence Co-authored-by: Antigravity <antigravity@google.com>
…eedback score Co-authored-by: Antigravity <antigravity@google.com>
|
@gemini-cli /review |
|
🤖 Hi @MrOrz, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
|
@gemini-cli /review |
|
🤖 Hi @MrOrz, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
There was a problem hiding this comment.
This Pull Request introduces Langfuse user feedback (thumbs up/down) and improves the visual grouping of consecutive assistant messages. While the core feedback logic and Langfuse integration are sound, there are significant UI layout regressions and functional losses that should be addressed.
🔍 General Feedback
- UI Layout: Removing the global
space-yinChatAreacauses all message bubbles to touch vertically, which makes the conversation hard to read. - Functionality: The "Copy to clipboard" button for agent responses was removed. This is a highly useful feature and should ideally be retained.
- Feedback Visibility: Feedback buttons are hidden for the entire chat history whenever a new message is being generated, which may be overly restrictive.
- Date Separator: The removal of the date separator means there is no longer any time-based context in the chat history.
| - Next message has different trace id or doesn't exist | ||
| */ | ||
| msg.langfuseTraceId && | ||
| !isStreaming && |
There was a problem hiding this comment.
🟡 The check !isStreaming globally hides the FeedbackButtons for all messages whenever the agent is generating any response.
This prevents users from giving feedback on previous messages while a new one is being generated. Consider using the individual message's streaming state or checking if only the current trace is active.
| !isStreaming && | |
| msg.langfuseTraceId && | |
| !msg.isStreaming && | |
| (!nextMsg?.langfuseTraceId || msg.langfuseTraceId !== nextMsg.langfuseTraceId) && | |
| <FeedbackButtons traceId={msg.langfuseTraceId} /> |
There was a problem hiding this comment.
This is legitimate. @jules please use msg.isStreaming instead of isStreaming here
There was a problem hiding this comment.
I have extracted the Langfuse feedback into a FeedbackWidget and correctly placed it in ChatArea.tsx using msg.isStreaming as requested.
There was a problem hiding this comment.
@claude please use msg.isStreaming instead of isStreaming here
There was a problem hiding this comment.
Claude finished @MrOrz's task in 46s —— View job
Fix applied: use msg.isStreaming instead of isStreaming
- Read
src/components/ChatArea.tsxto locate theisStreamingusage - Replace
isStreamingwithmsg.isStreamingin the feedback condition - Commit and push
Changed src/components/ChatArea.tsx line 54 from !isStreaming to !msg.isStreaming. Feedback buttons for previous messages are now shown even while a new message is still streaming.
08bd4c4 to
b860347
Compare
…isibility Only hide feedback buttons for the specific message that is streaming, not globally for all messages. Co-authored-by: Johnson Liang <MrOrz@users.noreply.github.com>
Goal
Improve the user feedback experience by ensuring feedback buttons are correctly associated with model traces and cleaning up the chat UI for better readability.
feedback.mp4
Technical Choices & Rationale
1. Centralized Feedback Logic in
ChatAreaFeedbackButtonsfrom individualAgentMessagecomponents into theChatArearendering loop.ChatArea, we can:langfuseTraceIdbetween consecutive messages.2. Message State & Internal Filtering
applyEventToStateto filter outfunctionResponseparts and storelangfuseTraceIddirectly in the message object.functionResponsemessages are used for agent orchestration and do not contribute to the user-facing UI. Removing them from themessagesarray prevents "empty" bubbles and ensures message index-based logic (like feedback placement) remains accurate.3. UI Consolidation for Consecutive Messages
showAvatarprop toAgentMessageand implemented logic inChatAreato only show the header (Avatar + Agent Name) for the first message in a consecutive sequence.4. Numeric Feedback Scores
+1for Thumbs Up,-1for Thumbs Down) instead of booleans.Verification Results
+1or-1score to the associated Langfuse trace ID.Co-authored-by: Antigravity antigravity@google.com