From 5ebb7c2b3fb4cd317a21340940f08a4d7cf6ca02 Mon Sep 17 00:00:00 2001 From: Matthew Valancy Date: Sat, 13 Jun 2026 22:26:29 -0700 Subject: [PATCH] Edges attach to node borders + edge-label width is a hard minimum edge length Two long-standing graph-layout problems, measured with the geometry diagnostic before changing anything (baseline: edges 0px from node centers; a 200px edge couldn't fit its 104px "Depends On" label -> +74px overflow onto the cards). 1. BORDER ATTACHMENT (dynamic anchor). New pure, unit-tested edgeGeometry.ts: rectBorderPoint() returns where the center line crosses a card's border; edgeBorderEndpoints() gives border-to-border endpoints. updateEdgePositions now draws the line + hitbox border-to-border and puts the arrow on the TARGET border. The anchor slides around the border as nodes move around each other -> always the shortest border-to-border connection. 2. LABEL WIDTH = MINIMUM EDGE LENGTH. minEdgeLength() = halfDiag(src) + halfDiag(tgt) + labelW + pad guarantees the label fits in the border gap at ANY angle. forceLink.distance is floored to it (rest length), and a new position-based 'minEdge' constraint force (like forceCollide, per-edge, respecting pinned nodes) makes it a HARD floor, not just a spring preference. Verified on the live stack via the diagnostic: border attachment: endpoint-to-center 0px -> 85px (on the border) for every edge. unpinned cluster (hub + 5 spokes, before the force): 1 edge overflowing, 2 labels overlapping their cards, minClearSpan 72 < labelW 106. unpinned cluster (after): 0 overflowing, 0 overlapping, minClearSpan 116 >= 107. No regressions: web units 113, perf budgets 3/3 (settle/tick/drift within budget), THE GATE 5/5, living-graph 3/3. (Pinned/user-placed nodes are deliberately left where the user put them; the floor governs the auto-layout. Drag-time clamping is a possible follow-up.) Co-Authored-By: Claude Opus 4.8 (1M context) --- .../InteractiveGraphVisualization.tsx | 82 +++++++++++++++---- .../src/lib/__tests__/edgeGeometry.test.ts | 75 +++++++++++++++++ packages/web/src/lib/edgeGeometry.ts | 77 +++++++++++++++++ tests/diagnostics/graph-geometry.spec.ts | 38 +++++++++ 4 files changed, 254 insertions(+), 18 deletions(-) create mode 100644 packages/web/src/lib/__tests__/edgeGeometry.test.ts create mode 100644 packages/web/src/lib/edgeGeometry.ts diff --git a/packages/web/src/components/InteractiveGraphVisualization.tsx b/packages/web/src/components/InteractiveGraphVisualization.tsx index 23ac750..766ae0f 100644 --- a/packages/web/src/components/InteractiveGraphVisualization.tsx +++ b/packages/web/src/components/InteractiveGraphVisualization.tsx @@ -46,6 +46,7 @@ import { mergeSimulationNodes, mergeSimulationEdges } from '../lib/graphDataMerg import { edgeLabelPlacement, clearSegment, slideTFromPointer, chooseLabelT } from '../lib/edgeLabelLayout'; import { PerfMeter, DriftMeter } from '../lib/perfMeter'; import { DEFAULT_PHYSICS, collisionRadius, linkDistance, linkMaxDistance, linkStrength } from '../lib/physicsConfig'; +import { edgeBorderEndpoints, minEdgeLength } from '../lib/edgeGeometry'; import { spawnCelebration } from '../lib/celebration'; import { buildNeighborhood } from '../lib/graphAdjacency'; import { UndoStack } from '../lib/undoStack'; @@ -1969,13 +1970,47 @@ export function InteractiveGraphVisualization({ onResetLayout }: InteractiveGrap // physicsConfig.ts — see that file (and the debug console's drift metrics) // to reason about / tune why nodes settle and drift the way they do. const phys = DEFAULT_PHYSICS; + + // Hard minimum-edge-length constraint: connected nodes may never sit closer + // than their edge label needs to display (d._minLen, cached by the link + // distance accessor = halfDiag(src)+halfDiag(tgt)+labelW+pad). Position-based + // like forceCollide, so it's a real floor, not just a spring preference. It + // respects pinned nodes (fx set): a free node yields, two pinned nodes hold. + const minEdgeForce = () => { + for (const e of validatedEdges as any[]) { + const s = e.source, t = e.target; + if (!s || !t || typeof s.x !== 'number' || typeof t.x !== 'number') continue; + const min = e._minLen || 0; + if (min <= 0) continue; + let dx = t.x - s.x, dy = t.y - s.y; + let dist = Math.hypot(dx, dy); + if (dist === 0) { dx = 1; dy = 0; dist = 1; } // arbitrary separation dir + if (dist >= min) continue; + const corr = ((min - dist) / dist) * 0.5; // ease toward the floor + const ox = dx * corr, oy = dy * corr; + const sFixed = s.fx != null, tFixed = t.fx != null; + if (sFixed && tFixed) continue; + if (sFixed) { t.x += ox * 2; t.y += oy * 2; } + else if (tFixed) { s.x -= ox * 2; s.y -= oy * 2; } + else { s.x -= ox; s.y -= oy; t.x += ox; t.y += oy; } + } + }; + simulation .force('link', d3.forceLink(validatedEdges) .id((d: any) => d.id) .distance((d: any) => { - const currentDistance = Math.hypot(d.target.x - d.source.x, d.target.y - d.source.y); + const currentDistance = Math.hypot((d.target.x || 0) - (d.source.x || 0), (d.target.y || 0) - (d.source.y || 0)); const maxDistance = linkMaxDistance(width, height, phys); - return currentDistance > maxDistance ? maxDistance : linkDistance(width, height, phys); + const preferred = currentDistance > maxDistance ? maxDistance : linkDistance(width, height, phys); + // Floor: never pull connected nodes closer than their edge label + // needs to display — the label width sets a minimum edge length so + // it always fits in the border-to-border gap (edgeGeometry.minEdgeLength). + const label = getRelationshipConfig(d.type as RelationshipType)?.label || ''; + const labelW = label.length * 6.2 + 28; // 10px/600 text + icon + padding + const minLen = minEdgeLength(getNodeDimensions(d.source), getNodeDimensions(d.target), labelW); + d._minLen = minLen; // cached for the hard min-edge constraint below + return Math.max(preferred, minLen); }) .strength((d: any) => { const currentDistance = Math.hypot(d.target.x - d.source.x, d.target.y - d.source.y); @@ -1995,6 +2030,7 @@ export function InteractiveGraphVisualization({ onResetLayout }: InteractiveGrap .strength(phys.collision.strength) .iterations(phys.collision.iterations) ) + .force('minEdge', minEdgeForce) .force('hierarchy', d3.forceLink() .id((d: any) => d.id) .links(createHierarchicalLinks(nodes)) @@ -3370,27 +3406,37 @@ export function InteractiveGraphVisualization({ onResetLayout }: InteractiveGrap let labelAvoidCounter = 0; const updateEdgePositions = () => { + // Border-to-border anchors: the edge starts/ends where the center line + // crosses each card's border, not at the buried center. Computed once per + // edge per tick (shared datum) so line, hitbox and arrow agree. The anchor + // slides around the border as the nodes move — shortest border path. + linkElements.each(function (d: any) { + d._ep = edgeBorderEndpoints( + { x: d.source.x || 0, y: d.source.y || 0 }, getNodeDimensions(d.source), + { x: d.target.x || 0, y: d.target.y || 0 }, getNodeDimensions(d.target) + ); + }); + // Update visible edge positions linkElements - .attr('x1', (d: any) => d.source.x) - .attr('y1', (d: any) => d.source.y) - .attr('x2', (d: any) => d.target.x) - .attr('y2', (d: any) => d.target.y); - - // Update clickable edge positions + .attr('x1', (d: any) => d._ep.x1) + .attr('y1', (d: any) => d._ep.y1) + .attr('x2', (d: any) => d._ep.x2) + .attr('y2', (d: any) => d._ep.y2); + + // Update clickable edge positions clickableEdges - .attr('x1', (d: any) => d.source.x) - .attr('y1', (d: any) => d.source.y) - .attr('x2', (d: any) => d.target.x) - .attr('y2', (d: any) => d.target.y); - - // Update arrow positions + .attr('x1', (d: any) => d._ep.x1) + .attr('y1', (d: any) => d._ep.y1) + .attr('x2', (d: any) => d._ep.x2) + .attr('y2', (d: any) => d._ep.y2); + + // Arrow sits at the TARGET border, pointing into the node. arrowElements .attr('transform', (d: any) => { - const midX = (d.source.x + d.target.x) / 2; - const midY = (d.source.y + d.target.y) / 2; - const angle = Math.atan2(d.target.y - d.source.y, d.target.x - d.source.x) * 180 / Math.PI; - return `translate(${midX},${midY}) rotate(${angle})`; + const ep = d._ep; + const angle = Math.atan2(ep.y2 - ep.y1, ep.x2 - ep.x1) * 180 / Math.PI; + return `translate(${ep.x2},${ep.y2}) rotate(${angle})`; }); // Edge labels: auto-centered in the clear span between the two node diff --git a/packages/web/src/lib/__tests__/edgeGeometry.test.ts b/packages/web/src/lib/__tests__/edgeGeometry.test.ts new file mode 100644 index 0000000..d0cda9c --- /dev/null +++ b/packages/web/src/lib/__tests__/edgeGeometry.test.ts @@ -0,0 +1,75 @@ +import { describe, it, expect } from 'vitest'; +import { rectBorderPoint, edgeBorderEndpoints, halfDiagonal, minEdgeLength } from '../edgeGeometry'; + +describe('rectBorderPoint', () => { + const dims = { width: 100, height: 60 }; // hw=50, hh=30 + + it('hits the vertical border for a horizontal ray', () => { + expect(rectBorderPoint({ x: 0, y: 0 }, dims, { x: 100, y: 0 })).toEqual({ x: 50, y: 0 }); + expect(rectBorderPoint({ x: 0, y: 0 }, dims, { x: -100, y: 0 })).toEqual({ x: -50, y: 0 }); + }); + + it('hits the horizontal border for a vertical ray', () => { + expect(rectBorderPoint({ x: 0, y: 0 }, dims, { x: 0, y: 100 })).toEqual({ x: 0, y: 30 }); + expect(rectBorderPoint({ x: 0, y: 0 }, dims, { x: 0, y: -100 })).toEqual({ x: 0, y: -30 }); + }); + + it('hits the nearer border on a diagonal', () => { + // toward (100,100): sx=50/100=0.5, sy=30/100=0.3 -> s=0.3 -> (30,30) + expect(rectBorderPoint({ x: 0, y: 0 }, dims, { x: 100, y: 100 })).toEqual({ x: 30, y: 30 }); + }); + + it('respects the center offset', () => { + expect(rectBorderPoint({ x: 200, y: 100 }, dims, { x: 400, y: 100 })).toEqual({ x: 250, y: 100 }); + }); + + it('returns the center when toward equals center', () => { + expect(rectBorderPoint({ x: 5, y: 5 }, dims, { x: 5, y: 5 })).toEqual({ x: 5, y: 5 }); + }); + + it('always lands on the border (distance from center = exactly one half-extent)', () => { + for (const angle of [0.1, 0.7, 1.2, 2.5, -1.9, 3.0]) { + const p = rectBorderPoint({ x: 0, y: 0 }, dims, { x: Math.cos(angle) * 1000, y: Math.sin(angle) * 1000 }); + const onVertical = Math.abs(Math.abs(p.x) - 50) < 1e-9; + const onHorizontal = Math.abs(Math.abs(p.y) - 30) < 1e-9; + expect(onVertical || onHorizontal, `point ${JSON.stringify(p)} should be on a border`).toBe(true); + // and within the box on the other axis + expect(Math.abs(p.x)).toBeLessThanOrEqual(50 + 1e-9); + expect(Math.abs(p.y)).toBeLessThanOrEqual(30 + 1e-9); + } + }); +}); + +describe('edgeBorderEndpoints', () => { + it('connects the facing borders of two horizontally-separated cards', () => { + const e = edgeBorderEndpoints({ x: 0, y: 0 }, { width: 100, height: 60 }, { x: 300, y: 0 }, { width: 100, height: 60 }); + expect(e).toEqual({ x1: 50, y1: 0, x2: 250, y2: 0 }); + }); + + it('the drawn segment is shorter than center-to-center (it starts at borders)', () => { + const s = { x: 0, y: 0 }, t = { x: 300, y: 0 }; + const e = edgeBorderEndpoints(s, { width: 100, height: 60 }, t, { width: 100, height: 60 }); + const drawn = Math.hypot(e.x2 - e.x1, e.y2 - e.y1); + const center = Math.hypot(t.x - s.x, t.y - s.y); + expect(drawn).toBeLessThan(center); + expect(drawn).toBe(200); // 300 - 50 - 50 + }); +}); + +describe('minEdgeLength', () => { + it('is zero when there is no label', () => { + expect(minEdgeLength({ width: 170, height: 105 }, { width: 170, height: 105 }, 0)).toBe(0); + }); + + it('guarantees the label fits in the border gap at any angle', () => { + const a = { width: 170, height: 105 }; + const b = { width: 160, height: 100 }; + const labelW = 104; + const min = minEdgeLength(a, b, labelW, 16); + expect(min).toBeCloseTo(halfDiagonal(a) + halfDiagonal(b) + 104 + 16, 6); + // At the worst angle (toward a corner) the projections sum to the two half + // diagonals; the remaining gap must still cover the label. + const gap = min - halfDiagonal(a) - halfDiagonal(b); + expect(gap).toBeGreaterThanOrEqual(labelW); + }); +}); diff --git a/packages/web/src/lib/edgeGeometry.ts b/packages/web/src/lib/edgeGeometry.ts new file mode 100644 index 0000000..6fe4ada --- /dev/null +++ b/packages/web/src/lib/edgeGeometry.ts @@ -0,0 +1,77 @@ +/** + * Edge geometry: where an edge meets a node, and how far apart connected nodes + * must sit for their label to fit. Pure functions, no D3 — unit-testable and + * shared by the renderer (border attachment) and the force sim (min length). + * + * Model: a node card is an axis-aligned box centered at (x,y). An edge is the + * straight line between two node centers; it should *draw* from border to + * border (the point where that line crosses each card), and the two nodes + * should never sit so close that the edge's label can't fit in the gap. + */ + +export interface Pt { x: number; y: number; } +export interface Dims { width: number; height: number; } + +/** + * The point on a node card's border along the ray from its center toward + * `toward`. As the two nodes move around each other this point slides around + * the border, always giving the shortest border-to-border connection. + * + * If the cards overlap (the other center is inside this card) the scaled point + * lands outside the segment; we clamp to the border so the result is always on + * the card edge, never past `toward`. + */ +export function rectBorderPoint(center: Pt, dims: Dims, toward: Pt): Pt { + const dx = toward.x - center.x; + const dy = toward.y - center.y; + if (dx === 0 && dy === 0) return { x: center.x, y: center.y }; + const hw = dims.width / 2; + const hh = dims.height / 2; + const sx = dx !== 0 ? hw / Math.abs(dx) : Infinity; + const sy = dy !== 0 ? hh / Math.abs(dy) : Infinity; + // Smaller scale = the first border (vertical vs horizontal) the ray hits. + const s = Math.min(sx, sy); + return { x: center.x + dx * s, y: center.y + dy * s }; +} + +export interface EdgeEndpoints { x1: number; y1: number; x2: number; y2: number; } + +/** + * Border-to-border endpoints for an edge: from the source card's border (facing + * the target) to the target card's border (facing the source). + */ +export function edgeBorderEndpoints( + source: Pt, + sourceDims: Dims, + target: Pt, + targetDims: Dims +): EdgeEndpoints { + const p1 = rectBorderPoint(source, sourceDims, target); + const p2 = rectBorderPoint(target, targetDims, source); + return { x1: p1.x, y1: p1.y, x2: p2.x, y2: p2.y }; +} + +/** Half the box diagonal — the largest distance from center to border (a + * corner), i.e. the worst-case projection of the card onto any edge angle. */ +export function halfDiagonal(dims: Dims): number { + return Math.hypot(dims.width, dims.height) / 2; +} + +/** + * Minimum CENTER-to-CENTER distance so the edge label always fits in the + * border-to-border gap, at ANY angle. The visible gap = centerLen − proj(src) − + * proj(tgt); projection peaks at the half-diagonal (edge toward a corner), so + * requiring centerLen ≥ halfDiag(src) + halfDiag(tgt) + labelWidth + pad + * guarantees gap ≥ labelWidth regardless of how the nodes are oriented. + * + * Returns 0 for a zero-width label (no constraint). + */ +export function minEdgeLength( + sourceDims: Dims, + targetDims: Dims, + labelWidth: number, + pad = 16 +): number { + if (!(labelWidth > 0)) return 0; + return halfDiagonal(sourceDims) + halfDiagonal(targetDims) + labelWidth + pad; +} diff --git a/tests/diagnostics/graph-geometry.spec.ts b/tests/diagnostics/graph-geometry.spec.ts index 14e6562..5ed44bd 100644 --- a/tests/diagnostics/graph-geometry.spec.ts +++ b/tests/diagnostics/graph-geometry.spec.ts @@ -226,5 +226,43 @@ test.describe('graph geometry diagnostic @geometry', () => { await gql(page, `mutation($id:ID!){deleteEdges(where:{source:{graph:{id:$id}}}){nodesDeleted}}`, { id: graphId }); await gql(page, `mutation($id:ID!){deleteWorkItems(where:{graph:{id:$id}}){nodesDeleted}}`, { id: graphId }); await gql(page, `mutation($id:ID!){deleteGraphs(where:{id:$id}){nodesDeleted}}`, { id: graphId }); + + // ── Scenario 2: an UNPINNED cluster the sim lays out (positionX/Y=0 => + // unplaced). This exercises the physics floor: every auto-laid edge should + // settle long enough for its label (clearSpan >= labelW). Pinned scenario + // above can't test this (user-placed nodes aren't moved by the sim). + const g2 = await gql(page, `mutation($i:[GraphCreateInput!]!){createGraphs(input:$i){graphs{id}}}`, + { i: [{ name: `${TEST_GRAPH_PREFIX} GeometryFlow ${Date.now()}`, type: 'PROJECT', status: 'ACTIVE', createdBy: userId, isShared: true }] }); + const flowId = g2.data.createGraphs.graphs[0].id; + const flowNodes = ['hub', 's1', 's2', 's3', 's4', 's5']; + const c2 = await gql(page, `mutation($i:[WorkItemCreateInput!]!){createWorkItems(input:$i){workItems{id title}}}`, + { i: flowNodes.map((t) => ({ type: 'TASK', title: t, status: 'IN_PROGRESS', priority: 0.5, positionX: 0, positionY: 0, positionZ: 0, owner: { connect: { where: { node: { id: userId } } } }, graph: { connect: { where: { node: { id: flowId } } } } })) }); + const fids: Record = {}; + for (const w of c2.data.createWorkItems.workItems) fids[w.title] = w.id; + const fEdge = (a: string, b: string, type: string) => ({ type, weight: 0.6, source: { connect: { where: { node: { id: fids[a] } } } }, target: { connect: { where: { node: { id: fids[b] } } } } }); + await gql(page, `mutation($i:[EdgeCreateInput!]!){createEdges(input:$i){edges{id}}}`, + { i: ['s1', 's2', 's3', 's4', 's5'].map((s, idx) => fEdge('hub', s, ['DEPENDS_ON', 'IS_PART_OF', 'RELATES_TO', 'BLOCKS', 'DEPENDS_ON'][idx])) }); + + await page.evaluate((gid) => { localStorage.setItem('currentGraphId', gid); localStorage.setItem('graphdone.quality.override', 'HIGH'); }, flowId); + await page.reload(); + await page.waitForTimeout(9000); // unplaced nodes flow + settle + await page.evaluate(() => (window as any).miniMapNavigate?.(0, 0)); + await page.waitForTimeout(800); + await page.screenshot({ path: path.join(OUT, 'flow-cluster.png') }); + const flow = await readGeometry(page); + const flowSummary = { + edgeCount: flow.edges.length, + labelsOverflowing: flow.edges.filter((e) => e.labelOverflowPx > 0).length, + labelsOverlappingCards: flow.edges.filter((e) => e.labelOverlapsCard).length, + minClearSpan: Math.min(...flow.edges.map((e) => e.clearSpanPx)), + maxLabelW: Math.max(...flow.edges.map((e) => e.labelW)), + }; + fs.writeFileSync(path.join(OUT, 'report-flow.json'), JSON.stringify({ summary: flowSummary, edges: flow.edges }, null, 2)); + // eslint-disable-next-line no-console + console.log('[geometry:flow] ' + JSON.stringify(flowSummary)); + + await gql(page, `mutation($id:ID!){deleteEdges(where:{source:{graph:{id:$id}}}){nodesDeleted}}`, { id: flowId }); + await gql(page, `mutation($id:ID!){deleteWorkItems(where:{graph:{id:$id}}){nodesDeleted}}`, { id: flowId }); + await gql(page, `mutation($id:ID!){deleteGraphs(where:{id:$id}){nodesDeleted}}`, { id: flowId }); }); });