Skip to content

fix(autolayout): relocate notes that overlap blocks after layout#4888

Merged
waleedlatif1 merged 2 commits into
stagingfrom
waleedlatif1/abuja-v4
Jun 4, 2026
Merged

fix(autolayout): relocate notes that overlap blocks after layout#4888
waleedlatif1 merged 2 commits into
stagingfrom
waleedlatif1/abuja-v4

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

Summary

  • Auto-layout excluded note blocks entirely, so repositioned workflow blocks landed on top of notes — fixed by relocating overlapping notes below the flow after layout
  • Notes in clear space are never moved; only overlapping ones are stacked beneath their parent group's blocks in reading order
  • Applied to both layout paths: the auto-layout button (relocates any overlap) and targeted layout used by copilot/diff (relocates only overlaps the pass introduced, via a pre-layout baseline comparison, so unrelated edits never move notes)
  • Handles notes inside loop/parallel containers, is idempotent, and consolidates a duplicate position-validity helper

Type of Change

  • Bug fix

Testing

Tested manually. Added unit tests for both full and targeted modes (overlap relocation, clear-note preservation, multi-note stacking, pre-existing-overlap preservation, new-block-onto-note).

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
Copy Markdown

vercel Bot commented Jun 4, 2026

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jun 4, 2026 9:36pm

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented Jun 4, 2026

PR Summary

Low Risk
Canvas layout-only changes with guarded finite positions and targeted-mode baseline checks; covered by new unit tests.

Overview
Workflow auto-layout still skips note blocks, so repositioned steps could cover them. This PR adds a resolveNoteOverlaps pass after layout that moves only overlapping notes into a vertical stack below their parent group’s blocks (root or loop/parallel children), using note-specific sizing and ignoring invalid positions.

Full auto-layout (applyAutoLayout) relocates any note that overlaps a laid-out block. Targeted layout (applyTargetedLayout, e.g. copilot/diff) passes a previousBlocks snapshot so notes move only when the current pass introduced the overlap—not when overlap already existed.

Shared hasFinitePosition replaces a duplicate helper in targeted layout; NOTE_BLOCK_TYPE centralizes the excluded type. New unit tests cover full vs targeted behavior, stacking, and NaN positions.

Reviewed by Cursor Bugbot for commit 8c1c3e0. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 4, 2026

Greptile Summary

This PR fixes a bug where running auto-layout would reposition workflow blocks on top of existing note blocks, since notes are excluded from the topological layout pass. The fix adds a post-layout resolveNoteOverlaps step that detects and relocates overlapping notes below the laid-out workflow, applied to both the full layout button and the targeted (copilot/diff) path.

  • Full layout: any note overlapping a laid-out block is moved; notes in clear space are untouched, producing idempotent results on repeated runs.
  • Targeted layout: a pre-layout snapshot (previousBlocks) is diffed so only overlaps introduced by the current pass trigger relocation, preserving pre-existing note arrangements.
  • Robustness: NaN/Infinity block positions are guarded with hasFinitePosition; the local hasPosition duplicate in targeted.ts is removed and replaced with the now-exported utility; the note string literal is extracted to a NOTE_BLOCK_TYPE constant.

Confidence Score: 5/5

Safe to merge — the change is well-scoped, self-contained, and does not modify any existing layout logic.

The core resolveNoteOverlaps logic is sound: stackY is always initialised from maxBottom, so relocated notes can never collide with retained notes or each other. NaN/Infinity propagation is blocked by hasFinitePosition. Targeted-mode baseline comparison correctly attributes newly-introduced overlaps while preserving pre-existing arrangements. Tests cover all key scenarios.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/lib/workflows/autolayout/utils.ts Adds hasFinitePosition, getNoteDimensions, noteOverlappedBlockBefore, and the main resolveNoteOverlaps function with correct NaN-guarding, targeted-mode baseline comparison, and idempotent stacking logic.
apps/sim/lib/workflows/autolayout/targeted.ts Wires resolveNoteOverlaps with the pre-layout blocks snapshot for targeted mode and replaces the local hasPosition helper with the now-exported hasFinitePosition from utils.
apps/sim/lib/workflows/autolayout/index.ts Adds the post-layout resolveNoteOverlaps call in the full auto-layout path, correctly positioned after layoutContainers.
apps/sim/lib/workflows/autolayout/constants.ts Extracts NOTE_BLOCK_TYPE constant and reuses it inside AUTO_LAYOUT_EXCLUDED_TYPES, avoiding magic string duplication.
apps/sim/lib/workflows/autolayout/utils.test.ts New test file covering overlap relocation, clear-note preservation, multi-note stacking, NaN-position robustness, and all three targeted-mode cases.

Reviews (2): Last reviewed commit: "fix(autolayout): harden note overlap res..." | Re-trigger Greptile

Comment thread apps/sim/lib/workflows/autolayout/utils.ts
Comment thread apps/sim/lib/workflows/autolayout/utils.ts
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Copy link
Copy Markdown

@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.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 8c1c3e0. Configure here.

@waleedlatif1 waleedlatif1 merged commit 80ea0dd into staging Jun 4, 2026
14 checks passed
@waleedlatif1 waleedlatif1 deleted the waleedlatif1/abuja-v4 branch June 4, 2026 21: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