diff --git a/packages/diagram-server/src/server/source-model-storage.ts b/packages/diagram-server/src/server/source-model-storage.ts index cffd091..3f3149a 100644 --- a/packages/diagram-server/src/server/source-model-storage.ts +++ b/packages/diagram-server/src/server/source-model-storage.ts @@ -98,8 +98,38 @@ const RUN_OUTPUT_FALLBACK_MAX_RESULTS = 50; * a diagram they can read, the result is persisted immediately so it happens * once, and this failure is at least visible where the other was silent. */ -export function savedLayoutIsUsable(hasSavedPositions: boolean, unpositionedNodeCount: number): boolean { - return hasSavedPositions && unpositionedNodeCount === 0; +export function savedLayoutIsUsable( + hasSavedPositions: boolean, + unpositionedNodeCount: number, + coincidentNodeCount = 0 +): boolean { + return hasSavedPositions && unpositionedNodeCount === 0 && coincidentNodeCount === 0; +} + +/** + * How many nodes share a position with another node. + * + * Covering every node turned out not to be enough. A layout captured before its + * nodes were ever placed names all of them and puts several at the same point — + * the origin, usually, since that is where a node starts. It passes a coverage + * check, suppresses the layout run, and is restored faithfully on every open, + * so the diagram opens stacked forever and only a layout by hand fixes it. + * + * Two nodes at the same coordinates is the reliable signature. No layout engine + * ever produces it, and nobody drags one node exactly on top of another, so it + * means the positions were recorded rather than computed. + */ +export function countCoincidentNodes(positions: Iterable<{ x: number; y: number }>): number { + const seen = new Set(); + let coincident = 0; + for (const { x, y } of positions) { + const key = `${x},${y}`; + if (seen.has(key)) { + coincident++; + } + seen.add(key); + } + return coincident; } /** @@ -484,7 +514,8 @@ export class WorkflowSourceModelStorage implements SourceModelStorage { const hasPersistedLayout = savedLayoutIsUsable( loadedLayoutPositions !== undefined, - unpositionedNodeCount + unpositionedNodeCount, + countCoincidentNodes(loadedLayoutPositions?.values() ?? []) ); const hasPersistedEdgeRoutesEffective = hasPersistedLayout && hasPersistedEdgeRoutes; diff --git a/packages/diagram-server/test/saved-layout-coverage.test.ts b/packages/diagram-server/test/saved-layout-coverage.test.ts index 125094a..cbbd4a0 100644 --- a/packages/diagram-server/test/saved-layout-coverage.test.ts +++ b/packages/diagram-server/test/saved-layout-coverage.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { savedLayoutIsUsable } from '../src/server/source-model-storage'; +import { countCoincidentNodes, savedLayoutIsUsable } from '../src/server/source-model-storage'; /** * When a saved layout may stand, and when the diagram must be laid out again. @@ -42,4 +42,52 @@ describe('savedLayoutIsUsable', () => { // by a different runtime names nothing in this graph. expect(savedLayoutIsUsable(true, 9)).toBe(false); }); + + it('is false when the layout stacks nodes on one another', () => { + // Covering every node is not enough. A layout captured before its nodes + // were placed names all of them and puts several at the same point, so + // it passes coverage, suppresses the run, and is restored faithfully + // every time — the diagram opens stacked forever, and only a layout by + // hand ever fixes it. + expect(savedLayoutIsUsable(true, 0, 1)).toBe(false); + expect(savedLayoutIsUsable(true, 0, 0)).toBe(true); + }); +}); + +describe('countCoincidentNodes', () => { + it('counts a node sharing a position with another', () => { + expect( + countCoincidentNodes([ + { x: 0, y: 0 }, + { x: 0, y: 0 }, + { x: 10, y: 4 } + ]) + ).toBe(1); + }); + + it('is zero for a layout that placed everything apart', () => { + expect( + countCoincidentNodes([ + { x: 0, y: 0 }, + { x: 10, y: 0 }, + { x: 0, y: 10 } + ]) + ).toBe(0); + }); + + it('is zero for nothing at all', () => { + // No saved positions is a different case, already handled by the + // coverage check, and must not be mistaken for a stacked layout. + expect(countCoincidentNodes([])).toBe(0); + }); + + it('counts every extra at a shared point, not just the pair', () => { + expect( + countCoincidentNodes([ + { x: 5, y: 5 }, + { x: 5, y: 5 }, + { x: 5, y: 5 } + ]) + ).toBe(2); + }); });