Skip to content

feat(ui): the Workspaces diagram viewer becomes the diagram engine (ui 0.41.0, core 0.25.4) - #1560

Merged
backnotprop merged 10 commits into
mainfrom
feat/diagram-engine
Sep 17, 2026
Merged

backnotprop merged 10 commits into
mainfrom
feat/diagram-engine

Conversation

@backnotprop

@backnotprop backnotprop commented Sep 17, 2026

Copy link
Copy Markdown
Owner

The Workspaces diagram viewer becomes THE diagram engine in @plannotator/ui (0.41.0)

Owner rulings (Workspaces deviations log rows 2249 and 2251): "only the viewer, get it done" and "this needs to be the new engine, not an option; it can replace what we have." Brief: research/handoffs/BRIEF-diagram-viewer-into-plannotator-ui-2026-09-17.md. Source copied (never imported) from Workspaces main at 72e2a40e. One Opus review follows. Do not merge before it.

MermaidBlock and GraphvizBlock keep fence parsing, diagramLanguages.ts and the lazy-retry contract, and render through ONE renderer slot and ONE canvas. Their own viewBox math, applyView and per-block zoom controls are deleted. The popout is the same DiagramViewer at full size in the PopoutDialog chrome (the TablePopout precedent). A comment composed on a diagram part is an Annotation on the document.

File table

Workspaces file Here Notes
plannotator/diagram-render.ts packages/ui/utils/diagram-render.ts renderer slot; DiagramTheme is { colorTheme, mode }; errors carry runtimeUnavailable; one automatic load re-attempt; sanitizeDiagramSvg split into parseDiagramSvg + scrubDiagramSvg; widenEdgeHitAreas
(new) packages/ui/utils/graphviz.ts Graphviz runtime slot, same shape as utils/mermaid.ts; @viz-js/viz pinned exactly 3.30.0 (published 2026-09-01, past the 7-day gate; no exclude added)
components/diagram/diagram-anchor.ts packages/core/diagram-anchor.ts (pure half) + packages/ui/utils/diagram-anchor.ts (DOM finders, re-exports core) the anchor type lives in core beside html-anchor, because the external-annotation validator and the feedback archive (both vendored to Pi) run the parser
diagram-anchor-graphviz.ts packages/ui/utils/diagram-anchor-graphviz.ts keyed on g.node > title, never nodeN
diagram-projection.ts packages/ui/utils/diagram-projection.ts unchanged
useDiagramViewport.ts, useDiagramRender.ts packages/ui/components/diagram/ render hook takes retryToken
DiagramCanvas.tsx, DiagramOverlay.tsx, DiagramComposer.tsx, DiagramViewer.tsx, DiagramSourcePane.tsx packages/ui/components/diagram/ host coupling removed (adapter below); arrow-key pan added
(new) components/diagram/useDiagramComments.ts, useDiagramSourceDraft.ts, DiagramPopout.tsx, index.ts; components/DiagramBlock.tsx the viewer-side halves of what useDiagramAnnotations / useDiagramDraft did, the popout, the barrel, the fence side
test/fixtures/diagrams/* packages/ui/test-setup/fixtures/diagrams/ 7 captured svgs + Chromium geometry (excluded from the published tarball with test-setup)
codec / projection / viewer tests packages/core/diagram-anchor.test.ts, packages/ui/utils/diagramAnchor.test.ts, diagramAnchorGraphviz.test.ts (real viz engine), components/diagram/DiagramViewer.test.tsx, components/DiagramBlock.anchor.test.tsx, plus export, archive, POST (Bun + Pi), share-pin and highlighter-skip cases ported from vitest/jsdom to bun/happy-dom

Not copied, per the brief: useDiagramDraft.ts, useDiagramAnnotations.ts, DiagramDocumentBody.tsx, DiagramFenceOverlay.tsx, diagram-slice.ts, diagram-source.ts, diagram-kind.ts and the icons. Deleted here: components/mermaidSvg.ts.

The adapter

DiagramViewer: kind, source, theme, comments: DiagramComment[] (id, anchor, text, author, resolved), onCreateComment(anchor, text, additionalTargets), optional onSave(source) => Promise<{ status: 'ok' } | { status: 'stale', currentSource }> (no onSave: no Source pane), optional readOnlySource. Additive optionals a host needs for parity with what Workspaces' viewer took: sourceOpen, selectedCommentId / onSelectComment, onUnanchoredChange, onDismiss, renderId, sourceLineOffset, maxAdditionalTargets (default 0), commentingDisabledReason, retryToken, onRenderState, renderFallback, autoFocus. Everything is exported (@plannotator/ui/components/diagram, utils/diagram-render, utils/graphviz, utils/diagram-anchor*, utils/diagram-projection, @plannotator/core/diagram-anchor); smoke:package passes.

Anchors

One optional diagramAnchor?: DiagramAnchor on Annotation (the htmlAnchor precedent): useAnnotationHighlighter skips rows carrying it; exportAnnotations prints Diagram node <label> (<nodeId>), line <n>; the feedback archive records the validated anchor; POST /api/external-annotations accepts it on plan comments in both runtimes (400 when malformed); share links drop it (sharing.multiTarget.test.ts gained the diagram case). In plan review and annotate the comment carries the fence's blockId and DOCUMENT lines, lists in the rail, exports, drafts, and restores after reload through findDiagramTarget (id, label, line, unanchored), with the existing "Unanchored" chip for the last case via Viewer.onRestoreReport.

Interaction

Click-to-select, drag-to-pan: a press that travels under the drag threshold is a click and opens the composer there (a slightly moving click still selects); one that travels further is a pan and never opens it. The threshold is pointer-type aware: 4 px for a mouse or pen, 10 px for a finger. Hover targeting was removed at the owner's request: it read as messy and fought the pan hand, so nothing highlights on a plain mouse-over. The one pre-click affordance left is the ring under the pointer while the platform modifier is held (Cmd on macOS, Ctrl elsewhere, isModKeyHeld), and it disarms on the modifier's release, on any other key, and on window blur.

Widened edge hit area, and what was actually covering the edges. The owner could only catch Mermaid edges at odd spots while Graphviz edges were fine. Measured in headless Chromium (elementsFromPoint at 20 points along every edge of a nested-subgraph flowchart, a state, a class and an ER diagram, then real clicks at the same points): the suspected cause, nested g.root groups and cluster rects, is NOT it — Mermaid 12 + ELK emits one g.root even with nested subgraphs, and clusters are painted before the edge paths. The only thing over any edge is that edge's own label box (p inside g.edgeLabel's foreignObject), covering 5–40% of every labelled edge (worst 12 of 20 points reachable), centred on its midpoint, which is exactly where a person clicks an edge. Unlabelled edges were reachable at 20 of 20 points, and Graphviz labels are svg <text> inside the g.edge group, which is why it felt fine. The fix is structural, so it also holds for a renderer that does nest: every edge gets a bare hit path (geometry only, stroke: transparent, fill: none, stroke-width: 14 set !important, pointer-events: stroke, no id / class / data-* / markers / dash pattern) in ONE layer appended last in the svg root, its transform composed from the ancestors' transforms, mapped back to its edge by diagramHitSource. Because that layer sits over the nodes, the canvas resolves a click by priority over everything under the pointer (elementsFromPoint: a node, then an edge, then a cluster), never by event.target. An edge label is itself a target and resolves to its edge. After the fix every edge in all four families is reachable at 20 of 20 points. Covered: Mermaid g.edgePaths path, path.flowchart-link, path.transition, path.relation, path.relationshipLine, sequence .messageLine0/1; Graphviz g.edge path.

Sequence diagrams are addressable. Mermaid gives sequence parts classes, not ids, so the codec gains a sequence family (additive within v: 1): actors are nodes whose id is the actor's name (top and bottom boxes are one target), messages are edges msg-<n> (the message text is clickable for its message; from / to from the line's data-from / data-to), notes are nodes note-<n>, loop / alt / opt frames are clusters frame-<n>. Because an ordinal moves when a statement is inserted above it, restore checks the label too (the ordinal when its label matches, else the one part with that label, else the ordinal).

A click never does nothing. A click that resolves no part opens the composer on the WHOLE diagram: additive kind diagram, no id, label = the diagram's first source line, sourceLine = the fence's full range; export line Diagram (<family>), lines a–b (the export's existing en-dash range style); ring on the content bounds, badge top-left. This covers gitGraph, pie and anything future.

Badges and rings for existing comments are unchanged. DOM tests: a click on an edge's hit path and on its label resolve that edge; where a hit path and a node are both under the pointer the node wins; a 20 px drag does not open the composer and a 2 px one does; an 8 px finger wobble is still a tap. Chromium tests (the parity driver): the midpoint of EVERY edge of the flowchart, state, class and ER diagrams opens the composer on that edge (8/8, 5/5, 3/3, 3/3); at a point where a hit path is genuinely topmost over a node (path[hit] > polygon > rect > svg) the composer opens on the node; a sequence comment made by clicking a message's TEXT survives a reload onto the same message; a click on empty canvas opens the whole-diagram composer.

Sanitizer delta

mermaidSvg.ts (normalizeMermaidSvgMarkup) did no sanitizing: it baked max-width: none, preserveAspectRatio and height="100%" into the root tag for the old innerHTML mount, and the old Graphviz path was a regex recolor. So there was nothing of ours to merge in the security direction; sanitizeDiagramSvg is a strict superset. The one case ours covered that theirs did not: a root sized only by width="206pt" height="188pt" with no viewBox (our parseViewBoxFromMarkup accepted pt). That is merged into svgContentSize, which now accepts pt/px lengths as its fallback. Consolidated per the brief: Graphviz default strokes move from var(--muted-foreground) to var(--foreground) (the moved themeGraphvizSvg), a visible change; cluster outlines use var(--border), which is faint in one-light (see the light dot-fence pair on the parity page) and is a one-token tune if the owner wants it stronger.

Structural note: happy-dom cannot host DOMPurify (foreign-realm fragment, mislabelled svg namespaces, a cached Node.prototype.nodeName getter), so DOM tests swap only the PARSE step for an inert XML parse through __setDiagramSvgParserForTests; the scrub runs on every test render. The real DOMPurify parse is proven in headless Chromium (on the parity page): hostile svg in, no script / on* / javascript: / data: / anchor target / <use> out, foreignObject labels kept, the captured fixture whole (95 of 95 elements), nothing executed.

Test-harness changes to pinned files, assertions untouched: DiagramBlock.lazyRetry.test.tsx and MermaidBlock.theme.test.tsx install that parser, and the Graphviz stand-in answers render() (the slot drives the engine through render, a value with status and errors, not renderString). MermaidBlock.test.ts keeps the securityLevel: 'strict' pin and loses the tests of the deleted normalizer.

Sizes (single-file builds, bytes)

Bundle main 2a51b26e this branch delta gzip main gzip branch
apps/hook/dist/index.html 24,616,376 24,659,661 +43,285 (+0.18%) 7,559,515 7,574,108
apps/hook/dist/review.html 18,274,838 18,278,786 +3,948 (+0.02%) 5,747,280 5,748,232

No new dependency: @viz-js/viz, DOMPurify, CodeMirror and lucide were already in. The review bundle carries no engine (no flowchart-v2, no data-diagram-canvas); its delta is stylesheet classes. build:opencode and build:portal pass; on the portal mermaid.core (640 KB) and viz (1,350 KB) stay lazy chunks. guides.show: the viewer script is byte-identical (viewer.KTNT-M2b.js), so the diagram engine is NOT in the portable bundle (the guide chain renders no fences). The manifest is repinned for the stylesheet only (349,765 → 352,239 bytes): the viewer's Tailwind scan covers packages/ui/components and picks up the new components' classes (verified: cursor-grabbing, md:order-first etc. are in the new file and not the old).

Parity proof

Local sign-off page (never an artifact): /private/tmp/claude-501/-Users-ramos-plannotator-plannotator/34d56d5a-33a8-4878-92b2-3f4318214ae2/scratchpad/diagram-engine/index.html. Both builds served by bun apps/hook/server/index.ts annotate fixture.md from their own trees, headless Chromium (Playwright 1.63), theme pinned through an isolated PLANNOTATOR_DATA_DIR config.json AND cookies with documentElement.className asserted before every shot. 55 of 55 checks pass (42 parity + 13 owner-findings checks; the page also carries the edge-coverage tables before and after the fix and the Chromium sanitizer proof): each of the three diagrams (flowchart, sequence, dot) before/after in plannotator dark and one-light; the pending state; the popout; keyboard zoom in, arrow-key pan, fit (fit returns exactly to the arrival transform); no hover surface on a plain mouse-over; the edge click 6 px off the stroke; a comment on node D with its ring and badge, listed in the rail; the export containing Diagram node Approve? (D), line 9; reload offering the recovered draft and restoring the comment onto the same node (ring center inside the node's box, not unanchored); a text annotation on the prose still working beside it; a forced failed engine load showing the error panel with the source and Retry, and Retry recovering, on both builds.

Verification

  • bun run typecheck: clean (all nine projects, Pi vendored).
  • bun test packages/ui/utils packages/core packages/shared packages/server: 2884 tests, 0 fail (95 pre-existing skips).
  • DOM_TESTS=1 bun test --isolate packages/ui/components packages/ui/hooks packages/editor: 1001 pass, 0 fail.
  • bun test apps/pi-extension: 281 pass, 0 fail.
  • bun test scripts/dom-test-allowlist.test.ts: pass (five new DOM files added to .github/workflows/test.yml); tests/entry-assets.test.ts: 26 pass against the final bundles (the viz lazy-import pin now points at utils/graphviz.ts).
  • bun run --cwd packages/ui smoke:package: packs 0.41.0 and installs externally with core 0.25.4.
  • bun run --cwd apps/guides-show check:manifest: passes after the stylesheet repin.

Publish order

@plannotator/ui 0.41.0 pins @plannotator/core 0.25.4 exactly and imports @plannotator/core/diagram-anchor, which 0.25.3 does not export. Publish core 0.25.4 first, then ui 0.41.0, by hand from main after merge. Then Workspaces re-pins, deletes its twelve copies and keeps its six host files; the .mmd / .dot renderAs PR follows this one.

Review fixes (Opus review of this PR)

Blockers:

  1. Touch scroll trap. The inline canvas was touch-none, so a phone reader could not scroll past a diagram. It is now touch-action: pan-y; only the popout passes touch-none (canvasClassName). DOM tests pin both.
  2. The zoom strip printed over every diagram. It carries data-print-hide (print.css already has the rule), as do the block's hover controls; rings and badges still print, they are the comments.
  3. exportLinkedDocAnnotations and exportAnnotationEntry dropped the location line. Both print it now; tests for both, plus the whole-diagram form.
  4. A WebMCP reply dropped the anchor. Replies inherit diagramAnchor, and the diagram block and viewer root are annotation-exclude, so text restore can never wrap a <mark> inside a diagram svg. Tests for both.

Nits, all fixed:

  • Hit paths keep no data-* (or id, class, markers, style): they are built bare rather than cloned, so [data-id="L_A_B_0"] matches what it matched before (pinned).
  • <style> is scoped, not dropped. scopeDiagramCss keeps only rules whose every selector starts at the svg's own root id, plus @keyframes; @import, any fetching function (url( that is not a #fragment, image-set(, …, CSS escapes decoded first), unscoped rules and @font-face go; grouping rules are filtered recursively. Proven with the reviewer's payload in Chromium: the outside #victim keeps its color, no request reaches the hostile host, the scoped rule still applies. Mermaid's own styles survive: every rule of six captured families is kept (unit), and in Chromium the computed node, label and edge styles equal main's on three palettes (plannotator dark, one-light, dracula dark) with identical rule counts (59→59, 55→55). That is an equality check against main rather than a re-run of the 78-combination contrast sweep, whose harness is not in this repo.
  • External POST anchors resolve. A comment whose blockId names no diagram block ("external", or a deleted fence) is resolved by anchor against every diagram block through DiagramAnchorClaims: the first block in document order whose finder resolves it shows it; when every block has answered and none did it is reported Unanchored (a document with no diagram: Viewer reports it). The earlier "Known limits" text said the opposite of what the code did; corrected below.
  • The label fallback is skipped when more than one part carries the label (falls through to the source line).
  • Canvas keys ignore Meta / Ctrl / Alt, so Mod+0, Mod+-, Alt+Arrow pass through to the browser.
  • The modifier-gated ring disarms on keyup, blur and any other key.
  • 10 px click threshold for pointerType === 'touch'.
  • The wheel's |deltaY| < 0.1 guard is back.
  • HANDOFF's "Publishing & versioning" names 0.41.0 / 0.25.4.

Known limits

  • Plannotator passes maxAdditionalTargets 0 (one part per comment, since Annotation gains ONE field per the brief) and no onSave (no Source pane; the in-block Show-source toggle remains).
  • gitGraph and pie carry neither ids nor stable classes: a comment there is a whole-diagram comment. Sequence activations are not targets.
  • Sequence message text and line pair by document order; if a renderer ever splits one message into several text.messageText elements the counts differ and the text stops resolving (the line still does, with an empty label).
  • A whole-diagram composer opens at the content's top-left rather than at the click point.

…d the feedback archive

DiagramAnchor ({ v, family, kind, id | from + to, label, sourceLine }) and its fail-closed parser live in @plannotator/core/diagram-anchor beside html-anchor. POST /api/external-annotations validates an optional diagramAnchor on plan comments in both runtimes (the module is vendored to Pi), and the feedback archive records the validated anchor as an additive field.
MermaidBlock and GraphvizBlock keep fence parsing, diagramLanguages and the lazy-retry contract and render through one renderer slot (utils/diagram-render) and one canvas (components/diagram); their own viewBox math, applyView and zoom controls are gone, and the popout is the same DiagramViewer at full size in the PopoutDialog chrome. Graphviz gets a runtime slot beside Mermaid's (utils/graphviz, @viz-js/viz pinned 3.30.0). mermaidSvg.ts is replaced by sanitizeDiagramSvg (DOMPurify parse + in-place scrub). A comment composed on a diagram part is an Annotation with diagramAnchor and the fence's document lines: the highlighter skips it, the block restores it through the engine's finder and reports unanchored rows, the export prints its location line, share links drop it. Interaction per owner feedback: click selects, drag pans (4 px threshold), no hover targeting without the platform modifier, and an invisible 14 px hit path beside every edge.
The viewer script is byte-identical (viewer.KTNT-M2b.js): the guide chain renders no fences, so the diagram engine is not in the portable bundle. Only the stylesheet hash moves (349,765 to 351,932 bytes) because the viewer's Tailwind scan covers packages/ui/components and picks up the new diagram components' classes.
…DOFF corrections

HANDOFF gains 'Diagram engine (0.41.0)' (every new export, the adapter props, the anchor shape, the runtime slots, the sanitizer delta, the interaction model, migration notes, publish order). The Mermaid 12 section is corrected: the sweep is 78 combinations / 12,870 text pairs / 7,410 line pairs; consumers add their own lodash-es override; state diagrams gain the barbEnd-margin marker id; ELK is the effective default through per-diagram defaults while config.layout still reads dagre.
…hor codec

Both additive within v: 1. Sequence parts carry classes, not ids, so the codec's ids are the actor's name, msg-<n>, note-<n>, frame-<n>; sourceLine for an ordinal is the n-th statement of its kind. Kind 'diagram' has no id: it is what a click that resolves no part anchors to, exported as 'Diagram (<family>), lines a–b'.
Review blockers: the inline canvas is touch-action pan-y (only the popout is touch-none); the zoom strip and block controls never print; exportLinkedDocAnnotations and exportAnnotationEntry print the diagram location line; a WebMCP reply inherits diagramAnchor and diagrams are annotation-exclude so text restore never wraps a mark inside an svg. Nits: bare hit paths (no data-*), <style> scoped to the svg's root id (no @import, no fetching url(), no rule that can restyle the page), comments that name no diagram block resolve by anchor across blocks or list as Unanchored, label fallback only for a unique label, canvas keys ignore Meta/Ctrl/Alt, the modifier ring disarms on keyup/blur/other key, 10 px touch threshold, the wheel deltaY guard. Owner findings: measured in Chromium, the only thing covering a Mermaid edge is its own label box at its midpoint, so hit paths move to one top layer with composed transforms, a label resolves to its edge, and clicks resolve by priority over elementsFromPoint (node, edge, cluster); sequence diagrams are addressable; a click on no part comments on the whole diagram.
…nd, scoped styles, unowned anchors; HANDOFF names 0.41.0 / 0.25.4
@backnotprop
backnotprop merged commit b6c475a into main Sep 17, 2026
28 checks passed
@backnotprop
backnotprop deleted the feat/diagram-engine branch September 17, 2026 20:43
backnotprop added a commit that referenced this pull request Sep 18, 2026
…scope the code-nav root exclusion exemption (#1564)

* fix(api): validate PATCH bodies on /api/external-annotations, and read diagramAnchor defensively

PATCH merged its body into the stored annotation verbatim in both runtimes,
so any local process could store a value POST refuses. `{"diagramAnchor":
null}` was answered 200 and the SSE broadcast then blanked the open page:
DiagramBlock read `.family` off it during render.

Two layers:

- `validateAnnotationPatch` in @plannotator/core/external-annotation (the
  module both handlers already import) allowlists and field-validates the
  patch with the same validators POST applies — diagramAnchor through
  parseDiagramAnchor, htmlAnchor / elementContext / the target arrays through
  their own parsers, the scalars by type and cap. Unknown keys are dropped,
  `id` and `source` stay immutable, `null` clears an optional field and is
  refused on an anchor or a structural one.
- The renderer no longer trusts the field: DiagramBlock and Viewer read it
  with `!= null` / `?.`, so no ingest — API, draft or share link — can make a
  render throw.

* fix(code-nav): keep the root vendor/ tree excluded for a first-party package of the same name

The origin-file exemption (#1558) lifted a directory exclusion whenever the
ORIGIN path contained that segment anywhere, which lifted it tree-wide: a
request from `src/main/java/com/example/vendor/app/Widget.java` also returned
matches from the repo-root `vendor/` third-party tree it exists to exclude.

Narrowed to the directory INSTANCE the origin lives in:

- a root-only glob (`!/vendor`) is only lifted when the origin's FIRST segment
  is that directory — a deep same-named package was never pruned by it anyway;
- `isCodeNavPathAllowed` post-filters every result, so an always-ignored name
  lifted for an origin under one `node_modules` no longer returns matches from
  a different one.

A file that really lives under an excluded root still finds its own siblings.

* fix(print): print the light half of the palette, so a dark-theme page prints on white paper

The print stylesheet has always assumed white paper: it paints the ground
white and the text near-black. A dark-palette page kept its own tokens under
it, and #1560's diagram engine made that visible — Mermaid 12 draws node and
edge labels as real HTML inside `<foreignObject>`, so the blanket
`div, span, p { color: #1a1a1a !important }` repainted them near-black on a
near-black node fill and the flowchart printed as empty boxes.

- ThemeProvider renders the LIGHT half of the user's pair while printing. The
  class write happens inside the `beforeprint` handler, because a real print
  snapshot is taken before React would flush; the `print` media query drives
  the same switch for headless emulation and preview, where the async Mermaid
  re-render also lands. The stored preference is never touched, and a
  light-mode user sees no change.
- print.css exempts diagram content from the typography rules
  (`:not([data-diagram-block] *)`): a diagram colours itself, and its own
  `<style>` has no `!important` to defend itself with.
- usePrintMode shares the same subscription, so `.plannotator-print` is
  applied under print emulation too.

* chore(guides-show): repin the viewer manifest after the print and diagram changes

* docs: PATCH validation on external annotations, and the print light-half rule
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant