agents icon indicating copy to clipboard operation
agents copied to clipboard

Fix: Data components render without page refresh in Try It mode

Open amikofalvy opened this issue 1 month ago • 3 comments

Summary

Fixed the issue where data components created during "TRY IT" mode wouldn't render without a page refresh.

Changes

1. Agent Component ()

  • Modified to read , , and from the agent store instead of just using static props
  • Added fallback logic to use props for initial load, but prefer store values when available
  • Now when updates the store (which already happens in the copilot), all components using these lookups get the fresh data

2. Playground Component ()

  • Added hook that listens for ikp-data-operation events (same pattern as copilot chat)
  • When a tool result operation is detected for the current conversation, it refetches data components and updates the store
  • Ensures the Playground chat gets the updated component registry immediately after creation

How It Works

The copilot chat already had a refreshAgentGraph mechanism that fetches and updates all lookups in the store. By making the Agent component read from the store instead of just the props, and adding a similar event listener in the Playground, we ensure data components are dynamically updated without requiring a page refresh.

Result

Data components created during "TRY IT" mode now render immediately without needing to refresh the page! ✨

amikofalvy avatar Nov 20 '25 17:11 amikofalvy

⚠️ No Changeset found

Latest commit: 0374fa41ad1e1631708e690ccd5aca5a61fb36e6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

changeset-bot[bot] avatar Nov 20 '25 17:11 changeset-bot[bot]

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

Project Deployment Preview Comments Updated (UTC)
agents-manage-api Ready Ready Preview Comment Nov 20, 2025 5:53pm
agents-manage-ui Ready Ready Preview Comment Nov 20, 2025 5:53pm
agents-run-api Ready Ready Preview Comment Nov 20, 2025 5:53pm
1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
agents-docs Skipped Skipped Nov 20, 2025 5:53pm

vercel[bot] avatar Nov 20 '25 17:11 vercel[bot]

Claude finished @amikofalvy's task —— View job


PR Review: Data Components Render Fix

I've conducted a comprehensive review of this PR that addresses data components not rendering without page refresh in "TRY IT" mode. Here's my detailed analysis:

Summary of Changes

This PR successfully implements a solution to synchronize data components across the Agent and Playground views when new components are generated during runtime. The core strategy involves:

  1. Store-First Architecture: The Agent component now reads from the Zustand store instead of relying solely on props
  2. Event-Driven Updates: Both Copilot and Playground listen for ikp-data-operation events and refresh component lookups
  3. Synchronous API: Added a new sync endpoint for component generation to support MCP integration
  4. Props Validation: Enhanced validation to ensure all JSON Schema properties have descriptions for LLM compatibility

Technical Implementation Review

Strengths

  1. Clean Architecture Pattern: The fallback logic between store and props (Object.keys(storeDataComponentLookup).length > 0 ? storeDataComponentLookup : dataComponentLookup) is well-designed and maintains backward compatibility.

  2. Consistent Event Handling: The ikp-data-operation event pattern is already established in the copilot and now properly extended to the playground.

  3. Proper State Management: The use of Zustand store for shared state between components follows React best practices.

  4. Generated Code Quality: The MCP package updates appear well-structured and follow OpenAPI specifications correctly.

  5. Comprehensive Validation: The props validation enhancement ensures LLM compatibility by requiring descriptions on all schema properties.

🔍 Areas for Consideration

System Design & Architecture

  1. Event Naming Consistency: The event name ikp-data-operation could be more specific. Consider ikp-data-component-updated or similar to better describe the specific operation being broadcast.

  2. Race Conditions: In the Playground component (playground.tsx:55-73), there's a potential race condition where the event listener checks conversationId in the closure. If the conversation changes rapidly, updates might apply to the wrong conversation.

    // Current implementation
    if (event.detail.type === 'tool_result' && event.detail.conversationId === conversationId) {
    

    Recommendation: Use a ref to ensure the latest conversationId is always checked.

  3. Error Handling: The refetchDataComponents function in Playground only logs errors to console. Consider propagating errors to the user through toast notifications or error states.

Performance & Scalability

  1. Multiple Event Listeners: The same event (ikp-data-operation) is being listened to by multiple components (Agent, Copilot, Playground). While functional, this could become inefficient with many components. Consider a centralized event dispatcher pattern.

  2. Unnecessary Refetches: The Playground refetches data components on every tool_result event, even if no data components were actually modified. Consider adding more specific event types or checking if the operation actually involved data components.

Code Quality & Maintainability

  1. Magic Numbers: The Agent component uses Object.keys(storeDataComponentLookup).length > 0 pattern repeatedly. This could be abstracted into a utility function like hasStoreData().

  2. Type Safety: In several places, the code uses as any type assertions (e.g., line 402-403 in agent.tsx). Consider defining proper TypeScript interfaces for better type safety.

  3. Component Coupling: The dependency on specific event detail structure (event.detail.type, event.detail.conversationId) creates tight coupling. Consider defining proper event interfaces.

Customer Impact & API Design

  1. Breaking Changes: The addition of required descriptions in props validation is technically a breaking change for existing components without descriptions. Consider a migration strategy or warning period.

  2. API Consistency: The new sync endpoint returns the same schema as the streaming endpoint but synchronously. Ensure both endpoints maintain consistent behavior for edge cases.

Testing & Validation

  1. Missing Test Coverage: The new event listening logic in Playground component doesn't appear to have corresponding test coverage. The existing tests focus on the validation logic but not the integration patterns.

  2. Props Validation Edge Cases: The validation requires descriptions for all properties, but what about nested object properties? The current implementation only checks top-level properties.

Specific Code Suggestions

1. Improve Race Condition Safety in Playground

// playground.tsx
const conversationIdRef = useRef(conversationId);
conversationIdRef.current = conversationId;

useEffect(() => {
  const refetchDataComponents = async (event: any) => {
    if (event.detail.type === 'tool_result' && event.detail.conversationId === conversationIdRef.current) {
      // ... existing logic
    }
  };
  // ... rest of the effect
}, [tenantId, projectId, setDataComponentLookup]); // Remove conversationId from deps

2. Define Proper Event Interfaces

interface IkpDataOperationEvent {
  detail: {
    type: 'tool_result' | 'tool_call' | 'completion';
    conversationId?: string;
    data?: any;
  };
}

3. Abstract Store Check Logic

// utils/store-helpers.ts
export const hasStoreData = (storeLookup: Record<string, any>) => 
  Object.keys(storeLookup).length > 0;

export const getEffectiveLookup = <T>(storeLookup: Record<string, T>, propsLookup: Record<string, T>) =>
  hasStoreData(storeLookup) ? storeLookup : propsLookup;

4. Enhanced Error Handling

// In Playground component
} catch (error) {
  console.error('Failed to refetch data components:', error);
  toast.error('Failed to update data components', {
    description: 'Please try refreshing the page if components appear outdated.'
  });
}

Security & Performance

  • No Security Concerns: The changes don't introduce any security vulnerabilities
  • Performance Impact: Minimal - the event-driven approach is efficient and only triggers when needed
  • Memory Management: Event listeners are properly cleaned up in useEffect return functions

Verdict

This is a solid, well-architected solution that addresses the core problem effectively. The implementation follows established patterns in the codebase and maintains good separation of concerns. The suggested improvements are primarily focused on robustness, maintainability, and future scalability rather than fundamental issues.

Recommended Actions:

  1. Address the race condition in Playground component (High Priority)
  2. Add proper error handling with user feedback (Medium Priority)
  3. Consider the breaking change impact of required descriptions (Medium Priority)
  4. Add test coverage for the new event handling patterns (Low Priority)

The PR successfully achieves its goal and represents quality engineering work that follows the codebase patterns well.


claude[bot] avatar Nov 20 '25 17:11 claude[bot]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs.

If this PR is still relevant:

  • Rebase it on the latest main branch
  • Add a comment explaining its current status
  • Request a review if it's ready

Thank you for your contributions!

github-actions[bot] avatar Dec 25 '25 00:12 github-actions[bot]