Skip to content

fix(autolayout): targetted autolayout heuristic restored#3536

Merged
icecrasher321 merged 9 commits intofeat/mothership-copilotfrom
fix/restore-targetted-al
Mar 12, 2026
Merged

fix(autolayout): targetted autolayout heuristic restored#3536
icecrasher321 merged 9 commits intofeat/mothership-copilotfrom
fix/restore-targetted-al

Conversation

@icecrasher321
Copy link
Collaborator

Summary

Restore targeted autolayout for copilot edits to workflows.

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 Mar 12, 2026

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Mar 12, 2026 8:36pm

Request Review

@cursor
Copy link

cursor bot commented Mar 12, 2026

PR Summary

Medium Risk
Touches workflow graph mutation, layout, and persistence paths; mistakes could drop edges, misplace blocks, or overwrite DB records during collaborative edits.

Overview
Restores targeted autolayout for copilot-driven workflow edits by replacing full applyAutoLayout calls with applyTargetedLayout and narrowing layout work to only blocks that are new or were moved into a subflow with position reset to (0,0).

Improves subflow move/extract behavior by resetting positions when inserting into a container, converting relative→absolute positions on extraction, and adding a post-processing pass to drop scope-crossing/dangling edges once final parentIds are known.

Refines targeted layout heuristics (better anchor selection; treat only non-finite coordinates as invalid) and adds height estimation for unmeasured blocks to reduce overlaps. Updates diff-generation to preserve server-assigned UUIDs, preserve positions when scope is unchanged, and standardize on targeted layout.

Hardens socket DB writes by switching block/edge/subflow inserts to upserts (onConflictDoUpdate) to avoid conflicts when IDs already exist.

Written by Cursor Bugbot for commit f483723. Configure here.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 12, 2026

Greptile Summary

This PR restores and improves the targeted autolayout heuristic for copilot-driven workflow edits. Instead of running a full applyAutoLayout on every copilot change (which repositioned all blocks and discarded user placements), it now uses applyTargetedLayout with a focused set of "blocks needing layout" — blocks that are genuinely new or whose parent container changed. Unchanged blocks act as positional anchors so the rest of the graph stays put.

Key changes across the files:

  • diff-engine.ts: Replaces the heavyweight computeStructuralLayoutImpact (200+ LOC with adjacency expansion, edge-diff analysis, and parent traversal) with a concise filter: new block IDs + blocks whose parentId changed. Existing block positions are now always preserved as anchors (position: existingBlock.position). A new isUuid-based branch avoids minting fresh UUIDs for server-generated IDs, preventing foreign-key mismatches on subsequent socket operations.
  • targeted.ts: Adds selectBestAnchor (prefers edge-neighbor anchors over arbitrary ones), moves updateContainerDimensions to after repositioning (so container sizes reflect final snapped positions), and replaces the isDefaultPosition(0,0) guard with the stricter hasPosition (NaN/Infinity only), which avoids false-positives for manually-placed blocks.
  • utils.ts: Adds estimateBlockHeight for blocks not yet rendered by the browser, reducing overlaps in the initial copilot-generated layout.
  • engine.ts: Adds a post-pass removeInvalidScopeEdges that cleans up dangling and cross-scope edges after all batch operations have been applied.
  • operations.ts: insert_into_subflow now resets block position to (0,0) (React Flow requires container-relative coords); extract_from_subflow converts relative position to absolute.
  • database/operations.ts: All insert statements for blocks, edges, and subflows gain .onConflictDoNothing() for idempotency against retries.

The one dead-code issue (hasExistingBaseline) and a fragile implicit coupling between extract_from_subflow's (0,0) fallback and the layout-trigger heuristic in index.ts are flagged inline.

Confidence Score: 4/5

  • Safe to merge with minor follow-up: the unused hasExistingBaseline variable should be removed, and the (0,0)-fallback/layout-trigger coupling in operations.ts/index.ts should be documented or hardened.
  • The core logic is sound — replacing full autolayout with targeted layout is a clear improvement, removeInvalidScopeEdges is well-reasoned, and the UUID-preservation fix prevents a real DB integrity issue. The two concerns (dead variable causing a lint warning, and the fragile implicit coupling between the extract fallback and the layout heuristic) are not blocking but worth addressing before further iteration on this code path. onConflictDoNothing() is broadly safe for the retry-idempotency use case but carries a minor silent-data-drop risk.
  • apps/sim/lib/workflows/diff/diff-engine.ts (unused variable), apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.ts and index.ts (fragile (0,0) coupling), apps/sim/socket/database/operations.ts (onConflictDoNothing semantics).

Important Files Changed

Filename Overview
apps/sim/lib/workflows/diff/diff-engine.ts Large refactor: removes computeStructuralLayoutImpact (200+ LOC) and replaces it with a simpler "new blocks + parent-changed blocks" heuristic; preserves existing block positions as anchors; hasExistingBaseline is declared but never used (dead variable).
apps/sim/lib/copilot/tools/server/workflow/edit-workflow/index.ts Switches from full applyAutoLayout to applyTargetedLayout for all copilot edits; correctly identifies blocks needing layout by comparing pre/post-operation state; the (0,0) position heuristic for parent-changed blocks creates a fragile coupling with handleExtractFromSubflowOperation's fallback path.
apps/sim/lib/copilot/tools/server/workflow/edit-workflow/engine.ts Adds removeInvalidScopeEdges post-processing step that correctly cleans up dangling and cross-scope edges after all operations are applied; well-documented and logically sound.
apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.ts Adds position reset to (0,0) on insert_into_subflow and absolute-position computation on extract_from_subflow; the (0,0) fallback in the extract path creates implicit coupling with the layout trigger heuristic in index.ts.
apps/sim/lib/workflows/autolayout/targeted.ts Adds selectBestAnchor helper for smarter anchor selection (edge-neighbor preference); moves updateContainerDimensions to after block repositioning so container sizes reflect final positions; removes stale isDefaultPosition dependency in favour of the stricter hasPosition guard.
apps/sim/lib/workflows/autolayout/utils.ts Adds estimateBlockHeight for blocks that haven't been measured by the browser yet, preventing layout overlaps for newly-added blocks; the estimation approach is reasonable.
apps/sim/lib/workflows/autolayout/constants.ts Adds two new layout estimation constants (ESTIMATED_SUBBLOCK_HEIGHT, ESTIMATED_BLOCK_BOTTOM_PADDING) used by the new height estimation heuristic; straightforward and well-documented.
apps/sim/socket/database/operations.ts All block, edge, and subflow inserts in handleBlocksOperationTx and handleEdgesOperationTx gain .onConflictDoNothing() for idempotency; silent data-drop on conflict could hide stale records if a re-transmitted payload contains updated properties.

Sequence Diagram

sequenceDiagram
    participant Client
    participant EditWorkflowTool as edit-workflow/index.ts
    participant Engine as engine.ts (applyOperationsToWorkflowState)
    participant Operations as operations.ts
    participant DiffEngine as diff-engine.ts (WorkflowDiffEngine)
    participant TargetedLayout as autolayout/targeted.ts

    Client->>EditWorkflowTool: copilot edit request
    EditWorkflowTool->>Engine: applyOperationsToWorkflowState(ops)
    loop Each operation
        Engine->>Operations: handleInsertIntoSubflow / handleExtractFromSubflow / etc.
        Operations-->>Engine: modifiedState (position reset to 0,0 for subflow inserts)
    end
    Engine->>Engine: removeInvalidScopeEdges(modifiedState)
    Engine->>Engine: generateLoopBlocks / generateParallelBlocks
    Engine-->>EditWorkflowTool: { state, validationErrors }

    EditWorkflowTool->>EditWorkflowTool: compute blocksNeedingLayout\n(new blocks + parent-changed blocks with pos 0,0)
    alt blocksNeedingLayout.length > 0
        EditWorkflowTool->>TargetedLayout: applyTargetedLayout(blocks, edges, {changedBlockIds})
        TargetedLayout->>TargetedLayout: selectBestAnchor (edge-neighbor preference)
        TargetedLayout->>TargetedLayout: layoutGroup per scope
        TargetedLayout-->>EditWorkflowTool: repositioned blocks
    end
    EditWorkflowTool->>Client: persist final workflow state

    Note over DiffEngine: Client-side path (applyProposedState)
    Client->>DiffEngine: applyProposedState(proposedState)
    DiffEngine->>DiffEngine: id-map: preserve UUIDs, mint for non-UUID IDs
    DiffEngine->>DiffEngine: merge: keep existingBlock.position for unchanged blocks
    DiffEngine->>DiffEngine: compute blocksNeedingLayout (new + parent-changed)
    DiffEngine->>TargetedLayout: applyTargetedLayout(finalBlocks, edges, {changedBlockIds})
    TargetedLayout-->>DiffEngine: repositioned blocks
    DiffEngine-->>Client: merged + laid-out workflow state
Loading

Last reviewed commit: fc9c9c1

@icecrasher321
Copy link
Collaborator Author

bugbot run

@icecrasher321
Copy link
Collaborator Author

@greptile

@icecrasher321
Copy link
Collaborator Author

bugbot run

@icecrasher321
Copy link
Collaborator Author

@greptile

@icecrasher321
Copy link
Collaborator Author

bugbot run

@icecrasher321
Copy link
Collaborator Author

bugbot run

@icecrasher321
Copy link
Collaborator Author

bugbot run

@icecrasher321
Copy link
Collaborator Author

bugbot run

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

@icecrasher321 icecrasher321 merged commit 0dd70b7 into feat/mothership-copilot Mar 12, 2026
4 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/restore-targetted-al branch March 13, 2026 05:50
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.

1 participant