Per-message user feedback mechanism #509
Conversation
This comment has been minimized.
This comment has been minimized.
- Introduced a new langfuseClient module to encapsulate LangfuseWeb client initialization using only the public key. - Updated the feedback route to build context from the session and send feedback directly to Langfuse using the new SDK, eliminating the need for secret keys. - Simplified the feedback submission process by removing unnecessary payload preparation and API call logic. This change enhances security and simplifies the feedback submission process.
- Updated button and checkbox components to include a cursor pointer style for better user interaction feedback. - This change improves the overall usability and accessibility of the UI elements.
- Enhanced FeedbackButtons to include cursor pointer style for better interaction. - Simplified FeedbackModal by removing unnecessary fragments and updating text for clarity. - Adjusted placeholder text and button labels for a more intuitive user experience. These changes aim to improve usability and accessibility in the feedback submission process.
- Replaced the AlertTriangle icon with Info for a clearer privacy indication. - Updated the label from "Include full transcript" to "Include previous messages" for better understanding. - Simplified the privacy disclaimer text to clarify how feedback and messages will be stored. These changes aim to improve user comprehension and enhance the overall feedback experience.
- Updated the feedback comment construction to focus on user-provided content. - Introduced a metadata object to encapsulate structured session information (project, session, user). - Simplified the feedback submission process by adjusting how comments and metadata are sent to the LangfuseWeb SDK. These changes enhance the clarity and organization of feedback data being sent, improving overall feedback management.
- Added an optional 'workflow' field to the feedback API request type for improved context. - Updated the feedback submission process to include the workflow in the metadata sent to Langfuse. - Modified the FeedbackModal to conditionally include the active workflow in the feedback context. These changes aim to provide better tracking of user feedback related to specific workflows, enhancing the overall feedback management experience.
…ckModal for improved clarity and completeness.
Claude Code ReviewSummaryThis PR implements a user feedback mechanism with thumbs up/down buttons on agent messages, allowing users to provide feedback that gets sent to Langfuse. The implementation is clean and follows most frontend patterns, but there are several critical security and privacy issues that must be addressed before merge, plus some architectural concerns. Issues by Severity🚫 Blocker Issues1. CRITICAL: Message Content Privacy Violation
2. CRITICAL: Missing Secret Key in Langfuse Client
3. CRITICAL: No Authentication/Authorization on Feedback API
// Add authentication check
const session = await getServerSession(); // or your auth method
if (!session) {
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 });
}🔴 Critical Issues4. Type Safety Violation -
5. Missing Error Handling in Feedback Submission
} catch (error) {
console.error('Error submitting feedback:', error);
if (error instanceof LangfuseError) {
return NextResponse.json(
{ error: 'Failed to record feedback', details: error.message },
{ status: 502 }
);
}
return NextResponse.json(
{ error: 'Internal server error' },
{ status: 500 }
);
}6. Missing
🟡 Major Issues7. FeedbackContext Performance Concern
const messagesRef = useRef(messages);
useEffect(() => {
messagesRef.current = messages;
}, [messages]);8. Environment Variables Not Validated
9. Missing Loading States in FeedbackButtons
10. No Success Toast/Notification
🔵 Minor Issues11. Inconsistent Error Message Formatting
12. Unused
13. Magic Numbers in Code
14. Missing JSDoc Comments
15. Cursor CSS Rule Duplication
Positive Highlights✅ Excellent UX Design: The inline feedback buttons with visual confirmation (check mark) are intuitive and non-intrusive ✅ Good Use of Shadcn Components: Properly uses Dialog, Button, Textarea, Checkbox from the UI library ✅ Privacy Notice: Good job including the privacy disclaimer in the modal ✅ Context Provider Pattern: Clean separation of concerns with FeedbackContext ✅ Graceful Degradation: Components check for context availability and render nothing if unavailable ✅ Type Safety (mostly): Good use of TypeScript types for FeedbackRequest, FeedbackType, etc. RecommendationsPriority 1 (Must Fix Before Merge)
Priority 2 (Should Fix Before Merge)
Priority 3 (Nice to Have)
Testing RecommendationsBefore merge, manually test:
References
Overall Assessment: This is a well-designed feature with good UX, but has critical security and privacy issues that must be fixed before merge. The transcript feature in particular needs serious reconsideration given the platform's privacy-first approach to Langfuse integration. 🔍 View AI decision process (logs available for 90 days) 📋 View memory system files loaded (click to expand)What Amber Loaded for Code ReviewAmber automatically loaded these repository standards from the memory system:
Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines. |
…enhance security and simplify environment variable management.
…streamline environment variable configuration.
Claude Code ReviewSummaryThis PR implements a user feedback mechanism for messages in the platform, allowing users to provide thumbs up/down feedback on agent responses with optional comments and full transcript inclusion. The implementation integrates with Langfuse for feedback collection. Overall Assessment: This is a well-structured implementation with good UX design and privacy considerations. However, there are several critical security and architecture issues that must be addressed before merging. Issues by Severity🚫 Blocker Issues1. Secret Key Exposure in Frontend Deployment (components/manifests/overlays/local-dev/frontend-patch.yaml:41-46)
2. Missing Secret Key Protection in API Route (components/frontend/src/lib/langfuseClient.ts:19-24)
3. No Error Handling for Langfuse SDK Failures (components/frontend/src/app/api/feedback/route.ts:108-114)
🔴 Critical Issues4. Type Safety Violation - Any Type Usage (components/frontend/src/components/feedback/FeedbackModal.tsx:95-110)
5. Missing Authentication/Authorization (components/frontend/src/app/api/feedback/route.ts:37-60)
6. Potential PII/Sensitive Data Exposure (components/frontend/src/app/api/feedback/route.ts:84-88)
7. No Request Rate Limiting (components/frontend/src/app/api/feedback/route.ts)
🟡 Major Issues8. Message State Not Persisted (components/frontend/src/components/feedback/FeedbackButtons.tsx:28)
9. Inconsistent Type Definitions (components/frontend/src/app/api/feedback/route.ts:24-35)
10. Missing Loading States (components/frontend/src/components/feedback/FeedbackButtons.tsx:61-82)
11. Error State Not Cleared on Modal Close (components/frontend/src/components/feedback/FeedbackModal.tsx:129-134)
12. Accessibility - Missing ARIA Live Region (components/frontend/src/components/feedback/FeedbackModal.tsx:210-215)
13. Environment Variable Documentation Incomplete (components/frontend/.env.example:34-40)
🔵 Minor Issues14. Unused Import in FeedbackButtons (components/frontend/src/components/feedback/FeedbackButtons.tsx:18)
15. Magic Numbers in UI (components/frontend/src/components/feedback/FeedbackButtons.tsx:65-66)
16. Missing JSDoc Comments (All new files)
17. Console.warn Instead of Proper Logging (components/frontend/src/lib/langfuseClient.ts:23, route.ts:66)
18. Button className Pattern Inconsistency (components/frontend/src/components/ui/button.tsx:10)
19. Checkbox className Pattern Inconsistency (components/frontend/src/components/ui/checkbox.tsx:16)
20. Hardcoded API Endpoint (components/frontend/src/components/feedback/FeedbackModal.tsx:95)
Positive Highlights✅ Excellent UX Design: The feedback buttons are well-designed with:
✅ Good Privacy Considerations:
✅ Proper Component Structure:
✅ TypeScript Type Safety (mostly):
✅ Graceful Degradation:
✅ Follows Shadcn UI Patterns:
RecommendationsImmediate Actions (Before Merge)
High Priority (Should Address)
Nice to Have (Future Improvements)
Architecture DiscussionQuestion: Should feedback collection be a frontend API route or a backend endpoint? Current: Frontend Next.js API route ( Pros of Backend Approach:
Cons of Backend Approach:
Recommendation: Consider moving to backend in future iteration for better security and consistency. Pre-Commit Checklist StatusBased on frontend checklist (CLAUDE.md:1122-1136):
Checklist Score: 4/7 applicable items passed Security Checklist StatusBased on security standards (security-standards.md:206-241): Authentication:
Authorization:
Secrets & Tokens:
Input Validation:
Container Security:
Security Score: 2/6 items passed Final RecommendationStatus: This is a well-designed feature with good UX, but has critical security issues that must be addressed:
Once these issues are resolved, this will be a solid addition to the platform. The core idea and implementation approach are sound. Estimated Effort to Fix: 4-6 hours
Note: This is a WIP PR as indicated by the title. The above issues should be addressed before removing the WIP status. 🔍 View AI decision process (logs available for 90 days) 📋 View memory system files loaded (click to expand)What Amber Loaded for Code ReviewAmber automatically loaded these repository standards from the memory system:
Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines. |
Claude Code ReviewSummaryThis PR implements a per-message user feedback mechanism with thumbs up/down buttons and a modal for detailed feedback submission to Langfuse. The implementation follows frontend patterns well but has several security, error handling, and architecture issues that need to be addressed. Overall Assessment: Good UX implementation with clean component design, but requires security hardening, error handling improvements, and better API client patterns before merging. Issues by Severity🚫 Blocker Issues1. Missing Authentication/Authorization on API Route (Security Critical)
2. Manual
🔴 Critical Issues3. No Error Logging in API Route (Observability)
console.error('Error submitting feedback:', {
error: error instanceof Error ? error.message : String(error),
username,
projectName,
sessionName,
traceId: effectiveTraceId,
});4. Langfuse Client Singleton Pattern Issues (Architecture)
export function getLangfuseClient(): LangfuseWeb | null {
const publicKey = process.env.LANGFUSE_PUBLIC_KEY;
const host = process.env.LANGFUSE_HOST;
if (\!publicKey || \!host) {
return null;
}
// Create fresh instance (Langfuse SDK is lightweight)
return new LangfuseWeb({ publicKey, baseUrl: host });
}5. Missing Input Validation on Transcript Data (Security)
if (includeTranscript && Array.isArray(transcript)) {
const validTranscript = transcript.filter(
m => m && typeof m === 'object' &&
typeof m.role === 'string' &&
typeof m.content === 'string'
);
// ... process validTranscript
}🟡 Major Issues6. Context Provider Re-renders on Every Message (Performance)
// Option 1: Don't memoize messages, access directly when needed
const value = useMemo(
() => ({
projectName,
sessionName,
username,
initialPrompt,
activeWorkflow,
traceId,
getMessages: () => messages, // Function reference is stable
}),
[projectName, sessionName, username, initialPrompt, activeWorkflow, traceId]
);7.
8. Missing Loading State During Feedback Submission (UX)
const controller = new AbortController();
const timeoutId = setTimeout(() => controller.abort(), 10000);
try {
const response = await fetch('/api/feedback', {
method: 'POST',
signal: controller.signal,
// ...
});
clearTimeout(timeoutId);
// ...
} catch (err) {
clearTimeout(timeoutId);
if (err.name === 'AbortError') {
setError('Request timed out. Please try again.');
}
// ...
}9. Environment Variables Not Documented in Deployment Manifests (DevOps)
🔵 Minor Issues10. Inconsistent Button Styling (UI Consistency)
11. Type Import Style Inconsistency (Code Style)
12. Missing JSDoc Comments on Public API (Documentation)
/**
* Feedback buttons component that displays thumbs up/down for user feedback.
* Requires FeedbackProvider in parent tree.
*
* @param messageContent - The message content to provide context for feedback
* @param messageTimestamp - Timestamp of the message
*/
export function FeedbackButtons({ ... }) { ... }13. Hardcoded Strings Should Be Constants (Maintainability)
const FEEDBACK_SCORE_NAME = 'user-feedback' as const;
langfuse.score({
// ...
name: FEEDBACK_SCORE_NAME,
// ...
});14. FeedbackContext TypeScript Type Could Use
export type FeedbackContextValue = {
projectName: string;
// ... (already correct\!)
};Note: This is already correct, no action needed Positive Highlights✅ Excellent Component Composition - Clean separation between FeedbackButtons, FeedbackModal, and FeedbackContext ✅ Good UX Design - Tooltips, loading states, visual feedback on submission, and privacy disclaimer ✅ Proper Use of Shadcn UI - Dialog, Button, Textarea, Checkbox all correctly used ✅ Accessibility - aria-labels on buttons, proper semantic HTML ✅ Privacy-Conscious - Clear opt-in for transcript inclusion with privacy notice ✅ Type Safety - Good use of TypeScript types ( ✅ Error Handling in UI - Error state displayed to users in modal ✅ Follows Submission Pattern - Disable submit button, show spinner, handle success/error RecommendationsPriority 1 (Blocker - Must Fix Before Merge)
Priority 2 (Critical - Should Fix Before Merge)
Priority 3 (Major - Address Soon)
Priority 4 (Minor - Nice to Have)
Testing RecommendationsBefore merging:
Architecture NotesThis PR introduces a new integration point with Langfuse for user feedback. Consider:
Estimated effort to address blockers: 2-3 hours Great work on the UX! The feedback mechanism looks polished. Focus on the security and architecture issues before merging, and this will be a solid addition to the platform. 🔍 View AI decision process (logs available for 90 days) 📋 View memory system files loaded (click to expand)What Amber Loaded for Code ReviewAmber automatically loaded these repository standards from the memory system:
Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines. |
Claude Code ReviewSummaryThis PR implements a per-message user feedback mechanism with thumbs up/down buttons and a detailed feedback modal. The implementation integrates with Langfuse for tracking feedback scores. Overall Assessment: Good implementation with solid UX patterns, but has several critical security and architectural issues that must be addressed before merge. Issues by SeverityBlocker Issues1. CRITICAL: Missing Input Validation (route.ts:54-60)
2. CRITICAL: Missing Error Context Logging (route.ts:122-128)
3. CRITICAL: Environment Variable Documentation Gap (langfuseClient.ts:19-20)
Critical Issues4. Architectural: No React Query Integration (FeedbackModal.tsx:95-110)
5. Missing Loading States (FeedbackButtons.tsx:62-81)
Major Issues6. Error Handling: Silent Langfuse Failures (route.ts:65-71)
7. Privacy Concern: Full Transcript Inclusion (FeedbackModal.tsx:91-93)
Positive Highlights✅ Excellent UX Patterns - Smooth feedback flow, clear visual feedback, disabled state Final VerdictCritical issues that must be fixed before merge:
Estimated Time to Fix: 2-4 hours for blockers + critical issues 📋 Next Steps: Please address blocker issues #1-3 and critical issue #4, then request re-review. 🔍 View AI decision process (logs available for 90 days) 📋 View memory system files loaded (click to expand)What Amber Loaded for Code ReviewAmber automatically loaded these repository standards from the memory system:
Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines. |
- Added a `sanitizeString` function to clean user inputs by removing control characters and normalizing line endings. - Updated the feedback submission process to sanitize various fields including username, project name, session name, comment, workflow, context, and trace ID. - Ensured transcript entries are also sanitized before processing. These changes enhance security by mitigating the risk of log injection attacks, improving the overall integrity of the feedback data submitted.
Claude Code ReviewSummaryThis PR implements a user feedback mechanism for messages using Langfuse's Scores API. The implementation adds thumbs up/down buttons for agent messages with an optional detailed feedback modal. Overall implementation is clean with good security practices (input sanitization, proper secret handling). However, there are several important issues to address before merging. Issues by Severity🚫 Blocker Issues1. Frontend Design Guidelines Violation -
🔴 Critical Issues2. Missing Type Definitions for
3. Security - LangfuseWeb Client Not Properly Scoped
4. Type Safety - Direct Type Assertions Without Checking
🟡 Major Issues5. Missing React Query Integration
6. Error Handling Pattern Inconsistency
7. Missing Loading States in Shadcn Components
8. Accessibility - Missing ARIA Labels
🔵 Minor Issues9. Input Sanitization Could Be More Robust
10. Console Warnings for Missing Config
11. TypeScript - Prefer
12. Component Size
Positive Highlights✅ Excellent Security Implementation
✅ Good UX Design
✅ Clean Code Organization
✅ Shadcn UI Usage
✅ Good Comments and Documentation
RecommendationsPriority 1 (Must Fix Before Merge)
Priority 2 (Should Fix Before Merge)
Priority 3 (Nice to Have)
Testing RecommendationsAdd tests for:
Architecture Compliance
Overall Assessment: Good implementation with strong security practices, but needs to address the 4 critical issues and align with React Query patterns before merging. The core functionality is solid and the UX is well thought out. Estimated effort to fix blockers + critical issues: 2-3 hours 🔍 View AI decision process (logs available for 90 days) 📋 View memory system files loaded (click to expand)What Amber Loaded for Code ReviewAmber automatically loaded these repository standards from the memory system:
Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines. |
andybraren
changed the title
1 participant
Implementation of a basic user feedback mechanism for messages using Langfuse's Scores API.
This adds thumbs up/down buttons below messages from agents and a modal for the user to provide more details before submitting their feedback.
The project, session ID, username, and selected workflow get stored in the Metadata field, and the user's comment and selected chat message get stored in the Comment field. If the user selects "Include all messages" the full transcript is included in the Comment field as well.
Future enhancements
Screenshots