diff --git a/AGENTS.md b/AGENTS.md index e096be25..66efbe4e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -291,7 +291,7 @@ Files: - **Levelling after measurement** (`lib/diagram/geometry/levelNearAligned.ts`, run from `Canvas.tsx` when measured sizes/positions settle and nothing is dragged): a card renders at its own size (a wrapped subtitle makes it taller), so nodes the layout lined up by their tops can end up with centres a few px apart and a kinked edge. Nodes joined by an edge off-centre by 0.5–12px are levelled, moving the lighter end, only when that strictly reduces the misalignment of the edges touching it, never into another node and never out of a group. It is idempotent, skips groups and cross-group edges, and records no undo step (`setNodes`). - **Principles for any diagram** (`layoutUniformity.test.ts`, seeded random diagrams, flat/grouped, sparse/dense, LR/TD): the hard ones hold for every diagram (layout succeeds, no node overlaps, every node inside its group's frame); fan centring and straight links are best effort and ratcheted on means, because interlocking fans contradict each other and no layout can centre them all (known limits: `mvc`, `repo_to_diagram`, `archdraw_self`). `symmetrizeFans` keeps the group range as a bound only as a second attempt (when the plain solve applies nothing). `settleLayout` keeps its final result only if it beats the raw layout (more than 2px off the worst fan or the total miss) or loses no level edge (`levelEdges`: an edge whose ends are level is drawn without a bend); the landing hero is sensitive to a few px of nudging, so never apply the passes unconditionally. All scorers go through `measureLayoutResult`, which turns the engine's absolute child positions into canvas-relative ones; scoring raw engine nodes as canvas nodes double-counts group origins and misleads every grouped score. - **Final straightening passes** (end of `LayoutStage`, each step kept only when it adds no hard violation, main-path failure, crossing, label collision or bend, keeps fans centred and straight links straight): `alignMainSpine` (`spineAlign.ts`) lines the top-level main path up on one line; `packGroupColumns` (`columnPacking.ts`) closes wide gaps inside a group column so a tier reads as one stack; `alignGroupCentres` (same file) moves whole top-level groups onto a shared centre line, all at once and then one at a time. On dense diagrams the routing usually rejects the group moves (edges between tiers would cut through other tiers), which is intended. -- **Edge channels:** edges of one fan share one vertical channel (`fanChannelX` in `edgeRouteBuilder.ts`); the optimizer (`edgeRouteOptimizer.ts`) treats a shared trunk from a common source/target as a merge, not an overlap (`sharedRun`, anchors < 8px apart count as one point), and a lane offset on an ordinary left-to-right edge moves its channel instead of building a stubbed path. +- **Edge channels:** edges of one fan are routed to one vertical channel (`fanChannelX` in `edgeRouteBuilder.ts`) and then drawn on separate lanes 14px apart, nested so the fan never crosses itself (`separateFanLanes` in `fanLaneSeparation.ts`, last step of `optimizeEdgeRoutes`; a lane is used only when it clears every card and adds no crossing); the optimizer (`edgeRouteOptimizer.ts`) treats a shared trunk from a common source/target as a merge, not an overlap (`sharedRun`, anchors < 8px apart count as one point), and a lane offset on an ordinary left-to-right edge moves its channel instead of building a stubbed path. - Edge sides (`handlerPairScorer.ts`): a target at least 40px downstream (LR: later column, TD: later row) prefers the flow sides Right→Left / Bottom→Top; same-rank and backward edges use the facing sides. ### Alternate path @@ -401,7 +401,7 @@ The hosted `/api/mcp` serves the npm package, so a new tool reaches it only afte **Preview links are on by default.** `generate_diagram`, `apply_template` (and `update_diagram` for already-published diagrams) return `shareUrl` (read-only preview) and `diagramUrl` (editor) in the same response — `publish` defaults to `true`; `publish:false` is the private opt-out. Tool text puts the link block first via `formatPreviewLinks` (`mcp-server/src/lib/preview-links.ts`) with an explicit "show these links to the user" line, because text-only agents can't render the widget. A failed publish returns the diagram plus `linkError`; it must never fail the generation. -**Inline preview (MCP App).** `mcp-server/src/ui/diagram-widget.html` (`ui://archdraw/diagram-v2.html`) is a canvas only: title, Open in ArchDraw, Expand, zoom, Download PNG. It shows the diagram drawn by `generatePureSVG` (light Studio style, as the share page shows it), which `POST /api/diagram/load` returns as `previewSvg` when called with `preview: true`; `generate_diagram` / `apply_template` pass it to the preview in the tool result's `_meta['archdraw/previewSvg']`, which hosts keep out of the model's context. The preview must report its height (`ui/notifications/size-changed`) or hosts render it at zero height. Never draw diagrams with a separate renderer in the widget. Bump the `ui://` URI when the widget changes, since hosts may cache it. +**Inline preview (MCP App).** `mcp-server/src/ui/diagram-widget.html` (`ui://archdraw/diagram-v3.html`) is a canvas only: title, Open in ArchDraw, Expand, zoom, Download PNG. It shows the diagram drawn by `generatePureSVG` (light Studio style, as the share page shows it), which `POST /api/diagram/load` returns as `previewSvg` when called with `preview: true`; `generate_diagram` / `apply_template` pass it to the preview in the tool result's `_meta['archdraw/previewSvg']`, which hosts keep out of the model's context. The preview must report its height (`ui/notifications/size-changed`) or hosts render it at zero height. Never draw diagrams with a separate renderer in the widget. Bump the `ui://` URI when the widget changes, since hosts may cache it. Because every MCP generation now creates a `SharedCanvas` row: `POST /api/diagram/load` is throttled per user / client IP (create 30 anon · 60 signed-in, dry-run preview 300, mcp-update 300 per hour) via `checkRateLimitWithFallback` (Redis, in-memory fallback — never use it for AI/billing quotas, those fail closed), and `/api/cron/cleanup-expired-shares` deletes expired shares daily. diff --git a/SEO.md b/SEO.md index faf93d9a..7ac0b320 100644 --- a/SEO.md +++ b/SEO.md @@ -237,6 +237,14 @@ Agentic Browsing includes an "agent-centric accessibility" check, so accessibili - **Dimmed content still has to pass contrast:** scroll effects that fade inactive steps are measured in their faded state. Compute the lowest opacity that keeps the faintest text at 4.5:1 (3:1 for 24px+ or 18.66px+ bold) and never go below it. On the night page, the muted body text needed at least 0.82; the steps had been at 0.38 (1.8:1). - **Source maps:** `productionBrowserSourceMaps: true` clears "Missing source maps for large first-party JavaScript" and makes production errors readable. Only do this when the code may be public (ArchDraw is open source). +## 11c. Performance: measure before you optimise + +- **Lighthouse's mobile LCP is simulated.** With the default `throttlingMethod: simulate`, it replays the page as a slow phone on a slow network and charges LCP for every script and stylesheet requested before the first paint. Read `metrics.observedLargestContentfulPaint` in the JSON report, too. On the ArchDraw home page, the hero paragraph really painted at **224 ms**, together with the first paint, while the simulated LCP said 3.0 s (production) or 3.9 s (local build). Real-user Core Web Vitals in PageSpeed Insights' field data are what count for Google. +- **Compare like with like.** Build `main` and the branch the same way, serve both locally, run Lighthouse three times each and compare medians. A single run, or local against production, is noise. +- **Find the biggest unused chunk and trace it to its import.** Minified chunks hide library names, but fingerprints such as hundreds of `viewBox` values give them away. Here a 241 KB chunk, 96% unused, was the full `@react-icons-cloud/aws` set (about 500 icons in one pre-bundled file that can't be tree-shaken). It was pulled into every canvas, including the landing hero, which uses none of it. Loading it on demand, the first time an AWS component renders, cut unused JavaScript from **292 to 43 KiB**. +- **Lazy isn't lazy if it fires during hydration.** An `IntersectionObserver` with a 200px margin fired at once on phones, so the hero map's JavaScript competed with the first paint. Now, when the map is merely visible, it waits for the `load` event and `requestIdleCallback`; interaction still loads it immediately. That cut total blocking time by 19% (219 → 177 ms) and Speed Index by 7% (2,529 → 2,347 ms). +- **Not every Lighthouse suggestion helps.** `experimental.inlineCss` removed the render-blocking stylesheets but inlined *all* of the app's CSS. The home page HTML grew to 946 KB, and the score fell from 85 to 75. Measure each suggestion; don't apply them in bulk. + ## 12. Launch and measurement checklist After each deploy: @@ -286,4 +294,5 @@ After each deploy: | 2026-11-07 | Free tools (AI generator, text-to-diagram, system design calculator) and comparison pages; this playbook | #32 | | 2026-11-07 | Clean-ups: lint errors to zero, current MCP install commands in docs | #33 | | 2026-11-07 | Search Console: sitemap read (116 pages), 8 indexing requests, Rich Results Test; WebMCP origin trial registered | #34 | -| 2026-11-07 | Lighthouse / PageSpeed fixes on the home page: inert hero toggle, AA contrast for dimmed steps, production source maps | not yet opened | +| 2026-11-07 | Lighthouse / PageSpeed fixes on the home page: inert hero toggle, AA contrast for dimmed steps, production source maps | #35 | +| 2026-11-07 | Home page performance: AWS icon set loaded on demand (unused JS 292 → 43 KiB), hero map loads after the page is idle | #36 | diff --git a/frontend/components/canvas/Canvas.tsx b/frontend/components/canvas/Canvas.tsx index 6de9e8bb..2f7f2db0 100755 --- a/frontend/components/canvas/Canvas.tsx +++ b/frontend/components/canvas/Canvas.tsx @@ -31,7 +31,6 @@ import { anyNodeInView } from '@/lib/diagram/canvas/nodesInView'; import { ContextMenu, type ContextMenuState } from '@/components/editor/ContextMenu'; import { useSnapping } from '@/hooks/useSnapping'; import { CometTrailCanvas } from '@/components/canvas/CometTrailCanvas'; -import { useMiddleMousePan } from '@/hooks/useCanvasInteractions'; import { useCallback, useEffect, useRef, useState, useMemo, Suspense } from 'react'; import { useShallow } from 'zustand/react/shallow'; import { useCanvasTheme } from '@/lib/theme/appTheme'; @@ -53,11 +52,9 @@ import { CANVAS_CONFIG, DEFAULT_EDGE_OPTIONS } from '@/lib/config'; import { useGrouping } from '@/hooks/useGrouping'; import { toast } from 'sonner'; import type { Node } from 'reactflow'; -import { resolveNodeCollisions } from '@/lib/diagram/geometry/resolveNodeCollisions'; import { useEdgeColors } from '@/lib/theme/edgeColors'; import { calculateNodeDimensions } from '@/lib/diagram/nodes/nodeSizing'; import { createNode, createEdge, createBlankShapeNode, createTextLabelNode } from '@/lib/component-library/factory'; -import { nodeCentre } from '@/lib/diagram/geometry/nodeBox'; import { levelNearAlignedNodes } from '@/lib/diagram/geometry/levelNearAligned'; import { reactFlowRef } from '@/lib/client/reactFlowRef'; import { resolveNodeDropConnection } from '@/lib/diagram/canvas/resolveNodeDropConnection'; @@ -231,12 +228,11 @@ function CanvasInner() { const isMobile = useIsMobile(); const { - onNodesChange, onEdgesChange, onConnect, onReconnect, + onEdgesChange, onConnect, onReconnect, setSelectedNodeId, setSelectedNodeIds, setSelectedEdgeId, setPendingLabelEdgeId, setNodes, addNodeOnEdgeDrop, addNode, } = useDiagramStore(useShallow((s) => ({ - onNodesChange: s.onNodesChange, onEdgesChange: s.onEdgesChange, onConnect: s.onConnect, onReconnect: s.onReconnect, @@ -251,10 +247,9 @@ function CanvasInner() { const { isDark } = useCanvasTheme(); const reactFlowInstance = useReactFlow(); - const { onNodeDrag, onNodeDragStop: onNodeDragStopSnap } = useSnapping(); + const { onNodesChange, onNodeDragStop: onNodeDragStopSnap } = useSnapping(); // While a node is lined up with another, alignment wins over the grid so edges can line up exactly. const alignmentActive = useDiagramStore((s) => s.guideLines.length > 0); - useMiddleMousePan(); useGrouping(); // Live migration for persisted groups saved before the zIndex fix. @@ -410,82 +405,31 @@ function CanvasInner() { }; }, [reactFlowInstance]); + const dragRevision = useRef(0); + // One undo step per drag, recorded at drag start; the moves in between are not recorded. const onNodeDragStart = useCallback(() => { + dragRevision.current++; useDiagramStore.getState().pushHistory(); }, []); const onNodeDragStop = useCallback( (event: React.MouseEvent, node: Node, draggedNodes: Node[]) => { + dragRevision.current++; onNodeDragStopSnap(event, node, draggedNodes); - // Auto-straighten: if dragged node has incident edges that are almost horizontal/vertical, - // nudge it so the edge becomes perfectly straight — so user doesn't have to hunt pixel-perfect. + // The drop keeps the last snapped position. Layout repair is explicit; + // moving neighbours or straightening again here makes the release jump. const store = useDiagramStore.getState(); - const isLR = store.activeLayoutPresetId !== 'layered-tb'; - const edges = store.edges; - const STRAIGHT_THRESHOLD = 12; - let autoStraightened = false; - const straightenMap = new Map(); - const draggedIds = new Set(draggedNodes.map((n) => n.id)); - for (const dragged of draggedNodes) { - const incident = edges.filter((e) => e.source === dragged.id || e.target === dragged.id); - for (const edge of incident) { - const otherId = edge.source === dragged.id ? edge.target : edge.source; - // Nodes dragged together keep their offset; straightening one would shear the selection. - if (draggedIds.has(otherId)) continue; - const other = store.nodes.find((n) => n.id === otherId); - if (!other) continue; - // React Flow's own node carries the absolute position and measured size, - // which the store copy lacks for nodes inside a group. - const dC = nodeCentre(dragged as Node); - const oC = nodeCentre(reactFlowRef.instance?.getNode(otherId) ?? other); - // Determine if edge is primarily horizontal or vertical - const dx = oC.x - dC.x; - const dy = oC.y - dC.y; - if (isLR && Math.abs(dx) > Math.abs(dy) && Math.abs(dy) > 0 && Math.abs(dy) < STRAIGHT_THRESHOLD) { - const cur = straightenMap.get(dragged.id) ?? { x: 0, y: 0 }; - if (cur.y === 0 || Math.abs(dy) < Math.abs(cur.y)) cur.y = dy; - straightenMap.set(dragged.id, cur); - autoStraightened = true; - } else if (!isLR && Math.abs(dy) > Math.abs(dx) && Math.abs(dx) > 0 && Math.abs(dx) < STRAIGHT_THRESHOLD) { - const cur = straightenMap.get(dragged.id) ?? { x: 0, y: 0 }; - if (cur.x === 0 || Math.abs(dx) < Math.abs(cur.x)) cur.x = dx; - straightenMap.set(dragged.id, cur); - autoStraightened = true; - } - } - } - let currentNodes = store.nodes; - if (autoStraightened) { - currentNodes = currentNodes.map((n) => { - const delta = straightenMap.get(n.id); - if (!delta) return n; - return { ...n, position: { x: n.position.x + delta.x, y: n.position.y + delta.y } }; - }); - setNodes(currentNodes); - } - - // Collision resolution (after straightening) - const resolvedNodes = resolveNodeCollisions(currentNodes); - - const hasChanges = currentNodes.some((n, i) => - n.position.x !== resolvedNodes[i]?.position.x || - n.position.y !== resolvedNodes[i]?.position.y - ); - - const latestNodes = hasChanges ? resolvedNodes : useDiagramStore.getState().nodes; - - // Persistence is handled by the store's auto-save subscription, which - // saves the final (non-dragging) positions. - if (hasChanges) setNodes(resolvedNodes); - - // Recalculate connection handles from the post-drag node positions - useDiagramStore.getState().recalculateHandles(latestNodes); + store.recalculateHandles(store.nodes); }, - [onNodeDragStopSnap, setNodes] + [onNodeDragStopSnap] ); + const onSelectionDragStop = useCallback((event: React.MouseEvent, dragged: Node[]) => { + if (dragged[0]) onNodeDragStop(event, dragged[0], dragged); + }, [onNodeDragStop]); + const onConnectStart: OnConnectStart = useCallback( (_event, params) => { connectSucceededRef.current = false; @@ -885,14 +829,16 @@ function CanvasInner() { // edge are levelled (see levelNearAligned). It changes nothing when there is // nothing to fix, and a fix needs no further fix, so it settles after one pass. const levelKey = useMemo( - () => (projection ? '' : nodes.map((n) => `${n.id}:${Math.round(n.position.x)}:${Math.round(n.position.y)}:${n.width ?? ''}:${n.height ?? ''}`).join('|')), + () => (projection ? '' : nodes.map((n) => `${n.id}:${n.width ?? ''}:${n.height ?? ''}`).join('|')), [nodes, projection], ); useEffect(() => { if (!levelKey) return; + const revision = dragRevision.current; const timer = setTimeout(() => { const store = useDiagramStore.getState(); const current = store.nodes; + if (revision !== dragRevision.current) return; if (current.some((n) => (n as Node & { dragging?: boolean }).dragging)) return; const connected = new Set(store.edges.flatMap((e) => [e.source, e.target])); if (!current.every((n) => !connected.has(n.id) || (n.width && n.height))) return; // still measuring @@ -1033,8 +979,9 @@ function CanvasInner() { onReconnect={projection ? undefined : onReconnect} onConnectStart={projection ? undefined : onConnectStart} onConnectEnd={projection ? undefined : onConnectEnd} - onNodeDrag={projection ? undefined : onNodeDrag} onNodeDragStart={projection ? undefined : onNodeDragStart} + onSelectionDragStart={projection ? undefined : onNodeDragStart} + onSelectionDragStop={projection ? undefined : onSelectionDragStop} onNodeDragStop={detailView ? undefined : highLevel ? onHighLevelDragStop : onNodeDragStop} onPaneClick={onPaneClickOrPlace} onNodeClick={isLiveProject && (!highLevel || detailView) ? onNodeClick : undefined} diff --git a/frontend/components/canvas/CometTrailCanvas.tsx b/frontend/components/canvas/CometTrailCanvas.tsx index 66931a3d..9adf4dd2 100644 --- a/frontend/components/canvas/CometTrailCanvas.tsx +++ b/frontend/components/canvas/CometTrailCanvas.tsx @@ -1,7 +1,7 @@ 'use client'; import { useEffect, useRef } from 'react'; -import { useReactFlow, useViewport } from 'reactflow'; +import { useReactFlow } from 'reactflow'; import { useDiagramStore } from '@/store/diagramStore'; interface Point { @@ -18,18 +18,21 @@ interface Stroke { export function CometTrailCanvas() { const isPenModeActive = useDiagramStore((s) => s.isPenModeActive); const reactFlowInstance = useReactFlow(); - const { x, y, zoom } = useViewport(); const canvasRef = useRef(null); const strokesRef = useRef([]); const isDrawingRef = useRef(false); const currentStrokeIdRef = useRef(null); + const animationRef = useRef(null); + const startAnimationRef = useRef<() => void>(() => {}); + const sizeRef = useRef({ width: 0, height: 0 }); // Resize canvas handler to handle window sizing and Retina displays const resizeCanvas = () => { const canvas = canvasRef.current; if (!canvas) return; const rect = canvas.getBoundingClientRect(); + sizeRef.current = { width: rect.width, height: rect.height }; const dpr = window.devicePixelRatio || 1; canvas.width = rect.width * dpr; canvas.height = rect.height * dpr; @@ -46,24 +49,17 @@ export function CometTrailCanvas() { return () => window.removeEventListener('resize', resizeCanvas); }, []); - // Frame loop for 60fps trail decay and render + // Animate only while strokes remain; normal canvas navigation does no pen work. useEffect(() => { - let animationFrameId: number; - const render = () => { const canvas = canvasRef.current; - if (!canvas) { - animationFrameId = requestAnimationFrame(render); - return; - } + animationRef.current = null; + if (!canvas) return; const ctx = canvas.getContext('2d'); - if (!ctx) { - animationFrameId = requestAnimationFrame(render); - return; - } + if (!ctx) return; - const rect = canvas.getBoundingClientRect(); - ctx.clearRect(0, 0, rect.width, rect.height); + ctx.clearRect(0, 0, sizeRef.current.width, sizeRef.current.height); + const { x, y, zoom } = reactFlowInstance.getViewport(); const now = Date.now(); @@ -106,12 +102,18 @@ export function CometTrailCanvas() { } }); - animationFrameId = requestAnimationFrame(render); + if (strokesRef.current.length) animationRef.current = requestAnimationFrame(render); }; - animationFrameId = requestAnimationFrame(render); - return () => cancelAnimationFrame(animationFrameId); - }, [x, y, zoom]); + startAnimationRef.current = () => { + if (animationRef.current === null) animationRef.current = requestAnimationFrame(render); + }; + return () => { + if (animationRef.current !== null) cancelAnimationFrame(animationRef.current); + animationRef.current = null; + startAnimationRef.current = () => {}; + }; + }, [reactFlowInstance]); // Pointer Down const handlePointerDown = (e: React.PointerEvent) => { @@ -131,6 +133,7 @@ export function CometTrailCanvas() { const newStrokeId = Math.random().toString(36); currentStrokeIdRef.current = newStrokeId; + startAnimationRef.current(); strokesRef.current.push({ id: newStrokeId, points: [{ x: flowPos.x, y: flowPos.y, time: Date.now() }] diff --git a/frontend/components/canvas/__tests__/CometTrailCanvas.test.tsx b/frontend/components/canvas/__tests__/CometTrailCanvas.test.tsx new file mode 100644 index 00000000..fb2a572c --- /dev/null +++ b/frontend/components/canvas/__tests__/CometTrailCanvas.test.tsx @@ -0,0 +1,35 @@ +import { act, cleanup, fireEvent, render } from '@testing-library/react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { CometTrailCanvas } from '../CometTrailCanvas'; + +vi.mock('reactflow', () => ({ useReactFlow: () => flow })); +vi.mock('@/store/diagramStore', () => ({ useDiagramStore: (selector: (s: object) => unknown) => selector({ isPenModeActive: true }) })); +const flow = { + getViewport: () => ({ x: 0, y: 0, zoom: 1 }), + screenToFlowPosition: (point: object) => point, +}; +afterEach(() => { cleanup(); vi.restoreAllMocks(); vi.unstubAllGlobals(); }); + +describe('pen rendering', () => { + it('does no idle frame work, runs for a stroke, and stops after it expires', () => { + const frames: FrameRequestCallback[] = []; + const request = vi.fn((callback: FrameRequestCallback) => { frames.push(callback); return frames.length; }); + vi.stubGlobal('requestAnimationFrame', request); + vi.stubGlobal('cancelAnimationFrame', vi.fn()); + const ctx = { resetTransform: vi.fn(), scale: vi.fn(), clearRect: vi.fn() }; + vi.spyOn(HTMLCanvasElement.prototype, 'getContext').mockReturnValue(ctx as unknown as CanvasRenderingContext2D); + HTMLCanvasElement.prototype.setPointerCapture = vi.fn(); + const now = vi.spyOn(Date, 'now').mockReturnValue(1000); + const { container } = render(); + expect(request).not.toHaveBeenCalled(); + const canvas = container.querySelector('canvas')!; + fireEvent.pointerDown(canvas, { pointerId: 1, clientX: 10, clientY: 10 }); + expect(request).toHaveBeenCalledTimes(1); + act(() => frames.shift()!(0)); + expect(request).toHaveBeenCalledTimes(2); + now.mockReturnValue(4001); + act(() => frames.shift()!(0)); + expect(request).toHaveBeenCalledTimes(2); + expect(ctx.clearRect).toHaveBeenCalledTimes(2); + }); +}); diff --git a/frontend/components/canvas/edges/DiagramGeometryProvider.tsx b/frontend/components/canvas/edges/DiagramGeometryProvider.tsx index 5925c1de..dbed5b78 100644 --- a/frontend/components/canvas/edges/DiagramGeometryProvider.tsx +++ b/frontend/components/canvas/edges/DiagramGeometryProvider.tsx @@ -1,10 +1,12 @@ 'use client'; -import { createContext, useContext, useEffect, useMemo, useSyncExternalStore, useState, type ReactNode } from 'react'; +import { createContext, useContext, useEffect, useMemo, useRef, useSyncExternalStore, useState, type ReactNode } from 'react'; import { useStore, type Edge, type Node, type ReactFlowState } from 'reactflow'; import { useCanvasView } from '@/lib/diagram/canvas/CanvasHostContext'; import { computeDiagramGeometryPlan, geometrySnapshotKey, type DiagramGeometryPlan } from '@/lib/diagram/geometry/diagramGeometry'; import { computeDiagramGeometryInWorker } from '@/lib/diagram/geometry/diagramGeometryWorkerClient'; +import { repairDiagramLayout } from '@/lib/diagram/mermaid/repairLayout'; +import { useDiagramStore } from '@/store/diagramStore'; import type { EdgeRouteHandleAnchors } from '@/lib/diagram/edges/edgeRouteBuilder'; type EdgeGeometrySnapshot = { route?: DiagramGeometryPlan['routes'] extends ReadonlyMap ? R : never; label?: DiagramGeometryPlan['labels'] extends ReadonlyMap ? L : never; provisional: boolean }; @@ -79,17 +81,26 @@ export function DiagramGeometryProvider({ children: ReactNode; }) { const { direction, planKey: planStoreKey, renderStyleId, iconMode, readOnly } = useCanvasView(); + const [repairing, setRepairing] = useState(false); + const [repairMessage, setRepairMessage] = useState(''); const [workerPlan, setWorkerPlan] = useState(null); const settledPlan = useSyncExternalStore( (listener) => subscribeToSettledPlan(planStoreKey, listener), () => settledPlans.get(planStoreKey) ?? null, () => null, ); - const nodeInternals = useStore((state: ReactFlowState) => state.nodeInternals); + const isDragging = nodes.some((node) => node.dragging === true); + const heldInternals = useRef(null); + const nodeInternals = useStore((state: ReactFlowState) => { + if (isDragging && heldInternals.current) return heldInternals.current; + return state.nodeInternals; + }); + useEffect(() => { if (!isDragging) heldInternals.current = nodeInternals; }, [isDragging, nodeInternals]); + const geometryNodes = isDragging ? null : nodes; const measuredNodes = useMemo(() => { const internal = Array.from(nodeInternals.values()) as Node[]; - return internal.length ? internal : nodes; - }, [nodeInternals, nodes]); + return internal.length ? internal : (geometryNodes ?? []); + }, [nodeInternals, geometryNodes]); const handleAnchors = useMemo(() => { const result = new Map(); const byId = new Map(measuredNodes.map((node) => [node.id, node as Node & { @@ -110,27 +121,27 @@ export function DiagramGeometryProvider({ } return result; }, [measuredNodes, edges]); - const isDragging = nodes.some((node) => (node as Node & { dragging?: boolean }).dragging === true); // A label drag rewrites `labelT` on every pointer move; keep it out of the plan key so the other edges are not re-planned (and flicker to their fallback routes) mid-drag. - const options = useMemo(() => ({ direction, renderStyleId, iconMode, handleAnchors, ignoreLabelPosition: true }), [direction, renderStyleId, iconMode, handleAnchors]); + const options = useMemo(() => ({ direction, renderStyleId, iconMode, handleAnchors, ignoreLabelPosition: true, straightEdges: false }), [direction, renderStyleId, iconMode, handleAnchors]); const geometryKey = useMemo(() => geometrySnapshotKey(measuredNodes, edges, options), [measuredNodes, edges, options]); useEffect(() => { if (isDragging) return; const currentKey = geometryKey; if (settledPlans.get(planStoreKey)?.key === currentKey) return; + let cancelled = false; const timer = setTimeout(() => { void computeDiagramGeometryInWorker(measuredNodes, edges, options).then((next) => { - if (next.key !== currentKey) return; + if (cancelled || next.key !== currentKey) return; setWorkerPlan(next); publishSettledPlan(planStoreKey, next); }).catch((error: unknown) => { - if (error instanceof Error && error.name === 'ObsoleteGeometryRequest') return; + if (cancelled || (error instanceof Error && error.name === 'ObsoleteGeometryRequest')) return; // The worker may be unavailable in older browser/test hosts. const next = computeDiagramGeometryPlan(measuredNodes, edges, options); if (next.key === currentKey) { setWorkerPlan(next); publishSettledPlan(planStoreKey, next); } }); }, 60); - return () => clearTimeout(timer); + return () => { cancelled = true; clearTimeout(timer); }; }, [isDragging, geometryKey, measuredNodes, edges, options, planStoreKey]); // During a drag the diagram keeps the last plan it had, instead of rebuilding @@ -155,22 +166,52 @@ export function DiagramGeometryProvider({ useEffect(() => { geometryStore.set(plan); }, [plan]); useEffect(() => { geometryStore.setDragging(isDragging); }, [isDragging]); - // Only hard issues (an edge through a node, colliding nodes) interrupt the - // user. Soft ones are mostly ordinary crossings, which any dense - // architecture diagram has; a standing warning about them was noise. + // Surface collisions and substantial congestion; a single ordinary crossing + // should not leave a permanent warning on an otherwise readable diagram. const hardIssueCount = plan.violations.filter((issue) => issue.severity === 'hard').length; + const repair = async () => { + if (readOnly || repairing || isDragging) return; + const snapshot = useDiagramStore.getState(); + setRepairing(true); + setRepairMessage(''); + try { + const result = await repairDiagramLayout(snapshot.nodes, snapshot.edges, direction, { renderStyleId, iconMode }); + const current = useDiagramStore.getState(); + if (current.activeCanvasId !== snapshot.activeCanvasId || current.nodes !== snapshot.nodes || current.edges !== snapshot.edges) { + setRepairMessage('Diagram changed during repair. Try again.'); + return; + } + if (!result.repaired) { + setRepairMessage('No safer layout found. Try separating the crowded components manually.'); + return; + } + current.pushHistory(); + current.setNodes(result.nodes); + setRepairMessage('Layout improved. You can undo this change.'); + } catch { + setRepairMessage('Repair could not finish. Your diagram is unchanged.'); + } finally { + setRepairing(false); + } + }; return ( {children} {/* Viewers can't move nodes, so the advice is for the editor only. */} - {!readOnly && !plan.provisional && hardIssueCount > 0 && ( + {!readOnly && !plan.provisional && (hardIssueCount > 0 || plan.quality.crossings > 2 || plan.quality.unexplainedPrivateOverlapLength > 48) && (
issue.severity === 'hard').map((issue) => issue.code).join(', ')} > - {`${hardIssueCount} layout issue${hardIssueCount === 1 ? '' : 's'}: an edge runs through a node or nodes overlap. Move a node or try another layout.`} + {hardIssueCount > 0 + ? `${hardIssueCount} layout issue${hardIssueCount === 1 ? '' : 's'}: components, connections or labels collide.` + : 'Connections share crowded routes. Try repairing the layout.'} + + {repairMessage &&

{repairMessage}

}
)}
diff --git a/frontend/components/canvas/edges/SimpleFloatingEdge.tsx b/frontend/components/canvas/edges/SimpleFloatingEdge.tsx index c3fdbc09..3ea0639b 100644 --- a/frontend/components/canvas/edges/SimpleFloatingEdge.tsx +++ b/frontend/components/canvas/edges/SimpleFloatingEdge.tsx @@ -229,6 +229,7 @@ export default function SimpleFloatingEdge({ target: { x: _targetX, y: _targetY }, }, iconMode, + { straightEdges: false }, ); }, [ id, source, target, sourceHandleId, targetHandleId, data, diff --git a/frontend/components/canvas/edges/__tests__/DiagramGeometryProvider.test.tsx b/frontend/components/canvas/edges/__tests__/DiagramGeometryProvider.test.tsx new file mode 100644 index 00000000..967ea1cc --- /dev/null +++ b/frontend/components/canvas/edges/__tests__/DiagramGeometryProvider.test.tsx @@ -0,0 +1,46 @@ +import { act, cleanup, render } from '@testing-library/react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import type { Node } from 'reactflow'; +import { DiagramGeometryProvider } from '../DiagramGeometryProvider'; + +const mocks = vi.hoisted(() => ({ + internals: new Map(), + key: vi.fn(() => 'geometry'), + worker: vi.fn(), +})); +vi.mock('reactflow', () => ({ useStore: (selector: (s: object) => unknown) => selector({ nodeInternals: mocks.internals }) })); +vi.mock('@/lib/diagram/canvas/CanvasHostContext', () => ({ + useCanvasView: () => ({ direction: 'LR', planKey: 'provider-test', renderStyleId: 'precision', iconMode: 'off', readOnly: true }), +})); +vi.mock('@/lib/diagram/geometry/diagramGeometry', () => ({ geometrySnapshotKey: mocks.key, computeDiagramGeometryPlan: vi.fn() })); +vi.mock('@/lib/diagram/geometry/diagramGeometryWorkerClient', () => ({ computeDiagramGeometryInWorker: mocks.worker })); +vi.mock('@/lib/diagram/mermaid/repairLayout', () => ({ repairDiagramLayout: vi.fn() })); +vi.mock('@/store/diagramStore', () => ({ useDiagramStore: { getState: vi.fn() } })); +const edges: never[] = []; +const node = (x: number, dragging = false): Node => ({ id: 'a', position: { x, y: 0 }, data: {}, dragging }); + +afterEach(() => { cleanup(); vi.useRealTimers(); vi.clearAllMocks(); }); + +describe('geometry during dragging', () => { + it('does not rebuild snapshot keys or request routing for successive drag frames', () => { + vi.useFakeTimers(); + mocks.worker.mockReturnValue(new Promise(() => {})); + mocks.internals = new Map([['a', node(0)]]); + const view = (n: Node) =>
; + const { rerender } = render(view(node(0))); + mocks.internals = new Map([['a', node(1, true)]]); + rerender(view(node(1, true))); + const before = mocks.key.mock.calls.length; + for (let x = 2; x <= 20; x++) { + mocks.internals = new Map([['a', node(x, true)]]); + rerender(view(node(x, true))); + } + act(() => vi.advanceTimersByTime(100)); + expect(mocks.key).toHaveBeenCalledTimes(before); + expect(mocks.worker).not.toHaveBeenCalled(); + mocks.internals = new Map([['a', node(20)]]); + rerender(view(node(20))); + act(() => vi.advanceTimersByTime(60)); + expect(mocks.worker).toHaveBeenCalledTimes(1); + }); +}); diff --git a/frontend/components/editor/toolbar/Toolbar.tsx b/frontend/components/editor/toolbar/Toolbar.tsx index 5ff98661..a5c927d2 100644 --- a/frontend/components/editor/toolbar/Toolbar.tsx +++ b/frontend/components/editor/toolbar/Toolbar.tsx @@ -1,6 +1,6 @@ 'use client'; -import { useRef, useState, useEffect, useMemo } from 'react'; +import { useRef, useState, useEffect } from 'react'; import { MapLevelToggle } from '@/components/canvas/MapLevelToggle'; import { useProjectLiveStore } from '@/store/projectLiveStore'; import { Trash2, Upload, Undo2, Redo2, Share2, Radio, Loader2, Check, GraduationCap, MoreHorizontal, HelpCircle, PanelLeftClose, LayoutTemplate, FolderOpen, LayoutDashboard, Github, PenTool, Type, Moon, Sun } from 'lucide-react'; @@ -33,14 +33,14 @@ export function Toolbar() { const router = useRouter(); const { clearDiagram, - nodes, - edges, + nodeCount, + edgeCount, importDiagram, undo, redo, - past, - future, - canvases, + canUndo, + canRedo, + canvasName, activeCanvasId, removeCanvas, savingState, @@ -57,14 +57,14 @@ export function Toolbar() { } = useDiagramStore( useShallow((s) => ({ clearDiagram: s.clearDiagram, - nodes: s.nodes, - edges: s.edges, + nodeCount: s.nodes.length, + edgeCount: s.edges.length, importDiagram: s.importDiagram, undo: s.undo, redo: s.redo, - past: s.past, - future: s.future, - canvases: s.canvases, + canUndo: s.past.length > 0, + canRedo: s.future.length > 0, + canvasName: s.canvases.find((c) => c.id === s.activeCanvasId)?.name, activeCanvasId: s.activeCanvasId, removeCanvas: s.removeCanvas, savingState: s.savingState, @@ -85,7 +85,7 @@ export function Toolbar() { const tier = getUserTier(user?.id); const isGuest = tier === 'guest'; const _isSequenceDiagram = activeCanvasId ? !!sequenceDiagrams[activeCanvasId] : false; - const canShowHighLevel = useMemo(() => hasHighLevelView(nodes), [nodes]); + const canShowHighLevel = useDiagramStore((s) => hasHighLevelView(s.nodes)); // Live projects switch between the overview (this canvas) and the detailed map. const mapLevel = useProjectLiveStore((s) => s.level); const setMapLevel = useProjectLiveStore((s) => s.setLevel); @@ -112,17 +112,13 @@ export function Toolbar() { const darkMode = useDiagramStore((s) => s.darkMode); const toggleDarkMode = useDiagramStore((s) => s.toggleDarkMode); - // Count nodes/edges in current canvas for delete confirmation - const currentCanvasForDelete = canvases.find(c => c.id === activeCanvasId); - const deleteNodeCount = currentCanvasForDelete?.nodes.length ?? 0; - const deleteEdgeCount = currentCanvasForDelete?.edges.length ?? 0; + const deleteNodeCount = nodeCount; + const deleteEdgeCount = edgeCount; const handleDeleteCanvas = () => { setConfirmDeleteId(activeCanvasId); }; - const activeCanvas = canvases.find((c) => c.id === activeCanvasId); - const sanitizeFilename = (name: string): string => { return name .replace(/[^\w\s-]/g, '') // Remove special characters @@ -133,7 +129,6 @@ export function Toolbar() { }; const getExportFilename = (extension: string): string => { - const canvasName = activeCanvas?.name; if (!canvasName) return `archdraw-export.${extension}`; return `${sanitizeFilename(canvasName)}.${extension}`; }; @@ -160,6 +155,7 @@ export function Toolbar() { }; const doShare = async () => { + const { nodes, edges } = useDiagramStore.getState(); const currentUser = useAuthStore.getState().user; const userEmail = currentUser?.email || 'owner@local'; const userName = currentUser?.name || currentUser?.email?.split('@')[0] || 'Owner'; @@ -172,7 +168,7 @@ export function Toolbar() { body: JSON.stringify({ nodes, edges, - label: activeCanvas?.name ?? 'Shared Diagram', + label: canvasName ?? 'Shared Diagram', source: 'manual', accessType: 'anyone', linkPermission: 'viewer', @@ -257,7 +253,7 @@ export function Toolbar() { const handleClear = () => { // A live project's map is published by its agents; clearing it would wipe it. - if (nodes.length === 0 || isLiveProject) return; + if (nodeCount === 0 || isLiveProject) return; if (window.confirm('Clear all nodes and edges from the canvas?')) { clearDiagram(); toast.success('Canvas cleared'); @@ -323,12 +319,12 @@ export function Toolbar() { {/* CENTER: Context info */}
- {nodes.length} + {nodeCount} nodes - {edges.length} + {edgeCount} edges @@ -353,7 +349,7 @@ export function Toolbar() { variant="ghost" size="icon" onClick={undo} - disabled={!past.length} + disabled={!canUndo} className="!w-8 sm:!w-9 !h-8 sm:!h-9 disabled:opacity-30" title="Undo (Cmd+Z)" aria-label="Undo" @@ -364,7 +360,7 @@ export function Toolbar() { variant="ghost" size="icon" onClick={redo} - disabled={!future.length} + disabled={!canRedo} className="!w-8 sm:!w-9 !h-8 sm:!h-9 disabled:opacity-30" title="Redo (Cmd+Shift+Z)" aria-label="Redo" @@ -467,7 +463,7 @@ export function Toolbar() { variant="ghost" size="icon" onClick={handleShare} - disabled={isSharing || nodes.length === 0} + disabled={isSharing || nodeCount === 0} className="!w-8 sm:!w-9 !h-8 sm:!h-9 disabled:opacity-40" aria-label="Share diagram" title="Share diagram" @@ -516,7 +512,7 @@ export function Toolbar() { variant="ghost" size="sm" onClick={() => { undo(); setMoreOpen(false); }} - disabled={!past.length} + disabled={!canUndo} className="w-full justify-start gap-2 rounded-none disabled:opacity-30" > @@ -526,7 +522,7 @@ export function Toolbar() { variant="ghost" size="sm" onClick={() => { redo(); setMoreOpen(false); }} - disabled={!future.length} + disabled={!canRedo} className="w-full justify-start gap-2 rounded-none disabled:opacity-30" > @@ -554,7 +550,7 @@ export function Toolbar() { variant="ghost" size="sm" onClick={() => { handleShare(); setMoreOpen(false); }} - disabled={isSharing || nodes.length === 0} + disabled={isSharing || nodeCount === 0} className="w-full justify-start gap-2 rounded-none disabled:opacity-30" > @@ -625,7 +621,7 @@ export function Toolbar() { variant="ghost" size="sm" onClick={() => { handleClear(); setMoreOpen(false); }} - disabled={nodes.length === 0 || isLiveProject} + disabled={nodeCount === 0 || isLiveProject} title={isLiveProject ? 'Live project maps are published by your agents and cannot be cleared' : undefined} className="w-full justify-start gap-2 rounded-none !text-destructive hover:!bg-destructive/10 disabled:opacity-40" > @@ -736,7 +732,7 @@ export function Toolbar() { title="Delete canvas?" description={ <> - This will delete “{currentCanvasForDelete?.name || 'this canvas'}” and remove {deleteNodeCount} node{deleteNodeCount !== 1 ? 's' : ''} and {deleteEdgeCount} edge{deleteEdgeCount !== 1 ? 's' : ''}. + This will delete “{canvasName || 'this canvas'}” and remove {deleteNodeCount} node{deleteNodeCount !== 1 ? 's' : ''} and {deleteEdgeCount} edge{deleteEdgeCount !== 1 ? 's' : ''}. } confirmText="Delete" diff --git a/frontend/components/icons/ProviderServiceIcon.tsx b/frontend/components/icons/ProviderServiceIcon.tsx index 794f7daf..8e216e59 100644 --- a/frontend/components/icons/ProviderServiceIcon.tsx +++ b/frontend/components/icons/ProviderServiceIcon.tsx @@ -1,7 +1,8 @@ 'use client'; -import { AWS_BRAND_ICON_MAP } from '@/lib/icons/cloud/awsBrandIconMap'; +import { useEffect, useState } from 'react'; import { CloudProviderIcon } from '@/components/icons/CloudProviderIcon'; +import type { AwsBrandIconComponent } from '@/lib/icons/cloud/awsBrandIconMap'; import type { CloudProviderId } from '@/lib/icons/cloud/types'; interface ProviderServiceIconProps { @@ -11,17 +12,43 @@ interface ProviderServiceIconProps { color?: string; } +type AwsIconMap = Record; + /** - * Renders the official AWS architecture icon when available, otherwise the - * lazy-loaded path glyph for AWS/Azure dictionary keys. + * The official AWS icons come from `@react-icons-cloud/aws`, a single pre-bundled file of + * ~500 icons (~250 KB compressed) that the bundler cannot tree-shake. Loading it statically + * put it in every canvas bundle, including the landing hero, which shows no AWS components. + * It is now imported the first time an AWS component renders and shared by all of them. */ -export function ProviderServiceIcon({ provider, serviceKey, size = 18, color }: ProviderServiceIconProps) { - if (provider === 'aws') { - const AwsIcon = AWS_BRAND_ICON_MAP[serviceKey]; - if (AwsIcon) { - return ; - } - } +let awsIcons: AwsIconMap | null = null; +let awsIconsLoading: Promise | null = null; +function loadAwsIcons(): Promise { + awsIconsLoading ??= import('@/lib/icons/cloud/awsBrandIconMap').then((m) => { + awsIcons = m.AWS_BRAND_ICON_MAP; + return awsIcons; + }); + return awsIconsLoading; +} + +function useAwsIcons(enabled: boolean): AwsIconMap | null { + const [icons, setIcons] = useState(awsIcons); + useEffect(() => { + if (!enabled || icons) return; + let live = true; + loadAwsIcons().then((map) => { if (live) setIcons(map); }).catch(() => {}); + return () => { live = false; }; + }, [enabled, icons]); + return icons; +} + +/** + * Renders the official AWS architecture icon when available, otherwise the path glyph for + * AWS/Azure dictionary keys (also shown for the moment before the AWS icons load). + */ +export function ProviderServiceIcon({ provider, serviceKey, size = 18, color }: ProviderServiceIconProps) { + const icons = useAwsIcons(provider === 'aws'); + const AwsIcon = provider === 'aws' ? icons?.[serviceKey] : undefined; + if (AwsIcon) return ; return ; } diff --git a/frontend/components/landing/live/LazyLiveMapHero.tsx b/frontend/components/landing/live/LazyLiveMapHero.tsx index b9b1592e..18d5d257 100644 --- a/frontend/components/landing/live/LazyLiveMapHero.tsx +++ b/frontend/components/landing/live/LazyLiveMapHero.tsx @@ -37,14 +37,30 @@ export function LazyLiveMapHero({ graphs }: { graphs: HeroGraphs }) { // Visible, or the visitor starts scrolling or moving: either way it is // wanted soon, and the first paint never waits for it. const load = () => setNear(true); + // On a phone the map frame starts within the margin of the first screen, so the + // observer fired during hydration and the map's JavaScript competed with the first + // paint (LCP). When it is merely visible, wait for the page's load event and an idle + // moment; any interaction below still loads it at once. + let idle = 0; + const loadWhenIdle = () => { + const run = () => { + const ric = (window as Window & { requestIdleCallback?: (cb: () => void, o?: { timeout: number }) => number }).requestIdleCallback; + idle = ric ? ric(load, { timeout: 1500 }) : window.setTimeout(load, 200); + }; + if (document.readyState === 'complete') run(); + else window.addEventListener('load', run, { once: true }); + }; const observer = new IntersectionObserver((entries) => { - if (entries.some((e) => e.isIntersecting)) load(); + if (entries.some((e) => e.isIntersecting)) loadWhenIdle(); }, { rootMargin: '200px 0px' }); observer.observe(el); const events = ['scroll', 'pointermove', 'touchstart', 'keydown'] as const; for (const type of events) window.addEventListener(type, load, { once: true, passive: true }); return () => { observer.disconnect(); + const cancel = (window as Window & { cancelIdleCallback?: (id: number) => void }).cancelIdleCallback; + if (idle && cancel) cancel(idle); + else if (idle) window.clearTimeout(idle); for (const type of events) window.removeEventListener(type, load); }; }, [near]); diff --git a/frontend/components/landing/live/heroGraph.ts b/frontend/components/landing/live/heroGraph.ts index f14af0af..5043ac75 100644 --- a/frontend/components/landing/live/heroGraph.ts +++ b/frontend/components/landing/live/heroGraph.ts @@ -52,7 +52,7 @@ export interface HeroGraph { nodes: Node[]; edges: Edge[] } export interface HeroGraphs { base: HeroGraph; mapped: HeroGraph; change: StructureChange } async function layout(mermaid: string): Promise { - const result = await runMermaidPipeline(mermaid, { layoutProfile: 'studio-live', areaOrder: 'flow', foldLongFlows: true }); + const result = await runMermaidPipeline(mermaid, { renderStyleId: 'studio-metallic', layoutProfile: 'studio-live', areaOrder: 'flow', foldLongFlows: true }); if (!isDomainSuccess(result)) throw new Error('Landing hero diagram failed to lay out'); return { nodes: result.data.nodes as unknown as Node[], edges: result.data.edges as unknown as Edge[] }; } diff --git a/frontend/components/landing/live/heroRoutes.test.ts b/frontend/components/landing/live/heroRoutes.test.ts index a401df67..bbd315a9 100644 --- a/frontend/components/landing/live/heroRoutes.test.ts +++ b/frontend/components/landing/live/heroRoutes.test.ts @@ -5,7 +5,7 @@ import { buildHeroGraphs } from './heroGraph'; async function hero() { const { base } = await buildHeroGraphs(); - const plan = computeDiagramGeometryPlan(base.nodes, base.edges, { direction: 'LR' }); + const plan = computeDiagramGeometryPlan(base.nodes, base.edges, { direction: 'LR', straightEdges: false, renderStyleId: 'studio-metallic' }); const route = (source: string, target: string) => { const edge = base.edges.find((e) => e.source === source && e.target === target)!; return plan.routes.get(edge.id)!.waypoints; @@ -22,25 +22,29 @@ describe('landing hero layout and routing', () => { expect(score.columnSpacingSpread).toBeLessThan(1); }); - it('draws the gateway fan-out as one shared trunk, with a straight line to the middle target', async () => { + it('routes gateway fan branches through shared orthogonal channels', async () => { const { route } = await hero(); - expect(route('gateway', 'auth')).toHaveLength(4); - expect(route('gateway', 'orders')).toHaveLength(4); + expect(route('gateway', 'auth').length).toBeGreaterThanOrEqual(3); + expect(route('gateway', 'auth').length).toBeLessThanOrEqual(4); + expect(route('gateway', 'orders').length).toBeGreaterThanOrEqual(3); + expect(route('gateway', 'orders').length).toBeLessThanOrEqual(4); expect(route('gateway', 'payments')).toHaveLength(2); const trunkX = new Set(['auth', 'orders'].map((target) => Math.round(route('gateway', target)[1].x))); expect(trunkX.size).toBe(1); }); - it('merges Web and Mobile into the gateway through one channel', async () => { + it('routes Web and Mobile through one orthogonal channel', async () => { const { route } = await hero(); const web = route('web', 'gateway'); const mobile = route('mobile', 'gateway'); - expect(web).toHaveLength(4); - expect(mobile).toHaveLength(4); + expect(web.length).toBeGreaterThanOrEqual(3); + expect(web.length).toBeLessThanOrEqual(4); + expect(mobile.length).toBeGreaterThanOrEqual(3); + expect(mobile.length).toBeLessThanOrEqual(4); expect(Math.round(web[1].x)).toBe(Math.round(mobile[1].x)); }); - it('routes edges between two groups through one channel instead of lanes at both ends', async () => { + it('routes edges between two groups without crossings or unnecessary bends', async () => { const { plan, route } = await hero(); for (const [source, target] of [['auth', 'redis'], ['orders', 'email'], ['orders', 'search'], ['payments', 'stripe']]) { expect(route(source, target).length).toBeLessThanOrEqual(4); @@ -53,7 +57,7 @@ describe('landing hero layout and routing', () => { // Orders feeds both Workers and Data, so those groups must sit in the order of their sources // (Workers above Data); otherwise Orders → Workers would have to cross Payments → Stripe. expect(plan.quality.unexplainedPrivateOverlapLength).toBe(0); - expect(route('orders', 'email')[2].y).toBeLessThan(route('orders', 'postgres')[2].y); - expect(route('payments', 'stripe')[2].y).toBeLessThan(route('auth', 'redis')[2].y); + expect(route('orders', 'email').at(-1)!.y).toBeLessThan(route('orders', 'postgres').at(-1)!.y); + expect(route('payments', 'stripe').at(-1)!.y).toBeLessThan(route('auth', 'redis').at(-1)!.y); }); }); diff --git a/frontend/components/viewers/SharedCanvasViewer.tsx b/frontend/components/viewers/SharedCanvasViewer.tsx index 694eebf1..ab3b2787 100644 --- a/frontend/components/viewers/SharedCanvasViewer.tsx +++ b/frontend/components/viewers/SharedCanvasViewer.tsx @@ -376,8 +376,8 @@ function Viewer({ canvas }: { canvas: SharedCanvas }) { elevateNodesOnSelect={false} elevateEdgesOnSelect={false} panOnDrag - panOnScroll - zoomOnScroll={false} + // A read-only viewer: the wheel zooms (as on a map) and dragging pans. + zoomOnScroll zoomOnPinch fitView fitViewOptions={{ padding: 0.15 }} diff --git a/frontend/data/edgeTypes.ts b/frontend/data/edgeTypes.ts index c7c157f0..7d079eb7 100644 --- a/frontend/data/edgeTypes.ts +++ b/frontend/data/edgeTypes.ts @@ -100,8 +100,13 @@ export interface EdgeData { labelT?: number; /** Keep a label at its rendered-path midpoint instead of collision-aware placement. */ labelPlacement?: 'auto' | 'midpoint'; + /** Last side hint; floating routing recomputes it as node geometry changes. */ sourceSide?: EdgePortSide; + /** Last side hint; floating routing recomputes it as node geometry changes. */ targetSide?: EdgePortSide; + /** Pin a floating attachment when a user or imported diagram explicitly requires that port. */ + sourceSideLocked?: boolean; + targetSideLocked?: boolean; curvature?: number; async?: boolean; importance?: 'primary' | 'secondary' | 'supporting' | 'diagnostic' | 'optional'; diff --git a/frontend/data/templates/archdraw.json b/frontend/data/templates/archdraw.json index db8757ef..68b83a52 100644 --- a/frontend/data/templates/archdraw.json +++ b/frontend/data/templates/archdraw.json @@ -1526,7 +1526,9 @@ "data": { "label": "no template → LLM", "connectionType": "sync", - "edgeVariant": "solid" + "edgeVariant": "solid", + "flowRole": "main", + "flowRoleSource": "explicit" } }, { @@ -1540,7 +1542,9 @@ "data": { "label": "architecture plan", "connectionType": "sync", - "edgeVariant": "solid" + "edgeVariant": "solid", + "flowRole": "main", + "flowRoleSource": "explicit" } }, { @@ -1554,7 +1558,9 @@ "data": { "label": "mermaid code", "connectionType": "sync", - "edgeVariant": "solid" + "edgeVariant": "solid", + "flowRole": "main", + "flowRoleSource": "explicit" } }, { @@ -1568,7 +1574,9 @@ "data": { "label": "RF nodes/edges", "connectionType": "sync", - "edgeVariant": "solid" + "edgeVariant": "solid", + "flowRole": "main", + "flowRoleSource": "explicit" } }, { @@ -1582,7 +1590,9 @@ "data": { "label": "diagram score", "connectionType": "sync", - "edgeVariant": "solid" + "edgeVariant": "solid", + "flowRole": "main", + "flowRoleSource": "explicit" } }, { @@ -1795,6 +1805,20 @@ "edgeVariant": "solid" } }, + { + "id": "e_shapenode_canvas", + "source": "nd_shapenode", + "target": "nd_canvas", + "sourceHandle": null, + "targetHandle": null, + "type": "simpleFloating", + "label": "rendered shapes", + "data": { + "label": "rendered shapes", + "connectionType": "sync", + "edgeVariant": "solid" + } + }, { "id": "e_floatedge_canvas", "source": "nd_floatedge", diff --git a/frontend/data/templates/archdraw.ts b/frontend/data/templates/archdraw.ts index 1f4063fa..3b03fb5e 100644 --- a/frontend/data/templates/archdraw.ts +++ b/frontend/data/templates/archdraw.ts @@ -1,5 +1,5 @@ // Typed wrapper over data/templates/archdraw.json — the ArchDraw -// self-architecture template graph (56 nodes / 54 edges) as pure data. +// self-architecture template graph (56 nodes / 55 edges) as pure data. import template from './archdraw.json'; import type { Node, Edge } from 'reactflow'; diff --git a/frontend/hooks/__tests__/useSnapping.test.ts b/frontend/hooks/__tests__/useSnapping.test.ts new file mode 100644 index 00000000..ea988a1f --- /dev/null +++ b/frontend/hooks/__tests__/useSnapping.test.ts @@ -0,0 +1,58 @@ +import { act, cleanup, renderHook } from '@testing-library/react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import type { Node, NodeChange } from 'reactflow'; +import { useSnapping } from '../useSnapping'; +import { useDiagramStore } from '@/store/diagramStore'; + +const node = (id: string, x: number, y: number, parentNode?: string): Node => ({ + id, position: { x, y }, width: 100, height: 100, data: {}, parentNode, +}); +function load(nodes: Node[]) { + useDiagramStore.setState({ activeCanvasId: 'snap-test', canvases: [{ id: 'snap-test', name: 'Snap', nodes, edges: [] }] }); +} +const move = (id: string, x: number, y: number): NodeChange => ({ + id, type: 'position', position: { x, y }, dragging: true, +}); +afterEach(() => { cleanup(); vi.restoreAllMocks(); }); + +describe('canvas snapping', () => { + it('snaps once without mutating incoming positions, and holds the snap on release', () => { + load([node('a', 0, 0), node('b', 300, 100)]); + const { result } = renderHook(useSnapping); + const change = move('a', 0, 96); + Object.freeze((change as { position: object }).position); + Object.freeze(change); + act(() => result.current.onNodesChange([change])); + expect(useDiagramStore.getState().nodes[0].position).toEqual({ x: 0, y: 100 }); + act(() => result.current.onNodesChange([{ id: 'a', type: 'position', dragging: false, position: { x: 0, y: 96 } }])); + expect(useDiagramStore.getState().nodes[0].position).toEqual({ x: 0, y: 100 }); + expect(useDiagramStore.getState().nodes[0].dragging).toBe(false); + }); + + it('preserves the offset between nodes dragged together', () => { + load([node('a', 0, 0), node('b', 120, 0), node('guide', 400, 100)]); + const { result } = renderHook(useSnapping); + act(() => result.current.onNodesChange([move('a', 0, 96), move('b', 120, 96)])); + const [a, b] = useDiagramStore.getState().nodes; + expect(a.position.y).toBe(100); + expect(b.position).toEqual({ x: a.position.x + 120, y: a.position.y }); + }); + + it('resolves child positions relative to their parent when finding guides', () => { + load([node('parent', 500, 500), node('child', 0, 0, 'parent'), node('guide', 800, 600)]); + const { result } = renderHook(useSnapping); + act(() => result.current.onNodesChange([move('child', 0, 96)])); + expect(useDiagramStore.getState().nodes[1].position).toEqual({ x: 0, y: 100 }); + }); + + it('allows Alt to bypass snapping and resets the modifier on blur', () => { + load([node('a', 0, 0), node('b', 300, 100)]); + const { result } = renderHook(useSnapping); + act(() => window.dispatchEvent(new KeyboardEvent('keydown', { key: 'Alt', altKey: true }))); + act(() => result.current.onNodesChange([move('a', 0, 96)])); + expect(useDiagramStore.getState().nodes[0].position.y).toBe(96); + act(() => window.dispatchEvent(new Event('blur'))); + act(() => result.current.onNodesChange([move('a', 0, 96)])); + expect(useDiagramStore.getState().nodes[0].position.y).toBe(100); + }); +}); diff --git a/frontend/hooks/useHandleSlotLayout.ts b/frontend/hooks/useHandleSlotLayout.ts index 7236edf7..7c9d4270 100644 --- a/frontend/hooks/useHandleSlotLayout.ts +++ b/frontend/hooks/useHandleSlotLayout.ts @@ -148,13 +148,14 @@ function resolveScorerSide( if (!sourceNode || !targetNode) return undefined; const sourceRect = getNodeRect(sourceNode, nodes, renderStyleId, iconMode); const targetRect = getNodeRect(targetNode, nodes, renderStyleId, iconMode); - // Explicit manual sides pin; lane sides only bias the scorer. - // Handle ids do NOT pin sides — floating behavior. + // Saved side values are hints from earlier geometry and may be stale after + // nodes move. Only an explicit lock pins the handle; floating handles follow + // the current graph geometry and let stored sides act as scorer preferences. if (edge.source === nodeId) { - const manual = sideFromDataString(data?.sourceSide); + const manual = data?.sourceSideLocked === true ? sideFromDataString(data?.sourceSide) : undefined; if (manual !== undefined) return manual; } else if (edge.target === nodeId) { - const manual = sideFromDataString(data?.targetSide); + const manual = data?.targetSideLocked === true ? sideFromDataString(data?.targetSide) : undefined; if (manual !== undefined) return manual; } else { return undefined; @@ -171,8 +172,8 @@ function resolveScorerSide( ]); const obstacles = buildScorerObstacles(nodes, excluded, passableGroupIds, renderStyleId, iconMode); - const manualSource = sideFromDataString(data?.sourceSide); - const manualTarget = sideFromDataString(data?.targetSide); + const manualSource = data?.sourceSideLocked === true ? sideFromDataString(data?.sourceSide) : undefined; + const manualTarget = data?.targetSideLocked === true ? sideFromDataString(data?.targetSide) : undefined; const laneSource = sideFromDataString(data?.laneSourceSide); const laneTarget = sideFromDataString(data?.laneTargetSide); diff --git a/frontend/hooks/useSnapping.ts b/frontend/hooks/useSnapping.ts index a9e62e02..b52ca4ae 100755 --- a/frontend/hooks/useSnapping.ts +++ b/frontend/hooks/useSnapping.ts @@ -1,7 +1,7 @@ 'use client'; -import { useCallback, useRef } from 'react'; -import { Node, NodeDragHandler } from 'reactflow'; +import { useCallback, useEffect, useRef } from 'react'; +import { type Node, type NodeDragHandler, type NodeChange } from 'reactflow'; import { useDiagramStore } from '@/store/diagramStore'; import { computeAlignmentGuides } from '@/lib/diagram/geometry/alignmentGuides'; import { nodeSize, nodeTopLeft } from '@/lib/diagram/geometry/nodeBox'; @@ -20,22 +20,74 @@ export function useSnapping() { const setGuideLines = useDiagramStore((s) => s.setGuideLines); const clearTimer = useRef | null>(null); - const onNodeDrag: NodeDragHandler = useCallback( - (event, draggedNode, draggedNodes) => { - if (!draggedNode) return; - // Alt disables snap/guides - const e = event as unknown as MouseEvent & KeyboardEvent; - const altDisabled = e?.altKey; - if (altDisabled) { - setGuideLines([]); + const altDown = useRef(false); + useEffect(() => { + const update = (event: KeyboardEvent) => { altDown.current = event.altKey; }; + const reset = () => { altDown.current = false; }; + window.addEventListener('keydown', update); + window.addEventListener('keyup', update); + window.addEventListener('blur', reset); + return () => { + window.removeEventListener('keydown', update); + window.removeEventListener('keyup', update); + window.removeEventListener('blur', reset); + if (clearTimer.current) clearTimeout(clearTimer.current); + }; + }, []); + + // Transform React Flow's position changes before committing them. There is + // one graph write per frame, and event nodes are never mutated. + const onNodesChange = useCallback((incoming: NodeChange[]) => { + const store = useDiagramStore.getState(); + // React Flow's release can carry its unsnapped drag coordinates. + // Keep the displayed position rather than jumping back on mouseup. + const changes = incoming.map((change) => { + if (change.type !== 'position' || change.dragging !== false) return change; + const node = store.nodes.find((n) => n.id === change.id); + if (!node?.dragging) return change; + return { ...change, position: node.position, positionAbsolute: node.positionAbsolute }; + }); + const positions = new Map(changes.flatMap((change) => + change.type === 'position' && change.dragging && change.position ? [[change.id, change] as const] : [])); + if (!positions.size || altDown.current) { + if (altDown.current) setGuideLines([]); + store.onNodesChange(changes); return; } - - const allNodes = useDiagramStore.getState().nodes; - const edges = useDiagramStore.getState().edges; - // For multi-select, compute guides against nodes not in selection - const draggedIds = new Set((draggedNodes ?? [draggedNode]).map((n) => n.id)); - const others = allNodes.filter((n) => !draggedIds.has(n.id)); + if (clearTimer.current) clearTimeout(clearTimer.current); + const allNodes = store.nodes; + const edges = store.edges; + const draggedIds = new Set(positions.keys()); + // Resolve absolute coordinates from the pending relative positions, + // including nested parents. Persisted nodes may lack positionAbsolute. + const byId = new Map(allNodes.map((n) => [n.id, n])); + const absolute = (node: Node, visited = new Set()): { x: number; y: number } => { + const position = positions.get(node.id)?.position ?? node.position; + if (visited.has(node.id)) return position; + visited.add(node.id); + const parentId = node.parentNode ?? (node as Node & { parentId?: string }).parentId; + const parent = parentId ? byId.get(parentId) : undefined; + if (!parent) return position; + const at = absolute(parent, visited); + return { x: at.x + position.x, y: at.y + position.y }; + }; + const measured = allNodes.map((n) => ({ ...n, positionAbsolute: absolute(n) })); + // Descendants already move with a dragged parent, so cannot guide it. + const movesWithSelection = (node: Node): boolean => { + const seen = new Set(); + let current: Node | undefined = node; + while (current && !seen.has(current.id)) { + if (draggedIds.has(current.id)) return true; + seen.add(current.id); + const parentId: string | undefined = current.parentNode ?? (current as Node & { parentId?: string }).parentId; + current = parentId ? byId.get(parentId) : undefined; + } + return false; + }; + const others = measured.filter((n) => !movesWithSelection(n)); + const draggedNodes = measured.filter((n) => draggedIds.has(n.id)); + const draggedNode = draggedNodes[0]; + if (!draggedNode) { store.onNodesChange(changes); return; } // If multi-drag, use bounding box center; else single node let dragProxy: Node | { position: { x: number; y: number }; width: number; height: number; id: string } = draggedNode; @@ -69,50 +121,23 @@ export function useSnapping() { setGuideLines(guides); - // Magnetic snap: apply delta to dragged nodes via store - if ((snapX !== null && snapX !== 0) || (snapY !== null && snapY !== 0)) { - const dx = snapX ?? 0; - const dy = snapY ?? 0; - if (dx !== 0 || dy !== 0) { - const _toUpdate = draggedNodes && draggedNodes.length > 1 ? draggedNodes : [draggedNode]; - // Mutate position for immediate visual feedback and update store - // React Flow will still emit onNodesChange, but we preemptively snap - const currentNodes = useDiagramStore.getState().nodes; - const snapped = currentNodes.map((n) => { - if (!draggedIds.has(n.id)) return n; - // For bbox mode, apply same delta to each - const abs = (n as unknown as { positionAbsolute?: { x: number; y: number } }).positionAbsolute; - if (abs) { - // Convert absolute snap to relative (if inside group, position is relative) - // Simplest: snap in absolute then convert back via parent offset - // For now apply dx/dy directly to position (works for top-level; nested groups rare during drag) - } - return { - ...n, - position: { x: n.position.x + dx, y: n.position.y + dy }, - }; - }); - // Use setNodes to avoid history push on every drag frame - useDiagramStore.getState().setNodes(snapped); - // Also mutate the event node for RF internal state (prevents jitter) - draggedNode.position.x += dx; - draggedNode.position.y += dy; - if (draggedNodes) { - for (const dn of draggedNodes) { - dn.position.x += dx; - dn.position.y += dy; - } - } - } - } - }, - [setGuideLines] - ); + const dx = snapX ?? 0, dy = snapY ?? 0; + store.onNodesChange(changes.map((change) => { + if (change.type !== 'position' || !change.position || !positions.has(change.id)) return change; + return { + ...change, + position: { x: change.position.x + dx, y: change.position.y + dy }, + ...(change.positionAbsolute ? { positionAbsolute: { + x: change.positionAbsolute.x + dx, y: change.positionAbsolute.y + dy, + } } : {}), + }; + })); + }, [setGuideLines]); const onNodeDragStop: NodeDragHandler = useCallback(() => { if (clearTimer.current) clearTimeout(clearTimer.current); clearTimer.current = setTimeout(() => setGuideLines([]), 400); }, [setGuideLines]); - return { onNodeDrag, onNodeDragStop }; + return { onNodesChange, onNodeDragStop }; } diff --git a/frontend/lib/diagram/edges/__tests__/edgeLabelLayout.test.ts b/frontend/lib/diagram/edges/__tests__/edgeLabelLayout.test.ts index e97c26f6..bf069c4f 100644 --- a/frontend/lib/diagram/edges/__tests__/edgeLabelLayout.test.ts +++ b/frontend/lib/diagram/edges/__tests__/edgeLabelLayout.test.ts @@ -44,6 +44,16 @@ function overlaps(a: { x: number; y: number; w: number; h: number }, b: { x: num return a.x < b.x + b.w && a.x + a.w > b.x && a.y < b.y + b.h && a.y + a.h > b.y } +/** The pill as drawn (estimateLabelSize before LABEL_SAFE_SCALE): 5.8px a character + 14, min 32, 22 high. */ +function drawnRectOf(anchor: EdgeLabelAnchor, text: string) { + const w = Math.max(32, text.length * 5.8 + 14) + return { x: anchor.x - w / 2, y: anchor.y - 11, w, h: 22 } +} + +function boxOf(node: Node) { + return { x: node.position.x, y: node.position.y, w: node.width!, h: node.height! } +} + describe('pointAtFraction', () => { it('interpolates along a straight line by arc length', () => { const segs = buildPathSegments([{ x: 0, y: 0 }, { x: 200, y: 0 }]) @@ -159,8 +169,10 @@ describe('computeEdgeLabelLayout', () => { const expectedA = pointAtFraction(buildPathSegments(routeA.waypoints), a.t) const expectedB = pointAtFraction(buildPathSegments(routeB.waypoints), b.t) expect(Math.hypot(a.x - expectedA.x, a.y - expectedA.y)).toBeLessThan(1) - expect(Math.hypot(b.x - expectedB.x, b.y - expectedB.y)).toBeLessThan(1) - expect(Math.abs(a.y - b.y)).toBeGreaterThan(1) + // The lanes are too close for both pills, so the second steps just beside its line + // rather than covering the first label. + expect(Math.hypot(b.x - expectedB.x, b.y - expectedB.y)).toBeLessThan(20) + expect(overlaps(drawnRectOf(a, 'REQUEST'), drawnRectOf(b, 'RESPONSE'))).toBe(false) }) it('resolves overlaps across several labeled edges', () => { @@ -260,7 +272,7 @@ describe('computeEdgeLabelLayout', () => { expect(overlaps(rectOf(a2, 'stores comment'), rectOf(a3, 'stores like'))).toBe(false) }) - it('places labels on vertical drops into a shared horizontal bus', () => { + it('places labels along straight drops from a shared sink', () => { const nodes = [ makeNode('notif', 80, 40, 200, 80), makeNode('comment', 320, 40, 200, 80), @@ -283,19 +295,29 @@ describe('computeEdgeLabelLayout', () => { edges[2].sourceHandle = 'source-bottom' edges[2].targetHandle = 'target-top' - const res = computeEdgeLabelLayout(edges, nodeInternals(nodes), 'TD') + const routes = new Map(edges.map(e => [ + e.id, + computeEdgeRoute(e, nodes, edges, 'TD', undefined, undefined, undefined, { straightEdges: true }), + ])) + const res = computeEdgeLabelLayout(edges, nodeInternals(nodes), 'TD', undefined, routes) const a1 = res.get('e1')! const a2 = res.get('e2')! const a3 = res.get('e3')! - // Labels must sit on the private vertical legs (near each source's x), - // not on the shared horizontal bus at y≈260. - expect(a1.x).toBeCloseTo(80 + 100, 0) - expect(a2.x).toBeCloseTo(320 + 100, 0) - expect(a3.x).toBeCloseTo(560 + 100, 0) - expect(a1.y).toBeLessThan(250) - expect(a2.y).toBeLessThan(250) - expect(a3.y).toBeLessThan(250) + // Straight wires from each source's bottom to the sink's top: every label + // sits on its own wire, left-to-right in source order, between the ends. + expect(a1.x).toBeGreaterThan(180) + expect(a1.x).toBeLessThan(500) + expect(a2.x).toBeGreaterThan(420) + expect(a2.x).toBeLessThan(500) + expect(a3.x).toBeGreaterThan(500) + expect(a3.x).toBeLessThan(660) + expect(a1.y).toBeGreaterThan(152) + expect(a2.y).toBeGreaterThan(152) + expect(a3.y).toBeGreaterThan(152) + expect(a1.y).toBeLessThan(388) + expect(a2.y).toBeLessThan(388) + expect(a3.y).toBeLessThan(388) expect(Math.abs(a1.x - a2.x)).toBeGreaterThan(100) expect(Math.abs(a2.x - a3.x)).toBeGreaterThan(100) }) @@ -355,9 +377,11 @@ describe('computeEdgeLabelLayout', () => { const a = res.get('e1')! expect(a).toBeDefined() - // Label is centered on the horizontal edge between the nodes (uniform 100px height). - expect(a.y).toBeCloseTo(150, 0) + // The 113px pill does not fit the 100px gap, so centred on the line it would cover both + // cards: it steps beside the line instead, centred along it, and covers neither. expect(a.t).toBeCloseTo(0.5, 1) + for (const node of nodes) expect(overlaps(drawnRectOf(a, 'controls pressure'), boxOf(node))).toBe(false) + expect(Math.abs(a.y - 150)).toBeLessThan(60) }) it('never overlaps any node at the doubled scale across a busy diagram', () => { @@ -454,6 +478,11 @@ describe('computeEdgeLabelLayout', () => { }) describe('composeEdgeDisplayLabel', () => { + it('retains operation qualifiers and complete request/response meaning', () => { + expect(composeEdgeDisplayLabel('Read account transaction history', 'Return paginated transaction results')) + .toBe('Read account transaction history / Return paginated transaction results') + }) + it('joins request and response labels with a separator', () => { expect(composeEdgeDisplayLabel('order', 'receipt')).toBe('order / receipt') }) @@ -501,7 +530,10 @@ describe('splitComposedEdgeLabel', () => { it('moves a label off the bends onto a straight leg when the middle leg is too short', () => { // A Z-shaped route whose middle (vertical) leg is only 60px: its midpoint is on the bends. const nodes = [makeNode('a', 0, 0, 200, 80), makeNode('b', 600, 60, 200, 80)] - const edges = [makeEdge('e1', 'a', 'b', { label: 'queries user' })] + const edges = [makeEdge('e1', 'a', 'b', { + label: 'queries user', + customWaypoints: [{ x: 400, y: 50 }, { x: 400, y: 110 }], + })] const route = computeEdgeRoute(edges[0], nodes, edges, 'LR') const bendX = route.waypoints[1].x expect(route.waypoints[1].y).not.toBe(route.waypoints[2].y) diff --git a/frontend/lib/diagram/edges/__tests__/edgePathGeometry.test.ts b/frontend/lib/diagram/edges/__tests__/edgePathGeometry.test.ts index 71134e7b..a5557615 100644 --- a/frontend/lib/diagram/edges/__tests__/edgePathGeometry.test.ts +++ b/frontend/lib/diagram/edges/__tests__/edgePathGeometry.test.ts @@ -2,6 +2,17 @@ import { describe, expect, it } from 'vitest'; import { buildEdgePathGeometry, pointAtRenderedPath, validateRenderedPath } from '../edgePathGeometry'; describe('shared rendered edge path geometry', () => { + it('preserves obstacle-avoiding bends with square corners', () => { + const path = buildEdgePathGeometry({ + kind: 'orthogonal', + waypoints: [{ x: 0, y: 0 }, { x: 100, y: 0 }, { x: 100, y: 100 }], + cornerRadius: 0, + }); + expect(path.d).toBe('M 0,0 L 100,0 L 100,100'); + expect(path.length).toBeCloseTo(200); + expect(validateRenderedPath(path, [{ id: 'middle', rect: { x: 40, y: 40, width: 20, height: 20 } }], 0)).toEqual([]); + }); + it('keeps the rounded SVG path, commands, and samples consistent', () => { const path = buildEdgePathGeometry({ kind: 'orthogonal', diff --git a/frontend/lib/diagram/edges/__tests__/edgeRouteBuilder.snap.test.ts b/frontend/lib/diagram/edges/__tests__/edgeRouteBuilder.snap.test.ts index 8cc9af4e..9511edc7 100644 --- a/frontend/lib/diagram/edges/__tests__/edgeRouteBuilder.snap.test.ts +++ b/frontend/lib/diagram/edges/__tests__/edgeRouteBuilder.snap.test.ts @@ -31,40 +31,65 @@ describe('computeEdgeRoute routing', () => { expect(route.svgPath).toBe('M 312,153 L 488,153'); }); - it('does not straighten a near-aligned link through another node', () => { + it('does not straighten a link through another node', () => { const nodes = [ - makeNode('a', 100, 100), - makeNode('b', 500, 106), - makeNode('blocker', 355, 120, 80, 60), + makeNode('a', 0, 100), + makeNode('b', 800, 100), + makeNode('blocker', 400, 60), ]; const edge = makeEdge('e1', 'a', 'b'); - const route = computeEdgeRoute(edge, nodes, [edge], 'LR'); + // Force the flow sides so the direct wire has to cross the blocker. + edge.data = { sourceSide: 'right', targetSide: 'left' }; + const route = computeEdgeRoute(edge, nodes, [edge], 'LR', undefined, undefined, undefined, { straightEdges: true }); expect(route.waypoints.length).toBeGreaterThan(2); expect(route.waypoints[0]).toEqual(route.sourcePoint); expect(route.waypoints[route.waypoints.length - 1]).toEqual(route.targetPoint); }); - it('keeps a tiny off-axis jog as a routed path (no straight-snap)', () => { + it('draws a straight diagonal for a tiny off-axis jog', () => { const nodes = [makeNode('a', 100, 100), makeNode('b', 500, 112)]; const edge = makeEdge('e1', 'a', 'b'); - const route = computeEdgeRoute(edge, nodes, [edge], 'LR'); + const route = computeEdgeRoute(edge, nodes, [edge], 'LR', undefined, undefined, undefined, { straightEdges: true }); - expect(route.waypoints.length).toBeGreaterThan(2); - expect(route.waypoints[0]).toEqual({ x: 312, y: 150 }); - expect(route.waypoints[route.waypoints.length - 1]).toEqual({ x: 488, y: 162 }); + // Nothing between the handles: the shortest wire is the straight one. + expect(route.waypoints).toEqual([ + { x: 312, y: 150 }, + { x: 488, y: 162 }, + ]); + expect(route.svgPath).toBe('M 312,150 L 488,162'); }); - it('keeps the routed Z path for a large vertical offset', () => { + it('draws a straight diagonal for a large vertical offset', () => { const nodes = [makeNode('a', 100, 100), makeNode('b', 500, 300)]; const edge = makeEdge('e1', 'a', 'b'); - const route = computeEdgeRoute(edge, nodes, [edge], 'LR'); + const route = computeEdgeRoute(edge, nodes, [edge], 'LR', undefined, undefined, undefined, { straightEdges: true }); - expect(route.waypoints.length).toBeGreaterThan(2); - expect(route.waypoints[0]).toEqual({ x: 312, y: 150 }); - expect(route.waypoints[route.waypoints.length - 1]).toEqual({ x: 488, y: 350 }); + expect(route.waypoints).toEqual([ + { x: 312, y: 150 }, + { x: 488, y: 350 }, + ]); + expect(route.svgPath).toBe('M 312,150 L 488,350'); }); + it('draws a straight fan branch instead of a shared channel when clear', () => { + const nodes = [ + makeNode('a', 100, 200), + makeNode('b', 600, 120), + makeNode('c', 600, 300), + ] + const e1 = makeEdge('e1', 'a', 'b') + const e2 = makeEdge('e2', 'a', 'c') + const opts = { straightEdges: true } + const r1 = computeEdgeRoute(e1, nodes, [e1, e2], 'LR', undefined, undefined, undefined, opts) + const r2 = computeEdgeRoute(e2, nodes, [e1, e2], 'LR', undefined, undefined, undefined, opts) + + // Both branches leave the same anchor: a shared orthogonal channel would + // draw them on one wire, but the straight diagonals already diverge. + expect(r1.waypoints.length).toBe(2) + expect(r2.waypoints.length).toBe(2) + }) + it('keeps parallel-edge separation (does not snap overlapping straights)', () => { const nodes = [makeNode('a', 100, 100), makeNode('b', 500, 112)]; const e1 = makeEdge('e1', 'a', 'b'); @@ -83,7 +108,7 @@ describe('computeEdgeRoute routing', () => { const nodes = [makeNode('a', 100, 100), makeNode('b', 500, 100)]; const pinned = { ...makeEdge('a-b', 'a', 'b'), - data: { sourceSide: 'top', targetSide: 'top' }, + data: { sourceSide: 'top', targetSide: 'top', sourceSideLocked: true, targetSideLocked: true }, } as Edge; const reverse = makeEdge('b-a', 'b', 'a'); @@ -99,7 +124,7 @@ describe('computeEdgeRoute routing', () => { const nodes = [makeNode('a', 100, 100), makeNode('b', 500, 100)]; const pinned = { ...makeEdge('a-b', 'a', 'b'), - data: { sourceSide: side, targetSide: side }, + data: { sourceSide: side, targetSide: side, sourceSideLocked: true, targetSideLocked: true }, } as Edge; const reverse = makeEdge('b-a', 'b', 'a'); @@ -118,7 +143,7 @@ describe('computeEdgeRoute routing', () => { ]; const edge = { ...makeEdge('a-b', 'a', 'b'), - data: { sourceSide: 'right', targetSide: 'left' }, + data: { sourceSide: 'right', targetSide: 'left', sourceSideLocked: true, targetSideLocked: true }, } as Edge; const route = computeEdgeRoute(edge, nodes, [edge], 'LR'); @@ -138,7 +163,7 @@ describe('computeEdgeRoute routing', () => { ]; const edge = { ...makeEdge('a-b', 'a', 'b'), - data: { sourceSide: 'right', targetSide: 'left' }, + data: { sourceSide: 'right', targetSide: 'left', sourceSideLocked: true, targetSideLocked: true }, } as Edge; const route = computeEdgeRoute(edge, nodes, [edge], 'LR'); diff --git a/frontend/lib/diagram/edges/__tests__/edgeRouteGroupAvoidance.test.ts b/frontend/lib/diagram/edges/__tests__/edgeRouteGroupAvoidance.test.ts index b6f3fa9a..8bf2f4b6 100644 --- a/frontend/lib/diagram/edges/__tests__/edgeRouteGroupAvoidance.test.ts +++ b/frontend/lib/diagram/edges/__tests__/edgeRouteGroupAvoidance.test.ts @@ -70,10 +70,11 @@ function pathHitsRect(points: Array<{ x: number; y: number }>, rect: Rect): bool return false; } -describe('computeEdgeRoute group-node avoidance', () => { - it('routes an edge inside a group around an unrelated group that blocks the corridor', () => { +describe('computeEdgeRoute group-node handling', () => { + it('passes an edge inside a group straight through another group’s empty space', () => { // Nodes A and B both live in grpA; grpB is a separate group sitting in the - // middle of the direct path between them. + // middle of the direct path between them. Groups are passable: only cards + // block an edge, so the shortest route is a straight line. const grpA = makeGroup('grpA', 0, 0, 900, 200); const s = makeChild('s', 'grpA', 80, 70); const t = makeChild('t', 'grpA', 700, 70); @@ -83,14 +84,12 @@ describe('computeEdgeRoute group-node avoidance', () => { const edge = makeEdge('e1', 's', 't'); const route = computeEdgeRoute(edge, nodes, [edge], 'LR'); - // The direct corridor (y≈105, x 200→700) crosses grpB (x 400–540, y 55–145). - expect(route.waypoints.length).toBeGreaterThan(2); - expect(pathHitsRect(route.waypoints, { x: 400, y: 55, w: 140, h: 90 })).toBe(false); + expect(route.waypoints.length).toBe(2); expect(route.waypoints[0]).toEqual(route.sourcePoint); expect(route.waypoints[route.waypoints.length - 1]).toEqual(route.targetPoint); }); - it('routes an edge between two groups around a third unrelated group', () => { + it('passes an edge between two groups straight through a third group’s space', () => { const grpA = makeGroup('grpA', 0, 0, 300, 200); const grpC = makeGroup('grpC', 600, 0, 300, 200); const s = makeChild('s', 'grpA', 80, 70); @@ -101,11 +100,12 @@ describe('computeEdgeRoute group-node avoidance', () => { const edge = makeEdge('e1', 's', 't'); const route = computeEdgeRoute(edge, nodes, [edge], 'LR'); - expect(route.waypoints.length).toBeGreaterThan(2); - expect(pathHitsRect(route.waypoints, { x: 350, y: 55, w: 200, h: 90 })).toBe(false); + expect(route.waypoints.length).toBe(2); + expect(route.waypoints[0]).toEqual(route.sourcePoint); + expect(route.waypoints[route.waypoints.length - 1]).toEqual(route.targetPoint); }); - it('routes an edge from a group to a free node around an unrelated group', () => { + it('passes an edge from a group to a free node straight through an unrelated group’s space', () => { const grpA = makeGroup('grpA', 0, 0, 300, 200); const s = makeChild('s', 'grpA', 80, 70); const t = makeNode('t', 700, 70); @@ -115,8 +115,9 @@ describe('computeEdgeRoute group-node avoidance', () => { const edge = makeEdge('e1', 's', 't'); const route = computeEdgeRoute(edge, nodes, [edge], 'LR'); - expect(route.waypoints.length).toBeGreaterThan(2); - expect(pathHitsRect(route.waypoints, { x: 350, y: 55, w: 200, h: 90 })).toBe(false); + expect(route.waypoints.length).toBe(2); + expect(route.waypoints[0]).toEqual(route.sourcePoint); + expect(route.waypoints[route.waypoints.length - 1]).toEqual(route.targetPoint); }); it('does not treat a node as an obstacle when it is inside the same group', () => { diff --git a/frontend/lib/diagram/edges/__tests__/edgeRouteOptimizer.rollback.test.ts b/frontend/lib/diagram/edges/__tests__/edgeRouteOptimizer.rollback.test.ts index 6b38c8bb..40dc2800 100644 --- a/frontend/lib/diagram/edges/__tests__/edgeRouteOptimizer.rollback.test.ts +++ b/frontend/lib/diagram/edges/__tests__/edgeRouteOptimizer.rollback.test.ts @@ -4,9 +4,37 @@ import { buildEdgePathGeometry } from '../edgePathGeometry'; import { optimizeEdgeRoutes } from '../edgeRouteOptimizer'; const route = vi.hoisted(() => vi.fn()); -vi.mock('../edgeRouteBuilder', () => ({ computeEdgeRoute: route, createEdgeRoutingContext: () => ({}) })); +vi.mock('../edgeRouteBuilder', async (importOriginal) => ({ + ...(await importOriginal()), + computeEdgeRoute: route, + createEdgeRoutingContext: () => ({}), +})); describe('route repair acceptance', () => { + it('detects sibling crossings away from their shared source', () => { + let crossing = true; + route.mockImplementation((edge: Edge) => { + const waypoints = edge.id === 'sa' + ? [{ x: 0, y: 0 }, { x: 100, y: 0 }, { x: 100, y: 200 }, { x: 300, y: 200 }] + : [{ x: 0, y: 0 }, { x: 50, y: 0 }, { x: 50, y: crossing ? 100 : -100 }, { x: 250, y: crossing ? 100 : -100 }]; + const path = { kind: 'orthogonal' as const, waypoints, cornerRadius: 0 }; + const renderedPath = buildEdgePathGeometry(path); + return { + sourcePosition: 'right', targetPosition: 'left', sourcePoint: waypoints[0], targetPoint: waypoints.at(-1), + waypoints, svgPath: renderedPath.d, path, renderedPath, + status: 'ok', violations: [], expandedStates: 0, + }; + }); + const nodes = ['s', 'a', 'b'].map(id => ({ + id, type: 'systemNode', data: { label: id }, position: { x: 0, y: 0 }, + })) as Node[]; + const edges = [{ id: 'sa', source: 's', target: 'a' }, { id: 'sb', source: 's', target: 'b' }] as Edge[]; + const input = { nodes, edges, direction: 'LR' as const, searchBudget: { maxCandidateRoutes: 2, ripUpPasses: 0 } }; + expect(optimizeEdgeRoutes(input).diagnostics.filter(item => item.kind === 'crossing')).toHaveLength(1); + crossing = false; + expect(optimizeEdgeRoutes(input).diagnostics.filter(item => item.kind === 'crossing')).toHaveLength(0); + }); + it('restores the prior routes when a rip-up pass worsens the objective', () => { let calls = 0; route.mockImplementation(() => { diff --git a/frontend/lib/diagram/edges/__tests__/fanLaneSeparation.test.ts b/frontend/lib/diagram/edges/__tests__/fanLaneSeparation.test.ts new file mode 100644 index 00000000..706a24f0 --- /dev/null +++ b/frontend/lib/diagram/edges/__tests__/fanLaneSeparation.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, it } from 'vitest'; +import { Position, type Edge, type Node } from 'reactflow'; +import { buildEdgePathGeometry, getRenderedPathInteraction } from '../edgePathGeometry'; +import type { EdgeRouteResult } from '../edgeRouteBuilder'; +import { FAN_LANE_GAP, separateFanLanes } from '../fanLaneSeparation'; +import { composeEdgeDisplayLabel } from '../edgeLabelLayout'; + +type Pt = { x: number; y: number }; + +function routeOf(waypoints: Pt[]): EdgeRouteResult { + const path = { kind: 'orthogonal' as const, waypoints, cornerRadius: 12 }; + const renderedPath = buildEdgePathGeometry(path); + return { + sourcePosition: Position.Right, targetPosition: Position.Left, + sourcePoint: waypoints[0], targetPoint: waypoints[waypoints.length - 1], + waypoints, svgPath: renderedPath.d, path, renderedPath, status: 'ok', violations: [], + }; +} + +const node = (id: string, x: number, y: number): Node => + ({ id, type: 'shapeNode', position: { x, y }, width: 200, height: 80, data: { label: id, nodeWidth: 200, nodeHeight: 80 } }) as Node; + +describe('separateFanLanes', () => { + // s fans out to a (just below) and b (far below): both leave s's right anchor and share x = 300. + const nodes = [node('s', 0, 0), node('a', 500, 120), node('b', 500, 320)]; + const edges = [{ id: 'sa', source: 's', target: 'a' }, { id: 'sb', source: 's', target: 'b' }] as Edge[]; + const shared = () => new Map([ + ['sa', routeOf([{ x: 200, y: 40 }, { x: 300, y: 40 }, { x: 300, y: 160 }, { x: 500, y: 160 }])], + ['sb', routeOf([{ x: 200, y: 40 }, { x: 300, y: 40 }, { x: 300, y: 360 }, { x: 500, y: 360 }])], + ]); + + it('gives each edge of a fan its own lane, nested so they do not cross', () => { + const routes = shared(); + separateFanLanes(routes, edges, nodes); + const sa = routes.get('sa')!, sb = routes.get('sb')!; + const laneA = sa.waypoints[1].x, laneB = sb.waypoints[1].x; + expect(Math.abs(laneA - laneB)).toBe(FAN_LANE_GAP); + // Turning away from the source: the farther target keeps the inner lane. + expect(laneB).toBeLessThan(laneA); + expect(getRenderedPathInteraction(sa.renderedPath, sb.renderedPath).crossing).toBe(false); + // Anchors and arrowheads stay where they were. + expect(sa.waypoints[0]).toEqual({ x: 200, y: 40 }); + expect(sb.waypoints.at(-1)).toEqual({ x: 500, y: 360 }); + }); + + it('keeps the shared line when a lane would run into a card', () => { + // A card right where the outer lane would go. + const blocked = [...nodes, node('wall', 306, 60)]; + const routes = shared(); + separateFanLanes(routes, edges, blocked); + expect(routes.get('sa')!.waypoints[1].x).toBe(300); + expect(routes.get('sb')!.waypoints[1].x).toBe(300); + }); +}); + +describe('composeEdgeDisplayLabel', () => { + it('preserves qualifiers and every listed operation', () => { + expect(composeEdgeDisplayLabel('index text & metadata', undefined)).toBe('index text & metadata'); + expect(composeEdgeDisplayLabel('list, rename, move, delete', undefined)).toBe('list, rename, move, delete'); + expect(composeEdgeDisplayLabel('thumbnails & previews', undefined)).toBe('thumbnails & previews'); + }); +}); diff --git a/frontend/lib/diagram/edges/__tests__/handleSlotOrder.test.ts b/frontend/lib/diagram/edges/__tests__/handleSlotOrder.test.ts index f62c9117..9381e8ae 100644 --- a/frontend/lib/diagram/edges/__tests__/handleSlotOrder.test.ts +++ b/frontend/lib/diagram/edges/__tests__/handleSlotOrder.test.ts @@ -289,7 +289,7 @@ describe('computeDynamicSlotOffsets', () => { makeEdge('e1', 'above', 'center', 'source-left', 'target-left'), { ...makeEdge('e2', 'center', 'below', 'source-right', 'target-right'), - data: { sourceSide: 'right', targetSide: 'right' }, + data: { sourceSide: 'right', targetSide: 'right', sourceSideLocked: true, targetSideLocked: true }, } as Edge, ]; diff --git a/frontend/lib/diagram/edges/__tests__/handleSlotSwap.test.ts b/frontend/lib/diagram/edges/__tests__/handleSlotSwap.test.ts index 67b95cab..f422388f 100644 --- a/frontend/lib/diagram/edges/__tests__/handleSlotSwap.test.ts +++ b/frontend/lib/diagram/edges/__tests__/handleSlotSwap.test.ts @@ -54,10 +54,10 @@ describe('getEdgeShiftOffset role → dedicated tip', () => { it('never places incoming and outgoing on the same tip when both share a side via data overrides', () => { const edges: Edge[] = [ - { id: 'i1', source: 'a', target: 'hub', data: { targetSide: 'right' } }, - { id: 'i2', source: 'b', target: 'hub', data: { targetSide: 'right' } }, - { id: 'o1', source: 'hub', target: 'e', data: { sourceSide: 'right' } }, - { id: 'o2', source: 'hub', target: 'f', data: { sourceSide: 'right' } }, + { id: 'i1', source: 'a', target: 'hub', data: { targetSide: 'right', targetSideLocked: true } }, + { id: 'i2', source: 'b', target: 'hub', data: { targetSide: 'right', targetSideLocked: true } }, + { id: 'o1', source: 'hub', target: 'e', data: { sourceSide: 'right', sourceSideLocked: true } }, + { id: 'o2', source: 'hub', target: 'f', data: { sourceSide: 'right', sourceSideLocked: true } }, ]; const inOnRight = getEdgeShiftOffset('hub', 'i1', Position.Right, edges, internals); const outOnRight = getEdgeShiftOffset('hub', 'o1', Position.Right, edges, internals); diff --git a/frontend/lib/diagram/edges/__tests__/lowBendReroute.test.ts b/frontend/lib/diagram/edges/__tests__/lowBendReroute.test.ts index ed6dc9da..9a2fbdd6 100644 --- a/frontend/lib/diagram/edges/__tests__/lowBendReroute.test.ts +++ b/frontend/lib/diagram/edges/__tests__/lowBendReroute.test.ts @@ -56,6 +56,18 @@ describe('low-bend collision reroutes', () => { ]) }) + it('chooses the shorter side route instead of keeping a bottom attachment from the initial heuristic', () => { + const source = makeNode('auth', 0, 0, 304, 100) + const target = makeNode('jwt', 674, 90, 304, 110) + const edge = { id: 'token', source: 'auth', target: 'jwt', data: {} } as Edge + + const route = computeEdgeRoute(edge, [source, target], [edge], 'LR') + + expect(route.sourcePosition).toBe('right') + expect(route.targetPosition).toBe('left') + expect(route.waypoints.length).toBeLessThanOrEqual(4) + }) + it('wraps a mid-corridor blocker with a clean U (≤4 bends, no terminal S)', () => { const source = makeNode('source', 0, 100) const target = makeNode('target', 500, 100) diff --git a/frontend/lib/diagram/edges/edgeLabelLayout.ts b/frontend/lib/diagram/edges/edgeLabelLayout.ts index 1176d9f8..f5b53e5e 100644 --- a/frontend/lib/diagram/edges/edgeLabelLayout.ts +++ b/frontend/lib/diagram/edges/edgeLabelLayout.ts @@ -48,7 +48,7 @@ const BORDER_RADIUS = 24 */ const PARALLEL_LABEL_STACK_GAP = 0 -function estimateLabelSize(text: string, renderStyle?: DiagramRenderStyleId): { w: number; h: number } { +export function estimateLabelSize(text: string, renderStyle?: DiagramRenderStyleId): { w: number; h: number } { const brutal = renderStyle === 'neubrutalism' const sketch = renderStyle === 'sketch' const wpc = brutal ? BRUTAL_WIDTH_PER_CHAR : sketch ? SKETCH_WIDTH_PER_CHAR : LABEL_WIDTH_PER_CHAR @@ -284,12 +284,10 @@ export function composeEdgeDisplayLabel( // half must never leave a dangling "/". Inner slashes ("read/write", // "/api/users") are content and pass through untouched. const raw = base && resp ? `${base} / ${resp}` : base || resp - const words = raw ? raw.split(/\s+/).filter(Boolean) : [] - if (words.length === 0) return '' - const full = words.length <= 3 ? raw : words.slice(0, 3).join(' ') - // Defensive: strip a leftover separator from legacy data (slash with - // adjacent whitespace), but preserve content slashes like "/api/users". - return full.replace(/^\s*\/\s+/, '').replace(/\s+\/\s*$/, '').trim() + // Preserve operation qualifiers and response semantics. Layout reserves the + // displayed text's full width rather than silently discarding words. + return raw.replace(/^\s*\/\s+/, '').replace(/\s+\/\s*$/, '').trim() + } /** @@ -312,7 +310,7 @@ export function splitComposedEdgeLabel(draft: string): { base: string; response: }; } -function getDisplayLabel(edge: Edge): string { +export function getDisplayLabel(edge: Edge): string { const data = edge.data as Record | undefined const base = typeof edge.label === 'string' && edge.label.trim() @@ -340,7 +338,7 @@ function getPreferredT(edge: Edge, parallelEdges: Edge[], labelOrder: number, ve // along the line. The spot is counted from the first edge's source: a reply runs the other way, // so its own t is mirrored. Otherwise a request at 0.4 and its reply at 0.6 (counted from the // reply's own source) landed on the same spot and covered each other. - const t = Math.max(0.2, Math.min(0.8, 0.5 + (labelOrder - (parallelEdges.length - 1) / 2) * 0.3)) + const t = Math.max(0.2, Math.min(0.8, 0.5 + (labelOrder - (parallelEdges.length - 1) / 2) * 0.5)) return edge.source === parallelEdges[0].source ? t : 1 - t } return 0.5 @@ -452,6 +450,10 @@ const LABEL_CORNER_GAP = 6 * 1.5x safe scale, so world gap is ~30px. */ const NODE_LABEL_GAP = 20 +/** Gap between the line and a label stepped beside it (only when every spot on the line covers a card). */ +const LABEL_ESCAPE_GAP = 6 +/** Multiples of (half the pill height + gap) tried on each side of the line. */ +const LABEL_ESCAPE_STEPS = [1, 2, 3] const BRUTAL_NODE_LABEL_GAP = 22 function totalPathLength(segs: PathSegment[]): number { @@ -651,6 +653,7 @@ function buildCandidates( userPlaced = false, /** Height of the pill in world px: its extent along a vertical edge. */ labelHeight = labelWidth, + paired = false, /** * Keep the pill off bends: spots where the whole pill sits on one straight run come first, and * each end's clearance is measured along the run there (see computeEdgeLabelLayout). @@ -695,7 +698,7 @@ function buildCandidates( // Measured on the canvas: the arrowhead plus its handle dot take about 24px of the end, // and labels are drawn up to 1.5x larger when zoomed out. Leave a visible stretch of // line between the pill and the arrowhead (about 24px) on top of that. - const sourceGap = brutal ? 30 : 24 + const sourceGap = brutal ? 30 : paired ? 12 : 24 const targetGap = brutal ? 60 : 48 const marginOf = (px: number, floor: number, extent: number) => (totalLen > 1 ? Math.min(0.45, Math.max(floor, (extent + px) / totalLen)) : 0.45) // The target end carries the arrowhead and its handle, so it keeps a firmer floor; the @@ -786,6 +789,9 @@ function computeLayout( const y = (n.positionAbsolute?.y ?? n.position.y) - nodeGap return { x, y, w: w + 2 * nodeGap, h: h + 2 * nodeGap } }) + const cardBoxes: PlacedRect[] = nodeObstacles.map((r) => ({ + x: r.x + nodeGap, y: r.y + nodeGap, w: r.w - 2 * nodeGap, h: r.h - 2 * nodeGap, + })) // Pass 1: route every edge so fan-in/fan-out sharing can be detected even // against unlabeled siblings that occupy the same corridor. @@ -860,12 +866,16 @@ function computeLayout( const stem = pathMerges ? findUniqueStemRange(item.samples, peerSamples) : { uniqueEndFromSource: 0.5, uniqueStartFromTarget: 0.5 } - const preferredT = pathMerges + // A request and its reply should use opposite stretches of the shared + // route. Unique-stem detection sees their offset terminal lanes as private + // and can put both pills back at the midpoint, where they collide. + const reversePair = parallelEdges.some((peer) => peer.id !== edge.id && peer.source === edge.target && peer.target === edge.source) + const preferredT = pathMerges && !reversePair ? preferredTForUniqueStem(edge, parallelEdges, labelOrder, stem, segs) : getPreferredT(edge, parallelEdges, labelOrder, isMostlyVertical(segs)) let preferredRange: { lo: number; hi: number } | undefined - if (pathMerges && typeof data?.labelT !== 'number') { + if (pathMerges && !reversePair && typeof data?.labelT !== 'number') { const fromSourceLen = stem.uniqueEndFromSource const fromTargetLen = 1 - stem.uniqueStartFromTarget if (fromSourceLen >= MIN_UNIQUE_STEM && fromSourceLen >= fromTargetLen) { @@ -895,7 +905,7 @@ function computeLayout( segs, basePerp: perpOffset, paired: parallelEdges.length > 1, - candidates: buildCandidates(segs, preferredT, perpOffset, preferredRange, renderStyle, estimateLabelSize(item.label, renderStyle).w, typeof data?.labelT === 'number', estimateLabelSize(item.label, renderStyle).h, avoidCorners), + candidates: buildCandidates(segs, preferredT, perpOffset, preferredRange, renderStyle, estimateLabelSize(item.label, renderStyle).w, typeof data?.labelT === 'number', estimateLabelSize(item.label, renderStyle).h, parallelEdges.length > 1, avoidCorners), sortX: preferred.x, sortY: preferred.y, }) @@ -927,6 +937,13 @@ function computeLayout( }) const clearOfNodes = (cand: LabelCandidate) => !nodeObstacles.some((n) => rectsOverlap(n, cssRectAt(cand))) + /** The drawn pill does not cover a card (the safe size and the clearance margin may be touched). */ + const drawnW = item.size.w / LABEL_SAFE_SCALE + const drawnH = item.size.h / LABEL_SAFE_SCALE + const clearOfCards = (cand: LabelCandidate) => { + const rect = { x: cand.x - drawnW / 2, y: cand.y - drawnH / 2, w: drawnW, h: drawnH } + return !cardBoxes.some((n) => rectsOverlap(n, rect)) + } const clearOfLabels = (cand: LabelCandidate) => { const rect = { x: cand.x - item.size.w / 2, y: cand.y - item.size.h / 2, w: item.size.w, h: item.size.h } return !placed.some((p) => rectsOverlap(p, rect)) @@ -979,6 +996,28 @@ function computeLayout( } } } + // No label may cover a card or another label. On the line first: crossing another line is + // fine, then so is sitting inside a card's clearance margin, as long as the card itself is clear. + const firstOnLine = (accept: (cand: LabelCandidate) => boolean) => item.candidates.find(accept) ?? null + if (!chosen) chosen = firstOnLine((cand) => clearOfNodes(cand) && clearOfLabels(cand)) + if (!chosen) chosen = firstOnLine((cand) => clearOfCards(cand) && clearOfLabels(cand)) + // Only when every spot on the line covers something, step the pill beside the line + // (either side, nearest first). + if (!chosen) { + const step = drawnH / 2 + LABEL_ESCAPE_GAP + const escapes = LABEL_ESCAPE_STEPS.flatMap((k) => [k * step, -k * step]) + const beside = (accept: (cand: LabelCandidate) => boolean) => { + for (const perp of escapes) { + for (const base of item.candidates) { + const cand = pointFor(base.t, perp) + if (accept(cand)) return cand + } + } + return null + } + chosen = beside((cand) => clearOfNodes(cand) && clearOfLabels(cand) && clearOfOtherEdges(cand)) + ?? beside((cand) => clearOfCards(cand) && clearOfLabels(cand)) + } if (!chosen) { for (const cand of item.candidates) { if (clearOfNodes(cand) && clearOfOtherEdges(cand)) { @@ -1014,6 +1053,7 @@ function labelObstructions( anchors: ReadonlyMap, edges: Edge[], nodeInternals: ReadonlyMap, + renderStyle?: DiagramRenderStyleId, ): { onNodes: Set; onLabels: number } { const boxes = [...nodeInternals.values()].map((node) => { const data = node.data as { nodeWidth?: number; nodeHeight?: number } | undefined @@ -1032,8 +1072,8 @@ function labelObstructions( const anchor = anchors.get(edge.id) const label = getDisplayLabel(edge) if (!anchor || !label) continue - const w = Math.min(260, Math.max(48, label.length * 8 + 24)) - const rect = { x: anchor.x - w / 2, y: anchor.y - 12, w, h: 24 } + const { w, h } = estimateLabelSize(label, renderStyle) + const rect = { x: anchor.x - w / 2, y: anchor.y - h / 2, w, h } const hit = boxes.some((box) => box.group ? rectsOverlap(rect, { x: box.x, y: box.y, w: box.w, h: Math.min(40, box.h) }) : box.node.id !== edge.source && box.node.id !== edge.target && rectsOverlap(rect, box)) @@ -1104,8 +1144,8 @@ export function computeEdgeLabelLayout( const hasBends = [...(routes?.values() ?? [])].some(route => route.waypoints.length > 2) || !routes if (hasBends && edges.some(edge => getDisplayLabel(edge))) { const cornerFree = computeLayout(edges, nodeInternals, direction, renderStyle, routes, iconMode, true) - const a = labelObstructions(cornerFree, edges, nodeInternals) - const b = labelObstructions(plain, edges, nodeInternals) + const a = labelObstructions(cornerFree, edges, nodeInternals, renderStyle) + const b = labelObstructions(plain, edges, nodeInternals, renderStyle) // No label may end up covering something it did not cover before. if ([...a.onNodes].every(id => b.onNodes.has(id)) && a.onLabels <= b.onLabels) result = cornerFree } diff --git a/frontend/lib/diagram/edges/edgePathGeometry.ts b/frontend/lib/diagram/edges/edgePathGeometry.ts index a79718bb..5b304307 100644 --- a/frontend/lib/diagram/edges/edgePathGeometry.ts +++ b/frontend/lib/diagram/edges/edgePathGeometry.ts @@ -112,7 +112,11 @@ function orthogonalCommands(points: readonly Point[], requestedRadius: number): const cross = incoming.x * outgoing.y - incoming.y * outgoing.x; if (Math.abs(cross) < 1e-6) continue; const radius = Math.max(0, Math.min(requestedRadius, distance(prev, corner) / 2, distance(corner, next) / 2)); - if (radius <= 0) continue; + if (radius <= 0) { + if (distance(cursor, corner) > 0.001) commands.push({ kind: 'line', from: cursor, to: corner }); + cursor = corner; + continue; + } const start = { x: corner.x - incoming.x * radius, y: corner.y - incoming.y * radius }; const end = { x: corner.x + outgoing.x * radius, y: corner.y + outgoing.y * radius }; if (distance(cursor, start) > 0.001) commands.push({ kind: 'line', from: cursor, to: start }); diff --git a/frontend/lib/diagram/edges/edgeRouteBuilder.ts b/frontend/lib/diagram/edges/edgeRouteBuilder.ts index 04f1d081..eb251032 100644 --- a/frontend/lib/diagram/edges/edgeRouteBuilder.ts +++ b/frontend/lib/diagram/edges/edgeRouteBuilder.ts @@ -53,6 +53,26 @@ function completeRoute= 2) { + const points = [...result.waypoints]; + const last = points[points.length - 1]; + const previous = points[points.length - 2]; + const box = terminals.target; + const side = result.targetPosition; + const tip = side === Position.Top ? { x: last.x, y: box.y } + : side === Position.Bottom ? { x: last.x, y: box.y + box.h } + : side === Position.Left ? { x: box.x, y: last.y } + : { x: box.x + box.w, y: last.y }; + if (Math.hypot(tip.x - last.x, tip.y - last.y) <= 16) { + const normalLeg = side === Position.Top || side === Position.Bottom + ? Math.abs(previous.x - last.x) < 0.01 : Math.abs(previous.y - last.y) < 0.01; + if (normalLeg) points[points.length - 1] = tip; + else points.push(tip); + result = { ...result, waypoints: points, targetPoint: tip }; + } + } const validation = routeStatus(result.waypoints, nodes, excludedNodeIds, terminals) const obstacleViolations = (routeOutcome?.violatedObstacleIds ?? []).map(obstacleId => ({ code: 'OBSTACLE_INTERSECTION' as const, @@ -145,8 +165,8 @@ function isAxisAligned( /** A measured handle anchor is trustworthy only when it sits on the scored anchor. */ const HANDLE_ANCHOR_TOLERANCE = 6 -const HAIRPIN_ALTERNATIVE_PAIRS = 6 -const ROUTE_STATUS_RANK: Record = { ok: 0, invalid: 1, failed: 2 } +const SIDE_CHANGE_STABILITY_BIAS = 8 +const ROUTE_BEND_COST = 28 /** * True when the route reverses direction across a leg shorter than `maxLeg` @@ -254,12 +274,27 @@ function getAbsolutePosition(node: Node, nodes: Node[]): { x: number; y: number return { x, y } } -function getNodeRect(node: Node, nodes: Node[], renderStyleId?: string, iconMode?: NodeIconMode): ObstacleRect { +export function getNodeRect(node: Node, nodes: Node[], renderStyleId?: string, iconMode?: NodeIconMode): ObstacleRect { const pos = getAbsolutePosition(node, nodes) const { width: w, height: h } = getEffectiveNodeDimensions(node, renderStyleId, iconMode) return { x: pos.x, y: pos.y, w, h } } +function __passable(a: string, b: string, nodes: Node[]): Set { + if (!process.env.EXP_GROUP_WALLS) return allGroupIds(nodes) + const byId = new Map(nodes.map(n => [n.id, n])) + const out = new Set() + for (const id of [a, b]) { + let cur = byId.get(id); const seen = new Set() + while (cur && !seen.has(cur.id)) { seen.add(cur.id); const p = cur.parentId || (cur as Node & { parentNode?: string }).parentNode; if (!p) break; const parent = byId.get(p); if (!parent) break; if (isGroupNode(parent)) out.add(p); cur = parent } + } + return out +} + +function allGroupIds(nodes: Node[]): Set { + return new Set(nodes.filter(isGroupNode).map(node => node.id)) +} + export function isGroupNode(node: Node): boolean { return ( node.type === 'groupNode' || @@ -278,30 +313,6 @@ function sideToPosition(side: string | undefined): Position | undefined { return undefined } -/** - * Collects the ids of every group node that contains `nodeId` (all ancestors - * through the parent chain). An edge may always cross its own groups' - * boundaries because its endpoints live inside them, so these groups must - * not be treated as routing obstacles — otherwise the edge could never leave - * (or stay inside) its container. - */ -function getAncestorGroupIds(nodeId: string, nodes: Node[]): Set { - const result = new Set() - const nodeById = new Map(nodes.map((n) => [n.id, n])) - const visited = new Set() - let currentId = nodeById.get(nodeId) - while (currentId && !visited.has(currentId.id)) { - visited.add(currentId.id) - const parentId = currentId.parentId || (currentId as Node & { parentNode?: string }).parentNode - if (!parentId) break - const parent = nodeById.get(parentId) - if (!parent) break - if (isGroupNode(parent)) result.add(parentId) - currentId = parent - } - return result -} - function buildBlockingNodeRects( nodes: Node[], excludedIds: Set, @@ -397,6 +408,7 @@ function computeDirectWaypoints( borderRadius: number = 24, trustMeasuredTerminals: boolean = false, maxExpandedStates?: number, + preferStraight: boolean = false, ): { waypoints: Array<{ x: number; y: number }>; outcome?: RoutePathOutcome } { // A tiny height difference should read as one straight connector. Snap to // the midpoint so the wire is pixel-straight instead of a shallow diagonal; @@ -422,6 +434,24 @@ function computeDirectWaypoints( } } + // Shortest distance between the two anchors: draw a straight wire whenever + // nothing is in the way. This is the common case (a clear corridor); when a + // card blocks it, fall through to the orthogonal route and its detour. The + // caller only enables this when no lane offset applies (parallel / + // bidirectional edges keep their offset and never collapse here); a shared + // fan channel is ignored because straight wires already diverge. + const outwardProjection = (side: Position, dx: number, dy: number) => + side === Position.Right ? dx : side === Position.Left ? -dx : side === Position.Bottom ? dy : -dy; + const leavesSource = outwardProjection(sourcePosition, th.x - sh.x, th.y - sh.y) > 0.5; + const entersTarget = outwardProjection(targetPosition, sh.x - th.x, sh.y - th.y) > 0.5; + if (preferStraight && leavesSource && entersTarget) { + const straight = [sh, th]; + const blocked = nodeRects && pathCollidesWithRects(straight, nodeRects, DIRECT_FOREIGN_NODE_CLEARANCE); + if (!blocked && !pathEntersTerminalInterior(straight, sourceRect, targetRect)) { + return { waypoints: straight }; + } + } + const directWaypoints = buildDefaultOrthogonalWaypoints( sh, th, sourcePosition, targetPosition, edgeOffset, ) @@ -581,22 +611,23 @@ function resolveEdgeSideOnNode( const sourceHandlerRect = { x: sourceRect.x, y: sourceRect.y, width: sourceRect.w, height: sourceRect.h } const targetHandlerRect = { x: targetRect.x, y: targetRect.y, width: targetRect.w, height: targetRect.h } const bidirectionalSides = resolveBidirectionalFacingSides(e, edges, sourceHandlerRect, targetHandlerRect) + const storedSource = sideToPosition(data?.sourceSide as string) + const storedTarget = sideToPosition(data?.targetSide as string) + const lockedSource = data?.sourceSideLocked === true ? storedSource : undefined + const lockedTarget = data?.targetSideLocked === true ? storedTarget : undefined if (e.source === nodeId) { - const manual = sideToPosition(data?.sourceSide as string) + const manual = lockedSource if (manual !== undefined) return manual } else { - const manual = sideToPosition(data?.targetSide as string) + const manual = lockedTarget if (manual !== undefined) return manual } if (bidirectionalSides) { return e.source === nodeId ? bidirectionalSides.sourceSide : bidirectionalSides.targetSide } const excluded = new Set([e.source, e.target]) - // Match useHandleSlotLayout: per-edge obstacles, not the main edge's. - const passableGroupIds = new Set([ - ...getAncestorGroupIds(e.source, nodes), - ...getAncestorGroupIds(e.target, nodes), - ]) + // Match useHandleSlotLayout: per-edge obstacles, not the main edge's (groups are never obstacles). + const passableGroupIds = __passable(e.source, e.target, nodes) const obstacles = buildScorerObstacles(nodes, excluded, passableGroupIds, renderStyleId, iconMode) let pair = selectBestHandlerPair( sourceHandlerRect, @@ -604,13 +635,13 @@ function resolveEdgeSideOnNode( direction, obstacles.size > 0 ? obstacles : undefined, excluded, - sideToPosition(data?.sourceSide as string), - sideToPosition(data?.targetSide as string), + lockedSource, + lockedTarget, sideToPosition(data?.laneSourceSide as string), sideToPosition(data?.laneTargetSide as string), ) - const manualSource = sideToPosition(data?.sourceSide as string) - const manualTarget = sideToPosition(data?.targetSide as string) + const manualSource = lockedSource + const manualTarget = lockedTarget const usesCustomWaypoints = Array.isArray(data?.customWaypoints) && data.customWaypoints.length > 0 if ((manualSource === undefined || manualTarget === undefined) && !usesCustomWaypoints) { if (orthogonalPenetratesTerminal(sourceHandlerRect, targetHandlerRect, pair.sourceSide, pair.targetSide)) { @@ -655,6 +686,14 @@ export interface EdgeRouteOptions { searchBudget?: { maxExpandedStates?: number }; /** Shared only within one immutable graph pass, never across edits. */ context?: EdgeRoutingContext; + /** + * Draw the shortest wire between the handles when nothing is in the way. + * Rendering (canvas / export) turns this on; the layout scorer keeps the + * orthogonal route so its aesthetic tuning stays unchanged. + */ + straightEdges?: boolean; + /** Forced only by the global optimizer while comparing crossing costs. */ + attachmentSides?: { sourceSide: Position; targetSide: Position }; } export interface EdgeRoutingContext { @@ -946,17 +985,14 @@ export function computeEdgeRoute( // this library free of store reads prevents an inverted layering dependency // where a geometry utility would import the UI state container. - const laneSourcePreference = sideToPosition(edgeData?.laneSourceSide as string) - const laneTargetPreference = sideToPosition(edgeData?.laneTargetSide as string) + let laneSourcePreference = sideToPosition(edgeData?.laneSourceSide as string) + let laneTargetPreference = sideToPosition(edgeData?.laneTargetSide as string) const excludedIds = new Set([edge.source, edge.target]) - // Groups containing either endpoint are passable (the edge starts/ends - // inside them); every other group is an obstacle so edges of one group - // never cut across another group's rectangle. - const passableGroupIds = new Set([ - ...getAncestorGroupIds(edge.source, nodes), - ...getAncestorGroupIds(edge.target, nodes), - ]) + // Groups are never obstacles: an edge takes the shortest way and goes around only what is in its + // way (cards). Treating every group an edge does not belong to as a wall sent long edges around + // the outside of whole subsystems instead of through their empty space. + const passableGroupIds = __passable(edge.source, edge.target, nodes) // Validation must skip the same groups routing treats as passable, or every // edge leaving its own group is reported as crossing it. const validationExcludedIds = new Set([...excludedIds, ...passableGroupIds]) @@ -969,14 +1005,15 @@ export function computeEdgeRoute( } const bidirectionalSides = resolveBidirectionalFacingSides(edge, edges, srcHandlerRect, tgtHandlerRect) - let manualSourceSide = sideToPosition(edgeData?.sourceSide as string) - let manualTargetSide = sideToPosition(edgeData?.targetSide as string) - // The facing sides of a request/reply pair are a preference, not the user's choice: when they - // cannot be routed cleanly (another card sits between the two), other sides are tried below. - const sidesFromPairOnly = Boolean(bidirectionalSides) && manualSourceSide === undefined && manualTargetSide === undefined + const storedSourceSide = sideToPosition(edgeData?.sourceSide as string) + const storedTargetSide = sideToPosition(edgeData?.targetSide as string) + // Side values are staleable geometry hints. Only an explicit lock pins a + // side; ordinary saved sides bias selection but must not block a shorter route. + const manualSourceSide = edgeData?.sourceSideLocked === true ? storedSourceSide : undefined + const manualTargetSide = edgeData?.targetSideLocked === true ? storedTargetSide : undefined if (bidirectionalSides) { - manualSourceSide = manualSourceSide ?? bidirectionalSides.sourceSide - manualTargetSide = manualTargetSide ?? bidirectionalSides.targetSide + laneSourcePreference ??= bidirectionalSides.sourceSide + laneTargetPreference ??= bidirectionalSides.targetSide } // Custom waypoints: keep stored sides when present; only score if missing. @@ -1077,8 +1114,8 @@ export function computeEdgeRoute( laneTargetPreference, ) - let sourcePosition = handlerPair.sourceSide - let targetPosition = handlerPair.targetSide + let sourcePosition = routeOptions.attachmentSides?.sourceSide ?? handlerPair.sourceSide + let targetPosition = routeOptions.attachmentSides?.targetSide ?? handlerPair.targetSide if (process.env.NEXT_PUBLIC_DEBUG_EDGES === 'true') { logger.debug('[EdgeDebug:4-Final] edge=%s src=%s→%s tgt=%s→%s', @@ -1086,7 +1123,8 @@ export function computeEdgeRoute( } // Terminal penetration guard on orthogonal path; respect lane prefs when falling back. - const hasUserOverride = manualSourceSide !== undefined && manualTargetSide !== undefined + const hasUserOverride = routeOptions.attachmentSides !== undefined + || (manualSourceSide !== undefined && manualTargetSide !== undefined) if (!hasUserOverride && orthogonalPenetratesTerminal( srcHandlerRect, tgtHandlerRect, sourcePosition, targetPosition, )) { @@ -1118,8 +1156,8 @@ export function computeEdgeRoute( } } - if (manualSourceSide === undefined && handleAnchors?.sourceSide) sourcePosition = handleAnchors.sourceSide - if (manualTargetSide === undefined && handleAnchors?.targetSide) targetPosition = handleAnchors.targetSide + if (!routeOptions.attachmentSides && manualSourceSide === undefined && handleAnchors?.sourceSide) sourcePosition = handleAnchors.sourceSide + if (!routeOptions.attachmentSides && manualTargetSide === undefined && handleAnchors?.targetSide) targetPosition = handleAnchors.targetSide const routeForSides = (sourcePosition: Position, targetPosition: Position): EdgeRouteResult => { const nodeMap = context.nodeById @@ -1142,11 +1180,13 @@ export function computeEdgeRoute( // meets the handle it is drawn next to, perpendicular to it. const scoredSh = getBoundaryAnchor( sourceRect.x, sourceRect.y, sourceRect.w, sourceRect.h, - sourcePosition, sourceShift, + sourcePosition, sourceShift, 0, + routeOptions.straightEdges && sourcePosition === targetPosition ? 0 : undefined, ) const scoredTh = getBoundaryAnchor( targetRect.x, targetRect.y, targetRect.w, targetRect.h, - targetPosition, targetShift, + targetPosition, targetShift, 0, + routeOptions.straightEdges && sourcePosition === targetPosition ? 0 : undefined, ) const measuredSh = handleAnchors?.source const measuredTh = handleAnchors?.target @@ -1214,6 +1254,7 @@ export function computeEdgeRoute( measuredTh && pointsNear(measuredTh, scoredTh), ), routeOptions.searchBudget?.maxExpandedStates, + routeOptions.straightEdges === true && edgeOffset === 0, ) let waypoints = directRoute.waypoints let svgPath = buildSmoothStepSvg(waypoints, curveRadius) @@ -1279,42 +1320,45 @@ export function computeEdgeRoute( } const primary = routeForSides(sourcePosition, targetPosition) - if (sidesFromPairOnly && primary.status !== 'ok' - && handleAnchors?.sourceSide === undefined && handleAnchors?.targetSide === undefined) { - const pairs = applyLaneBias( - scoreAllHandlerPairs(srcHandlerRect, tgtHandlerRect, direction, scorerObstacles, excludedIds), - laneSourcePreference, - laneTargetPreference, - ) - for (const { pair } of pairs) { - if (pair.sourceSide === primary.sourcePosition && pair.targetSide === primary.targetPosition) continue - if (orthogonalPenetratesTerminal(srcHandlerRect, tgtHandlerRect, pair.sourceSide, pair.targetSide)) continue - const alternative = routeForSides(pair.sourceSide, pair.targetSide) - if (alternative.status === 'ok') return alternative - } - } - // A side pair that faces away from the other node forces the router to - // overshoot and double back (a tight U next to the handle). When the sides - // are ours to choose, try the next-best pairs and keep the first clean - // route that validates at least as well. + // `edge.data.*Side` is an explicit user choice; bidirectional facing sides + // and measured floating handles are only preferences. Evaluate the actual + // smooth-step geometry for every legal pair so a short side connection can + // beat a heuristically preferred but unnecessarily long route. const sidesPinned = manualSourceSide !== undefined || manualTargetSide !== undefined - || handleAnchors?.sourceSide !== undefined || handleAnchors?.targetSide !== undefined - if (sidesPinned || !hasHairpinTurn(primary.waypoints, curveRadius * 2)) return primary + || routeOptions.attachmentSides !== undefined + if (sidesPinned) return primary + const alternatives = applyLaneBias( scoreAllHandlerPairs(srcHandlerRect, tgtHandlerRect, direction, scorerObstacles, excludedIds), laneSourcePreference, laneTargetPreference, ) - for (const { pair } of alternatives.slice(0, HAIRPIN_ALTERNATIVE_PAIRS)) { + const routeCost = (route: EdgeRouteResult, heuristicScore: number, isPrimary: boolean): number => { + const statusCost = route.status === 'ok' ? 0 : route.status === 'invalid' ? 1_000_000 : 2_000_000 + const bendCount = Math.max(0, route.waypoints.length - 2) + const stabilityCost = isPrimary ? 0 : SIDE_CHANGE_STABILITY_BIAS + return statusCost + route.violations.length * 100_000 + + route.renderedPath.length + bendCount * ROUTE_BEND_COST + + heuristicScore * 0.01 + stabilityCost + } + let bestRoute = primary + const primaryScore = alternatives.find(({ pair }) => + pair.sourceSide === primary.sourcePosition && pair.targetSide === primary.targetPosition, + ) + let bestCost = routeCost(primary, primaryScore?.total ?? 0, true) + for (const { pair, ...score } of alternatives) { if (pair.sourceSide === primary.sourcePosition && pair.targetSide === primary.targetPosition) continue + if (manualSourceSide !== undefined && pair.sourceSide !== manualSourceSide) continue + if (manualTargetSide !== undefined && pair.targetSide !== manualTargetSide) continue if (orthogonalPenetratesTerminal(srcHandlerRect, tgtHandlerRect, pair.sourceSide, pair.targetSide)) continue const alternative = routeForSides(pair.sourceSide, pair.targetSide) - if (ROUTE_STATUS_RANK[alternative.status] <= ROUTE_STATUS_RANK[primary.status] - && !hasHairpinTurn(alternative.waypoints, curveRadius * 2)) { - return alternative + const cost = routeCost(alternative, score.total, false) + if (cost < bestCost) { + bestRoute = alternative + bestCost = cost } } - return primary + return bestRoute } /** diff --git a/frontend/lib/diagram/edges/edgeRouteOptimizer.ts b/frontend/lib/diagram/edges/edgeRouteOptimizer.ts index 835eb381..f6171ec1 100644 --- a/frontend/lib/diagram/edges/edgeRouteOptimizer.ts +++ b/frontend/lib/diagram/edges/edgeRouteOptimizer.ts @@ -1,9 +1,12 @@ import type { Edge, Node } from 'reactflow'; +import { Position } from './edgePositions'; import type { EdgeRouteDirection, EdgeRouteHandleAnchors, EdgeRouteResult } from './edgeRouteBuilder'; -import { computeEdgeRoute, createEdgeRoutingContext } from './edgeRouteBuilder'; +import { computeEdgeRoute, createEdgeRoutingContext, getNodeRect } from './edgeRouteBuilder'; import { getRenderedPathInteraction, type RenderedEdgePath } from './edgePathGeometry'; +import { scoreAllHandlerPairs } from './handlerPairScorer'; import type { NodeIconMode } from '../nodes/nodeIconVisibility'; import { resolveEdgeFlowSemantics, type ResolvedEdgeFlowAssignment } from '@/lib/diagram/mermaid/edgeFlowSemantics'; +import { separateFanLanes } from './fanLaneSeparation'; const RIP_UP_PASSES = 2; const LANE_CANDIDATE_DELTAS = [0, -28, 28] as const; @@ -20,6 +23,8 @@ export interface RouteOptimizationInput { iconMode?: NodeIconMode; handleAnchors?: ReadonlyMap; searchBudget?: { perRouteStates?: number; totalStates?: number; ripUpPasses?: number; maxCandidateRoutes?: number }; + /** Rendering asks for the shortest wire when clear; layout scoring keeps the orthogonal route. */ + straightEdges?: boolean; } export interface RouteOptimizationResult { @@ -43,6 +48,7 @@ function edgeKey(edge: Edge): string { return [edge.source, edge.target, String(edge.label ?? data?.label ?? '').trim().toLowerCase(), edge.id].join('\u0000'); } +// Proper crossings exclude shared endpoints; sibling edges can still cross farther along. const properCross = (a: RenderedEdgePath, b: RenderedEdgePath) => getRenderedPathInteraction(a, b).crossing; type Pt = { x: number; y: number }; @@ -121,7 +127,7 @@ function boundsMayInteract(a: RenderedEdgePath, b: RenderedEdgePath): boolean { } function conflictsWithRoutes( - edge: Edge, + _edge: Edge, route: EdgeRouteResult, fixed: ReadonlyMap, edgeById: ReadonlyMap, @@ -131,8 +137,7 @@ function conflictsWithRoutes( if (!boundsMayInteract(route.renderedPath, otherRoute.renderedPath)) continue; const other = edgeById.get(id); if (!other) continue; - const sharesNode = [edge.source, edge.target].some(nodeId => nodeId === other.source || nodeId === other.target); - if (!sharesNode && properCross(route.renderedPath, otherRoute.renderedPath)) return true; + if (properCross(route.renderedPath, otherRoute.renderedPath)) return true; if (privateOverlapLength(route, otherRoute) > 10) return true; } return false; @@ -161,8 +166,7 @@ function globalCost(edges: readonly Edge[], routes: ReadonlyMap id === right.source || id === right.target); - if (!sharesNode && properCross(a, b)) cost += CROSSING_COST; + if (properCross(a, b)) cost += CROSSING_COST; const overlap = privateOverlapLength(ra, rb); cost += Math.ceil(overlap / 10) * SHARED_PRIVATE_COST_PER_10PX; } @@ -176,23 +180,21 @@ function hasRouteConflicts(edges: readonly Edge[], routes: ReadonlyMap id === otherEdge.source || id === otherEdge.target); - if (!sharesNode && properCross(route.renderedPath, otherRoute.renderedPath)) return true; + if (properCross(route.renderedPath, otherRoute.renderedPath)) return true; if (privateOverlapLength(route, otherRoute) > 10) return true; } } return false; } -function incrementalCost(edge: Edge, route: EdgeRouteResult, fixed: ReadonlyMap, edgeById: ReadonlyMap): number { +function incrementalCost(_edge: Edge, route: EdgeRouteResult, fixed: ReadonlyMap, edgeById: ReadonlyMap): number { let cost = route.status === 'ok' ? 0 : 1_000_000 + route.violations.length * 100_000; cost += route.renderedPath.length + bendCount(route) * BEND_COST; for (const [id, otherRoute] of fixed) { const other = edgeById.get(id); if (!other) continue; if (!boundsMayInteract(route.renderedPath, otherRoute.renderedPath)) continue; - const sharesNode = [edge.source, edge.target].some(nodeId => nodeId === other.source || nodeId === other.target); - if (!sharesNode && properCross(route.renderedPath, otherRoute.renderedPath)) cost += CROSSING_COST; + if (properCross(route.renderedPath, otherRoute.renderedPath)) cost += CROSSING_COST; cost += Math.ceil(privateOverlapLength(route, otherRoute) / 10) * SHARED_PRIVATE_COST_PER_10PX; } return cost; @@ -219,6 +221,15 @@ export function optimizeEdgeRoutes(input: RouteOptimizationInput): RouteOptimiza // Reserve one base route for every edge; optional lanes and repairs have a // separate bounded pool so a dense graph can never starve its last edges. let extraCandidates = Math.max(0, (input.searchBudget?.maxCandidateRoutes ?? edges.length * 4) - edges.length); + const perEdgeAlternativeBudget = edges.length > 0 ? Math.ceil(extraCandidates / edges.length) : 0; + const alternativesUsedByEdge = new Map(); + const consumeAlternative = (edgeId: string): boolean => { + const used = alternativesUsedByEdge.get(edgeId) ?? 0; + if (used >= perEdgeAlternativeBudget || extraCandidates <= 0) return false; + alternativesUsedByEdge.set(edgeId, used + 1); + extraCandidates--; + return true; + }; const edgeById = new Map(input.edges.map(edge => [edge.id, edge])); let candidateRoutes = 0; let expandedStates = 0; @@ -226,7 +237,7 @@ export function optimizeEdgeRoutes(input: RouteOptimizationInput): RouteOptimiza const totalStateBudget = input.searchBudget?.totalStates ?? 400_000; const ripUpBudget = input.searchBudget?.ripUpPasses ?? RIP_UP_PASSES; - const routeAt = (edge: Edge, laneOffset: number) => { + const routeAt = (edge: Edge, laneOffset: number, attachmentSides?: { sourceSide: Position; targetSide: Position }) => { candidateRoutes++; const remainingBudget = Math.max(0, totalStateBudget - expandedStates); const route = computeEdgeRoute( @@ -240,6 +251,8 @@ export function optimizeEdgeRoutes(input: RouteOptimizationInput): RouteOptimiza { laneOffset, context, + straightEdges: input.straightEdges === true, + attachmentSides, searchBudget: { maxExpandedStates: Math.min(perRouteBudget, remainingBudget) }, }, ); @@ -254,23 +267,63 @@ export function optimizeEdgeRoutes(input: RouteOptimizationInput): RouteOptimiza const laneDeltas = feedback ? [...LANE_CANDIDATE_DELTAS, ...FEEDBACK_CORRIDOR_DELTAS] : LANE_CANDIDATE_DELTAS; const candidates = [...new Set(laneDeltas.map(delta => base + delta))] .sort((a, b) => Math.abs(a) - Math.abs(b) || (stableOrder % 2 ? b - a : a - b)); - if (repair && extraCandidates <= 0) return { route: routes.get(edge.id)!, offset: base }; - if (repair) extraCandidates--; + if (repair && !consumeAlternative(edge.id)) return { route: routes.get(edge.id)!, offset: base }; let winner = routeAt(edge, candidates[0]); let winnerOffset = candidates[0]; let winnerCost = incrementalCost(edge, winner, fixed, edgeById); if (!conflictsWithRoutes(edge, winner, fixed, edgeById)) return { route: winner, offset: winnerOffset }; + + // A route can be individually short yet create a crossing or private + // overlap. Before moving its lane, try alternate node attachments against + // the already routed graph and score each with the same global conflict + // cost used by the lane optimizer. + const data = edge.data as Record | undefined; + const pinnedSource = data?.sourceSideLocked === true && typeof data.sourceSide === 'string' + ? data.sourceSide.toLowerCase() : undefined; + const pinnedTarget = data?.targetSideLocked === true && typeof data.targetSide === 'string' + ? data.targetSide.toLowerCase() : undefined; + const sourceNode = input.nodes.find(node => node.id === edge.source); + const targetNode = input.nodes.find(node => node.id === edge.target); + const sourceRect = sourceNode ? getNodeRect(sourceNode, input.nodes as Node[], input.renderStyleId, input.iconMode) : undefined; + const targetRect = targetNode ? getNodeRect(targetNode, input.nodes as Node[], input.renderStyleId, input.iconMode) : undefined; + const allSidePairs = sourceRect && targetRect + ? scoreAllHandlerPairs( + { x: sourceRect.x, y: sourceRect.y, width: sourceRect.w, height: sourceRect.h }, + { x: targetRect.x, y: targetRect.y, width: targetRect.w, height: targetRect.h }, + input.direction, + ).map(({ pair }) => pair) + : [Position.Right, Position.Left, Position.Top, Position.Bottom] + .flatMap(sourceSide => [Position.Right, Position.Left, Position.Top, Position.Bottom] + .map(targetSide => ({ sourceSide, targetSide }))); + const hasParallelEdge = edges.some(candidate => + candidate.id !== edge.id && candidate.source === edge.source && candidate.target === edge.target, + ); + const sidePairs = allSidePairs + .filter(pair => pair.sourceSide !== winner.sourcePosition || pair.targetSide !== winner.targetPosition) + .filter(pair => !pinnedSource || pair.sourceSide === pinnedSource) + .filter(pair => !pinnedTarget || pair.targetSide === pinnedTarget) + // Parallel edges share visible handles. Resolve their overlap with private + // route lanes; changing one edge's attachment side makes the fan look + // detached and defeats the lane allocator. + .filter(() => !hasParallelEdge); + let bestConflictFree: { route: EdgeRouteResult; cost: number } | undefined; + for (const pair of sidePairs) { + if (!consumeAlternative(edge.id)) break; + const candidate = routeAt(edge, candidates[0], pair); + const score = incrementalCost(edge, candidate, fixed, edgeById); + if (score < winnerCost) { winner = candidate; winnerOffset = candidates[0]; winnerCost = score; } + if (!conflictsWithRoutes(edge, candidate, fixed, edgeById) + && (!bestConflictFree || score < bestConflictFree.cost)) { + bestConflictFree = { route: candidate, cost: score }; + } + } + if (bestConflictFree) return { route: bestConflictFree.route, offset: candidates[0] }; + for (const offset of candidates.slice(1)) { - if (extraCandidates <= 0) break; - extraCandidates--; + if (!consumeAlternative(edge.id)) break; const candidate = routeAt(edge, offset); const score = incrementalCost(edge, candidate, fixed, edgeById); if (score < winnerCost) { winner = candidate; winnerOffset = offset; winnerCost = score; } - // Stop once a deterministic lane clears every routed path. Additional - // alternatives cannot improve validity and multiply work on dense graphs. - if (!conflictsWithRoutes(edge, candidate, fixed, edgeById)) { - return { route: candidate, offset }; - } } return { route: winner, offset: winnerOffset }; }; @@ -311,6 +364,9 @@ export function optimizeEdgeRoutes(input: RouteOptimizationInput): RouteOptimiza } } + // Edges of one fan no longer share one drawn line: each gets its own lane (see fanLaneSeparation). + if (!process.env.EXP_NO_LANES) separateFanLanes(routes, input.edges, input.nodes, input.renderStyleId, input.iconMode); + const diagnostics: RouteOptimizationResult['diagnostics'][number][] = []; for (let i = 0; i < input.edges.length; i++) for (let j = i + 1; j < input.edges.length; j++) { const a = input.edges[i], b = input.edges[j]; @@ -318,8 +374,7 @@ export function optimizeEdgeRoutes(input: RouteOptimizationInput): RouteOptimiza if (!routeA || !routeB) continue; const pathA = routeA.renderedPath, pathB = routeB.renderedPath; if (!boundsMayInteract(pathA, pathB)) continue; - const sharesNode = [a.source, a.target].some(id => id === b.source || id === b.target); - if (!sharesNode && properCross(pathA, pathB)) diagnostics.push({ edgeId: a.id, otherEdgeId: b.id, kind: 'crossing' }); + if (properCross(pathA, pathB)) diagnostics.push({ edgeId: a.id, otherEdgeId: b.id, kind: 'crossing' }); const overlap = privateOverlapLength(routeA, routeB); if (overlap > 10) diagnostics.push({ edgeId: a.id, otherEdgeId: b.id, kind: 'private-overlap', amount: overlap }); } diff --git a/frontend/lib/diagram/edges/fanLaneSeparation.ts b/frontend/lib/diagram/edges/fanLaneSeparation.ts new file mode 100644 index 00000000..24cb2447 --- /dev/null +++ b/frontend/lib/diagram/edges/fanLaneSeparation.ts @@ -0,0 +1,186 @@ +import type { Edge, Node } from 'reactflow'; +import type { EdgeRouteResult } from './edgeRouteBuilder'; +import { getNodeRect, isGroupNode } from './edgeRouteBuilder'; +import { buildEdgePathGeometry, getRenderedPathInteraction } from './edgePathGeometry'; +import type { NodeIconMode } from '../nodes/nodeIconVisibility'; + +/** + * Edges that leave (or enter) a node from the same anchor all turn onto one line: two or more + * arrows drawn as a single wire, so a reader cannot tell them apart or follow each one. This pass + * gives every edge of such a fan its own lane, FAN_LANE_GAP apart, on the run where they would + * share the line (the trunk: the leg after the stub out of the shared anchor). + * + * Lanes are nested so the fan never crosses itself: when the edges turn away from the shared node + * after the trunk, the edge whose trunk is longest (the farthest end) takes the lane nearest the + * node; when they turn back towards it, that edge takes the outermost lane. A lane is only used if + * the moved route clears every card and keeps its stub; otherwise the edges keep the shared line. + */ +export const FAN_LANE_GAP = 14; +/** Trunks closer than this are drawn on top of each other. */ +const SAME_LINE = 3; +/** Shortest stub left between the anchor and a lane moved towards the node. */ +const MIN_STUB = 12; +/** Space kept between a moved lane and a card it passes. */ +const CARD_CLEARANCE = 4; + +type Pt = { x: number; y: number }; +type Rect = { id: string; x: number; y: number; w: number; h: number }; + +interface Trunk { + edgeId: string; + /** Waypoints oriented so index 0 is the shared anchor. */ + points: Pt[]; + reversed: boolean; + /** Axis of the trunk: 'x' for a vertical trunk (its coordinate is x), 'y' for a horizontal one. */ + axis: 'x' | 'y'; + coord: number; + /** +1 / -1: direction of the stub along the trunk's coordinate axis (away from the node). */ + outward: number; + /** Direction the trunk runs (+1 / -1 along the other axis). */ + runs: number; + length: number; + /** True when the leg after the trunk heads back towards the shared node. */ + turnsBack: boolean; +} + +const isOrthogonal = (points: readonly Pt[]) => + points.every((p, i) => i === 0 || Math.abs(p.x - points[i - 1].x) < 0.5 || Math.abs(p.y - points[i - 1].y) < 0.5); + +/** The trunk of a route seen from its anchor end, or null when it has none that a lane could move. */ +function trunkOf(edgeId: string, waypoints: readonly Pt[], reversed: boolean): Trunk | null { + const points = reversed ? [...waypoints].reverse() : [...waypoints]; + // anchor → stub end → trunk end → … : the trunk must not end at the far terminal, which cannot move. + if (points.length < 4 || !isOrthogonal(points)) return null; + const [a, b, c, d] = points; + const stubHorizontal = Math.abs(a.y - b.y) < 0.5 && Math.abs(a.x - b.x) >= 0.5; + const stubVertical = Math.abs(a.x - b.x) < 0.5 && Math.abs(a.y - b.y) >= 0.5; + if (!stubHorizontal && !stubVertical) return null; + const axis: 'x' | 'y' = stubHorizontal ? 'x' : 'y'; + const other: 'x' | 'y' = axis === 'x' ? 'y' : 'x'; + // The trunk runs across the stub: same coordinate on the stub's axis, moving along the other. + if (Math.abs(b[axis] - c[axis]) >= 0.5 || Math.abs(b[other] - c[other]) < 0.5) return null; + const outward = Math.sign(b[axis] - a[axis]); + const next = Math.sign(d[axis] - c[axis]); + return { + edgeId, + points, + reversed, + axis, + coord: b[axis], + outward, + runs: Math.sign(c[other] - b[other]), + length: Math.abs(c[other] - b[other]), + turnsBack: next !== 0 && next !== outward, + }; +} + +function segmentHitsRect(p: Pt, q: Pt, r: Rect): boolean { + const minX = Math.min(p.x, q.x), maxX = Math.max(p.x, q.x); + const minY = Math.min(p.y, q.y), maxY = Math.max(p.y, q.y); + return maxX > r.x - CARD_CLEARANCE && minX < r.x + r.w + CARD_CLEARANCE + && maxY > r.y - CARD_CLEARANCE && minY < r.y + r.h + CARD_CLEARANCE; +} + +function withLane(trunk: Trunk, coord: number): Pt[] { + const points = trunk.points.map(p => ({ ...p })); + points[1][trunk.axis] = coord; + points[2][trunk.axis] = coord; + return trunk.reversed ? points.reverse() : points; +} + +function rebuilt(route: EdgeRouteResult, waypoints: Pt[]): EdgeRouteResult { + const cornerRadius = route.path.kind === 'orthogonal' ? route.path.cornerRadius : 0; + const path = { kind: 'orthogonal' as const, waypoints, cornerRadius }; + const renderedPath = buildEdgePathGeometry(path); + return { ...route, waypoints, path, renderedPath, svgPath: renderedPath.d }; +} + +export function separateFanLanes( + routes: Map, + edges: readonly Edge[], + nodes: readonly Node[], + renderStyleId?: string, + iconMode?: NodeIconMode, +): void { + const edgeById = new Map(edges.map(edge => [edge.id, edge])); + const cards: Rect[] = nodes + .filter(node => !isGroupNode(node)) + .map(node => ({ id: node.id, ...getNodeRect(node, nodes as Node[], renderStyleId, iconMode) })); + + // Fans: edges sharing one anchor point on one node, seen from that end. + const fans = new Map(); + for (const [edgeId, route] of routes) { + const edge = edgeById.get(edgeId); + if (!edge || route.status !== 'ok' || route.path.kind !== 'orthogonal') continue; + if (Array.isArray((edge.data as { customWaypoints?: unknown[] } | undefined)?.customWaypoints) + && ((edge.data as { customWaypoints: unknown[] }).customWaypoints.length > 0)) continue; + for (const [nodeId, reversed] of [[edge.source, false], [edge.target, true]] as const) { + const trunk = trunkOf(edgeId, route.waypoints, reversed); + if (!trunk) continue; + const anchor = trunk.points[0]; + const key = `${nodeId}|${Math.round(anchor.x / 4)}|${Math.round(anchor.y / 4)}|${trunk.axis}|${trunk.outward}|${trunk.runs}`; + const fan = fans.get(key) ?? []; + fan.push(trunk); + fans.set(key, fan); + } + } + + const moved = new Set(); + for (const [key, fan] of fans) { + const nodeId = key.split('|')[0]; + // Only trunks drawn on one line need lanes (a fan whose edges already turn apart is fine). + const shared = fan.filter(t => !moved.has(t.edgeId) && fan.some(o => o !== t && Math.abs(o.coord - t.coord) <= SAME_LINE)); + if (shared.length < 2) continue; + const base = Math.min(...shared.map(t => t.outward * t.coord)) * shared[0].outward; + const anchorCoord = shared[0].points[0][shared[0].axis]; + // Nesting (see above): innermost lane first. + const ordered = [...shared].sort((a, b) => + (a.turnsBack ? a.length - b.length : b.length - a.length) || a.edgeId.localeCompare(b.edgeId)); + const outward = shared[0].outward; + + const tryLanes = (offsets: number[]): Map | null => { + const next = new Map(); + for (let i = 0; i < ordered.length; i++) { + const trunk = ordered[i]; + const coord = base + outward * offsets[i]; + if ((coord - anchorCoord) * outward < MIN_STUB) return null; + // The leg after the trunk keeps its direction (a lane past the next corner would double back). + const after = trunk.points[3][trunk.axis]; + if ((trunk.turnsBack ? coord - after : after - coord) * outward < MIN_STUB) return null; + const waypoints = withLane(trunk, coord); + const edge = edgeById.get(trunk.edgeId)!; + const ends = new Set([edge.source, edge.target, nodeId]); + for (let s = 1; s < waypoints.length; s++) { + for (const card of cards) { + if (ends.has(card.id)) continue; + if (segmentHitsRect(waypoints[s - 1], waypoints[s], card)) return null; + } + } + next.set(trunk.edgeId, rebuilt(routes.get(trunk.edgeId)!, waypoints)); + } + // Shared endpoints may meet, but sibling edges must not gain a crossing farther along. + for (const [edgeId, route] of next) { + const before = routes.get(edgeId)!; + for (const [otherId, otherRoute] of routes) { + if (otherId === edgeId) continue; + const other = edgeById.get(otherId); + if (!other) continue; + const drawn = next.get(otherId) ?? otherRoute; + if (getRenderedPathInteraction(route.renderedPath, drawn.renderedPath).crossing + && !getRenderedPathInteraction(before.renderedPath, otherRoute.renderedPath).crossing) return null; + } + } + return next; + }; + + const n = ordered.length; + const outwardOnly = ordered.map((_, i) => i * FAN_LANE_GAP); + const centred = ordered.map((_, i) => (i - (n - 1) / 2) * FAN_LANE_GAP); + const applied = tryLanes(outwardOnly) ?? tryLanes(centred); + if (!applied) continue; + for (const [edgeId, route] of applied) { + routes.set(edgeId, route); + moved.add(edgeId); + } + } +} diff --git a/frontend/lib/diagram/edges/handleSlotOrder.ts b/frontend/lib/diagram/edges/handleSlotOrder.ts index ba32b224..6da471c1 100644 --- a/frontend/lib/diagram/edges/handleSlotOrder.ts +++ b/frontend/lib/diagram/edges/handleSlotOrder.ts @@ -27,8 +27,8 @@ function sideFromDataString(value: unknown): Position | undefined { /** * Which side of `nodeId` an edge renders on, mirroring the floating-side - * router: explicit data override first, then lane override, then center-to-center geometry. - * Handle ids no longer pin sides. + * router: locked override, lane preference, then current graph geometry. + * Old sourceSide/targetSide values are only a fallback without node geometry. */ export function inferEdgeSide( edge: Edge, @@ -36,19 +36,22 @@ export function inferEdgeSide( nodePositions?: Map, ): Position | undefined { const data = edge.data as Record | undefined; - const manual = sideFromDataString( + const lockedSide = sideFromDataString( edge.source === nodeId ? data?.sourceSide : data?.targetSide, ); - if (manual !== undefined) return manual; + const isLocked = edge.source === nodeId + ? data?.sourceSideLocked === true + : data?.targetSideLocked === true; + if (isLocked && lockedSide !== undefined) return lockedSide; const lane = sideFromDataString( edge.source === nodeId ? data?.laneSourceSide : data?.laneTargetSide, ); if (lane !== undefined) return lane; - if (!nodePositions) return undefined; + if (!nodePositions) return isLocked ? lockedSide : undefined; const self = nodePositions.get(nodeId); const other = nodePositions.get(edge.source === nodeId ? edge.target : edge.source); - if (!self || !other) return undefined; + if (!self || !other) return isLocked ? lockedSide : undefined; const dx = other.x + other.width / 2 - (self.x + self.width / 2); const dy = other.y + other.height / 2 - (self.y + self.height / 2); diff --git a/frontend/lib/diagram/edges/simpleFloatingEdge.ts b/frontend/lib/diagram/edges/simpleFloatingEdge.ts index 22403977..6879c4c2 100644 --- a/frontend/lib/diagram/edges/simpleFloatingEdge.ts +++ b/frontend/lib/diagram/edges/simpleFloatingEdge.ts @@ -127,16 +127,17 @@ export function getBoundaryAnchor( position: Position, shiftOffset: number = 0, contentBiasY: number = 0, + endpointGap: number = EDGE_ENDPOINT_GAP, ): { x: number; y: number } { switch (position) { case Position.Left: - return { x: nodeX - EDGE_ENDPOINT_GAP, y: nodeY + height / 2 + shiftOffset + contentBiasY }; + return { x: nodeX - endpointGap, y: nodeY + height / 2 + shiftOffset + contentBiasY }; case Position.Right: - return { x: nodeX + width + EDGE_ENDPOINT_GAP, y: nodeY + height / 2 + shiftOffset + contentBiasY }; + return { x: nodeX + width + endpointGap, y: nodeY + height / 2 + shiftOffset + contentBiasY }; case Position.Top: - return { x: nodeX + width / 2 + shiftOffset, y: nodeY - EDGE_ENDPOINT_GAP }; + return { x: nodeX + width / 2 + shiftOffset, y: nodeY - endpointGap }; case Position.Bottom: - return { x: nodeX + width / 2 + shiftOffset, y: nodeY + height + EDGE_ENDPOINT_GAP }; + return { x: nodeX + width / 2 + shiftOffset, y: nodeY + height + endpointGap }; default: return { x: nodeX + width / 2, y: nodeY + height / 2 }; } diff --git a/frontend/lib/diagram/export/svg/index.ts b/frontend/lib/diagram/export/svg/index.ts index d8e8fac8..e8b86e0b 100644 --- a/frontend/lib/diagram/export/svg/index.ts +++ b/frontend/lib/diagram/export/svg/index.ts @@ -126,6 +126,7 @@ export function generatePureSVG( renderStyleId, iconMode, revision: 'svg-export', + straightEdges: false, }); const nodeBounds = calculateBounds(preparedNodes); diff --git a/frontend/lib/diagram/geometry/__tests__/diagramGeometry.test.ts b/frontend/lib/diagram/geometry/__tests__/diagramGeometry.test.ts index 5eb1c53f..e5e32f77 100644 --- a/frontend/lib/diagram/geometry/__tests__/diagramGeometry.test.ts +++ b/frontend/lib/diagram/geometry/__tests__/diagramGeometry.test.ts @@ -11,6 +11,42 @@ const edge = (id: string, label: string): Edge => ({ } as Edge); describe('computeDiagramGeometryPlan', () => { + it('uses orthogonal segments for staggered nodes under the canvas routing policy', () => { + const nodes = [node('a', 0, 0), node('b', 500, 200)]; + const edges: Edge[] = [{ id: 'step', source: 'a', target: 'b' }]; + const plan = computeDiagramGeometryPlan(nodes, edges, { direction: 'LR', straightEdges: false }); + const points = plan.routes.get('step')!.waypoints; + expect(points.length).toBeGreaterThan(2); + for (let i = 1; i < points.length; i++) { + expect(Math.min(Math.abs(points[i].x - points[i - 1].x), Math.abs(points[i].y - points[i - 1].y))).toBeLessThan(0.01); + } + }); + + it('connects bottom handles with terminal legs instead of a floating horizontal wire', () => { + const nodes = [{ ...node('a', 0, 0), height: 100 }, { ...node('b', 500, 0), height: 100 }]; + const edges: Edge[] = [{ id: 'messages-tools', source: 'b', target: 'a', label: 'messages + tools', data: { sourceSide: 'bottom', targetSide: 'bottom', sourceSideLocked: true, targetSideLocked: true } }]; + const plan = computeDiagramGeometryPlan(nodes, edges, { direction: 'LR', straightEdges: true }); + const points = plan.routes.get('messages-tools')!.waypoints; + expect(points.length).toBeGreaterThanOrEqual(4); + expect(points[1].x).toBe(points[0].x); + expect(points[1].y).toBeGreaterThan(points[0].y); + expect(points.at(-2)!.x).toBe(points.at(-1)!.x); + expect(points.at(-2)!.y).toBeGreaterThan(points.at(-1)!.y); + expect(points.at(-1)!.y).toBe(nodes[0].position.y + nodes[0].height!); + }); + + it('weights crossings independently of input edge order', () => { + const nodes = [node('a', 0, 0), node('b', 0, 400), node('c', 500, 400), node('d', 500, 0)]; + const edges: Edge[] = [ + { id: 'main', source: 'a', target: 'c', data: { flowRole: 'main', flowRoleSource: 'explicit' } }, + { id: 'feedback', source: 'b', target: 'd', data: { flowRole: 'feedback', flowRoleSource: 'explicit' } }, + ]; + const options = { direction: 'LR' as const, straightEdges: true }; + const forward = computeDiagramGeometryPlan(nodes, edges, options); + const reverse = computeDiagramGeometryPlan(nodes, [...edges].reverse(), options); + expect(forward.quality.weightedCrossings).toBe(reverse.quality.weightedCrossings); + }); + it('invalidates cached label placement for a top-level label edit', () => { const nodes = [node('a', 0, 0), node('b', 400, 0)]; const edges = [{ id: 'label-cache', source: 'a', target: 'b', label: 'short' }] as Edge[]; diff --git a/frontend/lib/diagram/geometry/alignmentGuides.ts b/frontend/lib/diagram/geometry/alignmentGuides.ts index 676535ea..30a0e371 100644 --- a/frontend/lib/diagram/geometry/alignmentGuides.ts +++ b/frontend/lib/diagram/geometry/alignmentGuides.ts @@ -58,11 +58,14 @@ export function computeAlignmentGuides( const xCandidates: Candidate[] = []; const yCandidates: Candidate[] = []; + const connectedIds = new Set(); + for (const edge of edges) { + if (draggedIds.has(edge.source)) connectedIds.add(edge.target); + if (draggedIds.has(edge.target)) connectedIds.add(edge.source); + } for (const other of others) { const o = getNodeBounds(other); - const isEdgeConnected = edges.some( - (e) => (draggedIds.has(e.source) && e.target === other.id) || (draggedIds.has(e.target) && e.source === other.id) - ); + const isEdgeConnected = connectedIds.has(other.id); const thX = isEdgeConnected ? EDGE_STRAIGHT_THRESHOLD : threshold; const thY = isEdgeConnected ? EDGE_STRAIGHT_THRESHOLD : threshold; const priority = isEdgeConnected ? 0 : 1; // edge-connected snaps win (straight edge) diff --git a/frontend/lib/diagram/geometry/diagramGeometry.ts b/frontend/lib/diagram/geometry/diagramGeometry.ts index 72df3f9e..74915ea6 100644 --- a/frontend/lib/diagram/geometry/diagramGeometry.ts +++ b/frontend/lib/diagram/geometry/diagramGeometry.ts @@ -3,13 +3,17 @@ import { withoutDecorNodes } from '@/lib/diagram/nodes/infographicNodes'; import type { DiagramRenderStyleId } from '@/lib/theme/renderStyles'; import type { NodeIconMode } from '../nodes/nodeIconVisibility'; import { computeEdgeRoute, createEdgeRoutingContext, type EdgeRouteDirection, type EdgeRouteHandleAnchors, type EdgeRouteResult } from '../edges/edgeRouteBuilder'; -import { computeEdgeLabelLayout, type EdgeLabelAnchor } from '../edges/edgeLabelLayout'; +import { computeEdgeLabelLayout, estimateLabelSize, getDisplayLabel, type EdgeLabelAnchor } from '../edges/edgeLabelLayout'; import { optimizeEdgeRoutes } from '../edges/edgeRouteOptimizer'; import { validateRenderedPath, getRenderedPathInteraction, type RenderedEdgePath } from '../edges/edgePathGeometry'; import type { GeometryStatus, GeometryViolation, Rect } from './geometryTypes'; import type { Position } from 'reactflow'; import { resolveEdgeFlowSemantics } from '@/lib/diagram/mermaid/edgeFlowSemantics'; +// Change when routing semantics change so persisted in-memory geometry plans +// from the prior router cannot keep drawing stale attachment choices. +const EDGE_ROUTING_ALGORITHM_REVISION = 'floating-sides-v2'; + export interface DiagramGeometryOptions { direction: EdgeRouteDirection; renderStyleId?: DiagramRenderStyleId; @@ -23,6 +27,11 @@ export interface DiagramGeometryOptions { * itself. Exports keep the full key. */ ignoreLabelPosition?: boolean; + /** + * Draw the shortest wire between handles when nothing is in the way. + * Scoring, repair, canvas and export must use the same value. + */ + straightEdges?: boolean; } export interface DiagramGeometryPlan { @@ -81,7 +90,7 @@ export function computeDiagramGeometryPreview( const context = createEdgeRoutingContext(nodes, edges, options.direction, options.renderStyleId, options.iconMode); for (const edge of edges) { if (!affected.has(edge.id)) continue; - const route = computeEdgeRoute(edge, nodes, edges, options.direction, options.renderStyleId, options.handleAnchors?.get(edge.id), options.iconMode, { context }); + const route = computeEdgeRoute(edge, nodes, edges, options.direction, options.renderStyleId, options.handleAnchors?.get(edge.id), options.iconMode, { context, straightEdges: options.straightEdges === true }); routes.set(edge.id, route); paths.set(edge.id, route.renderedPath); sourceSides.set(edge.id, route.sourcePosition); @@ -142,17 +151,6 @@ function getBounds(nodes: readonly Node[]): Map { })); } -function ancestors(node: Node, nodesById: ReadonlyMap): Set { - const result = new Set(); - let parent = node.parentNode ?? (node as Node & { parentId?: string }).parentId; - while (parent && !result.has(parent)) { - result.add(parent); - const parentNode = nodesById.get(parent); - parent = parentNode?.parentNode ?? (parentNode as Node & { parentId?: string } | undefined)?.parentId; - } - return result; -} - function countBends(route: EdgeRouteResult): number { const points = route.waypoints; let bends = 0; @@ -172,9 +170,11 @@ function withoutLabelPosition(data: unknown): unknown { export function geometrySnapshotKey(nodes: Node[], edges: Edge[], options: DiagramGeometryOptions, bounds: ReadonlyMap = getBounds(nodes)): string { return JSON.stringify([ options.revision ?? 0, + EDGE_ROUTING_ALGORITHM_REVISION, options.direction, options.renderStyleId ?? '', options.iconMode ?? '', + options.straightEdges === true, nodes.map(node => [node.id, node.type, node.parentNode, (node as Node & { parentId?: string }).parentId, bounds.get(node.id), node.data]), edges.map(edge => [edge.id, edge.source, edge.target, edge.sourceHandle, edge.targetHandle, edge.label, options.ignoreLabelPosition ? withoutLabelPosition(edge.data) : edge.data, edge.style]), [...(options.handleAnchors ?? new Map()).entries()], @@ -210,6 +210,7 @@ export function computeDiagramGeometryPlan( const routeOptimization = optimizeEdgeRoutes({ nodes, edges, direction: options.direction, renderStyleId: options.renderStyleId, iconMode: options.iconMode, handleAnchors: options.handleAnchors, + straightEdges: options.straightEdges === true, }); const optimizedRoutes = routeOptimization.routes; @@ -225,6 +226,7 @@ export function computeDiagramGeometryPlan( const route = optimizedRoutes.get(edge.id) ?? computeEdgeRoute( edge, nodes, edges, options.direction, options.renderStyleId, options.handleAnchors?.get(edge.id), options.iconMode, + { straightEdges: options.straightEdges === true }, ); routes.set(edge.id, route); paths.set(edge.id, route.renderedPath); @@ -237,13 +239,9 @@ export function computeDiagramGeometryPlan( violations.push({ severity: 'hard', code: 'TERMINAL_INTERSECTION', edgeId: edge.id, nodeId: violation.nodeId }); } } - const sourceNode = nodeById.get(edge.source), targetNode = nodeById.get(edge.target); - const passable = new Set([ - ...(sourceNode ? ancestors(sourceNode, nodeById) : []), - ...(targetNode ? ancestors(targetNode, nodeById) : []), - ]); + // Only cards are obstacles: an edge may cross any group's empty space (see edgeRouteBuilder). const obstacles = nodes - .filter(node => node.id !== edge.source && node.id !== edge.target && (!isGroup(node) || !passable.has(node.id))) + .filter(node => node.id !== edge.source && node.id !== edge.target && (!isGroup(node) || (Boolean(process.env.EXP_GROUP_WALLS) && !__ancestorsOf(edge, nodeById).has(node.id)))) .flatMap(node => { const rect = bounds.get(node.id); return rect ? [{ id: node.id, rect }] : []; @@ -275,7 +273,6 @@ export function computeDiagramGeometryPlan( let crossings = 0; for (let i = 0; i < edges.length; i++) for (let j = i + 1; j < edges.length; j++) { const a = edges[i], b = edges[j]; - if ([a.source, a.target].some(id => id === b.source || id === b.target)) continue; const pathA = paths.get(a.id), pathB = paths.get(b.id); if (pathA && pathB && getRenderedPathInteraction(pathA, pathB).crossing) { crossings++; @@ -292,11 +289,9 @@ export function computeDiagramGeometryPlan( for (const [edgeId, anchor] of labels) { const edge = edges.find(candidate => candidate.id === edgeId); if (!edge) continue; - const raw = edge.label ?? (edge.data as { label?: unknown; responseLabel?: unknown } | undefined)?.label; - const response = (edge.data as { responseLabel?: unknown } | undefined)?.responseLabel; - const label = [raw, response].filter(value => typeof value === 'string' && value.trim()).join(' / '); - if (!label.trim()) continue; - const width = Math.min(260, Math.max(48, label.length * 8 + 24)), height = 24; + const label = getDisplayLabel(edge); + if (!label) continue; + const { w: width, h: height } = estimateLabelSize(label, options.renderStyleId); const rect = { x: anchor.x - width / 2, y: anchor.y - height / 2, width, height }; const obstruction = leafNodes.find(node => { if (node.id === edge.source || node.id === edge.target) return false; @@ -330,8 +325,12 @@ export function computeDiagramGeometryPlan( }).length; const weightedCrossings = violations.filter((violation): violation is Extract => violation.code === 'EDGE_CROSSING') .reduce((sum, violation) => { - const role = flow.assignments.get(violation.edgeId)?.role; - return sum + (role === 'main' ? 10 : role === 'branch' ? 4 : role === 'feedback' ? 2 : 1); + const weight = (id: string) => { + const role = flow.assignments.get(id)?.role; + return role === 'main' ? 10 : role === 'branch' ? 4 : role === 'feedback' ? 2 : 1; + }; + // A crossing involving the main flow has the same cost in either edge order. + return sum + Math.max(weight(violation.edgeId), weight(violation.otherEdgeId)); }, 0); const unexplainedPrivateOverlapLength = violations.reduce((sum, violation) => sum + (violation.code === 'PRIVATE_ROUTE_OVERLAP' ? violation.length : 0), 0); const quality = { @@ -362,3 +361,12 @@ export function getCachedDiagramGeometryPlan(nodes: Node[], edges: Edge[], optio if (settledPlans.size > 8) settledPlans.delete(settledPlans.keys().next().value!); return plan; } + +function __ancestorsOf(edge: Edge, byId: ReadonlyMap): Set { + const out = new Set(); + for (const id of [edge.source, edge.target]) { + let p = byId.get(id)?.parentNode ?? (byId.get(id) as Node & { parentId?: string } | undefined)?.parentId; + while (p && !out.has(p)) { out.add(p); const n = byId.get(p); p = n?.parentNode ?? (n as Node & { parentId?: string } | undefined)?.parentId; } + } + return out; +} diff --git a/frontend/lib/diagram/mermaid/__tests__/geometry-pipeline-regressions.test.ts b/frontend/lib/diagram/mermaid/__tests__/geometry-pipeline-regressions.test.ts index edc97e88..b490baee 100644 --- a/frontend/lib/diagram/mermaid/__tests__/geometry-pipeline-regressions.test.ts +++ b/frontend/lib/diagram/mermaid/__tests__/geometry-pipeline-regressions.test.ts @@ -1,8 +1,16 @@ +import { computeDiagramGeometryPlan } from '@/lib/diagram/geometry/diagramGeometry'; import { describe, expect, it } from 'vitest'; import { runMermaidPipeline } from '../pipeline'; import { semanticEdgeKeyString } from '../edgeFlowSemantics'; describe('geometry and flow metadata pipeline regressions', () => { + it('reports the same visible geometry the canvas receives, including style and labels', async () => { + const result = await runMermaidPipeline('graph LR\n a[Client] -->|Read account transaction history| b[API]\n b --> c[(Database)]', { renderStyleId: 'studio-metallic', layoutProfile: 'studio-minimal', iconMode: 'off' }); + if (!result.success) throw result.error; + const visible = computeDiagramGeometryPlan(result.data.nodes, result.data.edges, { direction: 'LR', straightEdges: false, renderStyleId: 'studio-metallic', iconMode: 'off' }); + expect(result.data.geometrySummary).toMatchObject({ status: visible.status, ...visible.quality }); + }); + it('scores RL geometry in the same horizontal orientation as final validation', async () => { const source = 'graph LR\n a[Client] --> b[API]\n b --> c[Database]'; const lr = await runMermaidPipeline(source); diff --git a/frontend/lib/diagram/mermaid/__tests__/repairLayout.test.ts b/frontend/lib/diagram/mermaid/__tests__/repairLayout.test.ts new file mode 100644 index 00000000..997a20fd --- /dev/null +++ b/frontend/lib/diagram/mermaid/__tests__/repairLayout.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, it, vi } from 'vitest'; +import type { Edge, Node } from 'reactflow'; +import { computeDiagramGeometryPlan } from '../../geometry/diagramGeometry'; +import { acceptsLayoutRepair, repairDiagramLayout } from '../repairLayout'; +import { layoutDiagramViaMermaid } from '../relayout'; +vi.mock('../relayout', () => ({ layoutDiagramViaMermaid: vi.fn() })); +const nodes: Node[] = [ + { id: 'a', position: { x: 0, y: 0 }, width: 200, height: 88, data: { label: 'API' } }, + { id: 'b', position: { x: 50, y: 0 }, width: 200, height: 88, data: { label: 'DB' } }, +]; +describe('bounded layout repair', () => { + it('rejects exchanging collisions for crossings, failures or worse fit', () => { + const before = computeDiagramGeometryPlan(nodes, [], { direction: 'LR', straightEdges: true }); + const after = { ...before, status: 'ok' as const, violations: [], quality: { ...before.quality, hardViolations: 0 } }; + expect(acceptsLayoutRepair(before, after, 1, 0.95)).toBe(true); + expect(acceptsLayoutRepair(before, after, 1, 0.89)).toBe(false); + for (const field of ['weightedCrossings', 'criticalPathDisconnectCount', 'unexplainedPrivateOverlapLength'] as const) { + expect(acceptsLayoutRepair(before, { ...after, quality: { ...after.quality, [field]: 1 } }, 1, 1)).toBe(false); + } + expect(acceptsLayoutRepair(before, before, 1, 1)).toBe(false); + }); + it('preserves identity and input data when repairing overlap', async () => { + vi.mocked(layoutDiagramViaMermaid).mockResolvedValue({ nodes: nodes.map((node, i) => ({ ...node, position: { x: i * 200, y: 0 } })), edges: [], success: true, warnings: [] }); + const edges: never[] = []; + const result = await repairDiagramLayout(nodes, edges, 'LR'); + expect(result.repaired).toBe(true); + expect(result.edges).toBe(edges); + expect(result.nodes.map(node => node.id)).toEqual(['a', 'b']); + expect(nodes[1].position.x).toBe(50); + }); + it('never rewrites user edge metadata or removes custom routes', async () => { + const edges: Edge[] = [{ id: 'manual', source: 'a', target: 'b', label: 'Read account data', data: { customWaypoints: [{ x: 300, y: 200 }], responseLabel: 'Account result' } }]; + vi.mocked(layoutDiagramViaMermaid).mockResolvedValue({ nodes, edges: [], success: true, warnings: [] }); + const result = await repairDiagramLayout(nodes, edges, 'LR'); + expect(result.edges).toBe(edges); + expect(result.edges[0]).toBe(edges[0]); + expect(result.edges[0].data.customWaypoints).toEqual([{ x: 300, y: 200 }]); + }); + it('repairs overlapping cards through the real Mermaid/Dagre pipeline', async () => { + const actual = await vi.importActual('../relayout'); + vi.mocked(layoutDiagramViaMermaid).mockImplementationOnce(actual.layoutDiagramViaMermaid); + const result = await repairDiagramLayout(nodes, [], 'LR'); + expect(result.repaired).toBe(true); + expect(result.after.quality.hardViolations).toBe(0); + expect(result.nodes).toHaveLength(nodes.length); + }); + it('rejects a candidate that places a child outside its group', async () => { + const grouped: Node[] = [{ id: 'group', type: 'groupNode', position: { x: 0, y: 0 }, width: 400, height: 300, data: {} }, ...nodes.map(node => ({ ...node, parentNode: 'group' }))]; + vi.mocked(layoutDiagramViaMermaid).mockResolvedValue({ nodes: grouped.map(node => node.id === 'b' ? { ...node, position: { x: 450, y: 0 } } : node), edges: [], success: true, warnings: [] }); + const result = await repairDiagramLayout(grouped, [], 'LR'); + expect(result.repaired).toBe(false); + expect(result.nodes).toBe(grouped); + }); + it('keeps the graph unchanged on layout failure', async () => { + vi.mocked(layoutDiagramViaMermaid).mockResolvedValue({ nodes, edges: [], success: false, warnings: ['failed'] }); + const result = await repairDiagramLayout(nodes, [], 'LR'); + expect(result.repaired).toBe(false); + expect(result.nodes).toBe(nodes); + }); +}); diff --git a/frontend/lib/diagram/mermaid/pipeline-stages/LayoutStage.ts b/frontend/lib/diagram/mermaid/pipeline-stages/LayoutStage.ts index e7844e3c..afaafe56 100644 --- a/frontend/lib/diagram/mermaid/pipeline-stages/LayoutStage.ts +++ b/frontend/lib/diagram/mermaid/pipeline-stages/LayoutStage.ts @@ -1,3 +1,5 @@ +import type { DiagramRenderStyleId } from '@/lib/theme/renderStyles'; +import type { NodeIconMode } from '@/lib/diagram/nodes/nodeIconVisibility'; import { BaseStage, type StageResult, successResult } from '@/lib/diagram/pipeline-core'; import type { PipelineContext } from '@/lib/diagram/pipeline-core'; import { measureLayoutQuality } from '@/lib/diagram/pipeline-shared/layout/layoutQuality'; @@ -41,8 +43,8 @@ const COLUMN_PACK_GAP = 56; const COLUMN_PACK_MIN_GAP = 40; /** Tiers are lined up only in diagrams with at most this many top-level groups... */ const GROUP_ALIGN_MAX = 12; -/** ...without drawing the diagram more than this much smaller on screen. */ -const GROUP_ALIGN_MAX_SHRINK = 0.1; +/** Cosmetic alignment must preserve fitted text size. */ +const GROUP_ALIGN_MAX_SHRINK = 0; function foldTries(nodeCount: number): number { return nodeCount <= 20 ? 6 : nodeCount <= 40 ? 3 : 1; @@ -112,13 +114,15 @@ export class LayoutStage extends BaseStage { }; // Studio: large cards packed close, but not so close that a label pill cannot // sit between two columns: every label reserves its full width on top of - // rankSep, which leaves room for the arrowhead and a gap on both sides. A live - // project map keeps more room inside a column for the agent tags that hang - // below a component. Other appearances keep the shared default geometry. + // rankSep, which leaves room for the arrowhead and a gap on both sides. Inside a + // column the cards keep 72px apart, a corridor for edges that pass between them + // (at 48px they had none and were routed around the whole column), which also + // holds the agent tags of a live project map. Other appearances keep the shared + // default geometry. const profile = _context.metadata.layoutProfile; const layoutOptions = profile === 'studio-minimal' || profile === 'studio-live' ? { - nodeSep: profile === 'studio-live' ? 72 : 48, + nodeSep: process.env.EXP_NS48 ? (profile === 'studio-live' ? 72 : 48) : 72, rankSep: 60, } : undefined; @@ -154,7 +158,7 @@ export class LayoutStage extends BaseStage { const candidates = [ { nodes: canonicalNodes, options: layoutOptions || Object.keys(flowAreas).length ? { ...layoutOptions, ...flowAreas } : undefined }, { nodes: [...canonicalNodes].reverse(), options: layoutOptions || Object.keys(flowAreas).length ? { ...layoutOptions, ...flowAreas } : undefined }, - { nodes: canonicalNodes, options: { ...base, nodeSep: baseNodeSep + 32, ...flowAreas } }, + { nodes: canonicalNodes, options: { ...base, nodeSep: Math.max(COLUMN_PACK_MIN_GAP, baseNodeSep - 32), rankSep: Math.max(40, baseRankSep - 20), ...flowAreas } }, { nodes: canonicalNodes, options: { ...base, rankSep: baseRankSep + 48, ...flowAreas } }, ]; const qualityDirection = input.direction === 'TD' ? 'TB' : input.direction; @@ -173,7 +177,12 @@ export class LayoutStage extends BaseStage { // counted twice and grouped candidates receive incorrect scores. const normalized = applySubgraphBoundsToRf(laid.nodes); const quality = measureLayoutQuality(normalized, laid.edges, qualityDirection); - const geometry = getCachedDiagramGeometryPlan(normalized as never, laid.edges as never, { direction: geometryDirection }); + const geometry = getCachedDiagramGeometryPlan(normalized as never, laid.edges as never, { + direction: geometryDirection, + straightEdges: false, + renderStyleId: (_context.metadata.renderStyleId ?? (profile === 'studio-minimal' || profile === 'studio-live' ? 'studio-minimal' : undefined)) as DiagramRenderStyleId | undefined, + iconMode: _context.metadata.iconMode as NodeIconMode | undefined, + }); const labelCollisions = geometry.violations.filter(issue => issue.code === 'LABEL_COLLISION').length; return { quality, @@ -243,6 +252,8 @@ export class LayoutStage extends BaseStage { evaluated.add(signature); const { candidate, measured } = produce(candidateSpec); const { quality, geometry, candidateScore } = measured; + + const score = (value: typeof best) => value ? [ value.geometry.quality.hardViolations, @@ -261,7 +272,17 @@ export class LayoutStage extends BaseStage { for (let previous = 0; previous < index; previous++) if (candidateScore[previous] !== priorScore[previous]) return false; return value < priorScore[index]; }); - if (!best || improves) { + // Protect fitted typography: route tie-breaks cannot shrink a diagram. + // A smaller result must fix hard geometry or main-path failures. + const fixesSomething = candidateScore[0] < priorScore[0] + || (candidateScore[0] === priorScore[0] && candidateScore[1] < priorScore[1]); + const shrinks = best !== undefined && measured.fit < best.fit - 1e-6; + const improvesFit = best !== undefined && measured.fit > best.fit + 1e-6 + && geometry.quality.hardViolations <= best.geometry.quality.hardViolations + && geometry.quality.criticalPathDisconnectCount <= best.geometry.quality.criticalPathDisconnectCount + && geometry.quality.weightedCrossings <= best.geometry.quality.weightedCrossings + && geometry.quality.unexplainedPrivateOverlapLength <= best.geometry.quality.unexplainedPrivateOverlapLength; + if (!best || improvesFit || (improves && (fixesSomething || !shrinks))) { best = { nodeQuality: quality, geometry, fit: measured.fit }; // Keep Dagre's absolute coordinates for the next sizing stage; the // normalized copy is only the coordinate-space-correct scoring view. @@ -282,6 +303,28 @@ export class LayoutStage extends BaseStage { if ((best?.nodeQuality.weightedCrossings ?? 0) > 0 || (best?.geometry.quality.weightedCrossings ?? 0) > 0) { evaluate(candidates[1]); } + evaluate(candidates[2]); + evaluate({ ...candidates[2], nodes: [...canonicalNodes].reverse() }); + // Packing both axes can close a necessary corridor. Search each axis separately + // on small/medium maps; all candidates still pass the complete route score. + if (canonicalNodes.length <= 40 && best && best.fit < 1) { + for (const nodes of [canonicalNodes, [...canonicalNodes].reverse()]) { + evaluate({ nodes, options: { ...base, nodeSep: Math.max(COLUMN_PACK_MIN_GAP, baseNodeSep - 32), ...flowAreas } }); + evaluate({ nodes, options: { ...base, rankSep: Math.max(40, baseRankSep - 20), ...flowAreas } }); + evaluate({ nodes, options: { ...base, nodeSep: Math.max(48, baseNodeSep - 12), rankSep: Math.max(40, baseRankSep - 10), ...flowAreas } }); + } + } + + if (best && (best.geometry.quality.weightedCrossings > 0 || best.geometry.quality.unexplainedPrivateOverlapLength > 0)) { + // Congestion can need more room on one axis, not a globally looser diagram. + for (const nodes of [canonicalNodes, [...canonicalNodes].reverse()]) { + evaluate({ nodes, options: { ...base, nodeSep: baseNodeSep + 24, ...flowAreas } }); + evaluate({ nodes, options: { ...base, rankSep: baseRankSep + 16, ...flowAreas } }); + evaluate({ nodes, options: { ...base, rankSep: baseRankSep + 24, ...flowAreas } }); + evaluate({ nodes, options: { ...base, rankSep: baseRankSep + 32, ...flowAreas } }); + evaluate({ nodes, options: { ...base, rankSep: baseRankSep + 48, ...flowAreas } }); + } + } if (best?.geometry.quality.hardViolations) { // Two bounded Dagre spacing repairs. Route optimization then reallocates // the automatic edges around the new channels; fixed paths stay fixed. @@ -387,7 +430,9 @@ export class LayoutStage extends BaseStage { const fansBefore = fans(layouted.nodes); const worse = (measured: Measured, nodes: RFObjects['nodes']) => { const after = measured.geometry.quality; - if (after.hardViolations > before.hardViolations + if (measured.fit < settled.fit - 1e-6 + || after.unexplainedPrivateOverlapLength > before.unexplainedPrivateOverlapLength + || after.hardViolations > before.hardViolations || after.criticalPathDisconnectCount > before.criticalPathDisconnectCount || after.weightedCrossings > before.weightedCrossings || labelHits(measured.geometry) > labelHits(settled.geometry)) return true; diff --git a/frontend/lib/diagram/mermaid/pipeline.ts b/frontend/lib/diagram/mermaid/pipeline.ts index 301c01ef..750db467 100644 --- a/frontend/lib/diagram/mermaid/pipeline.ts +++ b/frontend/lib/diagram/mermaid/pipeline.ts @@ -1,3 +1,5 @@ +import type { DiagramRenderStyleId } from '@/lib/theme/renderStyles' +import type { NodeIconMode } from '@/lib/diagram/nodes/nodeIconVisibility' import { Pipeline, pipelineStages, toDomainResult } from '@/lib/diagram/pipeline-core' import type { DomainPipelineResult } from '@/lib/diagram/pipeline-core' import type { Stage } from '@/lib/diagram/pipeline-core/Stage' @@ -214,7 +216,7 @@ export function createMermaidPipelineStages(): Stage; export interface RelayoutOptions { + iconMode?: NodeIconMode; /** Used when the graph has no top heading yet (templates, manual diagrams). */ title?: string; /** Applies an appearance-specific spacing profile without changing topology. */ @@ -115,7 +117,7 @@ export async function layoutDiagramViaMermaid( if (!byEdges) { byEdges = new WeakMap(); relayoutCache.set(nodes, byEdges); } let byOptions = byEdges.get(edges); if (!byOptions) { byOptions = new Map(); byEdges.set(edges, byOptions); } - const cacheKey = JSON.stringify([direction, options.title ?? '', options.renderStyleId ?? '', options.layoutStyle ?? 'flow', options.foldLongFlows ?? false, options.mixGroupDirections ?? true, options.layoutProfile ?? '', options.areaOrder ?? '']); + const cacheKey = JSON.stringify([direction, options.title ?? '', options.renderStyleId ?? '', options.layoutStyle ?? 'flow', options.foldLongFlows ?? false, options.mixGroupDirections ?? true, options.layoutProfile ?? '', options.areaOrder ?? '', options.iconMode ?? '']); const cachedLayout = byOptions.get(cacheKey); if (cachedLayout) return cachedLayout; @@ -169,6 +171,8 @@ export async function layoutDiagramViaMermaid( const result = await runMermaidPipeline(mermaid, { layoutProfile: options.layoutProfile ?? (options.renderStyleId === 'studio-minimal' || options.renderStyleId === 'studio-metallic' ? 'studio-minimal' : undefined), + renderStyleId: options.renderStyleId, + iconMode: options.iconMode, edgeSemantics, layoutStyle: options.layoutStyle, foldLongFlows: options.foldLongFlows, diff --git a/frontend/lib/diagram/mermaid/repairLayout.ts b/frontend/lib/diagram/mermaid/repairLayout.ts new file mode 100644 index 00000000..bf25ba55 --- /dev/null +++ b/frontend/lib/diagram/mermaid/repairLayout.ts @@ -0,0 +1,54 @@ +import type { Edge, Node } from 'reactflow'; +import { getCachedDiagramGeometryPlan, type DiagramGeometryPlan } from '../geometry/diagramGeometry'; +import { measureLayoutAesthetics } from '../pipeline-shared/layout/layoutAesthetics'; +import { layoutDiagramViaMermaid, type LayoutDirection, type RelayoutOptions } from './relayout'; + +/** A repair must improve a conflict without buying a different kind of conflict. */ +export function acceptsLayoutRepair(before: DiagramGeometryPlan, after: DiagramGeometryPlan, beforeFit: number, afterFit: number): boolean { + const metrics = (plan: DiagramGeometryPlan) => [ + plan.quality.hardViolations, + plan.quality.criticalPathDisconnectCount, + plan.quality.weightedCrossings, + plan.quality.unexplainedPrivateOverlapLength, + plan.violations.filter(issue => issue.code === 'LABEL_COLLISION').length, + ]; + const previous = metrics(before), next = metrics(after); + const issueKey = (issue: DiagramGeometryPlan['violations'][number]) => + JSON.stringify(Object.entries(issue).sort(([a], [b]) => a.localeCompare(b))); + const existingHardIssues = new Set(before.violations.filter(issue => issue.severity === 'hard').map(issueKey)); + const introducesHardIssue = after.violations.some(issue => issue.severity === 'hard' && !existingHardIssues.has(issueKey(issue))); + return !introducesHardIssue && !after.provisional && after.status !== 'failed' + && afterFit >= beforeFit * 0.9 + && next.every((value, index) => value <= previous[index]) + && next.some((value, index) => value < previous[index]); +} + +/** One bounded canonical relayout, judged on restored user edges and visible geometry. */ +export async function repairDiagramLayout(nodes: Node[], edges: Edge[], direction: LayoutDirection, options: RelayoutOptions = {}) { + const geometryOptions = { direction, straightEdges: false, renderStyleId: options.renderStyleId, iconMode: options.iconMode }; + const before = getCachedDiagramGeometryPlan(nodes, edges, geometryOptions); + const unchanged = { nodes, edges, repaired: false, before, after: before }; + if (!nodes.length || before.provisional) return unchanged; + const candidate = await layoutDiagramViaMermaid(nodes, edges, direction, { ...options, foldLongFlows: false, mixGroupDirections: false }); + if (!candidate.success) return unchanged; + // Repair moves existing components only. Keep annotations and every edge's identity, + // handles, labels, custom waypoints and styling exactly as authored. + const positions = new Map(candidate.nodes.map(node => [node.id, node])); + if (nodes.some(node => !positions.has(node.id))) return unchanged; + const repairedNodes = nodes.map(node => { + const laid = positions.get(node.id)!; + return { ...node, position: laid.position, positionAbsolute: undefined, width: laid.width, height: laid.height, style: laid.style, data: { ...node.data, nodeWidth: laid.width, nodeHeight: laid.height } }; + }); + const byId = new Map(repairedNodes.map(node => [node.id, node])); + // Geometry routing validates card collisions; containment is a separate layout invariant. + for (const node of repairedNodes) { + const parent = node.parentNode ? byId.get(node.parentNode) : undefined; + if (parent && (node.position.x < 0 || node.position.y < 0 + || node.position.x + (node.width ?? 200) > (parent.width ?? 0) + || node.position.y + (node.height ?? 88) > (parent.height ?? 0))) return unchanged; + } + const after = getCachedDiagramGeometryPlan(repairedNodes, edges, geometryOptions); + const fit = (value: Node[]) => measureLayoutAesthetics(value, [], direction).fitScale; + if (!acceptsLayoutRepair(before, after, fit(nodes), fit(repairedNodes))) return unchanged; + return { nodes: repairedNodes, edges, repaired: true, before, after }; +} diff --git a/frontend/lib/diagram/pipeline-shared/__tests__/crossGroupAlignment.test.ts b/frontend/lib/diagram/pipeline-shared/__tests__/crossGroupAlignment.test.ts index 46fef769..d40b8b44 100644 --- a/frontend/lib/diagram/pipeline-shared/__tests__/crossGroupAlignment.test.ts +++ b/frontend/lib/diagram/pipeline-shared/__tests__/crossGroupAlignment.test.ts @@ -38,11 +38,11 @@ const interviewApp = `graph LR rest -->|"rows"| db `; -describe('cross-group edges are drawn level', () => { - it('puts a service and the external API it calls on the same level', async () => { +describe('cross-group edge alignment', () => { + it('keeps large endpoint offsets as distinct lanes instead of pulling groups out of place', async () => { const { centerY } = await layout(interviewApp); - expect(Math.abs(centerY('voice') - centerY('eleven'))).toBeLessThan(1); - expect(Math.abs(centerY('ai') - centerY('groq'))).toBeLessThan(1); + expect(Math.abs(centerY('voice') - centerY('eleven'))).toBeGreaterThanOrEqual(64); + expect(Math.abs(centerY('ai') - centerY('groq'))).toBeGreaterThanOrEqual(64); }); it('leaves large offsets (separate lanes) alone', async () => { diff --git a/frontend/lib/diagram/pipeline-shared/__tests__/layoutAesthetics.test.ts b/frontend/lib/diagram/pipeline-shared/__tests__/layoutAesthetics.test.ts index 9c561dbe..b0b08dd9 100644 --- a/frontend/lib/diagram/pipeline-shared/__tests__/layoutAesthetics.test.ts +++ b/frontend/lib/diagram/pipeline-shared/__tests__/layoutAesthetics.test.ts @@ -21,7 +21,7 @@ const DEFAULT_FAN_CEILING = 10; const STRAIGHT_FLOOR: Record = {}; -/** Key: `${template id}:${LR|TD}`. Every template not listed here must be within the default. */ +/** Key: template id. Every template not listed here must be within the default. */ const FAN_CEILING: Record = { // Dense and flat (56 nodes): interlocking fans that cannot all be centred. 'archdraw_self:LR': 350, diff --git a/frontend/lib/diagram/pipeline-shared/layout/CompoundLayout.ts b/frontend/lib/diagram/pipeline-shared/layout/CompoundLayout.ts index 26e913dc..6cb4309e 100644 --- a/frontend/lib/diagram/pipeline-shared/layout/CompoundLayout.ts +++ b/frontend/lib/diagram/pipeline-shared/layout/CompoundLayout.ts @@ -448,6 +448,8 @@ function alignCrossGroupEdges( positions.has(edge.source) && positions.has(edge.target) && parentOf.get(edge.source) !== parentOf.get(edge.target) && (outDegree.get(edge.source)?.size ?? 0) === 1 && + (inDegree.get(edge.source)?.size ?? 0) === 1 && + (outDegree.get(edge.target)?.size ?? 0) <= 1 && (inDegree.get(edge.target)?.size ?? 0) === 1); const level = new Set(); diff --git a/frontend/scripts/eval/layout/run.mts b/frontend/scripts/eval/layout/run.mts new file mode 100644 index 00000000..f7982087 --- /dev/null +++ b/frontend/scripts/eval/layout/run.mts @@ -0,0 +1,31 @@ +/** Reproducible diagram gallery: npx tsx scripts/eval/layout/run.mts [output-directory] [template-id] */ +import { mkdirSync, writeFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import { TEMPLATES } from '../../../data/templates'; +import { layoutDiagramViaMermaid } from '../../../lib/diagram/mermaid/relayout'; +import { computeDiagramGeometryPlan } from '../../../lib/diagram/geometry/diagramGeometry'; +import { measureLayoutAesthetics } from '../../../lib/diagram/pipeline-shared/layout/layoutAesthetics'; +import { generatePureSVG } from '../../../lib/diagram/export/svgExport'; + +const output = resolve(process.argv[2] ?? '/tmp/archdraw-layout-audit'); +mkdirSync(output, { recursive: true }); +const escape = (value: string) => value.replace(/[&<>"']/g, char => ({ '&': '&', '<': '<', '>': '>', '"': '"', "'": ''' })[char]!); +const results = []; +const panels: string[] = []; +for (const template of TEMPLATES.filter(t => !process.argv[3] || process.argv[3].split(',').includes(t.id))) { + for (const direction of ['LR', 'TD'] as const) { + const start = performance.now(); + const result = await layoutDiagramViaMermaid(template.nodes, template.edges, direction, { renderStyleId: 'studio-minimal', foldLongFlows: true }); + const geometry = computeDiagramGeometryPlan(result.nodes, result.edges, { direction, renderStyleId: 'studio-minimal', straightEdges: false }); + const aesthetics = measureLayoutAesthetics(result.nodes, result.edges, direction); + const metrics = { id: template.id, direction, ms: Math.round(performance.now() - start), ...geometry.quality, titlePxAtFit: aesthetics.titlePxAtFit }; + results.push(metrics); + console.log(JSON.stringify(metrics)); + const filename = `${template.id}-${direction}.svg`; + writeFileSync(resolve(output, filename), generatePureSVG(result.nodes, result.edges, false, '#ffffff', direction, 'studio-minimal', 'studio-minimal')); + panels.push(`

${escape(template.name)} · ${direction}

${escape(JSON.stringify(metrics))}

Review: identify entry point; trace request to storage; explain branches and feedback.

`); + } +} +writeFileSync(resolve(output, 'metrics.json'), JSON.stringify(results, null, 2)); +writeFileSync(resolve(output, 'index.html'), `ArchDraw layout audit

Layout audit · 1280 × 720

${panels.join('')}`); +console.log(`Wrote ${results.length} diagrams and metrics to ${output}`); diff --git a/layout-fix.md b/layout-fix.md new file mode 100644 index 00000000..056838c7 --- /dev/null +++ b/layout-fix.md @@ -0,0 +1,155 @@ +# Layout and edge readability investigation + +Date: 2026-11-08 +Status: implementation in progress. Root scoring/label/spacing fixes are implemented; two typography regressions and live interaction validation remain unresolved. + +## Outcome + +Yes: the current system has several concrete reasons why a technically valid diagram can be difficult to understand. It optimizes placement and routing in stages, but those stages do not consistently judge the final rendered result. Geometry correctness alone does not establish whether a reader can identify the entry point, main flow, branches, and system boundaries. + +The earlier crossing and square-corner fixes address real bugs, but do not solve this broader readability problem. + +## Evidence and limits + +This review inspected the current working tree, including existing uncommitted changes, in the canonical Mermaid/Dagre layout, route optimizer, geometry planner, label placement, and canvas geometry provider. The prior verification run passed 292 tests and TypeScript. Those results are regression evidence, not proof of visual readability. This investigation did not inspect a supplied failing diagram, run a user comprehension study, or complete a browser screenshot audit. Findings below distinguish code behavior from hypotheses needing visual measurement. + +## Findings + +### 1. Layout selection and the canvas evaluate different geometry — highest priority + +Evidence: +- `frontend/lib/diagram/mermaid/pipeline-stages/LayoutStage.ts`, `measureCandidate`, calls `getCachedDiagramGeometryPlan` with only `direction`. +- `frontend/components/canvas/edges/DiagramGeometryProvider.tsx` passes render style, icon mode, measured handle anchors, and `straightEdges: true`. +- `frontend/lib/diagram/geometry/diagramGeometry.ts` defaults straight routing to false. + +The candidate selected for low crossings and good labels is therefore not evaluated under the same routing policy as the visible canvas. Style and measured geometry can also differ. This mismatch is confirmed; the size of its effect on individual diagrams needs measurement. A good layout score cannot currently guarantee that the visible routes retain that quality. + +### 2. Fitting the graph can trade away traceable flow + +Evidence: `LayoutStage.ts` tries folded layouts and mixed group directions. Fold acceptance explicitly permits additional weighted crossings through `foldCrossingAllowance`, capped at 12, in exchange for improved fit. Mixed direction candidates use the same allowance. + +These features improve text size and should not simply be removed. However, serpentine rows and changes in local direction make the reader reconstruct the reading order. More readable text can coexist with a less understandable flow. The current score does not directly measure that cognitive cost. + +### 3. The optimization objective is not a comprehension objective + +Evidence: `LayoutStage.ts` compares candidates lexicographically by hard violations, main-path failures, private overlap, weighted crossings, label collisions, bends, path length, area, and node-quality penalty. Fit is handled through additional guards and candidate transformations, rather than a complete typography/readability acceptance contract. + +Small improvements in an earlier metric can dominate large changes in later metrics. This is not inherently wrong, but the relative priorities need explicit product decisions. There is no demonstrated guarantee that a first-time reader can identify a main path at the final fit zoom. + +### 4. Edge routing has a deliberately narrow search + +Evidence: `frontend/lib/diagram/edges/edgeRouteOptimizer.ts` uses normal lane deltas of 0 and ±28, additional feedback deltas of ±144, two repair passes, and bounded candidate/state budgets. It routes edges sequentially, then repairs conflicts. Route cost combines length, bend cost, crossing cost, and overlap cost. + +The bounds protect responsiveness. They also mean complex graphs may exhaust local alternatives without finding a globally clear arrangement. Increasing budgets indiscriminately would risk latency without fixing the objective. Measure which conflicts are caused by placement versus route choice before expanding search. + +### 5. Labels can lose architectural meaning + +Evidence: `composeEdgeDisplayLabel` in `frontend/lib/diagram/edges/edgeLabelLayout.ts` keeps only the first three words. Request and response labels are combined before shortening. The label placement search keeps labels on their wires (`NODE_ESCAPE_PERPS = [0]`). + +A label can therefore omit the operation, destination qualifier, or response semantics that distinguish two edges. On-wire placement also has limited options on short or shared segments. Placement already checks cards, other labels, and other edges; the issue is not an absence of collision handling, but limited escape space and potentially lossy content. + +Additional mismatch: `diagramGeometry.ts` estimates diagnostic label rectangles from raw label length with a capped width, while the display formatter shortens text. Share display text and metrics so the scorer judges the actual pill. + +### 6. Known bad geometry can be returned successfully + +Evidence: the end of `LayoutStage.ts` returns `successResult(layouted, geometryWarnings, ...)` even when geometry status reports remaining hard violations or main-path failures. The canvas provider shows a hard-issue warning suggesting moving a node or trying another layout. + +This is a useful graceful fallback, but it is not a readability guarantee. The system can knowingly deliver a result that requires manual repair. Preserve the user's graph and offer a concrete recovery path; do not silently remove connections to make the warning disappear. + +### 7. Fan handling had blind spots; earlier fixes need broader fixtures + +The earlier changes removed the blanket crossing exemption for edges sharing a node, guarded lane separation against new sibling crossings, and preserved orthogonal bends at zero corner radius. Existing tests pass. These are foundational fixes, not evidence that dense fan-in/fan-out and feedback diagrams are now understandable. + +### 8. Interaction stability needs browser verification + +Evidence: the canvas provider holds a previous plan during dragging, uses per-edge fallback routes while a new worker plan is pending, and recomputes after a delay. This could produce visibly different intermediate and settled paths. Treat this as a hypothesis, not a confirmed flicker bug. Test dragging, resizing, loading saved canvases, and switching render styles. + +## Repair plan + +### Phase 1 — Establish a reproducible visual baseline + +- Capture representative shipped templates and failing user diagrams: flat chains, grouped flows, dense branches, feedback, parallel edges, long labels, nested groups, and manually edited routes. +- Exercise LR/TD, relevant render styles, icon modes, and editor/share/export outputs. +- Save screenshots at a fixed laptop viewport and fit zoom alongside geometry metrics: node and label intersections, crossings, private overlap, bends, path length, and actual rendered title/label sizes. +- Include a simple comprehension review: identify entry point, follow the main request, locate storage and external services, explain one feedback loop. +- Preserve baseline fixtures and existing quality floors. Do not lower thresholds to accommodate a repair. + +Deliverable: reproducible before/after gallery and per-fixture measurements, including at least one real failing diagram when available. + +### Phase 2 — Make scoring match rendering + +- Define one explicit geometry policy passed through layout evaluation, canvas, viewers, and exports, including style, icon mode, and straight/orthogonal policy. +- Use shared displayed label text and dimensions for placement and diagnostics. +- Validate the final measured canvas geometry after node sizing; avoid a recursive layout/measurement loop. +- Preserve the canonical `layoutDiagramViaMermaid` → Dagre path and manual waypoint overrides. + +Acceptance: equivalent graph/style inputs produce consistent route topology and label bounds across scoring and rendering, within documented measurement tolerance. + +### Phase 3 — Make main flow clarity a layout constraint + +- Keep hard safety constraints first: no node overlaps, out-of-group nodes, route-through-card failures, or disconnected main path on supported regression fixtures. +- Add explicit minimum fitted typography targets, chosen from the visual baseline rather than guessed globally. +- Penalize main-path direction reversals, excessive detours, and ambiguous wrap transitions. +- Prefer a clear stage order and stable group directions. Accept folding or mixed directions only when the visible result passes readability checks; do not reward fit alone. +- For diagrams too dense to meet those constraints, propose an overview/detail workflow as a separate product decision. Never silently hide or delete relationships. + +Acceptance: readers can follow the main path without guessing the next row or branch; text remains readable at the target viewport. + +### Phase 4 — Repair congested routing and label space together + +- Detect congested corridors and distinguish shared intentional trunks from unrelated overlapping connections. +- Allocate space based on corridor demand and label size; feed unresolved congestion back into candidate placement instead of only moving wires by fixed offsets. +- Keep fan lanes nested and feedback routes distinguishable. Preserve source/target semantics and user overrides. +- Use bounded, deterministic alternative searches focused on problematic regions; retain a candidate only when the complete visible geometry improves. +- Replace unconditional three-word shortening with deliberate label sizing/wrapping and an explicit truncation treatment when necessary; preserve complete accessible text. + +Acceptance: no newly introduced crossings or card/label intersections; every visible connection remains traceable; labels retain the information distinguishing operations. + +### Phase 5 — Stable interaction and actionable recovery + +- Browser-test provisional versus settled geometry during drag, resize, load, and style changes. Repair stale geometry only where reproduced. +- Make remaining layout failure actionable through a bounded repair/re-layout operation, preserving manual edits and undo behavior. +- Report accurately when a graph cannot meet readability constraints instead of implying every successful generation is visually acceptable. + +## Validation and rollout + +Run touched Vitest suites, layout aesthetics/readability and hero regressions, TypeScript, and browser checks. Compare screenshots and metrics against Phase 1, including runtime measurements on larger fixtures. Verify export parity, read-only hosts, deterministic reruns, custom waypoints, and saved canvases. Use small independently reviewable changes in phase order; do not replace Dagre or add another layout engine before these specific mismatches are resolved. + +The first implementation priority is geometry-policy parity. Tuning spacing or routing against a different picture from the one users see would make subsequent work unreliable. + +## Repair model implementation (2026-11-08) + +Implemented a bounded editor recovery operation in `frontend/lib/diagram/mermaid/repairLayout.ts`: + +- One canonical Dagre relayout using the existing bounded candidate/spacing search, with folding and mixed group directions disabled for repair. +- Score restored nodes against the original authored edges under the canvas straight-edge policy, current render style and icon mode. +- Accept only a strict reduction in conflicts with no increase in hard violations, main-path failures, weighted crossings, private overlaps or label collisions; retain at least 90% of the prior fitted size. This 90% guard is a conservative implementation bound, not a measured typography target. +- Preserve all node identities/data and original edge objects, including manual routes; reject out-of-group results. Leave the graph unchanged if no safe improvement is found. +- Editor hard-issue notice offers **Repair layout**, records one undo step on acceptance, and rejects results if the canvas or graph changed while awaiting repair. Read-only viewers do not offer the action. +- Repair candidate scoring and final pipeline validation use straight routes and accept render-style/icon-mode inputs. Ordinary layout selection retains its existing orthogonal scoring policy: globally switching it caused fitted-text regression failures and needs a separate rollout. Visible-route label diagnostics share display text and estimated pill dimensions with placement; legacy orthogonal scoring retains its old estimates during migration. + +This implements the repair mechanism and geometry-policy corrections. The visual baseline/gallery, measured typography targets, broader semantic label redesign, congestion-driven placement search and browser interaction audit remain follow-up work; the full five-phase plan is not complete. + +Validation: routing/geometry/aesthetics/hero and pipeline regression run passed 299 tests; the expanded repair suite covers acceptance guards, original edge metadata, canonical Dagre repair, group containment, and unchanged-on-failure behavior. TypeScript and diff whitespace checks passed. The broader readability suite still reports fitted-title floor failures for `food_delivery:LR` (14.78 vs 14.9px) and `ecommerce:TD` (19.49 vs 20.2px) under the retained ordinary scoring policy; floors were not lowered. Browser interaction/screenshot validation has not been performed. + +## Root-cause fixes and balanced spacing + +Ordinary layout selection now scores the same straight-route policy as the canvas, including render style and icon mode; the legacy scoring exception has been removed. Visible label placement and diagnostics share the full displayed text and pill dimensions. Request/response labels retain all words rather than silently stopping at word three. + +Candidate selection protects fitted size when only soft route metrics improve. Cosmetic group alignment cannot shrink fitted text. Spacing search compares standard, compact and independently adjusted axes, with compact node spacing bounded at the existing 40px column-corridor minimum and rank spacing at 40px plus the engine's label reservation. Congested routes trigger intermediate 16/24/32px rank-gap candidates before a 48px expansion; all alternatives are routed and scored. Crossing cost now uses both participating edges' semantic roles, making it independent of edge array order. + +The repair notice also covers substantial edge congestion. Hero generation passes its actual metallic render style, and hero tests evaluate the visible routes rather than the orthogonal fallback. Existing crossing and typography thresholds were not lowered. + +A reproducible export gallery and metrics runner lives at `frontend/scripts/eval/layout/run.mts`: run `npx tsx scripts/eval/layout/run.mts /tmp/archdraw-layout-audit [template-id]` from `frontend/`. It emits fixed 1280×720 views, SVGs, geometry metrics, fitted-title sizes and timing. A Food Delivery export gallery was rendered and inspected in headless Chrome. This is export/browser-render evidence; editor drag/resize/share interaction coverage and a human comprehension study remain unverified. + +Final verification during this follow-up: 304 routing/geometry/repair/pipeline/aesthetics/visible-hero tests passed across 34 files, and TypeScript passed. The full typography suite still fails Netflix LR (9.204px vs 9.3px) and Food Delivery TD (11.852px vs 12.1px); the other two suite checks pass. These remain unresolved and the full plan must not be marked complete. + +The local editor browser capture was attempted against port 3100 but did not finish; its headless browser was stopped. No drag/resize/reload result is claimed. + +Final-pass correction: spine alignment now preserves the selected fit scale and cannot increase private route overlap. Previously its acceptance checks covered collisions/crossings and fan alignment but could still enlarge the bounding box after candidate selection. Typography and hero regressions are being checked against this correction. + +Spine correction verification: hero tests pass and TypeScript passes; the two typography failures remain unchanged (Netflix LR 9.204/9.3px; Food Delivery TD 11.852/12.1px). The correction therefore does not resolve those failures. + +## Consistent rounded-step routing (2026-11-09) + +Product direction updated: automatic canvas, fallback, layout scoring, repair and SVG export routes now use the same rounded orthogonal policy (`straightEdges: false`). Aligned nodes have straight axis-aligned links; staggered nodes use rounded right-angle bends. Diagonal shortcuts are disabled in every production caller. Hero channel tests are restored to that routing contract. Initial verification: 304 tests across 33 files and TypeScript pass. diff --git a/mcp-server/src/ui/diagram-widget.html b/mcp-server/src/ui/diagram-widget.html index 8c7a5e1a..d89dfffe 100644 --- a/mcp-server/src/ui/diagram-widget.html +++ b/mcp-server/src/ui/diagram-widget.html @@ -137,10 +137,11 @@

stage.addEventListener('wheel', (event) => { // Inline, a plain scroll keeps scrolling the chat; pinch (ctrl) or ⌘ zooms. + // Expanded, the diagram is the whole view: the wheel zooms and dragging pans. if (!natural || (displayMode !== 'fullscreen' && !event.ctrlKey && !event.metaKey)) return; event.preventDefault(); - if (event.ctrlKey || event.metaKey) zoomAt(Math.exp(-event.deltaY * 0.01), ...local(event)); - else { view = { ...view, x: view.x - event.deltaX, y: view.y - event.deltaY }; applyView(); } + const speed = event.ctrlKey || event.metaKey ? 0.01 : 0.0025; + zoomAt(Math.exp(-event.deltaY * speed), ...local(event)); }, { passive: false }); const pointers = new Map(); diff --git a/mcp-server/src/ui/diagram-widget.ts b/mcp-server/src/ui/diagram-widget.ts index 5334949e..4299b85f 100644 --- a/mcp-server/src/ui/diagram-widget.ts +++ b/mcp-server/src/ui/diagram-widget.ts @@ -1,6 +1,6 @@ import { readFileSync } from 'node:fs'; -export const DIAGRAM_WIDGET_URI = 'ui://archdraw/diagram-v2.html'; +export const DIAGRAM_WIDGET_URI = 'ui://archdraw/diagram-v3.html'; /** Tool-result `_meta` key holding the drawn diagram (SVG) for the preview. */ export const PREVIEW_SVG_META_KEY = 'archdraw/previewSvg'; export const DIAGRAM_WIDGET_HTML = readFileSync(new URL('./diagram-widget.html', import.meta.url), 'utf8');