You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The render function currently leaks internal store identifiers (ULIDs) into the rendered SVG as data-id attributes and SVG element id properties. These identifiers exist to serve the store's internal bookkeeping — referential integrity between messages and actors, undo stack entries, event targeting. They have no business appearing in the rendered output.
The leak causes two observable problems:
Non-deterministic snapshots. Every render produces different data-id values because ULIDs are minted per dispatch. Snapshot comparison requires a normalizeIds() pre-pass that strips them. This is a symptom of the leak, not a solution.
Canonical hashing is impossible without pre-normalization. Issue Canonical SVG serializer #51 ships a canonical SVG serializer. Without ULID stripping, the canonical form is different every render — defeating the "same sidewalk → same bytes" property the canonicalizer is supposed to provide.
The underlying pattern: internal identity has leaked through the presentation layer. This is the database-entity-in-DTO anti-pattern, applied to SVG. The fix is to treat the render function as an adapter that projects store state into SVG using business fields only, never internal identifiers.
The one reader
Grep of events section shows exactly one downstream consumer of the leaked data-id:
L71: const id = g.dataset.id;
A single event handler looks up the clicked store element by reading the DOM's data-id. Every other consumer (ARIA, drag, selection, ghost overlays) uses uiState.selected?.id or similar in-memory references that do not require the SVG to carry the ID.
The full refactor, therefore, reduces to: find a different way for that one handler to look up the store element.
Three viable mappings (pick one in this issue)
Each replaces g.dataset.id = el.id with an alternative mechanism:
A. WeakMap. Maintain a dom → storeElement WeakMap during render. The click handler queries the map. SVG stays clean. Memory is automatically reclaimed when elements are removed. Probably the cleanest.
B. Position-based lookup at event time. The click handler reads the click coordinates and asks the store "which element is at this position?". Requires a bounding-box test on the store's actor/message/note/fragment arrays. Makes hit-testing first-class, removes DOM coupling entirely. More code, more elegant, potentially slower.
C. Private attribute + canonicalizer strip rule. Rename data-id to something the canonicalizer is configured to strip (e.g. data-store-ref). The SVG still carries it; the canonical form doesn't. Smallest diff, preserves existing event-handler code, but pushes the leak into the canonicalizer rather than fixing it at source.
Recommendation: A (WeakMap). Smallest code change that actually fixes the leak at source, preserves O(1) lookup, and requires no canonicalizer awareness of ULIDs. C is tempting for minimal change but is architecturally the weakest — it just moves the leak.
Scope of this issue
In scope:
Remove data-id and data-type dataset writes from renderElement() (L76-77 of render section)
Remove data-id attribute emissions from _renderMsgUnconnected (L164), frag-resize-handle at L109, actor-move hoverboxes at L150, msg-endpoint handles at L351/L353
Introduce the chosen lookup mechanism (WeakMap recommended)
Update the one reader at events L71 to use the new mechanism
Verify all existing interactions still work: click-to-select, drag, resize, message endpoint reassignment, fragment corner resize, note drag, keyboard nav
Out of scope:
Removing normalizeIds() from the render gate. It stays until the follow-up issue wires hash-based comparison. (Reason: render-gate work is a separate issue; this issue is purely about the render layer.)
Any changes to the store, the canonicalizer, or the test suite beyond what's needed to verify render correctness
Changes to ARIA labels, tabindex, or other accessibility affordances — those remain
Verification
Manual (browser, via the running dev server):
Load each of the three demos from the Demo dropdown
Click each actor, message, note, and fragment — Properties panel populates correctly
Drag each element type — position updates
Resize a fragment from its SE corner — size updates
Drag a message arrow tip left/right — reassigns fromId/toId
Undo/redo each operation — state restores correctly
Tour walkthrough completes without errors
Export JSON, re-import — resulting diagram is identical
Automated:
9. GET /test passes (existing tests should not require changes, but watch for any that assert on data-id presence — those would be wrong to assert that in the first place and should be updated)
10. GET /lint passes
11. GET /test-render still green under existing normalizeIds() comparison — proves no regression in the current gate even though we haven't switched to hashing yet
Post-conditions
After this issue ships:
Rendered SVG contains no ULIDs anywhere
normalizeIds() still exists in the render gate (untouched) but has nothing to normalize — it becomes an identity function on the captured output
Canonical SVG hash becomes deterministic across runs by construction
The render gate refactor (hash-based comparison) can proceed in the follow-up issue with a much simpler scope
Done when
No data-id, dataset.id, or dataset.type writes in the render section
One lookup mechanism introduced and used consistently by all event handlers
Manual verification steps 1-8 all pass
GET /test passes
GET /lint passes
GET /test-render passes (15/15, though still comparing via current normalized-string path)
Canary: canonicalize(document.querySelector('#actors-layer').outerHTML) produces byte-identical output across two consecutive renders of the same demo (this is the proof that the leak is closed)
Why this blocks the hash gate
Issue #52-followup (hash-based render gate) depends on canonical SVG being deterministic. Today it isn't — each render produces different ULIDs, different canonical form, different hash. Fixing that at the canonicalizer level requires teaching the canonicalizer about ULIDs (wrong layer, wrong concern). Fixing it at the render level makes the canonicalizer's job trivial and the gate's job obvious.
This is the architectural precondition. Ship this first, then the hash gate is a natural follow-up.
Render: do not emit ULIDs in SVG output
Context
The render function currently leaks internal store identifiers (ULIDs) into the rendered SVG as
data-idattributes and SVG elementidproperties. These identifiers exist to serve the store's internal bookkeeping — referential integrity between messages and actors, undo stack entries, event targeting. They have no business appearing in the rendered output.The leak causes two observable problems:
Non-deterministic snapshots. Every render produces different
data-idvalues because ULIDs are minted per dispatch. Snapshot comparison requires anormalizeIds()pre-pass that strips them. This is a symptom of the leak, not a solution.Canonical hashing is impossible without pre-normalization. Issue Canonical SVG serializer #51 ships a canonical SVG serializer. Without ULID stripping, the canonical form is different every render — defeating the "same sidewalk → same bytes" property the canonicalizer is supposed to provide.
The underlying pattern: internal identity has leaked through the presentation layer. This is the database-entity-in-DTO anti-pattern, applied to SVG. The fix is to treat the render function as an adapter that projects store state into SVG using business fields only, never internal identifiers.
The one reader
Grep of
eventssection shows exactly one downstream consumer of the leakeddata-id:A single event handler looks up the clicked store element by reading the DOM's
data-id. Every other consumer (ARIA, drag, selection, ghost overlays) usesuiState.selected?.idor similar in-memory references that do not require the SVG to carry the ID.The full refactor, therefore, reduces to: find a different way for that one handler to look up the store element.
Three viable mappings (pick one in this issue)
Each replaces
g.dataset.id = el.idwith an alternative mechanism:A. WeakMap. Maintain a
dom → storeElementWeakMap during render. The click handler queries the map. SVG stays clean. Memory is automatically reclaimed when elements are removed. Probably the cleanest.B. Position-based lookup at event time. The click handler reads the click coordinates and asks the store "which element is at this position?". Requires a bounding-box test on the store's actor/message/note/fragment arrays. Makes hit-testing first-class, removes DOM coupling entirely. More code, more elegant, potentially slower.
C. Private attribute + canonicalizer strip rule. Rename
data-idto something the canonicalizer is configured to strip (e.g.data-store-ref). The SVG still carries it; the canonical form doesn't. Smallest diff, preserves existing event-handler code, but pushes the leak into the canonicalizer rather than fixing it at source.Recommendation: A (WeakMap). Smallest code change that actually fixes the leak at source, preserves O(1) lookup, and requires no canonicalizer awareness of ULIDs. C is tempting for minimal change but is architecturally the weakest — it just moves the leak.
Scope of this issue
In scope:
data-idanddata-typedataset writes fromrenderElement()(L76-77 of render section)data-idattribute emissions from_renderMsgUnconnected(L164),frag-resize-handleat L109,actor-movehoverboxes at L150,msg-endpointhandles at L351/L353Out of scope:
normalizeIds()from the render gate. It stays until the follow-up issue wires hash-based comparison. (Reason: render-gate work is a separate issue; this issue is purely about the render layer.)tabindex, or other accessibility affordances — those remainVerification
Manual (browser, via the running dev server):
fromId/toIdAutomated:
9.
GET /testpasses (existing tests should not require changes, but watch for any that assert ondata-idpresence — those would be wrong to assert that in the first place and should be updated)10.
GET /lintpasses11.
GET /test-renderstill green under existingnormalizeIds()comparison — proves no regression in the current gate even though we haven't switched to hashing yetPost-conditions
After this issue ships:
normalizeIds()still exists in the render gate (untouched) but has nothing to normalize — it becomes an identity function on the captured outputDone when
data-id,dataset.id, ordataset.typewrites in the render sectionGET /testpassesGET /lintpassesGET /test-renderpasses (15/15, though still comparing via current normalized-string path)canonicalize(document.querySelector('#actors-layer').outerHTML)produces byte-identical output across two consecutive renders of the same demo (this is the proof that the leak is closed)Why this blocks the hash gate
Issue #52-followup (hash-based render gate) depends on canonical SVG being deterministic. Today it isn't — each render produces different ULIDs, different canonical form, different hash. Fixing that at the canonicalizer level requires teaching the canonicalizer about ULIDs (wrong layer, wrong concern). Fixing it at the render level makes the canonicalizer's job trivial and the gate's job obvious.
This is the architectural precondition. Ship this first, then the hash gate is a natural follow-up.