improvement(workflow): remove useEffect anti-patterns

[waleedlatif1]

Summary

• Remove unnecessary useEffect for ref sync (assign during render instead)
• Move orphan node cleanup from reactive effect to batchRemoveBlocks store function
• Add missing syncPanelWithSelection dependency
• Improve diffBlocksRef pattern for clarity

Type of Change

• Improvement (code cleanup)

Testing

Tested manually - copy/paste, delete blocks, diff view

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]

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

1 Skipped Deployment

Project	Deployment	Review	Updated (UTC)
docs	Skipped		Jan 27, 2026 10:51pm

[greptile-apps]

Greptile Overview

Greptile Summary

This PR refactors React patterns by removing unnecessary useEffect hooks and moving reactive logic to the appropriate locations.

Key Changes:

• Ref sync improvement: Removed useEffect for autoConnectRef sync - now assigns directly during render (workflow.tsx:324-325)
• Orphan cleanup relocation: Moved orphaned node cleanup from reactive useEffect to batchRemoveBlocks store function (store.ts:447-480)
• Diff tracking pattern: Improved diffBlocksRef pattern by explicitly tracking when blocks change before deciding to reapply diff markers (workflow.tsx:464-473)
• Nested paste logic: Enhanced calculatePasteOffset to detect when all clipboard blocks are nested and use simple offset instead of viewport centering (workflow.tsx:124-127)
• Code cleanup: Removed unused variables (workspacePermissions, permissionsError) and permission logging effect
• Removed functions: Cleaned up duplicateBlock, toggleBlockAdvancedMode, setBlockTriggerMode, updateLoopCollection, and related helper functions that are no longer used

Issues Found:

• Missing syncPanelWithSelection dependency in onNodesChange callback (workflow.tsx:2323)
• Orphan cleanup logic uses currentBlocks instead of newBlocks for parent lookup, which will fail for multi-level parent chains (store.ts:466)

Confidence Score: 3/5

• This PR has good intentions but contains a critical logic bug in orphan cleanup and a missing dependency
• The refactoring improves React patterns by removing unnecessary useEffect hooks, which is a positive change. However, there's a critical bug in the orphan cleanup logic (store.ts:466) that uses the wrong block reference for parent lookup, and a missing dependency in the onNodesChange callback. These issues need to be fixed before merging.
• apps/sim/stores/workflows/workflow/store.ts needs attention for the orphan cleanup bug, and workflow.tsx needs the missing dependency added

Important Files Changed

Filename	Overview
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx	Improved ref sync pattern, fixed diff block tracking logic, enhanced paste logic for nested blocks
apps/sim/stores/workflows/workflow/store.ts	Moved orphan cleanup from reactive effect to batchRemoveBlocks, but position calculation needs verification

Sequence Diagram

sequenceDiagram
    participant User
    participant WorkflowUI as Workflow Component
    participant Store as Workflow Store
    participant Blocks as Block State

    Note over WorkflowUI,Store: useEffect Anti-pattern Removal

    User->>WorkflowUI: Delete blocks (batchRemoveBlocks)
    WorkflowUI->>Store: batchRemoveBlocks(blockIds)
    
    Note over Store: OLD: orphan cleanup in useEffect
    Note over Store: NEW: orphan cleanup in batchRemoveBlocks
    
    Store->>Store: Remove blocks and descendants
    Store->>Store: Filter edges connected to removed blocks
    
    rect rgb(240, 240, 255)
        Note over Store: Orphan Cleanup Logic
        Store->>Store: Check remaining blocks for invalid parentId
        Store->>Blocks: Look up parent in currentBlocks
        Store->>Store: Calculate absolute position with container offsets
        Store->>Store: Clear parentId and extent properties
    end
    
    Store->>Blocks: Update blocks state
    Store-->>WorkflowUI: State updated
    
    Note over WorkflowUI: Ref Sync Pattern Improvement
    
    User->>WorkflowUI: Toggle auto-connect
    WorkflowUI->>WorkflowUI: autoConnectRef.current = isAutoConnectEnabled (during render)
    Note over WorkflowUI: OLD: useEffect to sync ref<br/>NEW: direct assignment during render
    
    Note over WorkflowUI: Diff Blocks Ref Pattern
    
    WorkflowUI->>WorkflowUI: blocks state changes
    WorkflowUI->>WorkflowUI: Check if blocks !== diffBlocksRef.current
    WorkflowUI->>WorkflowUI: Update diffBlocksRef.current = blocks
    alt blocks changed AND hasActiveDiff
        WorkflowUI->>WorkflowUI: reapplyDiffMarkers()
    end

Loading

[greptile-apps]

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[waleedlatif1]

@greptile

[greptile-apps]

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[waleedlatif1]

@greptile

[greptile-apps]

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[waleedlatif1]

@greptile

[greptile-apps]

_{2 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Additional Comments (1)

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx
syncPanelWithSelection is used inside the callback but not included in the dependency array

    [blocks, syncPanelWithSelection]

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx
Line: 2323:2323

Comment:
`syncPanelWithSelection` is used inside the callback but not included in the dependency array

```suggestion
    [blocks, syncPanelWithSelection]
```

How can I resolve this? If you propose a fix, please make it concise.

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

[cursor]

Orphan cleanup in replaceWorkflowState doesn't convert position

Medium Severity

The newly added orphan handling in replaceWorkflowState clears parentId for blocks whose parent doesn't exist in the incoming state, but doesn't convert the position from relative to absolute. Child blocks store positions relative to their parent container (with header and padding offsets). When parentId is cleared without updating the position, the block appears at its relative position interpreted as an absolute position, causing it to display in the wrong location on the canvas. Compare with batchRemoveBlocks (lines 455-480) which correctly calculates absolute position by traversing the parent chain.

 

[cursor]

Test file documentation no longer matches test coverage

Low Severity

The header comment claims tests cover "Block operations (add, remove, duplicate, update)" and "Mode switching (basic/advanced)", but the tests for duplicateBlock, toggleBlockAdvancedMode, and the mode switching section were removed in this PR. The documentation no longer accurately describes the test coverage.