Common Render pipeline 1: ELK

[knsv-bot]

Summary

• Refactor ELK layout into prepare/layout-core phases for the common renderer pipeline.
• Keep ELK layout core DOM-free and reuse the common renderer for painting.
• Fix ELK edge marker clearance and subgraph child positioning after the refactor.
• Add elk.nodePlacementAlignment with RIGHTDOWN as the Mermaid default for Brandes-Koepf alignment.

Verification

• pnpm vitest run packages/mermaid-layout-elk/src/__tests__/geometry.spec.ts packages/mermaid-layout-elk/src/__tests__/render.spec.ts packages/mermaid/src/rendering-util/rendering-elements/edges.spec.js packages/mermaid/src/rendering-util/layout-algorithms/common/index.spec.ts
• pnpm vitest run packages/mermaid-layout-elk/src/__tests__/render.spec.ts packages/mermaid/src/config.spec.ts
• Browser check on decision-flow.mmd: default nodePlacementAlignment gives Task A - Merge = -23.378; NONE gives -90.255; BALANCED gives -56.816.
• git diff --check
• npx fallow audit --changed-since origin/develop --format json reports no dead code or duplication; still reports the known common-renderer complexity finding in positionRenderedEdgeLabel.

Base branch: develop.

[changeset-bot]

🦋 Changeset detected

Latest commit: cfc264e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
mermaid	Minor
@mermaid-js/layout-elk	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[netlify]

✅ Deploy Preview for mermaid-js ready!

Name	Link
🔨 Latest commit	cfc264e
🔍 Latest deploy log	https://app.netlify.com/projects/mermaid-js/deploys/6a22ba07c349000008acda71
😎 Deploy Preview	https://deploy-preview-7803--mermaid-js.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[pkg-pr-new]

Open in StackBlitz

@mermaid-js/examples

npm i https://pkg.pr.new/@mermaid-js/examples@7803

mermaid

npm i https://pkg.pr.new/mermaid@7803

@mermaid-js/layout-elk

npm i https://pkg.pr.new/@mermaid-js/layout-elk@7803

@mermaid-js/layout-tidy-tree

npm i https://pkg.pr.new/@mermaid-js/layout-tidy-tree@7803

@mermaid-js/mermaid-zenuml

npm i https://pkg.pr.new/@mermaid-js/mermaid-zenuml@7803

@mermaid-js/parser

npm i https://pkg.pr.new/@mermaid-js/parser@7803

@mermaid-js/tiny

npm i https://pkg.pr.new/@mermaid-js/tiny@7803

commit: cfc264e

[argos-ci]

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build	Status	Details	Updated (UTC)
default (Inspect)	👍 Changes approved	5 changed	Jun 5, 2026, 12:13 PM

[codecov]

Codecov Report

❌ Patch coverage is 0.25907% with 1155 lines in your changes missing coverage. Please review.
✅ Project coverage is 2.82%. Comparing base (1193d7e) to head (cfc264e).

Files with missing lines	Patch %	Lines
packages/mermaid-layout-elk/src/render.ts	0.00%	791 Missing ⚠️
...c/rendering-util/layout-algorithms/common/index.ts	0.47%	208 Missing ⚠️
...rc/rendering-util/layout-algorithms/dagre/index.js	0.00%	50 Missing ⚠️
...g-util/rendering-elements/shapes/requirementBox.ts	0.00%	26 Missing ⚠️
...g-util/layout-algorithms/swimlanes/adjustLayout.ts	0.00%	19 Missing ⚠️
...-util/rendering-elements/shapes/labelImageUtils.ts	0.00%	17 Missing ⚠️
packages/mermaid/src/rendering-util/createGraph.ts	0.00%	14 Missing ⚠️
...endering-util/layout-algorithms/swimlanes/index.ts	0.00%	8 Missing ⚠️
...aid/src/rendering-util/rendering-elements/edges.js	0.00%	8 Missing ⚠️
packages/mermaid/src/rendering-util/render.ts	20.00%	4 Missing ⚠️
... and 6 more

Additional details and impacted files

@@            Coverage Diff             @@
##           develop   #7803      +/-   ##
==========================================
- Coverage     2.83%   2.82%   -0.01%     
==========================================
  Files          668     669       +1     
  Lines        72273   72527     +254     
  Branches       989     990       +1     
==========================================
+ Hits          2047    2048       +1     
- Misses       70226   70479     +253     

Flag	Coverage Δ
unit	2.82% <0.25%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
packages/mermaid/src/config.type.ts	100.00% <ø> (ø)
packages/mermaid/src/defaultConfig.ts	48.97% <100.00%> (+0.20%)	⬆️
packages/mermaid/src/diagram-api/types.ts	100.00% <ø> (ø)
packages/mermaid/src/rendering-util/types.ts	100.00% <ø> (ø)
packages/mermaid/src/dagre-wrapper/shapes/util.js	0.00% <0.00%> (ø)
packages/mermaid/src/mermaid.ts	0.42% <0.00%> (-0.01%)	⬇️
...c/rendering-util/rendering-elements/shapes/util.ts	0.00% <0.00%> (ø)
.../src/diagrams/flowchart/flowRenderer-v3-unified.ts	0.00% <0.00%> (ø)
.../mermaid/src/diagrams/requirement/requirementDb.ts	0.00% <0.00%> (ø)
packages/mermaid/src/diagrams/class/shapeUtil.ts	0.00% <0.00%> (ø)
... and 10 more

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[knsv-bot]

[sisyphus-bot]

Review of #7803 — Refactor ELK onto common layout renderer (posted as a comment since GitHub won't let the PR author submit a formal review; verdict would be COMMENT — this is a draft, so nothing here is blocking).

Really nice piece of work — this is a clean, well-structured refactor of some of the gnarliest code in the repo. Notes below are "heads up for when you take it out of draft," not demands.

What's working well 🎉

• The createCommonLayoutRenderer seam is a genuinely good abstraction. Collapsing dagre, swimlanes and ELK onto a single prepareLayout → measureLayout → runLayoutCore → paintLayout → afterPaint pipeline removes a lot of duplicated orchestration — the old swimlanes adjustLayout edge-label/terminal-label positioning logic now lives once in paintLayoutData (common/index.ts) and everyone benefits.
• DOM-free ELK layout core, done properly. runElkLayoutCore only mutates plain data (nodeDb, layoutEdge.points, offsets) — all DOM measurement/painting is delegated to the shared pipeline. That's a real testability win, and you backed it up with DOM-decoupled unit tests (buildElkGraphFromLayoutData, runElkLayoutCore, ensureEndMarkerSegmentLength) in the DDLT spirit.
• The dagre refactor is behavior-preserving. recursiveRender = paintDagreLayoutCore(await measureAndRunDagreLayoutCore(...)) keeps the nested-subgraph recursion intact, and since nothing runs between core and paint at the top level, output ordering is unchanged. Markers + clears moved into the common renderer consistently. This was the highest-blast-radius part (dagre drives flowchart/state/class) and it looks safe.
• Unit coverage is strong — hook ordering, default-measure delegation, skipEdge/skipIntersect, edge-label insert+position, and the import surface (common-renderer-import.spec.ts) are all exercised.
• Config plumbing is clean — config.type.ts correctly regenerated from the schema, config.spec.ts covers the new default, and the registry contract render(data4Layout, svg, helpers, options, positions) still matches how rendering-util/render.ts invokes layout renderers (ELK reads context.helpers and would throw otherwise).

Things to address

🟡 [important] No e2e / visual-regression coverage for a shared-rendering + ELK rewrite. Per the repo guidance, shared rendering-util/ changes need the full e2e suite and renderer changes "must add or update e2e tests." This PR touches the shared layout orchestration (so flowchart/state/class via dagre) and rewrites the ELK pipeline, but ships unit tests only. The unit tests can't catch group z-order, nesting, or spacing regressions. Before un-drafting: run the full e2e/Argos suite and confirm flowchart + ELK snapshots are unchanged (or intentionally re-baselined).

🟡 [important] ELK's default layout output changes for every existing layout: elk diagram. develop never set elk.layered.nodePlacement.bk.fixedAlignment, so ELK used its built-in default (NONE); this PR makes RIGHTDOWN the default in both createRootElkGraph and buildSubgraphLayoutOptions — and the PR description itself documents node positions shifting (Task A → Merge from -90.255 to -23.378). Two things worth doing:

1. The changeset reads "expose elk.nodePlacementAlignment to configure…" but the user-visible effect is that the default ELK layout changes. Mermaid is re-rendered at scale by GitHub/GitLab, so the changeset should call that out explicitly.
2. Confirm RIGHTDOWN is the intended new default rather than preserving NONE (which would keep existing ELK diagrams stable while still exposing the option), and back the choice with ELK visual snapshots.

💡 [suggestion] New public API surface. mermaid.ts now exports createCommonLayoutRenderer, defaultMeasureLayout, paintLayoutData, clearLayoutRenderState and 5 types. These are needed by the external @mermaid-js/layout-elk package, but exporting them commits Mermaid to maintaining this layout-renderer contract as stable public API (changing it later would be breaking). Worth a deliberate call: treat it as supported/documented public API, or mark it as an internal seam for layout plugins — either is fine, just intentional.

🟢 [nit] Dead assignment in configureSubgraphNodes (render.ts:445). node.width = (node.width ?? 0) + 2 * (node.padding ?? 0) is immediately discarded by the delete node.width a few lines later (same for height). Looks like a refactor leftover — removing the assignment would avoid the head-scratch.

Nothing here changes the shape of the design, which is solid. The main gate before merge is visual verification of both the dagre-backed diagrams and ELK (especially given the RIGHTDOWN default). Happy to take another pass once the e2e run is green. 🚀

[knsv]

Thanks for the review.

On the e2e/visual-regression point: I do not think it is accurate that this PR ships unit tests only. The existing Cypress visual specs already cover the relevant shared-rendering/ELK paths:

• ELK nested subgraphs / group z-order:
  cypress/integration/rendering/flowchart/flowchart-elk.spec.js
  tests 50-elk, 51-elk, 52-elk, etc.

• ELK flowchart styling and linkStyle:
  cypress/integration/rendering/flowchart/flowchart-elk.spec.js
  includes styled nodes and linkStyle coverage.

• ELK class diagram without htmlLabels:
  cypress/integration/rendering/class/classDiagram-elk-v3.spec.js
  test ELK-12.1.

• Requirement diagrams with htmlLabels disabled, styles/classes, and edge labels:
  cypress/integration/rendering/requirement/requirementDiagram-unified.spec.js.

So I agree the full e2e/Argos suite should be green before undrafting, but I would push back on “no e2e / visual-regression coverage.” The coverage already exists and exercises the paths affected by this PR.

For the RIGHTDOWN point: agreed that the changeset should make the default behavior explicit. I pushed a follow-up commit clarifying that RIGHTDOWN is now the intended default and that users can set elk.nodePlacementAlignment: NONE if they want ELK’s no-fixed-alignment behavior.

For the dead assignment nit: agreed, removed in the same follow-up commit.

[knsv-bot]

[sisyphus-bot]

Follow-up review of #7803 — Common Render pipeline 1: ELK (round 2, posted as a comment since GitHub blocks the PR author from a formal review). Verdict-equivalent: looks good to merge once the full Argos/e2e run is confirmed green.

I took the round-1 discussion into account and re-verified everything against the current head (f8d802b). All three of my earlier points are addressed — and the most important one was resolved better than I'd asked.

Resolved since round 1

✅ ELK default behavior (was 🟡 important). The follow-up (5c88f9d6) sets DEFAULT_NODE_PLACEMENT_ALIGNMENT = 'NONE' (render.ts:109), applied in both createRootElkGraph (render.ts:137) and buildSubgraphLayoutOptions (render.ts:273). Since develop never set elk.layered.nodePlacement.bk.fixedAlignment — so ELK already used NONE — existing layout: elk diagrams are now unchanged. That removes the "every ELK diagram shifts" concern entirely, while still exposing elk.nodePlacementAlignment (incl. RIGHTDOWN/BALANCED) for opt-in, and the changeset now states the default explicitly. This is the safer choice — thanks for taking it.

✅ e2e / visual coverage (was 🟡 important) — you're right, I was wrong. I verified the pointers: flowchart-elk.spec.js (50/51/52-elk nested-subgraph / z-order), classDiagram-elk-v3.spec.js (ELK-12.1, no-htmlLabels), and requirementDiagram-unified.spec.js (htmlLabels-off + classes + edge labels) all exist and exercise the paths this PR touches. The PR's Argos check also ran and reports 5 changed snapshots, approved. So I retract "ships unit tests only / no e2e coverage" — the visual safety net is already in place. The only standing gate is confirming that full run is green, which we agree on.

✅ Dead assignment (was 🟢 nit). Gone — configureSubgraphNodes (render.ts:472) now does a clean delete node.width/height with no preceding throwaway assignment.

Re-verified independently

🎉 Security: clean. The new shared createCommonLayoutRenderer / paintLayoutData (common/index.ts) introduces no new DOM sinks (no innerHTML/.html()/insertAdjacentHTML/xlink:href/javascript:) and paints through the existing sanitized helpers (insertCluster, insertEdge, insertEdgeLabel). The changed shape/edge files add no new href/HTML sinks. This is a pure orchestration move, not a sanitization change.

🎉 Semver is right. minor for both mermaid and @mermaid-js/layout-elk is correct now that the default preserves behavior — additive config + additive exports, no breaking change.

One open item (non-blocking)

💡 Public API surface. mermaid.ts now exports createCommonLayoutRenderer, defaultMeasureLayout, paintLayoutData, clearLayoutRenderState + 5 types. The external @mermaid-js/layout-elk package needs them, but exporting them commits Mermaid to keeping this layout-renderer contract stable. Given the title ("pipeline 1") implies more layout engines will follow, a one-line note on whether this is supported public API or an internal layout-plugin seam would help future pipeline PRs know whether they can evolve it freely. No change needed for this PR.

Two small sanity checks before merge, both yours to wave off:

• Confirm the 5 changed Argos snapshots are the intended edge-marker-clearance + subgraph-child-positioning re-baselines (from the PR summary), not incidental drift.
• Codecov shows ~0% patch coverage on render.ts/common/index.ts — expected here, since those paint paths are covered by Argos/e2e and common/index.spec.ts adds 364 lines of DOM-decoupled unit tests. Flagging only so the codecov red doesn't read as alarming.

Design is unchanged and solid, and the follow-ups landed fast. 🚀

[pbrolin47]

@knsv Did a review on the last changes.

[sisyphos-bot]

---

PR #7803 — Common Render Pipeline 1: ELK (codex/elk-prep-extraction → develop)

Focus: fixes from knsv's previous review

Prior review summary

knsv left 5 inline comments on common/index.ts and approved the PR at 12:32 after the author addressed them. Here is the verification of each:

1. "This is an important function, but hard to read. Please add some comments" (line 87)
   ✅ Addressed. createCommonLayoutRenderer now has a leading doc comment ("Convenience struct containing everything you need to render…") and each pipeline step — prepare,
   measure, core, paint, afterPaint — has an explanatory comment above it.

2. "Explain what we're doing here" (line 102)
   ✅ Addressed. The measureLayout section now reads: "Use the provided measureLayout or default to createGraphWithElements if not provided…" and the factory step reads: "This is
   the actual factory step where the render function is created."

3. "Remove positions. I don't see why we need it here."
   ✅ Addressed. grep "positions" on common/index.ts shows the word only in comment text — not as a variable or property anywhere in the file.

4. "What does this do?" (line 129 — the paintLayout escape hatch)
   ✅ Addressed. Now reads: "Escape hatch: if a custom paintLayout is provided, we assume it handles everything…"

5. "dagre/index.js line 1: Should this not be in the DAGRE phase?"
   ✅ Resolved — knsv's own follow-up confirmed: "it is okay to have it here."

---

Tests

common/index.spec.ts (362 lines, 6 cases) covers the full pipeline:

1. Hook ordering: setup → prepare → measure → core → paint → post-paint
2. Custom measure results passed to custom paint hooks
3. Delegates to createGraphWithElements
4. Paints clusters and nodes, skips isLayoutOnly/skipped edges, forwards skipIntersect
5. Edge label insertion and positioning
6. Terminal labels without a center edge label

🎉 The test coverage directly mirrors the createCommonLayoutRenderer options API — easy to read, comprehensive.

---

Security

The new common/index.ts uses translate(${x}, ${y}) to position edge labels. The x and y values come from calcLabelPosition / calcTerminalLabelPosition → mathematical operations
on Point[] (typed as { x: number; y: number }). These are layout-engine outputs, not user text. No user-controlled string flows into any attribute or DOM sink in this file.

Edge labels themselves (user-controlled diagram text) flow through insertEdgeLabels → createText.ts, which sanitizes via sanitizeText() before any DOM insertion. The pipeline is
secure.

No XSS or injection issues identified in the changed files.

---

Verdict: APPROVE

All four substantive review items from knsv are addressed, the test suite is solid, and there are no security concerns. knsv has already approved — this is consistent with that
assessment.