Skip to content

Render gate uses canonical hashes #52

Description

@MeatPopSci1972

Render: do not emit ULIDs in SVG output

Context

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:

  1. 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.

  2. 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):

  1. Load each of the three demos from the Demo dropdown
  2. Click each actor, message, note, and fragment — Properties panel populates correctly
  3. Drag each element type — position updates
  4. Resize a fragment from its SE corner — size updates
  5. Drag a message arrow tip left/right — reassigns fromId/toId
  6. Undo/redo each operation — state restores correctly
  7. Tour walkthrough completes without errors
  8. 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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    architectureiceboxParked — not current cycleregressionDrop everything — broken behaviourrfcDesign spike — no implementation until gate condition mettestsTest suite only

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions