Feature/2833 add randomize config option for architecture diagrams#7457
Conversation
…zation demo - Re-enabled tests for architecture diagrams, ensuring deterministic layouts. - Added a new test for complex deeply-nested diagrams. - Introduced a new HTML demo for testing architecture randomization configurations.
…layout - Introduced a new `randomize` configuration option to allow random initial node positions in architecture diagrams. - Updated the configuration schema and types to include the new option, defaulting to `false` for deterministic layouts. - Modified the architecture renderer to utilize the `randomize` setting during layout calculations.
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🦋 Changeset detectedLatest commit: 3f4df67 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
@mermaid-js/examples
mermaid
@mermaid-js/layout-elk
@mermaid-js/layout-tidy-tree
@mermaid-js/mermaid-zenuml
@mermaid-js/parser
@mermaid-js/tiny
commit: |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #7457 +/- ##
==========================================
- Coverage 3.53% 3.53% -0.01%
==========================================
Files 490 495 +5
Lines 48968 49213 +245
Branches 766 772 +6
==========================================
+ Hits 1733 1741 +8
- Misses 47235 47472 +237
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
- Updated the syntax for connections and labels in architecture diagram tests for consistency. - Added a new test case to validate rendering with the `randomize` option set to true, ensuring no errors occur during the process.
|
Thanks, it makes sense! I will let my assistant do a review. I am trimming it in, hoping for greatness. It is ok to comment on the review if it comes out badly. I am reviewing it as well :) Btw, spontaneously, I would also want the ability to set the number. This is to make it behave deterministically while still allowing it to be affected. |
knsv-bot
left a comment
knsv-bot
left a comment
There was a problem hiding this comment.
[sisyphus-bot]
Review: PR #7457 — Add randomize config option for architecture diagrams
Thanks for this contribution, @jeroensmink98! Non-deterministic layouts have been a long-standing pain point for architecture diagrams (and the reason the E2E tests were skipped), so this is a really valuable fix. The approach is clean and well-considered — let's get it across the finish line.
What's working well
🎉 [praise] Excellent design decision to follow the schema-first config workflow. Adding randomize to config.schema.yaml and letting it flow through getConfigField() is exactly the right pattern — no manual wiring needed in architectureDb.ts or defaultConfig.ts.
🎉 [praise] Un-skipping the architecture E2E tests is a huge win. These have been describe.skip'd for a long time. Updating the edge syntax to the current format (db:L -- R:server instead of db L--R server) while you're at it is great housekeeping.
🎉 [praise] The screenshot: false option on the randomize: true E2E test is smart — it verifies the diagram renders without errors while avoiding flaky snapshot comparisons from the non-deterministic layout.
🎉 [praise] The demo page (architecture-randomize.html) with side-by-side deterministic vs randomized diagrams is a nice touch for manual verification. Screenshots in the PR body clearly demonstrate the fix.
Things to address
🟡 [important] Changeset bump vs prefix mismatch — The changeset uses patch bump but the message starts with feat:. Per project conventions, features should use minor and fixes should use patch. This change is arguably both — it fixes non-deterministic behavior (#2833) while adding a new config option. I'd suggest either:
- Change to
minorbump withfeat:prefix (since it adds a new public config key), or - Keep
patchbut change the prefix tofix:(since the primary value is deterministic layouts)
Either is defensible — just make them consistent. File: .changeset/fix-architecture-deterministic-layout.md
🟡 [important] Documentation is missing — The PR template checkbox for documentation is unchecked. Since randomize is a new user-facing config option, it would be great to add a brief mention in the architecture diagram docs (packages/mermaid/src/docs/syntax/architecture.md or equivalent). Just a note in the configuration section explaining the option and that layouts are deterministic by default. Remember to use the MERMAID_RELEASE_VERSION placeholder for the version.
🟢 [nit] Generated file committed directly — config.type.ts appears in the diff with 7 added lines. It looks like the regeneration was run correctly (the output matches the schema), but worth confirming this was generated via pnpm run --filter mermaid types:build-config rather than hand-edited. CI's types:verify-config check should catch any mismatch, so this is just a verification note.
💡 [suggestion] Consider adding randomize to the required array — The existing architecture config fields (padding, iconSize, fontSize) are listed in the required array in the schema (config.schema.yaml:965). The new randomize field is not in that list. While it works fine with a default, adding it to required would be consistent with the sibling fields. Not blocking — the default value ensures it works either way.
Security
No XSS or injection issues identified. The change passes a schema-validated boolean (true/false) directly to the fcose layout engine's randomize option. No user-controlled strings are introduced into DOM sinks, SVG output, or sanitization paths.
Self-check
- At least one 🎉 [praise] item exists
- No duplicate comments
- Severity tally: 🔴 0 / 🟡 2 / 🟢 1 / 💡 1 / 🎉 4
- Verdict matches criteria: COMMENT (1-2 🟡)
- Not a draft PR — COMMENT is appropriate
- No inline comments used
- Tone check: constructive and collaborative ✓
…in architecture diagrams - Introduced a new section in the architecture documentation detailing the `randomize` option for initial node positions. - Provided examples for enabling randomization via frontmatter and `mermaid.initialize()`. - Updated the configuration table to include the new option and its default behavior.
|
Thanks for the feedback! sofar I have made the following changes
|
3 participants
Summary
Add a
randomizeconfig option to architecture diagrams (architecture-beta) that controls whether initial node positions are randomized before running the fcose layout algorithm.The
cytoscape-fcoselibrary defaultsrandomizetotrue, which causes architecture diagrams to produce a different layout on every render. This change defaultsrandomizetofalse, making layouts deterministic and identical across renders. Users can opt into randomized layout by settingrandomize: truevia frontmatter ormermaid.initialize().Resolves #2833
Resolves #6167
Design Decisions
randomizeboolean is added toArchitectureDiagramConfiginconfig.schema.yaml, following the existing pattern used bypadding,iconSize, andfontSize. The TypeScript types inconfig.type.tsare auto-generated from the schema.defaultConfig.tsorarchitectureDb.ts: The default value (false) is defined in the schema and flows through the existinggetConfigField()mechanism, so no manual wiring in the DB or default config is needed.quality: 'proof'compatibility: The renderer already setsquality: 'proof', which is required whenrandomize: false(fcose logs a warning if quality is'draft'withrandomize: false).describe.skipprecisely because layouts were non-deterministic. With this fix, they are re-enabled and include a new complex deeply-nested diagram test to guard against regressions.demos/dev/architecture-randomize.htmlwith two versions of the same complex diagram (one deterministic, one randomized) for manual verification.Tasks
Make sure you
MERMAID_RELEASE_VERSIONis used for all new features.pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Screenshots
The screenshots below show the output of
demos/dev/architecture-randomize.html:randomize: false, default): The layout stays the same after a page refresh.randomize: true): The layout changes after each refresh.Screenshot 1:
Screenshot 2:
Screenshot 3:
This shows that the default configuration produces deterministic layouts, while
randomize: truerestores the previous randomized behavior for users who want it.