Fix node positions drifting on reload (snapshot-authoritative layout)#38
Merged
Conversation
The bug the user reported: arrange a graph neatly, reload, and it
'relaxes' into a different shape every time. Root cause found by
instrumenting live nodes (fx/fy were always null even for saved nodes):
1. initializeVisualization unconditionally ran node.fx = node.fy = null
on EVERY node on EVERY init ('like Reset Layout button') — so the
force sim always re-flowed from scratch, ignoring saved positions.
2. mergeSimulationNodes strips physics keys (correct, to preserve live
velocity for the edge-follow fix) — so placed nodes arriving via a
graph-switch or 2s poll came back unpinned and drifted too.
3. Positions were saved only on drag-end; a physics-laid-out graph the
user never dragged was never persisted.
Fix (snapshot-authoritative, per decision):
- On init, a node whose saved position != (0,0) loads PINNED (fx/fy =
saved), so the sim cannot move it; only new/unplaced nodes flow.
- Re-assert pins after every merge (graph-switch/poll).
- persistAllPositions(): batch-save any node moved >1px from its saved
position, fired when the sim settles (on('end')) and on pagehide/
visibilitychange — so physics-arranged and grown nodes become durable.
- Reset Layout stays the explicit re-flow escape: clears saved-position
intent, suspends pinning while it rearranges, persists the new layout.
Verified: arrange + reload drift went 536px → 0px. New smoke gate test
'layout persistence: an arranged node survives a reload' (≤25px) locks
it in. web 91 unit, lint 0 errors, typecheck clean, smoke 4/4.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🧪 Comprehensive Test Suite
Full-stack smoke gate runs in the CI workflow. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
Arrange a graph neatly → reload → it relaxes into a different shape every time. Reported by the user; root cause confirmed by instrumenting live nodes (
fx/fywere always null even for saved nodes).Root cause (three compounding issues)
initializeVisualizationrannode.fx = node.fy = nullon every node on every init — the force sim always re-flowed from scratch, ignoring saved positions.mergeSimulationNodesstrips physics keys (correct — preserves live velocity for the edge-follow fix), so placed nodes arriving via graph-switch / 2s poll came back unpinned and drifted too.Fix — snapshot-authoritative (per decision)
fx/fy= saved); only new/unplaced nodes flow.persistAllPositions()batch-saves moved nodes on settle (on('end')) and onpagehide/visibilitychange→ physics-arranged and grown layouts become durable.Verified
Arrange + reload drift 536px → 0px. New smoke-gate test
layout persistence: an arranged node survives a reload(≤25px tolerance) locks it in. web 91 unit · lint 0 errors · typecheck clean · smoke 4/4.Part of the deeper plan in
docs/design/spatial-stability-and-reporting.md(physics tuning + unified video report are the next slices).🤖 Generated with Claude Code