fix(visualization): make nested A -> B arrow terminals normal to target border

editors/jsfcstm/src/diagram/elk-graph.ts

@@ -24,6 +24,112 @@ const LINE_HEIGHT = 18;
 const STATE_TITLE_HEIGHT = 30;
 const LEAF_MIN_WIDTH = 140;
 const LEAF_MIN_HEIGHT = 58;
+export const MIN_TERMINAL_SEGMENT = 18;
+export const MIN_SELF_LOOP_SEGMENT = 18;
+const GEOMETRY_EPSILON = 0.75;
+
+export interface FcstmElkPoint {
+    x: number;
+    y: number;
+}
+
+export interface FcstmElkNodeBox {
+    left: number;
+    right: number;
+    top: number;
+    bottom: number;
+}
+
+export interface FcstmElkLayoutGeometry {
+    boxes: Map<string, FcstmElkNodeBox>;
+    edgeOffsets: Map<string, FcstmElkPoint>;
+}
+
+function terminalSegmentLength(previous: FcstmElkPoint, end: FcstmElkPoint): number {
+    return Math.hypot(end.x - previous.x, end.y - previous.y);
+}
+
+/**
+ * Classify the final orthogonal edge segment entering a target node.
+ *
+ * Endpoints in a corner tolerance are rejected deliberately: a segment that
+ * arrives at a corner can be parallel to one border while visually belonging
+ * to the adjacent border, so it is not a reliable normal entry. Callers must
+ * provide sections produced by the configured ``ORTHOGONAL`` ELK router;
+ * polyline or spline sections are intentionally rejected as non-normal.
+ */
+export function terminalApproach(
+    previous: FcstmElkPoint,
+    end: FcstmElkPoint,
+    box: FcstmElkNodeBox
+): {side: 'top' | 'right' | 'bottom' | 'left'; length: number} | null {
+    const horizontal = Math.abs(previous.y - end.y) <= GEOMETRY_EPSILON;
+    const vertical = Math.abs(previous.x - end.x) <= GEOMETRY_EPSILON;
+    const nearLeft = Math.abs(end.x - box.left) <= GEOMETRY_EPSILON;
+    const nearRight = Math.abs(end.x - box.right) <= GEOMETRY_EPSILON;
+    const nearTop = Math.abs(end.y - box.top) <= GEOMETRY_EPSILON;
+    const nearBottom = Math.abs(end.y - box.bottom) <= GEOMETRY_EPSILON;
+    const withinX = end.x >= box.left - GEOMETRY_EPSILON && end.x <= box.right + GEOMETRY_EPSILON;
+    const withinY = end.y >= box.top - GEOMETRY_EPSILON && end.y <= box.bottom + GEOMETRY_EPSILON;
+    // A tolerance admits small ELK rounding drift outside the border, but an
+    // endpoint that has crossed into the node must never be accepted as a
+    // border hit merely because it is numerically close to that border.
+    const onTopBoundary = nearTop && end.y <= box.top;
+    const onRightBoundary = nearRight && end.x >= box.right;
+    const onBottomBoundary = nearBottom && end.y >= box.bottom;
+    const onLeftBoundary = nearLeft && end.x <= box.left;
+
+    if ((nearLeft || nearRight) && (nearTop || nearBottom)) {
+        return null;
+    }
+    if (vertical && withinX && onTopBoundary && previous.y < box.top - GEOMETRY_EPSILON) {
+        return {side: 'top', length: terminalSegmentLength(previous, end)};
+    }
+    if (horizontal && withinY && onRightBoundary && previous.x > box.right + GEOMETRY_EPSILON) {
+        return {side: 'right', length: terminalSegmentLength(previous, end)};
+    }
+    if (vertical && withinX && onBottomBoundary && previous.y > box.bottom + GEOMETRY_EPSILON) {
+        return {side: 'bottom', length: terminalSegmentLength(previous, end)};
+    }
+    if (horizontal && withinY && onLeftBoundary && previous.x < box.left - GEOMETRY_EPSILON) {
+        return {side: 'left', length: terminalSegmentLength(previous, end)};
+    }
+    return null;
+}
+
+/**
+ * Collect node boxes and edge-owner offsets in absolute canvas coordinates.
+ *
+ * ELK stores edge sections relative to the node that owns the edge. Keeping
+ * this traversal beside the graph builder gives tests and external geometry
+ * scanners one coordinate-normalization implementation to share.
+ */
+export function collectElkLayoutGeometry(graph: FcstmElkNode): FcstmElkLayoutGeometry {
+    const boxes = new Map<string, FcstmElkNodeBox>();
+    const edgeOffsets = new Map<string, FcstmElkPoint>();
+
+    function visit(node: FcstmElkNode, parentX = 0, parentY = 0): void {
+        const x = parentX + (node.x ?? 0);
+        const y = parentY + (node.y ?? 0);
+        if (node.fcstm?.kind !== 'canvas') {
+            boxes.set(node.id, {
+                left: x,
+                right: x + (node.width ?? 0),
+                top: y,
+                bottom: y + (node.height ?? 0),
+            });
+        }
+        for (const edge of node.edges || []) {
+            edgeOffsets.set(edge.id, {x, y});
+        }
+        for (const child of node.children || []) {
+            visit(child, x, y);
+        }
+    }
+
+    visit(graph);
+    return {boxes, edgeOffsets};
+}
 
 /**
  * Glyphs used to visually distinguish transition-label components from
@@ -342,11 +448,25 @@ export function buildFcstmElkGraph(
             edges,
             layoutOptions: {
                 'elk.padding': '[top=54,left=28,bottom=28,right=28]',
-                // Repeat nodeSelfLoop on composites because ELK's
-                // ``INCLUDE_CHILDREN`` mode does not propagate this
-                // spacing into nested layouts — without this copy
-                // self-loops on states nested inside a composite
-                // collapse to the default 10-unit spacing.
+                // ELK does not propagate parent spacing into nested layouts
+                // under ``INCLUDE_CHILDREN``. Keep enough room between an
+                // edge layer and its target node so the arrow approaches the
+                // node along a visible normal segment instead of turning on,
+                // or running along, the node border. This also gives direct
+                // two-point entry edges enough room; the post-layout smoother
+                // cannot repair a section that has no bend point.
+                // Nested layouts need an explicit value because
+                // ``INCLUDE_CHILDREN`` does not inherit the canvas setting;
+                // keep ELK's compact default rather than multiplying the
+                // root-level whitespace inside every composite.
+                'elk.layered.spacing.nodeNodeBetweenLayers': '20',
+                'elk.layered.spacing.edgeNodeBetweenLayers': String(MIN_TERMINAL_SEGMENT),
+                // Keep parallel edge lanes separated inside nested composites;
+                // canvas-level spacing is not inherited by INCLUDE_CHILDREN.
+                'elk.layered.spacing.edgeEdgeBetweenLayers': '32',
+                // Repeat nodeSelfLoop for the same non-propagation reason;
+                // otherwise loops nested inside a composite collapse to ELK's
+                // default 10-unit spacing.
                 'elk.spacing.nodeSelfLoop': '28',
             },
             fcstm: {

editors/jsfcstm/src/diagram/index.ts

@@ -17,8 +17,13 @@ export {
 } from './render';
 export {
     buildFcstmElkGraph,
+    collectElkLayoutGeometry,
     measureLabel as measureFcstmElkLabel,
+    MIN_TERMINAL_SEGMENT,
+    MIN_SELF_LOOP_SEGMENT,
+    terminalApproach,
 } from './elk-graph';
+export type {FcstmElkLayoutGeometry, FcstmElkNodeBox, FcstmElkPoint} from './elk-graph';
 export {
     renderFcstmDiagramSvg,
 } from './svg-renderer';

editors/jsfcstm/test/diagram-elk.test.ts

@@ -1,12 +1,20 @@
 import assert from 'node:assert/strict';
+import * as fs from 'node:fs';
+import * as path from 'node:path';
+
+import ELK from 'elkjs/lib/elk.bundled.js';
 
 import {createDocument} from './support';
 import {
     buildFcstmDiagramFromDocument,
     buildFcstmDiagramWebviewPayload,
     buildFcstmElkGraph,
+    collectElkLayoutGeometry,
+    MIN_TERMINAL_SEGMENT,
+    MIN_SELF_LOOP_SEGMENT,
     renderFcstmDiagramSvg,
     resolveFcstmDiagramPreviewOptions,
+    terminalApproach,
 } from '@pyfcstm/jsfcstm/diagram';
 
 describe('jsfcstm ELK-based diagram pipeline', () => {
@@ -255,6 +263,202 @@ describe('jsfcstm ELK-based diagram pipeline', () => {
             'nodeNode spacing should be at least 72 for a non-cramped diagram');
         assert.ok(Number(graph.layoutOptions['elk.layered.spacing.nodeNodeBetweenLayers']) >= 96,
             'between-layer spacing should be at least 96');
+        const fleet = graph.children.find(child => child.fcstm?.qualifiedName === 'Fleet');
+        const running = (fleet?.children || []).find(child => child.fcstm?.qualifiedName === 'Fleet.Running');
+        assert.equal(
+            fleet?.layoutOptions?.['elk.layered.spacing.edgeNodeBetweenLayers'],
+            '18',
+            'root-state composite must reserve a visible edge-to-target approach'
+        );
+        assert.equal(
+            running?.layoutOptions?.['elk.layered.spacing.edgeNodeBetweenLayers'],
+            '18',
+            'nested composites must restate spacing because ELK does not inherit it'
+        );
+        assert.equal(
+            running?.layoutOptions?.['elk.layered.spacing.nodeNodeBetweenLayers'],
+            '20',
+            'nested composites must pin the compact ELK layer-spacing fallback'
+        );
+        assert.equal(
+            running?.layoutOptions?.['elk.layered.spacing.edgeEdgeBetweenLayers'],
+            '32',
+            'nested composites must restate parallel-edge spacing because ELK does not inherit it'
+        );
+    });
+
+    it('rejects a terminal segment that ends at a target corner', () => {
+        const box = {left: 0, right: 100, top: 0, bottom: 100};
+        assert.deepEqual(
+            terminalApproach({x: 50, y: -20}, {x: 50, y: 0}, box),
+            {side: 'top', length: 20}
+        );
+        assert.deepEqual(
+            terminalApproach({x: 120, y: 50}, {x: 100, y: 50}, box),
+            {side: 'right', length: 20}
+        );
+        assert.deepEqual(
+            terminalApproach({x: 50, y: 120}, {x: 50, y: 100}, box),
+            {side: 'bottom', length: 20}
+        );
+        assert.deepEqual(
+            terminalApproach({x: -20, y: 50}, {x: 0, y: 50}, box),
+            {side: 'left', length: 20}
+        );
+        assert.equal(
+            terminalApproach({x: -18, y: 0}, {x: 0, y: 0}, box),
+            null,
+            'a horizontal line ending at the top-left corner is not a normal entry'
+        );
+        assert.equal(
+            terminalApproach({x: 0, y: -18}, {x: 0, y: 0}, box),
+            null,
+            'a vertical line ending at the top-left corner is not a normal entry'
+        );
+        const epsilonOutsideShortEntry = terminalApproach(
+            {x: 50, y: -18},
+            {x: 50, y: -0.75},
+            box
+        );
+        assert.ok(epsilonOutsideShortEntry,
+            'an endpoint within position tolerance should still be classified as a top entry');
+        assert.equal(epsilonOutsideShortEntry!.length, 17.25,
+            'terminal length must use the actual endpoint-to-endpoint segment');
+        assert.ok(epsilonOutsideShortEntry!.length < MIN_TERMINAL_SEGMENT,
+            'a 17.25px actual segment must not satisfy the strict 18px threshold');
+        assert.equal(
+            terminalApproach({x: 50, y: -20}, {x: 50, y: 0.5}, box),
+            null,
+            'an endpoint that has crossed inside the top border must be rejected'
+        );
+        assert.equal(
+            terminalApproach({x: 120, y: 50}, {x: 99.5, y: 50}, box),
+            null,
+            'an endpoint that has crossed inside the right border must be rejected'
+        );
+        assert.equal(
+            terminalApproach({x: 50, y: 80}, {x: 50, y: 99.5}, box),
+            null,
+            'an endpoint that has crossed inside the bottom border must be rejected'
+        );
+        assert.equal(
+            terminalApproach({x: -20, y: 50}, {x: 0.5, y: 50}, box),
+            null,
+            'an endpoint that has crossed inside the left border must be rejected'
+        );
+    });
+
+    it('routes every non-self arrow into its target border along a visible outward normal', async () => {
+        const fixtureDir = path.join(__dirname, 'fixtures', 'visual');
+        const fixtureNames = fs.readdirSync(fixtureDir)
+            .filter(name => name.endsWith('.fcstm'))
+            .sort();
+        const elk = new ELK();
+        let checkedSections = 0;
+        let checkedNormalTransitions = 0;
+        let checkedForcedSections = 0;
+        const fixtureCoverage = new Map<string, {sections: number; normal: number}>();
+
+        for (const fixtureName of fixtureNames) {
+            const fixturePath = path.join(fixtureDir, fixtureName);
+            const source = fs.readFileSync(fixturePath, 'utf8');
+            const diagram = await buildFcstmDiagramFromDocument(createDocument(source, fixturePath));
+            assert.ok(diagram, `${fixtureName}: diagram IR should be produced`);
+
+            for (const direction of ['TB', 'LR'] as const) {
+                const coverageKey = `${fixtureName}/${direction}`;
+                fixtureCoverage.set(coverageKey, {sections: 0, normal: 0});
+                const options = resolveFcstmDiagramPreviewOptions({detailLevel: 'normal', direction});
+                const graph = buildFcstmElkGraph(diagram!, options);
+                const laid = await elk.layout(JSON.parse(JSON.stringify(graph))) as any;
+                const {boxes, edgeOffsets} = collectElkLayoutGeometry(laid);
+
+                function checkNode(node: any): void {
+                    for (const edge of node.edges || []) {
+                        const sourceId = edge.sources?.[0];
+                        const targetId = edge.targets?.[0];
+                        if (!sourceId || !targetId) {
+                            assert.fail(`${fixtureName}/${direction}/${edge.id}: edge endpoint id missing`);
+                        }
+                        if (sourceId === targetId) {
+                            assert.ok((edge.sections || []).length > 0,
+                                `${fixtureName}/${direction}/${edge.id}: self-loop has no routed section`);
+                            for (const section of edge.sections || []) {
+                                const points = [
+                                    section.startPoint,
+                                    ...(section.bendPoints || []),
+                                    section.endPoint,
+                                ];
+                                assert.ok(points.length >= 4,
+                                    `${fixtureName}/${direction}/${edge.id}: self-loop needs a visible orthogonal route`);
+                                const lengths = points.slice(1).map((point, index) => Math.hypot(
+                                    point.x - points[index].x,
+                                    point.y - points[index].y,
+                                ));
+                                assert.ok(Math.min(...lengths) >= MIN_SELF_LOOP_SEGMENT,
+                                    `${fixtureName}/${direction}/${edge.id}: self-loop segment is too short`);
+                            }
+                            continue;
+                        }
+                        const targetBox = boxes.get(targetId);
+                        const offset = edgeOffsets.get(edge.id);
+                        assert.ok(targetBox, `${fixtureName}/${direction}/${edge.id}: target box ${targetId} missing`);
+                        assert.ok(offset, `${fixtureName}/${direction}/${edge.id}: edge owner offset missing`);
+                        assert.ok((edge.sections || []).length > 0,
+                            `${fixtureName}/${direction}/${edge.id}: ELK returned no edge section`);
+
+                        if (edge.fcstm?.transitionKind === 'normal') {
+                            checkedNormalTransitions += 1;
+                            fixtureCoverage.get(coverageKey)!.normal += 1;
+                        }
+                        if (edge.fcstm?.transitionKind === 'normalAll') {
+                            checkedForcedSections += (edge.sections || []).length;
+                        }
+                        for (const section of edge.sections || []) {
+                            const points = [
+                                section.startPoint,
+                                ...(section.bendPoints || []),
+                                section.endPoint,
+                            ].map(point => ({x: point.x + offset!.x, y: point.y + offset!.y}));
+                            assert.ok(points.length >= 2,
+                                `${fixtureName}/${direction}/${edge.id}: section must contain two endpoints`);
+                            const previous = points[points.length - 2];
+                            const end = points[points.length - 1];
+                            const approach = terminalApproach(previous, end, targetBox!);
+                            const context = `${fixtureName}/${direction}/${edge.id}`;
+                            assert.ok(
+                                approach,
+                                `${context}: arrow must approach ${targetId} from outside, perpendicular to its border; ` +
+                                `previous=${JSON.stringify(previous)}, end=${JSON.stringify(end)}, ` +
+                                `box=${JSON.stringify(targetBox)}`
+                            );
+                            assert.ok(
+                                approach!.length >= MIN_TERMINAL_SEGMENT,
+                                `${context}: ${approach!.side} terminal segment is only ${approach!.length}px; ` +
+                                `expected at least ${MIN_TERMINAL_SEGMENT}px`
+                            );
+                            checkedSections += 1;
+                            fixtureCoverage.get(coverageKey)!.sections += 1;
+                        }
+                    }
+                    for (const child of node.children || []) {
+                        checkNode(child);
+                    }
+                }
+
+                checkNode(laid);
+            }
+        }
+
+        assert.ok(checkedSections >= 100,
+            `expected broad geometry coverage, checked only ${checkedSections} sections`);
+        assert.ok(checkedNormalTransitions >= 50,
+            `expected direct A -> B coverage, checked only ${checkedNormalTransitions} transitions`);
+        assert.ok(checkedForcedSections > 0, 'forced-expansion sections must be part of the geometry corpus');
+        for (const [coverageKey, coverage] of fixtureCoverage) {
+            assert.ok(coverage.sections > 0, `${coverageKey}: expected at least one checked edge section`);
+            assert.ok(coverage.normal > 0, `${coverageKey}: expected at least one ordinary A -> B transition`);
+        }
     });
 
     it('returns null payload when the document has no state machine', async () => {