perf: scope undo/redo invalidation to changed geometry and cleared previews (charter row 7) - #805
perf: scope undo/redo invalidation to changed geometry and cleared previews (charter row 7)#805wass08 wants to merge 11 commits into
Conversation
|
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 |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ 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 |
There was a problem hiding this comment.
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.
Reviewed by Cursor Bugbot for commit 14218f4. Configure here.
There was a problem hiding this comment.
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.
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.


Metric
rich-4×
undoreceipt after a wall-body move < 2 s (charter backlog row 7). Three same-load pairs, main vs this branch: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.tsrefreshSceneAfterHistoryJump, 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
history-invalidation.ts, pure + 20 tests): changed nodes plus old and new direct parents; for every changed wall the union ofgetAdjacentWallIdsover 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.spatial-grid-sync.ts: floor-placed objects overlapping either rendered boundary of an affected slab are marked; unrelated levels stay clean.Finding 11's "GPU queue backlog" turned out to be an accounting artefact of the
queuerow (overlapping submit-to-completion latencies summed), so no backpressure change is included.Gates
--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 --noEmitclean; biome clean. A pre-existing editor-store test-ordering flake is fixed in the second commit.wiki/architecture/systems.mddescribes scoped history invalidation and the subscriber owners.Charter:
plans/performance/editor-scalable-scene-runtime.mdrow 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/runRedocapture 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;
getHistoryDirtyNodeIdsis exported from core.Reviewed by Cursor Bugbot for commit 5aa6515. Bugbot is set up for automated code reviews on this repo. Configure here.