fix: palette-add mints unique node ids against live topology - #47
Conversation
makeNode uses a module counter that resets on page load, causing duplicate ids when adding nodes of the same kind. handleAddNode now overrides the id using freshId (exported from clipboard.ts), which scans the live node id set — matching the pattern freshAnnId already uses for annotations. Closes xevrion#46
|
@cursoragent is attempting to deploy a commit to the whoarrryou's projects Team on Vercel. A member of the Team first needs to authorize it. |
…ral tests handleAddNode now reads topoLiveRef.current instead of the React topology closure, preventing two rapid adds from dropping the first node. topology removed from the dependency array. New test file handleAddNode.wiring.test.ts: - Source assertion: handleAddNode spreads topoLiveRef.current, not the React topology binding, and freshId is called. - Behavioral test: two sequential cache adds against the default preset produce unique ids, both nodes survive, isTopology accepts.
|
This is a real fix and the diff reads well. Minting against the live topology is right, and picking up the stale-closure bug on the way (reading One change before I merge. Could you drop the I follow the argument in the comment, and you're right that a pure-function test can't catch a reintroduced stale closure. But a test that asserts on source text breaks when someone renames a local or reformats the callback, and the failure it gives them won't point at anything real. That maintenance cost sits on every future contributor who touches that function. The behavioural describe blocks in the same file, plus the Once that's out I'll merge. |
|
Going to close this one, and I want to be straight about why since you've been waiting on me. #65 came in for the same issue and puts the dedupe inside That's a call about structure, not about your work, and the four day wait was on me. Sorry for the wasted time. #52 and #55 are both open and unclaimed if you want another. |
3 participants
What this changes
Palette-add (
handleAddNode) now mints node ids against the live topology instead of a module counter that resets on page load. Closes #46.Why
makeNodeuses a module-levelnodeCounterthat resets on load. Two palette-adds of the same kind mint the same id.isTopologyrejects the save, andloadSessionsilently falls back toPRESETS[0].Changes
src/clipboard.ts: exportfreshId(contract unchanged)src/App.tsx:handleAddNodestill callsmakeNodefor kind/label/config/position, then overridesidwithfreshIdagainsttopoLiveRef.current.nodes.makeNodecounter left for presets.src/clipboard.test.ts: regression tests for unique mints, skip-existing,isTopologyacceptsKeep
Fork PR (same branch): Dahhrk#1