From a4a250b96321e0648eecfbadbfb17b1537dff691 Mon Sep 17 00:00:00 2001 From: Mukul Kumar Yadav Date: Sat, 16 May 2026 14:00:36 +0530 Subject: [PATCH 1/2] fix(venn): synthesize implied pairwise subsets for higher-arity unions The underlying venn.js layout requires explicit pairwise intersection sizes to make circles overlap. When a user declares only a higher-arity union (for example, `union A,B,C[label]`) without the underlying pairwise unions, the layout has no overlap constraints, the circles render disjointly, and there is no intersection region for the label. Before passing subsets to venn.js, synthesize any missing pairwise subsets implied by higher-arity unions. User-declared subsets keep their sizes, labels, and styles; only the missing implied pairs are filled in, with a default size. Adds: - expandImpliedSubsets helper in vennRenderer.ts (exported for tests) - unit tests covering no-op, 3-way, 4-way, and partial-overlap cases - a render-level jsdom test verifying the label renders for the reporter's exact input - Cypress visual tests for bare 3-way and 4-way unions Fixes #7656 --- .changeset/venn-implied-pairwise-subsets.md | 5 + .../integration/rendering/venn/venn.spec.ts | 25 ++++ .../src/diagrams/venn/vennRenderer.spec.ts | 112 +++++++++++++++++- .../mermaid/src/diagrams/venn/vennRenderer.ts | 31 ++++- 4 files changed, 171 insertions(+), 2 deletions(-) create mode 100644 .changeset/venn-implied-pairwise-subsets.md diff --git a/.changeset/venn-implied-pairwise-subsets.md b/.changeset/venn-implied-pairwise-subsets.md new file mode 100644 index 00000000000..38b683b9397 --- /dev/null +++ b/.changeset/venn-implied-pairwise-subsets.md @@ -0,0 +1,5 @@ +--- +'mermaid': patch +--- + +fix(venn): render labeled higher-arity unions when the underlying pairwise unions are not declared. The venn.js layout needs pairwise intersection sizes to make circles overlap, so `union A,B,C[label]` on its own previously rendered as three disjoint circles with no intersection region for the label. `vennRenderer` now synthesizes the missing pairwise subsets from any higher-arity union before passing the data to venn.js. User-declared subsets keep their sizes, labels, and styles. Resolves #7656. diff --git a/cypress/integration/rendering/venn/venn.spec.ts b/cypress/integration/rendering/venn/venn.spec.ts index a7e1127b1ac..4ab08607b58 100644 --- a/cypress/integration/rendering/venn/venn.spec.ts +++ b/cypress/integration/rendering/venn/venn.spec.ts @@ -237,4 +237,29 @@ describe('Venn Diagram', () => { { look: 'handDrawn', handDrawnSeed: 1, fontFamily: 'courier' } ); }); + + it('17: should render a three-set venn with only a labeled 3-way union (issue #7656)', () => { + imgSnapshotTest( + `venn-beta + title Innovation + set Desirable + set Feasible + set Viable + union Desirable, Feasible, Viable["Innovation"] + ` + ); + }); + + it('18: should render a four-set venn with only a 4-way union (issue #7656)', () => { + imgSnapshotTest( + `venn-beta + title Four sets + set A + set B + set C + set D + union A, B, C, D["AllFour"] + ` + ); + }); }); diff --git a/packages/mermaid/src/diagrams/venn/vennRenderer.spec.ts b/packages/mermaid/src/diagrams/venn/vennRenderer.spec.ts index 9dc6564baf1..ca3671e48c4 100644 --- a/packages/mermaid/src/diagrams/venn/vennRenderer.spec.ts +++ b/packages/mermaid/src/diagrams/venn/vennRenderer.spec.ts @@ -1,6 +1,7 @@ import { describe, expect, it, vi } from 'vitest'; -import { draw } from './vennRenderer.js'; +import { draw, expandImpliedSubsets } from './vennRenderer.js'; import type { Diagram } from '../../Diagram.js'; +import type { VennData } from './vennTypes.js'; import * as configModule from '../../config.js'; const createDiagram = (overrides: Partial> = {}) => { @@ -163,4 +164,113 @@ describe('vennRenderer', () => { const debugCircle = document.querySelector('.venn-text-debug-circle'); expect(debugCircle).not.toBeNull(); }); + + it('renders label for 3-way union without explicit pairwise unions (issue #7656)', async () => { + document.body.innerHTML = ''; + const diagram = createDiagram({ + getSubsetData: () => [ + { sets: ['A'], size: 10, label: undefined }, + { sets: ['B'], size: 10, label: undefined }, + { sets: ['C'], size: 10, label: undefined }, + { sets: ['A', 'B', 'C'], size: 1, label: 'Innovation' }, + ], + }); + + await draw('', 'venn', '1.0', diagram); + + const intersectionTexts = [...document.querySelectorAll('.venn-intersection text')]; + const labels = intersectionTexts.map((el) => el.textContent); + expect(labels).toContain('Innovation'); + }); +}); + +describe('expandImpliedSubsets', () => { + it('returns input unchanged when only singleton and pairwise subsets are present', () => { + const input: VennData[] = [ + { sets: ['A'], size: 10, label: undefined }, + { sets: ['B'], size: 10, label: undefined }, + { sets: ['A', 'B'], size: 2.5, label: 'AB' }, + ]; + expect(expandImpliedSubsets(input)).toBe(input); + }); + + it('returns input unchanged when 3-way union has all pairwise unions declared', () => { + const input: VennData[] = [ + { sets: ['A'], size: 10, label: undefined }, + { sets: ['B'], size: 10, label: undefined }, + { sets: ['C'], size: 10, label: undefined }, + { sets: ['A', 'B'], size: 2.5, label: undefined }, + { sets: ['A', 'C'], size: 2.5, label: undefined }, + { sets: ['B', 'C'], size: 2.5, label: undefined }, + { sets: ['A', 'B', 'C'], size: 1, label: 'ABC' }, + ]; + const result = expandImpliedSubsets(input); + expect(result).toHaveLength(input.length); + }); + + it('synthesizes missing pairwise subsets for a bare 3-way union', () => { + const input: VennData[] = [ + { sets: ['A'], size: 10, label: undefined }, + { sets: ['B'], size: 10, label: undefined }, + { sets: ['C'], size: 10, label: undefined }, + { sets: ['A', 'B', 'C'], size: 1, label: 'Innovation' }, + ]; + const result = expandImpliedSubsets(input); + const pairKeys = result + .filter((entry) => entry.sets.length === 2) + .map((entry) => entry.sets.join('|')) + .sort(); + expect(pairKeys).toEqual(['A|B', 'A|C', 'B|C']); + }); + + it('preserves user-declared pairwise subsets and only fills in missing ones', () => { + const input: VennData[] = [ + { sets: ['A'], size: 10, label: undefined }, + { sets: ['B'], size: 10, label: undefined }, + { sets: ['C'], size: 10, label: undefined }, + { sets: ['A', 'B'], size: 5, label: 'AB' }, + { sets: ['A', 'B', 'C'], size: 1, label: 'ABC' }, + ]; + const result = expandImpliedSubsets(input); + + const ab = result.find((entry) => entry.sets.join('|') === 'A|B'); + expect(ab).toEqual({ sets: ['A', 'B'], size: 5, label: 'AB' }); + + const synthesized = result + .filter((entry) => entry.label === undefined && entry.sets.length === 2) + .map((entry) => entry.sets.join('|')) + .sort(); + expect(synthesized).toEqual(['A|C', 'B|C']); + }); + + it('synthesizes C(N,2) pairs for a bare 4-way union', () => { + const input: VennData[] = [ + { sets: ['A'], size: 10, label: undefined }, + { sets: ['B'], size: 10, label: undefined }, + { sets: ['C'], size: 10, label: undefined }, + { sets: ['D'], size: 10, label: undefined }, + { sets: ['A', 'B', 'C', 'D'], size: 1, label: 'AllFour' }, + ]; + const result = expandImpliedSubsets(input); + const pairKeys = result + .filter((entry) => entry.sets.length === 2) + .map((entry) => entry.sets.join('|')) + .sort(); + expect(pairKeys).toEqual(['A|B', 'A|C', 'A|D', 'B|C', 'B|D', 'C|D']); + }); + + it('synthesized pairs have a default size and undefined label', () => { + const input: VennData[] = [ + { sets: ['A'], size: 10, label: undefined }, + { sets: ['B'], size: 10, label: undefined }, + { sets: ['C'], size: 10, label: undefined }, + { sets: ['A', 'B', 'C'], size: 1, label: undefined }, + ]; + const result = expandImpliedSubsets(input); + const synthesized = result.filter((entry) => entry.sets.length === 2); + for (const entry of synthesized) { + expect(entry.label).toBeUndefined(); + expect(entry.size).toBeGreaterThan(0); + } + }); }); diff --git a/packages/mermaid/src/diagrams/venn/vennRenderer.ts b/packages/mermaid/src/diagrams/venn/vennRenderer.ts index 3a02362fcee..b1a04897fc9 100644 --- a/packages/mermaid/src/diagrams/venn/vennRenderer.ts +++ b/packages/mermaid/src/diagrams/venn/vennRenderer.ts @@ -28,6 +28,35 @@ function buildStyleByKey(styleData: VennStyleData[]): Map s.sets.join('|'))); + const implied: VennData[] = []; + const defaultPairSize = 10 / 4; + for (const subset of subsets) { + if (subset.sets.length < 3) { + continue; + } + for (let i = 0; i < subset.sets.length; i++) { + for (let j = i + 1; j < subset.sets.length; j++) { + const pair = [subset.sets[i], subset.sets[j]].sort(); + const key = pair.join('|'); + if (existingKeys.has(key)) { + continue; + } + existingKeys.add(key); + implied.push({ sets: pair, size: defaultPairSize, label: undefined }); + } + } + } + return implied.length > 0 ? [...subsets, ...implied] : subsets; +} + export const draw: DrawDefinition = ( _text: string, id: string, @@ -49,7 +78,7 @@ export const draw: DrawDefinition = ( themeVariables.venn8, ].filter(Boolean); const title = db.getDiagramTitle?.(); - const sets = db.getSubsetData(); + const sets = expandImpliedSubsets(db.getSubsetData()); const textNodes = db.getTextData(); const styleByKey = buildStyleByKey(db.getStyleData()); From 4e51e9783ee3721906cc14b608abc3b5c36a40b0 Mon Sep 17 00:00:00 2001 From: Mukul Kumar Yadav Date: Mon, 18 May 2026 12:39:56 +0530 Subject: [PATCH 2/2] test(venn): strengthen jsdom render guard for issue #7656 fix Address review feedback on PR #7758. - vennRenderer.spec.ts: the existing jsdom render test for issue #7656 only asserted that the "Innovation" label appeared in the DOM, which venn.js emits for any declared labeled union regardless of whether the circles actually overlap. The test passed even with expandImpliedSubsets bypassed. Replace it with an assertion on the rendered geometry: the three implied pairwise intersections (A-B, A-C, B-C) must be present in addition to the user-declared A-B-C intersection, and the 3-way intersection path must not be the degenerate "M 0 0" placeholder venn.js emits when an area has no visible geometry. Verified to fail when the fix is bypassed. - vennRenderer.ts: extract the synthesized-pair default size into a defaultSubsetSize(arity) helper using the same expression as vennDB.addSubsetData (10 / arity^2), and add a comment so the two defaults stay in sync. - docs/syntax/venn.md: add a "Higher-arity unions" section with a 3-set Desirable/Feasible/Viable Innovation example, since bare 3-way unions now render correctly. --- docs/syntax/venn.md | 22 ++++++++++++ .../src/diagrams/venn/vennRenderer.spec.ts | 35 ++++++++++++++++--- .../mermaid/src/diagrams/venn/vennRenderer.ts | 9 ++++- packages/mermaid/src/docs/syntax/venn.md | 14 ++++++++ 4 files changed, 75 insertions(+), 5 deletions(-) diff --git a/docs/syntax/venn.md b/docs/syntax/venn.md index 5eaf66dc96d..518f561dea4 100644 --- a/docs/syntax/venn.md +++ b/docs/syntax/venn.md @@ -53,6 +53,28 @@ venn-beta union A,B["AB"] ``` +### Higher-arity unions + +`union` accepts three or more set names. The diagram renders the implied +pairwise overlaps automatically, so the label on the higher-arity union has a +visible region to sit in: + +```mermaid-example +venn-beta + set Desirable + set Feasible + set Viable + union Desirable,Feasible,Viable["Innovation"] +``` + +```mermaid +venn-beta + set Desirable + set Feasible + set Viable + union Desirable,Feasible,Viable["Innovation"] +``` + ### Sizes Use `:N` suffix to set the size of a set or union: diff --git a/packages/mermaid/src/diagrams/venn/vennRenderer.spec.ts b/packages/mermaid/src/diagrams/venn/vennRenderer.spec.ts index ca3671e48c4..69bed7d60d4 100644 --- a/packages/mermaid/src/diagrams/venn/vennRenderer.spec.ts +++ b/packages/mermaid/src/diagrams/venn/vennRenderer.spec.ts @@ -165,7 +165,7 @@ describe('vennRenderer', () => { expect(debugCircle).not.toBeNull(); }); - it('renders label for 3-way union without explicit pairwise unions (issue #7656)', async () => { + it('renders an overlapping layout for a bare 3-way union (issue #7656)', async () => { document.body.innerHTML = ''; const diagram = createDiagram({ getSubsetData: () => [ @@ -178,9 +178,36 @@ describe('vennRenderer', () => { await draw('', 'venn', '1.0', diagram); - const intersectionTexts = [...document.querySelectorAll('.venn-intersection text')]; - const labels = intersectionTexts.map((el) => el.textContent); - expect(labels).toContain('Innovation'); + // The label being present is necessary but not sufficient: venn.js emits a + // `.venn-intersection text` element for any declared labeled union, even + // when the circles do not actually overlap. + const intersectionLabels = [...document.querySelectorAll('.venn-intersection text')].map( + (el) => el.textContent + ); + expect(intersectionLabels).toContain('Innovation'); + + // What the fix actually guarantees is that the layout produces overlapping + // circles, which is visible in two complementary ways in the DOM: + // 1. venn.js renders the three implied pairwise intersections, in addition + // to the user-declared 3-way intersection. + // 2. The 3-way intersection's SVG path is a real region, not the + // degenerate `"M 0 0"` placeholder venn.js emits when an area has no + // visible geometry on screen. + const intersections = [...document.querySelectorAll('.venn-intersection')]; + const setsByPath = new Map(); + for (const node of intersections) { + const data = (node as unknown as { __data__?: { sets?: string[] } }).__data__; + const sets = data?.sets ?? []; + if (sets.length >= 2) { + const key = [...sets].sort().join('|'); + setsByPath.set(key, node.querySelector('path')?.getAttribute('d') ?? null); + } + } + expect([...setsByPath.keys()].sort()).toEqual(['A|B', 'A|B|C', 'A|C', 'B|C']); + const threeWayPath = setsByPath.get('A|B|C'); + expect(threeWayPath).toBeTruthy(); + expect(threeWayPath).not.toBe('M 0 0'); + expect((threeWayPath ?? '').length).toBeGreaterThan(20); }); }); diff --git a/packages/mermaid/src/diagrams/venn/vennRenderer.ts b/packages/mermaid/src/diagrams/venn/vennRenderer.ts index b1a04897fc9..9d89a758c54 100644 --- a/packages/mermaid/src/diagrams/venn/vennRenderer.ts +++ b/packages/mermaid/src/diagrams/venn/vennRenderer.ts @@ -34,10 +34,17 @@ function buildStyleByKey(styleData: VennStyleData[]): Map 10 / Math.pow(arity, 2); + export function expandImpliedSubsets(subsets: VennData[]): VennData[] { const existingKeys = new Set(subsets.map((s) => s.sets.join('|'))); const implied: VennData[] = []; - const defaultPairSize = 10 / 4; + const defaultPairSize = defaultSubsetSize(2); for (const subset of subsets) { if (subset.sets.length < 3) { continue; diff --git a/packages/mermaid/src/docs/syntax/venn.md b/packages/mermaid/src/docs/syntax/venn.md index 8e294c36c09..e0e424673de 100644 --- a/packages/mermaid/src/docs/syntax/venn.md +++ b/packages/mermaid/src/docs/syntax/venn.md @@ -32,6 +32,20 @@ venn-beta union A,B["AB"] ``` +### Higher-arity unions + +`union` accepts three or more set names. The diagram renders the implied +pairwise overlaps automatically, so the label on the higher-arity union has a +visible region to sit in: + +```mermaid-example +venn-beta + set Desirable + set Feasible + set Viable + union Desirable,Feasible,Viable["Innovation"] +``` + ### Sizes Use `:N` suffix to set the size of a set or union: