Skip to content

Conversation

@waleedlatif1
Copy link
Collaborator

Summary

  • fix subflow resizing on exit

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 10, 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 10, 2026 6:34am

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 10, 2026

Greptile Overview

Greptile Summary

Overview

This PR refactors workflow operations from individual updates to atomic batch operations, fixing the subflow resizing issue on exit. The core problem was that the old implementation updated blocks incrementally in a loop, causing multiple resize operations and potential race conditions.

What Changed

  • Store Operations: Added batch methods (batchAddBlocks, batchRemoveBlocks, batchToggleEnabled, batchToggleHandles, batchAddEdges, batchRemoveEdges, batchUpdateBlocksWithParent)
  • Collaborative Hook: Refactored to use batch operations throughout, removing individual operation handlers
  • Subflow Exit Fix: handleRemoveFromSubflow now collects all updates first, then applies them atomically via collaborativeBatchUpdateParent, ensuring container resizing happens once after all blocks are updated
  • Dead Code Removal: Removed unused operations (UPDATE_WIDE, UPDATE_TRIGGER_MODE, SUBFLOW_OPERATIONS.ADD/REMOVE)

How It Fixes Subflow Resizing

Old approach (problematic):

for each block:
  removeEdges(block)
  updateParent(block)  
  resize()  // Multiple resizes with inconsistent state

New approach (correct):

1. Collect all blocks and edges
2. Single atomic batch update (edges + parents)
3. derivedNodes effect triggers
4. Single resize with consistent final state

The fix ensures that:

  1. All blocks are updated atomically
  2. Container sees the final consistent state (not intermediate states)
  3. Only one resize operation occurs after all updates complete
  4. No race conditions from incremental updates

Architecture

The refactoring maintains proper separation of concerns:

  • Store layer: Pure state mutations via batch operations
  • Collaborative layer: Handles socket communication and undo/redo
  • UI layer: Triggers batch operations and responds to state changes via effects

All changes are backward compatible with the socket protocol (empty string represents parent removal, converted to undefined on receive).

Confidence Score: 4/5

  • This PR is safe to merge with low risk - it's a well-executed refactoring that fixes a real issue
  • Score of 4/5 reflects that this is a significant refactoring touching 15 files with complex state management logic. While the implementation is correct and fixes the subflow resizing issue, the lack of comprehensive automated tests for the batch operations introduces some risk. The PR would benefit from manual testing of edge cases like nested subflows and concurrent multi-user editing scenarios.
  • apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx - This file contains the critical subflow exit logic and should be tested thoroughly with multiple blocks, nested subflows, and boundary edge cases

Important Files Changed

File Analysis

Filename Score Overview
apps/sim/stores/workflows/workflow/store.ts 4/5 Added batch operations (batchAddBlocks, batchRemoveBlocks, batchToggleEnabled, batchToggleHandles, batchAddEdges, batchRemoveEdges, batchUpdateBlocksWithParent). All implementations are correct and handle edge cases properly.
apps/sim/hooks/use-collaborative-workflow.ts 4/5 Refactored to use batch operations instead of individual operations. Removed individual edge/block operations in favor of batch methods. Correctly handles remote operation processing and local state updates.
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx 4/5 Key fix: handleRemoveFromSubflow now uses single atomic batch update instead of loop-based incremental updates. This prevents race conditions and ensures container resizing happens correctly after all blocks are updated.
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-node-utilities.ts 5/5 Updated updateNodeParent signature to accept batch update functions instead of individual update functions. Logic remains correct and still calls resizeCallback at the end.
apps/sim/socket/validation/schemas.ts 5/5 Schemas properly define validation for all batch operations. BatchUpdateParentSchema accepts string for parentId (empty string represents removal).
apps/sim/stores/workflows/workflow/types.ts 5/5 Added type definitions for new batch operations. All signatures are correct and consistent with implementations.

Sequence Diagram

sequenceDiagram
    participant User
    participant UI as workflow.tsx
    participant Hook as use-collaborative-workflow
    participant Store as workflow store
    participant Socket as Socket Server
    participant Remote as Remote Clients

    Note over User,Remote: Removing Blocks from Subflow (New Batch Approach)
    
    User->>UI: Click "Remove from Subflow"
    UI->>UI: Collect all blocks and boundary edges
    UI->>UI: Calculate absolute positions BEFORE mutations
    UI->>UI: Build batch update array with edges
    UI->>Hook: collaborativeBatchUpdateParent(updates)
    
    Hook->>Store: batchRemoveEdges(edgeIds)
    Store-->>Store: Remove all edges atomically
    
    Hook->>Store: batchUpdateBlocksWithParent(updates)
    Store-->>Store: Update all blocks atomically
    Store-->>Store: Regenerate loops & parallels
    
    Hook->>Socket: Queue batch-update-parent operation
    Socket->>Remote: Broadcast batch-update-parent
    Remote->>Remote: Apply batch updates locally
    
    Store-->>UI: Trigger derivedNodes recalculation
    UI->>UI: useEffect detects derivedNodes change
    UI->>UI: resizeLoopNodesWrapper()
    UI-->>User: Container properly resized
    
    Note over User,Remote: Old Approach (Loop-based - PROBLEMATIC)
    Note over UI: for each block:
    Note over UI:   removeEdges(block)
    Note over UI:   updateParent(block)
    Note over UI:   resize() <- Multiple resizes!
    Note over UI: Result: Race conditions & incorrect sizing
Loading

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.

No files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@waleedlatif1 waleedlatif1 merged commit 6744043 into staging Jan 10, 2026
6 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/ops branch January 10, 2026 06:35
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