mcp tool fail display in traces

[shagun-singh-inkeep]

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: 1b74f659d9fa1b1bb3b794f88c34752f3efb9178

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages

Name	Type
@inkeep/agents-core	Patch
@inkeep/agents-manage-ui	Patch
@inkeep/agents-cli	Patch
@inkeep/agents-manage-api	Patch
@inkeep/agents-run-api	Patch
@inkeep/agents-manage-mcp	Patch
@inkeep/agents-sdk	Patch
@inkeep/ai-sdk-provider	Patch
@inkeep/create-agents	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
agents-manage-api	Ready	Preview	Comment	Dec 2, 2025 2:57am
agents-manage-ui	Ready	Preview	Comment	Dec 2, 2025 2:57am
agents-run-api	Ready	Preview	Comment	Dec 2, 2025 2:57am

1 Skipped Deployment

Project	Deployment	Preview	Comments	Updated (UTC)
agents-docs	Skipped			Dec 2, 2025 2:57am

[claude[bot]]

Claude finished @shagun-singh-inkeep's task —— View job

---

Code Review: MCP Tool Fail Display in Traces

Conducting comprehensive review as TypeScript Staff Engineer and System Architect.

Overview

This PR adds a new "Tool Calls" card to the conversation detail view, displaying MCP tool call statistics with success/failure counts and deep-linking to SigNoz for detailed traces.

Review Progress:

• [x] Read repository guidelines and context
• [x] Analyze modified files and understand changes
• [x] Research existing patterns and related code
• [x] Evaluate engineering quality and architecture
• [x] Assess customer impact and API design
• [x] Provide comprehensive feedback

Architectural & Design Review

✅ Strengths

1. Consistent Pattern Usage: The implementation follows established patterns in the codebase:

   • Uses the same Card structure as other summary cards
   • Follows existing icon and layout conventions
   • Consistent with SigNoz URL generation patterns

2. Good Data Modeling: The status tracking (success/error) maps cleanly to the existing ActivityItem type structure

3. Deep Integration: Leverages existing tracing infrastructure effectively with SigNoz integration

⚠️ Areas for Improvement

High Priority Issues

1. 🔴 Performance Concern - Inline Complex Logic (conversation-detail.tsx:265-343)

   // This 80-line IIFE should be extracted to a custom hook or utility function
   {(() => {
     const toolCalls = conversation?.activities?.filter(/*...*/);
     const mcpTools: Record<string, { success: number; failed: number }> = {};
     // ... complex aggregation logic
   })()}
   

   Impact: Violates React performance best practices and makes testing difficult.

   Solution: Extract to a useMCPToolStats hook or utility function.

2. 🔴 Type Safety Issue - Hard-coded Status Mapping (conversation-detail.tsx:275)

   if (tc.status === 'error') {
     mcpTools[toolName].failed += 1;
   } else {
     mcpTools[toolName].success += 1;
   }
   

   Impact: Assumes all non-error statuses are successes. What about 'pending' status?

   Solution: Explicit status handling with proper type guards.

3. 🔴 Inconsistent Filtering Logic (conversation-detail.tsx:266-268)

   const toolCalls = conversation?.activities?.filter(
     (a: ActivityItem) => a.type === ACTIVITY_TYPES.TOOL_CALL && a.toolType === TOOL_TYPES.MCP
   ) as ActivityItem[];
   

   Impact: The type assertion as ActivityItem[] is unnecessary and potentially misleading.

Medium Priority Issues

4. 🟡 Missing Error Boundaries & Loading States

   • No error handling for SigNoz URL generation failures
   • Missing loading states for the external link navigation

5. 🟡 Hard-coded UI Constraints (conversation-detail.tsx:294)

   <div className="space-y-2 max-h-32 overflow-y-auto">
   

   Impact: Fixed height of max-h-32 might not scale well with varying tool counts.

6. 🟡 URL Generation Inconsistency (signoz-links.ts:114)

   return `${url}/traces-explorer?compositeQuery=${encodedCompositeQuery}&relativeTime=2w`;
   // vs existing function uses relativeTime=1month
   

   Impact: Different time ranges without clear reasoning.

System Design Considerations

Extensibility & Future-Proofing

1. Limited Tool Type Support: Currently hardcoded to MCP tools only. Consider:

   // More flexible approach:
   const supportedToolTypes = [TOOL_TYPES.MCP, TOOL_TYPES.TOOL];
   const toolCalls = activities?.filter(a => 
     a.type === ACTIVITY_TYPES.TOOL_CALL && 
     supportedToolTypes.includes(a.toolType)
   );
   

2. Missing Abstraction: The tool statistics logic should be extracted into reusable utilities for consistency across the application.

Customer Experience Issues

3. 🟡 Accessibility Concerns:

   • External link buttons lack proper aria-labels
   • Color-only differentiation for success/failure states (green/red text)
   • No keyboard navigation consideration for scrollable tool list

4. 🟡 UX Inconsistency:

   • Grid layout changes from lg:grid-cols-3 to lg:grid-cols-4 but skeleton loading still shows 4 items, creating layout shift

Data Integrity & Edge Cases

5. Missing Null Safety:

   const toolName = tc.toolName || 'Unknown Tool';
   // Should validate that toolName is meaningful, not just truthy
   

6. Edge Case Handling: No consideration for:

   • Very long tool names (truncation is good, but no tooltip)
   • Tools with 0 calls
   • Mixed tool types in same conversation

Recommendations

Immediate Actions Needed

1. Extract Complex Logic:

   // Create custom hook
   function useMCPToolStats(activities?: ActivityItem[]) {
     return useMemo(() => {
       const toolCalls = activities?.filter(/*...*/) || [];
       // ... aggregation logic
       return { mcpTools, totalCalls };
     }, [activities]);
   }
   

2. Improve Type Safety:

   const isSuccessfulStatus = (status: ActivityItem['status']): boolean => 
     status === 'success';
   
   if (isSuccessfulStatus(tc.status)) {
     mcpTools[toolName].success += 1;
   } else if (tc.status === 'error') {
     mcpTools[toolName].failed += 1;
   }
   // Handle pending status appropriately
   

3. Add Error Boundaries:

   <Button onClick={() => {
     try {
       window.open(getSignozToolCallUrl(/*...*/), '_blank');
     } catch (error) {
       toast.error('Failed to open SigNoz trace');
     }
   }}>
   

Future Considerations

• Consider adding metrics aggregation at the API level rather than client-side
• Implement caching for tool statistics to improve performance
• Add unit tests for the new utility functions
• Consider adding tool-specific configuration for time ranges

Security & Performance Notes

• window.open usage is appropriate for external links
• Double URI encoding in SigNoz URL generation is correct for the query parameter structure
• Component renders efficiently with proper memoization opportunities

Overall, this is a solid feature addition that enhances observability. The main concerns are around code organization and type safety rather than fundamental architecture issues.

---