Skip to content

perf: scope undo/redo invalidation to changed geometry and cleared previews (charter row 7) - #805

Open
wass08 wants to merge 11 commits into
mainfrom
perf/undo-scoped-invalidation
Open

perf: scope undo/redo invalidation to changed geometry and cleared previews (charter row 7)#805
wass08 wants to merge 11 commits into
mainfrom
perf/undo-scoped-invalidation

Conversation

@wass08

@wass08 wass08 commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Metric

rich-4× undo receipt after a wall-body move < 2 s (charter backlog row 7). Three same-load pairs, main vs this branch:

fixture undo receipt frames first clean after dispatch
rich 4× 2 312 → 204 ms, 2 720 → 210, 2 340 → 208 (2nd undo 2 666 → 673, 3 191 → 605, 2 690 → 615) 84–87 → 5–6 1 611 → 125 ms
rich 2× 1 045 → 141 ms, 895 → 314, 895 → 129 41–42 → 7–8 690 → 34 ms
tower 2×18 2 299 → 206 ms, 2 723 → 168, 2 378 → 247 90–92 → 6–11 1 661 → 121 ms
Maxi 8× 3 999 (often a 5 s timeout) → 551 ms (body move); endpoint undo 4 787 → 2 119 — the remaining closure is the pre-existing slab-tracker cascade 142 → 14; 158 → 63 2 842 → 407 ms

Idle frame and like-gesture edit receipts unchanged; batch membership identical at dispatch and settle except the affected walls' openings.

Why

The editor's standalone undo/redo (history.ts refreshSceneAfterHistoryJump, introduced by ffa78f1 to restore canonical geometry after imperative previews) cleared live previews and then marked every scene node dirty on top of core's temporal changed-node/parent diff. One undo of a one-wall move rebuilt all 356 walls through the progressive pipeline. The runner's new undo sampling showed it directly: 10 ms after dispatch on Maxi 1×, 134 nodes of every kind were dirty for a one-wall move.

What changed — scoped invalidation

  1. Editor history: surviving ids captured from live transforms/overrides (incl. republished surface holes) plus their surviving direct parents are marked; the loop over every node is gone. Drafting cancellation, empty-command behaviour and the collab-delegate path are untouched.
  2. Core temporal diff (history-invalidation.ts, pure + 20 tests): changed nodes plus old and new direct parents; for every changed wall the union of getAdjacentWallIds over the before and after layouts (one hop: corners, T-junctions, curves — former neighbours re-miter); on wall thickness/height/curve change or add/remove, the hosted doors, windows and wall-side items of either snapshot (CSG holes, proxy depth, offsets); the existing deletion rule and dirty-id pruning kept.
  3. Existing scene-write subscribers keep owning their dependencies (spatial grid, slab tracker, space detection, stair/elevator openings — each verified to fire on the temporal write). One gap closed in spatial-grid-sync.ts: floor-placed objects overlapping either rendered boundary of an affected slab are marked; unrelated levels stay clean.
  4. Batching untouched: a one-wall undo releases only that wall's and its neighbours' openings (test).

Finding 11's "GPU queue backlog" turned out to be an accounting artefact of the queue row (overlapping submit-to-completion latencies summed), so no backpressure change is included.

Gates

  • e2e: 11/11 against the branch dists (undo/redo graph, perf-regression pack ×5, batched-wall and hidden-wall clicks, floor and wall placement, slab room paint). Independent audit: one blocker (undo/redo with a live wall preview left the preview-joined neighbour's miter stale — history now computes the closure of the effective override-merged layout before clearing) and one should-fix (linker-agnostic history probes for the private CI) closed; the new slab-boundary rule limited to changed boundary bands; endpoint-move closure pinned by test.
  • Unit: core+viewer+nodes+editor 5 006 pass (--randomize --seed=1); 20 dependency cases (disconnect/reconnect corner, T, reverse-T, curved endpoints; thickness/height; reparent; delete/restore), 11 real undo/redo cases with microtask flush, 2 spatial temporal cases; tsc --noEmit clean; biome clean. A pre-existing editor-store test-ordering flake is fixed in the second commit.
  • Docs: wiki/architecture/systems.md describes scoped history invalidation and the subscriber owners.

Charter: plans/performance/editor-scalable-scene-runtime.md row 7 (private PR follows with the evidence row and the runner's undo sampling).

🤖 Generated with Claude Code

https://claude.ai/code/session_01Jmmz2AMwTzcnKsSHHPEMhN


Note

Medium Risk
Changes undo/redo dirty marking and spatial slab-boundary invalidation—easy to miss dependents if the closure is wrong—but behavior is heavily tested and narrows rebuild scope rather than widening it.

Overview
Undo/redo no longer marks the whole scene dirty. A new pure closure (getHistoryDirtyNodeIds) drives both core’s temporal subscriber and the editor’s preview cleanup: changed nodes, parents, one-hop wall neighbours in before/after layouts, hosted openings/items on host dimension changes, and deletion sibling rules—replacing per-node scans and a stale “last snapshot” tracker with layouts taken from the temporal stacks.

Editor runUndo/runRedo capture override-merged layout before the jump, clear live transforms/overrides, then mark only the preview→committed dependency closure (including neighbours that existed only in the discarded preview) plus surviving preview targets and parents—not every node.

Spatial grid sync closes a gap for history/temporal writes: when walls or slabs change context but a slab’s stored polygon is unchanged, it diffs rendered slab bands and dirties floor-placed consumers overlapping only the gained/lost bands; slab reparent also refreshes covering dependents on both old and new levels.

Tests and docs expand around dependency closure, temporal slab support, and subprocess-based editor history probes; getHistoryDirtyNodeIds is exported from core.

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

@pascal

pascal Bot commented Sep 9, 2026

Copy link
Copy Markdown

I hit an error while handling your request (Model unavailable on AI Gateway free tier: Free tier users do not have access to this model. Upgrade to paid credits at https://vercel.com/d?to=%2F%5Bteam%5D%2F%7E%2Fai%3Fmodal%3Dtop-up for unrestricted…).

Please try again, rephrase, or reach out if it keeps failing.

Error id: 85b2b238-1a21-4987-9f0b-6bbe206da5b6

@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 2 potential issues.

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 14218f4. Configure here.

// writes (such as stair rise), unlike a snapshot saved at the last edit.
const snapshotBefore = didUndo
? state.futureStates[prevFutureLength]?.nodes
: state.pastStates[prevPastLength]?.nodes

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Later undos capture the wrong snapshot

Medium Severity

Undo reads futureStates[prevFutureLength] as the pre-jump layout. Zundo prepends the just-left snapshot at index 0 and shifts older futures back, so the second and later undos diff the wrong stack entry. Neighbours that were only adjacent in the intermediate layout can keep stale miters after systems have already cleared the first jump's dirty marks.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 14218f4. Configure here.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Checked against the installed zundo 2.3.0 source: undo removes from the end of pastStates and appends currentState to futureStates (dist/index.js:13–18); redo mirrors it (:28–32). So futureStates[prevFutureLength] is the just-left snapshot on every jump. Added a test with three adjacency-changing wall moves, then three undos and three redos, asserting the exact dirty set after each jump (aa9c58a); switching to index 0 makes the second undo mark the wrong neighbour. Leaving the index as is.

Comment thread packages/core/src/hooks/spatial-grid/spatial-grid-sync.ts
Zundo 2.3.0 appends the just-left snapshot to both destination stacks, so the existing pre-jump length indices are correct. Cover three adjacency-changing moves and each undo/redo with cleared marks and flushed microtasks.
Refresh covering dependents below both parent levels, deduplicating equal resolved levels. Cover reparent from level 2 to level 3 and undo with exact wall/ceiling sets and unrelated levels left clean.
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