fix(sequence): correct messageAlign label position for right-to-left arrows in sequence diagrams

[ekiauhce]

📑 Summary

Resolves #3594

• Root cause: textObj.x = startx and textObj.width = stopx - startx break when a message goes right-to-left, because width becomes negative and x is set to the right edge.
• Use Math.min(startx, stopx) for x and Math.abs(stopx - startx) for width, so all three alignment modes (left, center, right) compute correct positions regardless of arrow direction.

Added unit tests and tested fix locally

📋 Tasks

Make sure you

• 📖 have read the contribution guidelines
• 💻 have added necessary unit/e2e tests.
• 📓 have added documentation. Make sure MERMAID_RELEASE_VERSION is used for all new features.
• 🦋 If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

[changeset-bot]

🦋 Changeset detected

Latest commit: ab4e28d

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

This PR includes changesets to release 1 package

Name	Type
mermaid	Patch

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	ab4e28d
🔍 Latest deploy log	https://app.netlify.com/projects/mermaid-js/deploys/69e7f534c9cb910008d3e933
😎 Deploy Preview	https://deploy-preview-7632--mermaid-js.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

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@7632

mermaid

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

@mermaid-js/layout-elk

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

@mermaid-js/layout-tidy-tree

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

@mermaid-js/mermaid-zenuml

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

@mermaid-js/parser

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

@mermaid-js/tiny

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

commit: ab4e28d

[codecov]

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 3.31%. Comparing base (1bf8f4d) to head (ab4e28d).
⚠️ Report is 83 commits behind head on develop.

Files with missing lines	Patch %	Lines
.../mermaid/src/diagrams/sequence/sequenceRenderer.ts	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           develop   #7632      +/-   ##
==========================================
- Coverage     3.33%   3.31%   -0.02%     
==========================================
  Files          538     540       +2     
  Lines        56366   56699     +333     
  Branches       822     824       +2     
==========================================
+ Hits          1878    1880       +2     
- Misses       54488   54819     +331     

Flag	Coverage Δ
unit	3.31% <0.00%> (-0.02%)	⬇️

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

Files with missing lines	Coverage Δ
.../mermaid/src/diagrams/sequence/sequenceRenderer.ts	0.05% <0.00%> (ø)

... and 6 files with indirect coverage changes

🚀 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	3 changed	Apr 21, 2026, 10:19 PM

[knsv-bot]

[sisyphus-bot]

Thanks @ekiauhce — this is a really nicely done bug fix. The root-cause explanation in the PR body points directly at the problem ("width becomes negative and x is set to the right edge"), the fix is a surgical 2-line change, and the changeset is in place. 🎉

One important thing came up when I reviewed the tests:

🟡 [important] — the new tests don't actually exercise the fix

The three cases under describe('messageAlign anchor positioning') in svgDraw.spec.js:167-218 call svgDraw.drawText() directly with hardcoded positive x=100, width=200 and check that it produces the right anchor positions for 'left', 'center', and 'right'. But the bug isn't in drawText — it's in drawMessage's construction of textObj.x and textObj.width (sequenceRenderer.ts:471-473). drawText's anchor math is correct when given positive x/width, both before and after your fix.

I verified this locally by checking out this branch, reverting sequenceRenderer.ts to the pre-fix state, and re-running the tests: all 3 pass. So as regression tests for #3594 they wouldn't catch a regression of the exact bug you fixed.

Two directions that would close the gap, either is fine:

1. Keep the drawText tests but add a separate drawMessage test that spies on (or wraps) drawText and asserts that for a msgModel with startx > stopx, the textObj.x passed in is min(startx, stopx) and textObj.width is abs(stopx - startx).
2. Or rely on the e2e path — see the suggestion below — and update the comment // Covers issue #3594 so future readers aren't misled.

💡 [suggestion] — existing e2e snapshots likely already regress-test this

In cypress/integration/rendering/sequence/sequencediagram.spec.js:333-352 there are two tests that render Alice->>Bob + Bob->>Alice with messageAlign: 'left' and 'right'. The Bob->>Alice line hits the exact reverse-arrow path you fixed, so those existing snapshots should diff in Argos when this PR runs — i.e., the e2e coverage already exists. Reviewers will just need to approve the visual diff as the intended fix. Worth mentioning in the PR description so nobody is surprised by "Argos changes detected."

💡 [suggestion] — follow-up cleanup opportunity

Out of scope for this PR, but drawKatex at svgDraw.js:123-127 does the same normalization via a temp-swap:

if (startx > stopx) {
  const temp = startx;
  startx = stopx;
  stopx = temp;
}

Your Math.min / Math.abs pattern is cleaner — worth folding into drawKatex in a follow-up PR to remove the duplication and keep the two paths consistent.

🎉 Other things that stood out as well-done

• Minimal and surgical — only two lines of renderer change, no speculative refactoring around it.
• Forward-arrow behavior provably unchanged: min(startx, stopx) === startx and abs(stopx - startx) === stopx - startx when startx <= stopx. So no risk of a regression for the common case.
• Self-message case (startx == stopx) also unchanged: min returns startx, abs returns 0.

Security summary

No security surface touched. The change is purely numeric coordinate normalization — no DOM sinks, no user-input handling, no attribute interpolation.

---

Verdict: COMMENT. The fix itself is good to go; just needs either a test that actually exercises the reverse-arrow path in drawMessage, or a comment tweak to make clear that these are drawText-documentation tests rather than regression tests for #3594. Either is a small change.

[ekiauhce]

@knsv, hi, resolved bot's comment about tests

[ashishjain0512]

[sisyphus-bot] — round 2

Thanks @ekiauhce for the quick turnaround on the test feedback. 🎉 The new sequenceRenderer.spec.js is exactly the right shape — it mocks svgDraw.drawText, calls the real drawMessage with startx=320, stopx=80 (the reverse-arrow case), and asserts on textObj.x and textObj.width. That's the construction site where the original bug lived.

Verification

I checked out the branch and confirmed:

• The new test passes against the fix as-is.
• I reverted sequenceRenderer.ts:471-473 to the pre-fix state and re-ran the test — it fails with expected 80, received 320, which is precisely the bug from #3594. So this is a real regression test now, not just a documentation test.
• The existing svgDraw.spec.js messageAlign anchor positioning cases (11 tests) still pass and remain useful as drawText documentation.
• Argos: 3 changes already reviewed and approved upstream — the e2e snapshots in cypress/integration/rendering/sequence/sequencediagram.spec.js covered this visually as expected.

🎉 Things that stood out as well-done

• The mock setup is tight: vi.mock on svgDraw.js preserves all real exports and only intercepts drawText, so the rest of drawMessage's control flow runs unmodified. That's the right boundary to mock at.
• Good filter on messageText calls (messageRenderer.spec.js:90) — drawMessage calls drawText multiple times (for sequence numbers, etc.), and asserting on the right call is what makes the test specific.
• Forward-arrow correctness still provable: min(startx, stopx) === startx and abs(stopx - startx) === stopx - startx when startx <= stopx, and the self-message case (startx === stopx) collapses to (startx, 0) — same as before. No regression risk for the common path.

Notes (non-blocking)

• Exporting drawMessage (sequenceRenderer.ts:460) is purely additive — nothing else in the codebase imports it, and sequenceRenderer.ts isn't re-exported from mermaid.ts/mermaidAPI.ts, so this isn't a public API change.
• The drawKatex cleanup in svgDraw.js:123-127 is still a nice follow-up, but that's out of scope here as previously noted.

Security

Unchanged from round 1: numeric coordinate normalization, no DOM sinks, no user-input-to-SVG path touched.

---

Verdict: APPROVE. The round 1 testing gap is closed and the fix is good to merge. Thanks for the careful work on this one.

[mermaid-bot]

@ekiauhce, Thank you for the contribution!
You are now eligible for a year of Premium account on MermaidChart.
Sign up with your GitHub account to activate.