Skip to content

Conversation

@icecrasher321
Copy link
Collaborator

Summary

Redrawing edges should not lead to socket ops -- add check to collab function.

Type of Change

  • Bug fix

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link

vercel bot commented Jan 14, 2026

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

1 Skipped Deployment
Project Deployment Review Updated (UTC)
docs Skipped Skipped Jan 14, 2026 2:27am

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 14, 2026

Greptile Summary

This PR consolidates duplicate edge filtering logic introduced in commit 8ccf454 into a reusable filterNewEdges utility function. The refactor prevents redundant socket operations when edges are redrawn in collaborative editing sessions by consistently filtering out edges that already exist (same source, target, and handles) before attempting to add them.

Key Changes:

  • Extracted filterNewEdges function to apps/sim/stores/workflows/utils.ts to centralize logic for detecting duplicate edges and self-referencing edges
  • Refactored batchAddEdges in workflow store to use the new utility instead of inline duplicate checks
  • Updated collaborative workflow handlers (both receiving and sending) to filter edges before processing
  • Removed test case for self-referencing edges since that logic is now in the utility function

Issues Found:

  • Import statements are split across the file in utils.ts (some at top, function definition, then more imports), which violates the import patterns guide requiring all imports at the top

Confidence Score: 4/5

  • This PR is safe to merge with minimal risk after addressing the import ordering style issue
  • Score reflects a solid refactoring that consolidates existing functionality without changing behavior. The logic was already tested in production (from commit 8ccf454), and this PR simply extracts it into a reusable utility. The only issue found is a minor style violation with import ordering that doesn't affect functionality. No logical errors or bugs detected.
  • Pay attention to apps/sim/stores/workflows/utils.ts for the import ordering issue

Important Files Changed

Filename Overview
apps/sim/stores/workflows/utils.ts Adds filterNewEdges utility function to consolidate edge filtering logic; imports split across file violates style guide
apps/sim/stores/workflows/workflow/store.ts Refactors batchAddEdges to use new filterNewEdges utility, simplifies logic by removing duplicate filtering code
apps/sim/hooks/use-collaborative-workflow.ts Refactors collaborative edge operations to use filterNewEdges utility, preventing redundant socket operations when redrawing edges
apps/sim/stores/workflows/workflow/store.test.ts Renames test case and removes self-referencing edge test (logic moved to filterNewEdges)

Sequence Diagram

sequenceDiagram
    participant User as User/UI
    participant Hook as useCollaborativeWorkflow
    participant Filter as filterNewEdges
    participant Store as WorkflowStore
    participant Queue as Operation Queue
    participant Socket as WebSocket

    User->>Hook: collaborativeBatchAddEdges(edges)
    Hook->>Store: get current edges
    Store-->>Hook: currentEdges[]
    Hook->>Filter: filterNewEdges(edges, currentEdges)
    Note over Filter: Filter out:<br/>1. Self-referencing edges<br/>2. Duplicate connections<br/>(same source/target/handles)
    Filter-->>Hook: newEdges[]
    
    alt newEdges.length > 0
        Hook->>Queue: addToQueue(BATCH_ADD_EDGES, newEdges)
        Queue->>Socket: Send to collaborators
        Hook->>Store: batchAddEdges(newEdges)
        Note over Store: Only cycle check needed,<br/>duplicates already filtered
        Store-->>Hook: edges updated
        Hook->>Hook: recordAddEdge (undo/redo)
    else newEdges.length === 0
        Note over Hook: No-op: all edges already exist<br/>Prevents redundant socket ops
    end

    Note over Socket: Collaborator receives operation
    Socket->>Hook: BATCH_ADD_EDGES event
    Hook->>Store: get current edges
    Store-->>Hook: currentEdges[]
    Hook->>Filter: filterNewEdges(payload.edges, currentEdges)
    Filter-->>Hook: newEdges[]
    
    alt newEdges.length > 0
        Hook->>Store: batchAddEdges(newEdges)
        Store-->>Hook: edges updated
    else newEdges.length === 0
        Note over Hook: Skip: edges already exist<br/>from redraw/sync
    end
Loading

@icecrasher321
Copy link
Collaborator Author

@greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Additional Comments (1)

  1. apps/sim/stores/workflows/utils.ts, line 17-20 (link)

    style: imports split across the file - move all import statements to the top of the file before function definitions

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

    Context Used: Context from dashboard - Import patterns for the Sim application (source)

4 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@icecrasher321 icecrasher321 merged commit e3fa40a into staging Jan 14, 2026
11 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/add-dedup branch January 14, 2026 16:56
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.

2 participants