Skip to content

Fix composite node duplication placement#542

Merged
Aymericr merged 2 commits into
mainfrom
feat/realtime-collaboration-sfx
Jul 24, 2026
Merged

Fix composite node duplication placement#542
Aymericr merged 2 commits into
mainfrom
feat/realtime-collaboration-sfx

Conversation

@Aymericr

@Aymericr Aymericr commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • route subtree-capable nodes through the shared fresh-placement clone and commit lifecycle
  • preserve real stair geometry and fresh descendant IDs while moving duplicated stairs
  • clear the original selection and roll back failed duplicate drafts without leaving history paused

Validation

  • bun run check-types
  • bun test packages/core/src packages/editor/src packages/nodes/src (2100 passed, 1 skipped)
  • annotation performance test repeated 3x in isolation
  • bun run build
  • bunx biome check on changed files
  • architecture review: no dependency-direction, renderer-isolation, or 2D/3D parity violations found

Note

Medium Risk
Touches scene graph duplication, undo/history pause-resume, and stair segment parenting—user-visible placement bugs are possible if draft rollback or subtree wiring regresses, but behavior is covered by new tests and mirrors an existing cabinet path.

Overview
Unifies duplicate → place for composite nodes by extending the fresh-placement lifecycle and tightening who uses it.

duplicatesAsFreshSubtree now keys only on capabilities.duplicable.subtree (not whether children exist), so childless subtree kinds (e.g. curved stairs, empty cabinet runs) still get a full fresh-subtree draft. Non-subtree copies go through prepareFreshPlacementRootDuplicate, which strips id, clears children so the clone never aliases the source’s descendants, and sets metadata.isNew.

2D and 3D action menus share that split: subtree path uses createFreshPlacementSubtree with try/catch that deletes partial drafts and always resumes scene history; root path uses the helper instead of inline structuredClone. Floorplan duplicate also clears selection and sets 2D move origin after success.

Stairs declare duplicable: { subtree: true }; duplicateStairSubtree is rewritten on createFreshPlacementSubtree / commitFreshPlacementSubtree (no manual segment cloning or rAF registry wait). The floating menu’s bespoke stair duplicate branch is removed—stairs follow the generic subtree path like cabinets.

Tests cover root-only child isolation, childless subtree eligibility, and stair move drafts (segments rewired, selection cleared).

Reviewed by Cursor Bugbot for commit f1a14da. Bugbot is set up for automated code reviews on this repo. Configure here.

@Aymericr
Aymericr merged commit b95d737 into main Jul 24, 2026
2 checks passed

@cursor cursor Bot left a comment

Copy link
Copy Markdown

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 using high effort 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.

Reviewed by Cursor Bugbot for commit f1a14da. Configure here.

if (draft) {
setMovingNode(draft as any)
setSelection({ selectedIds: [] })
return

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stair duplicate drops placement offset

Medium Severity

3D duplicate now routes stairs through createFreshPlacementSubtree with no position patch, but the removed menu path used duplicateStairSubtree, which defaulted to a [1, 0, 1] offset. The draft is created at the source stair’s position, so it overlaps the original until the user moves it, unlike other positioned duplicates that get a small nudge.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit f1a14da. Configure here.

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