Skip to content

Conversation

@dimaMachina
Copy link
Collaborator

@dimaMachina dimaMachina commented Jan 19, 2026

@sarah-inkeep why we need them?

removed

I added background: 'red' for default marker, and it's rendered in

image

I added background: 'green' for selected marker, and it's rendered in

image

dark

image

light

image

@vercel
Copy link

vercel bot commented Jan 19, 2026

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

Project Deployment Review Updated (UTC)
agents-eval-api Ready Ready Preview, Comment Jan 20, 2026 5:41pm
agents-manage-api Ready Ready Preview, Comment Jan 20, 2026 5:41pm
agents-manage-ui Ready Ready Preview, Comment Jan 20, 2026 5:41pm
agents-run-api Ready Ready Preview, Comment Jan 20, 2026 5:41pm
1 Skipped Deployment
Project Deployment Review Updated (UTC)
agents-docs Skipped Skipped Jan 20, 2026 5:41pm

Request Review

@vercel vercel bot temporarily deployed to Preview – agents-docs January 19, 2026 12:16 Inactive
@changeset-bot
Copy link

changeset-bot bot commented Jan 19, 2026

🦋 Changeset detected

Latest commit: 5b14429

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

This PR includes changesets to release 10 packages
Name Type
@inkeep/agents-manage-ui Patch
@inkeep/agents-manage-api Patch
@inkeep/agents-run-api Patch
@inkeep/agents-eval-api Patch
@inkeep/agents-cli Patch
@inkeep/agents-core 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

@claude
Copy link

claude bot commented Jan 19, 2026

Claude finished @dimaMachina's task —— View job


Code Review - Issue Identified ⚠️

After thoroughly analyzing this PR, I've identified a critical issue that will cause visual bugs in the React Flow diagram.

Summary of Changes

This PR removes two SVG marker components (DefaultMarker and SelectedMarker) and their imports from the main agent page. These components defined SVG arrowhead markers for React Flow edges with IDs #marker-default and #marker-selected.

🚨 Critical Issue: Broken SVG References

The removed components are still referenced in two edge components:

  1. agents-manage-ui/src/components/agent/edges/agent-to-agent-edge.tsx:78
const getMarker = (isSelected: boolean) =>
  isSelected ? 'url(#marker-selected)' : 'url(#marker-default)';
  1. agents-manage-ui/src/components/agent/edges/self-loop-edge.tsx:63
const getMarker = (isSelected: boolean) =>
  isSelected ? 'url(#marker-selected)' : 'url(#marker-default)';

Impact Analysis

Visual Impact:

  • Edge arrows will not render correctly (likely no arrowheads will appear)
  • The React Flow diagram will be missing essential visual indicators for relationships between agents
  • Users will not be able to distinguish direction or state of connections

Technical Impact:

  • SVG references to non-existent marker definitions will fail silently
  • No JavaScript errors, but visual functionality is broken
  • Could cause confusion in the visual builder interface

System Design Considerations

1. Incomplete Refactoring:
The removal appears to be part of a cleanup effort, but it's incomplete. Either:

  • The marker components should be restored, OR
  • The references should be updated to use React Flow's built-in markers, OR
  • Custom CSS/SVG should be added to replace the functionality

2. React Flow Integration:
React Flow v12 (used in this codebase) provides built-in marker support, but requires proper configuration. The @xyflow/react/dist/style.css is imported in globals.css, but custom markers still need to be defined.

3. Missing Test Coverage:
This visual regression would not be caught by unit tests but would be immediately visible in the UI. The change lacks visual regression testing consideration.

Recommended Solutions

Option 1 (Recommended): Restore the markers and refactor properly

  • Restore the DefaultMarker and SelectedMarker components
  • Move them to a more appropriate location (e.g., a shared markers directory)
  • Ensure they're properly imported where needed

Option 2: Use React Flow's built-in markers

  • Update the edge components to use React Flow's default markers
  • Modify the CSS classes to style them appropriately
  • Update the getMarker functions accordingly

Option 3: Move markers to CSS

  • Define the markers in CSS/SVG directly in the global styles
  • Keep the same marker IDs to maintain compatibility

Code Quality Assessment

Positive aspects:

  • Clean removal of unused imports
  • Consistent with React 19 cleanup efforts mentioned in commit history

Areas for improvement:

  • Incomplete change that breaks functionality
  • No consideration for dependent components
  • Missing integration testing for visual components

Breaking Change Assessment

This is a breaking change that affects the core visual functionality of the agent builder. While it doesn't cause compilation errors, it significantly impacts user experience.

Recommendation

❌ Do not merge this PR as-is. The functionality needs to be properly addressed before merging.

Next steps:

  1. Choose one of the solutions above
  2. Test the visual output in the agent builder
  3. Consider adding visual regression tests for critical UI components
  4. Update any documentation if marker usage patterns change

Copy link
Contributor

@sarah-inkeep sarah-inkeep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are being used for the little arrows when connecting 2 agents to show which direction that relationship is. It seems there is a bug with the SelectedMarker not showing up though (it is supposed to make the arrow blue when the line is selected), I can look into that.
Screenshot 2026-01-19 at 5 30 46 PM

@dimaMachina dimaMachina reopened this Jan 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants