-
Notifications
You must be signed in to change notification settings - Fork 0
Add LangSmith observability UI for AI operation traces and metrics #83
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
… FE-4) Co-authored-by: kchia <7776562+kchia@users.noreply.github.com>
Co-authored-by: kchia <7776562+kchia@users.noreply.github.com>
Co-authored-by: kchia <7776562+kchia@users.noreply.github.com>
Co-authored-by: kchia <7776562+kchia@users.noreply.github.com>
Co-authored-by: kchia <7776562+kchia@users.noreply.github.com>
|
@claude review this PR |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
|
@claude review this PR |
|
Claude finished @kchia's task —— View job PR Review: LangSmith Observability UII've completed a comprehensive review of this PR implementing frontend observability components for Epic 004. Overall, this is a well-structured implementation that follows project guidelines and delivers the required functionality. Below is my detailed review: ✅ Strengths1. Component Architecture
2. TypeScript & Type Safety
3. Test Coverage
4. Accessibility
5. User Experience
6. Integration
7. Storybook Documentation
🔍 Issues FoundCritical Issues ⛔None! No critical bugs or security issues. High Priority Issues 🟡1. Inconsistent Progress Variant (app/src/components/observability/GenerationMetadataDisplay.tsx:157) <Progress
value={percentage}
variant="default" // ⚠️ Should use variant based on stage/performance
className="h-1"
/>Issue: All stage progress bars use Recommendation: Add visual differentiation: const getStageVariant = (stage: string, percentage: number) => {
if (percentage < 20) return "warning"; // Slow stages
if (stage === "llm_generating") return "default"; // Main stage in blue
return "success"; // Completed stages in green
};2. Environment Variable Not in .env.example Recommendation: Add to Medium Priority Issues 🟠1. Cost Estimation Not Implemented (app/src/app/preview/page.tsx:394) estimated_cost: undefined, // Backend will provide this in futureIssue: Frontend shows "N/A" for cost, but Epic 004 mentions cost tracking as a feature. Recommendation: Either:
2. Missing Fallback Message in Dashboard (app/src/app/page.tsx:372-376) <LangSmithTraceLink ... />
{!metadata.trace_url && (
<p className="text-xs text-muted-foreground italic">
Trace link will appear here when LangSmith is configured
</p>
)}Issue: The preview page has this helpful fallback, but it's inside the Card which only renders when Recommendation: The current implementation is actually correct - the entire observability section only shows when generation completes and metadata exists. This is intentional. 3. Stage Name Formatting (GenerationMetadataDisplay.tsx:151-152) <span className="capitalize text-muted-foreground">
{stage.replace(/_/g, " ")}
</span>Issue: Recommendation: Add proper formatting function: const formatStageName = (stage: string) => {
return stage
.replace(/_/g, " ")
.split(" ")
.map(word => word === "llm" ? word.toUpperCase() : word.charAt(0).toUpperCase() + word.slice(1))
.join(" ");
};Low Priority Issues 🟢1. Storybook Stories Not Checked 2. E2E Test Incomplete (app/e2e/observability.spec.ts)
Recommendation: This is acceptable for MVP. E2E tests should be completed when backend trace URL generation is fully implemented. 3. Missing Error States 📋 Code Quality Checklist
🎯 Epic 004 Requirements VerificationComparing against
🚀 RecommendationsBefore Merge (Optional but Recommended)
After Merge (Future Improvements)
📊 Overall AssessmentVerdict: ✅ APPROVE with minor suggestions This PR successfully implements the frontend requirements for Epic 004 with:
The minor issues mentioned above are non-blocking and can be addressed in follow-up PRs if desired. The current implementation provides solid functionality for displaying LangSmith trace data. Estimated effort to address issues: 30-45 minutes for all medium-priority items. |
Implements frontend display components for LangSmith trace links and AI operation metrics (latency, token usage, cost) as specified in Epic 004.
Components
LangSmithTraceLink - Clickable trace links with tooltips
GenerationMetadataDisplay - AI operation metrics visualization
Integration Points
Preview page (
app/src/app/preview/page.tsx)Dashboard (
app/src/app/page.tsx)Type Changes
Extended
GenerationMetadatainterface:Testing
observability.spec.tsBackend Contract
Expects API responses to include:
metadata.trace_url- LangSmith trace URLmetadata.session_id- Session identifiermetadata.llm_token_usage- Token breakdowntiming.{llm_generating_ms, validating_ms, post_processing_ms}- Stage latenciesFrontend handles missing fields gracefully (shows "N/A" or fallback messages).
Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
fonts.googleapis.com/usr/local/bin/node /home/REDACTED/work/component-forge/component-forge/app/node_modules/next/dist/compiled/jest-worker/processChild.js(dns block)storybook.js.orgnode (vitest)(dns block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.