Common Render pipeline 3: DAGRE phase 3

[knsv-bot]

Summary

• split DAGRE layout into explicit measure, core layout, and paint phases
• keep recursive subgraph rendering behavior local to DAGRE for this PR
• enforce that runDagreLayoutCore uses an already measured graph and does not perform DOM measurement

Validation

• pnpm build:types
• pnpm lint
• pnpm vitest run packages/mermaid/src/rendering-util/layout-algorithms/dagre/index.spec.js
• pnpm vitest run packages/mermaid/src/diagrams/block/parser/block.spec.ts packages/mermaid/src/rendering-util/rendering-elements/nodes.spec.ts packages/mermaid/src/rendering-util/rendering-elements/edges.spec.js packages/mermaid/src/rendering-util/rendering-elements/edgeMarker.spec.ts packages/mermaid/src/rendering-util/layout-algorithms/dagre/index.spec.js packages/mermaid/src/rendering-util/layout-algorithms/dagre/mermaid-graphlib.spec.js
• git diff --check
• npx fallow audit --changed-since origin/codex/common-render-pipeline-2-dagre --format json

[changeset-bot]

🦋 Changeset detected

Latest commit: 4a0e4bb

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

This PR includes changesets to release 1 package

Name	Type
mermaid	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

[codecov]

Codecov Report

❌ Patch coverage is 0.41494% with 240 lines in your changes missing coverage. Please review.
✅ Project coverage is 2.93%. Comparing base (95327b3) to head (4a0e4bb).

Files with missing lines	Patch %	Lines
...rc/rendering-util/layout-algorithms/dagre/index.js	0.00%	170 Missing ⚠️
packages/mermaid/src/rendering-util/createGraph.ts	0.00%	40 Missing ⚠️
...c/rendering-util/layout-algorithms/common/index.ts	0.00%	30 Missing ⚠️

Additional details and impacted files

@@                           Coverage Diff                           @@
##           codex/common-render-pipeline-2-dagre   #7808      +/-   ##
=======================================================================
- Coverage                                  2.94%   2.93%   -0.01%     
=======================================================================
  Files                                       655     655              
  Lines                                     69675   69837     +162     
  Branches                                    978     978              
=======================================================================
+ Hits                                       2052    2053       +1     
- Misses                                    67623   67784     +161     

Flag	Coverage Δ
unit	2.93% <0.41%> (-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/defaultConfig.ts	49.39% <100.00%> (+0.20%)	⬆️
...c/rendering-util/layout-algorithms/common/index.ts	0.44% <0.00%> (-0.04%)	⬇️
packages/mermaid/src/rendering-util/createGraph.ts	1.14% <0.00%> (-0.39%)	⬇️
...rc/rendering-util/layout-algorithms/dagre/index.js	0.00% <0.00%> (ø)

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

[argos-ci]

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

Build	Status	Details	Updated (UTC)
default (Inspect)	👍 Changes approved	156 changed, 1 added	Jun 15, 2026, 8:35 AM

[knsv-bot]

[sisyphus-bot]

Review of Common Render pipeline 3: DAGRE phase 3 (stacked on codex/common-render-pipeline-2-dagre).

Verdict: APPROVE — no blocking or important findings, nits and suggestions only. (Posted as a comment because GitHub does not allow the authoring account to formally approve its own PR.)

What's working well

• 🎉 [praise] The contract test in dagre/index.spec.js is exactly right for this refactor — asserting node('A').x is undefined after measureDagreLayout and a number only after runDagreLayoutCore proves the phase boundary instead of just exercising it. The companion test for the fail-fast error closes the loop.
• 🎉 [praise] Converting recursiveRender's six positional args into a named-options object (renderDagreSubgraph) is a real readability win, and the explicit throw in runDagreLayoutCore with a clear message beats the silent misbehavior we'd otherwise get from running layout on an unmeasured graph.
• 🎉 [praise] Full CI green including all five e2e shards and an approved Argos build — for a rendering-util/ change that's the signal that matters. Verified separately that nothing outside dagre/index.js imports the renamed/re-contracted exports, and recursion correctly stays local to DAGRE as described.

Security

XSS sub-agent review: no XSS or injection issues identified. The dagre change is a pure restructure (no new string-to-DOM paths; all DOM insertion still flows through the existing insertNode/insertEdge* helpers behind the unchanged sanitization boundary). The new usesLegacyFontAwesomeSyntax regex is linear-time (no ReDoS) and its output only gates a hardcoded CDN URL.

Things to address (all non-blocking)

• 💡 [suggestion] measureDagreLayout passes its result to the core phase by mutating prepared.measuredLayout (dagre/index.js:656) — a hidden side channel that exists only because runLayoutCore receives CommonLayoutRenderContext, which has no measure field, even though the common renderer computes measure immediately before calling it (common/index.ts:124-125). For a follow-up phase PR, consider extending the core-phase context with the measure result so the dependency is explicit in the contract and the runtime throw becomes structurally unnecessary. Fine as-is for this PR since changing the shared contract deserves its own change.
• 🟢 [nit] paintDagreLayout's coreResult ?? measure fallback (dagre/index.js:672): runDagreLayoutCore always returns the measured layout or throws, so the fallback is unreachable through the renderer — and if paint ever did run without the core phase, painting an un-laid-out graph (no x/y) should fail loudly rather than proceed. Worth dropping the ?? measure.
• 🟢 [nit] In measureDagreLayout (dagre/index.js:645-657), if preparedLayout is absent the measure is stashed on a locally created prepared that nothing can reach, so runDagreLayoutCore would throw even though measurement ran. Unreachable today (prepareLayout is always registered), but a short comment noting the assumption would help the next reader.
• 🟢 [nit] cypress/platform/viewer.js:19 — the legacy FontAwesome pattern covers every syntax variant currently in the suite (fa:/fab: prefixes and ::icon(fa fa-book)), which I verified by grepping the specs. One gap: ::icon(fab fa-x) would not match (fa\b doesn't match inside fab, and the first alternative requires a colon), so a future test using that form would silently render without the FA font. Consider widening to ::icon\(\s*fa[bklrs]?\b. Also, an explicit typeof code === 'string' guard would make the recursion robust to arbitrary ?graph= JSON shapes.
• 💡 [suggestion] The two cypress changes (conditional FA CDN loading, 15s rendered timeout) are test-infra hardening unrelated to the DAGRE phase split. Totally reasonable to keep the stack's CI stable, but a sentence in the PR description explaining why they ride along would help future archaeology.

The missing changeset is correct here — internal refactor with no user-facing behavior change, so the changeset-bot warning can be ignored.

Nice, clean step in the pipeline series — happy to see this one land. 🚀