diff --git a/plugins/visual-explainer/references/diagrams-svg.md b/plugins/visual-explainer/references/diagrams-svg.md index 0def1ef..1b33dc8 100644 --- a/plugins/visual-explainer/references/diagrams-svg.md +++ b/plugins/visual-explainer/references/diagrams-svg.md @@ -247,6 +247,23 @@ Compute endpoints from the node's own geometry, not by eyeballing coordinates: - **Diamond node centered at `(cx, cy)` with half-width `hw`, half-height `hh`** → right tip is `(cx + hw + gap, cy)`; bottom tip is `(cx, cy + hh + gap)`. - **Ellipse/pill** → project from center along the connector angle to the ellipse boundary, then add `gap`. +Tag the relationship so browser verification checks the rendered geometry, including +CSS/SVG coordinate transforms: + +```html + +... +``` + +Use `left-center`, `right-center`, `top-center`, or `bottom-center` only when the +connector is meant to hit the midpoint of that edge. Omit the anchor attribute for +intentional off-center entry points; the air-gap and inside-node checks still run. + If the arrow must route around a sibling node, add an orthogonal elbow (one or two right-angle segments) rather than letting a diagonal segment graze other nodes. An arrow that originates or terminates **visually inside** any node — its own or a neighbor — is a routing bug, not a styling choice. ### Arrow label masking diff --git a/plugins/visual-explainer/scripts/verify/checks.json b/plugins/visual-explainer/scripts/verify/checks.json index f811c1f..cd17f31 100644 --- a/plugins/visual-explainer/scripts/verify/checks.json +++ b/plugins/visual-explainer/scripts/verify/checks.json @@ -1226,7 +1226,7 @@ { "family": "F3_diagrams", "id": "diagram-arrow-endpoint-air-gap", - "title": "Arrow tips stop 6-10px from node borders, never inside", + "title": "Arrow tips align to declared node anchors with a 6-10px air gap", "profiles": [ "page", "slides", @@ -1237,9 +1237,9 @@ "stage": "browser", "severity": "error", "applies_when": "Arrows are tagged/detectable and connect to nodes with resolvable bounding boxes.", - "spec": "Render at 1440x900 (both schemes). For each arrow terminal point and its connected node getBBox(), compute perpendicular distance from the endpoint to the nearest node edge and assert 6-10px; assert the endpoint fails a point-in-rect test against its own node bbox and every neighboring node bbox.", - "fp_guards": "Requires role tagging to pair arrows with nodes.", - "fixture_violation": "An arrow whose arrowhead point lands 0px inside (piercing) the target node's border.", + "spec": "Render at 1440x900 (both schemes). Resolve terminal points for line, path, polyline, and polygon connectors into viewport coordinates with getScreenCTM(). Pair data-diagram-target with the node's data-diagram-id when supplied; otherwise use the nearest tagged node. Assert a 6-10px endpoint air gap and no point-in-rect intersection. When data-diagram-target-anchor is left-center, right-center, top-center, or bottom-center, also assert the endpoint lands on that side and aligns to the edge midpoint within 3px.", + "fp_guards": "Requires role tagging to pair arrows with nodes. Center-anchor alignment runs only when the connector declares data-diagram-target-anchor.", + "fixture_violation": "A polyline tagged for left-center whose arrowhead is 18px below the target node's midpoint, even though it happens to sit near another edge.", "source_rules": [ "arrow-endpoint-air-gap" ] diff --git a/plugins/visual-explainer/scripts/verify/lib/browser.mjs b/plugins/visual-explainer/scripts/verify/lib/browser.mjs index 5b3bd34..0366023 100644 --- a/plugins/visual-explainer/scripts/verify/lib/browser.mjs +++ b/plugins/visual-explainer/scripts/verify/lib/browser.mjs @@ -830,29 +830,80 @@ function diagramArrowMetrics(doc, compact) { const nodes = [...doc.querySelectorAll('[data-diagram-role~="node"], [data-diagram-role~="step"], [data-diagram-role~="decision"]')]; if (!arrows.length || !nodes.length) return { skipped: true, reason: 'No tagged arrows/nodes' }; const offenders = []; - const nodeBoxes = nodes.map((node) => ({ node, box: node.getBBox ? node.getBBox() : node.getBoundingClientRect() })); + const nodeBoxes = nodes.map((node) => ({ node, box: node.getBoundingClientRect() })); + const nodesById = new Map( + nodeBoxes + .map((item) => [item.node.getAttribute('data-diagram-id'), item]) + .filter(([id]) => id) + ); for (const arrow of arrows) { - let point = null; - if (arrow.tagName.toLowerCase() === 'line') { - point = { x: Number(arrow.getAttribute('x2')), y: Number(arrow.getAttribute('y2')) }; - } else if (arrow.tagName.toLowerCase() === 'path') { - const len = arrow.getTotalLength?.() || 0; - point = len ? arrow.getPointAtLength(len) : null; - } + const point = arrowTerminalPoint(arrow); if (!point) continue; - const nearest = nodeBoxes.map(({ node, box }) => ({ node, box, d: distanceToRectEdge(point, box), inside: point.x > box.x && point.x < box.x + box.width && point.y > box.y && point.y < box.y + box.height })).sort((a, b) => a.d - b.d)[0]; - if (nearest && (nearest.inside || nearest.d < 6 || nearest.d > 10)) { - offenders.push({ arrow: compact(arrow), node: compact(nearest.node), distance: Math.round(nearest.d * 10) / 10, inside: nearest.inside }); + const targetId = arrow.getAttribute('data-diagram-target'); + const target = targetId + ? nodesById.get(targetId) + : nodeBoxes + .map((item) => ({ ...item, d: distanceToRectEdge(point, item.box) })) + .sort((a, b) => a.d - b.d)[0]; + if (!target) { + offenders.push({ arrow: compact(arrow), targetId, reason: 'Target node not found' }); + continue; + } + const d = distanceToRectEdge(point, target.box); + const inside = point.x > target.box.left && point.x < target.box.right && point.y > target.box.top && point.y < target.box.bottom; + const anchor = arrow.getAttribute('data-diagram-target-anchor'); + const anchorResult = anchor ? diagramAnchorAlignment(point, target.box, anchor) : { alignment: 0, wrongSide: false }; + if (inside || d < 6 || d > 10 || anchorResult.alignment > 3 || anchorResult.wrongSide) { + offenders.push({ + arrow: compact(arrow), + node: compact(target.node), + targetId, + anchor, + distance: Math.round(d * 10) / 10, + alignment: Math.round(anchorResult.alignment * 10) / 10, + wrongSide: anchorResult.wrongSide, + inside, + }); } } return { offenders: offenders.slice(0, 10) }; } +function arrowTerminalPoint(arrow) { + let point = null; + const tag = arrow.tagName.toLowerCase(); + if (tag === 'line') { + point = { x: Number(arrow.getAttribute('x2')), y: Number(arrow.getAttribute('y2')) }; + } else if (typeof arrow.getTotalLength === 'function' && typeof arrow.getPointAtLength === 'function') { + const len = arrow.getTotalLength(); + point = Number.isFinite(len) && len >= 0 ? arrow.getPointAtLength(len) : null; + } else if ((tag === 'polyline' || tag === 'polygon') && arrow.points?.numberOfItems) { + point = arrow.points.getItem(arrow.points.numberOfItems - 1); + } + if (!point || !Number.isFinite(point.x) || !Number.isFinite(point.y)) return null; + const matrix = arrow.getScreenCTM?.(); + if (!matrix) return point; + return { + x: point.x * matrix.a + point.y * matrix.c + matrix.e, + y: point.x * matrix.b + point.y * matrix.d + matrix.f, + }; +} + +function diagramAnchorAlignment(point, box, anchor) { + const centerX = (box.left + box.right) / 2; + const centerY = (box.top + box.bottom) / 2; + if (anchor === 'left-center') return { alignment: Math.abs(point.y - centerY), wrongSide: point.x >= box.left }; + if (anchor === 'right-center') return { alignment: Math.abs(point.y - centerY), wrongSide: point.x <= box.right }; + if (anchor === 'top-center') return { alignment: Math.abs(point.x - centerX), wrongSide: point.y >= box.top }; + if (anchor === 'bottom-center') return { alignment: Math.abs(point.x - centerX), wrongSide: point.y <= box.bottom }; + return { alignment: 0, wrongSide: false }; +} + function distanceToRectEdge(point, box) { - const left = Math.abs(point.x - box.x); - const right = Math.abs(point.x - (box.x + box.width)); - const top = Math.abs(point.y - box.y); - const bottom = Math.abs(point.y - (box.y + box.height)); + const left = Math.abs(point.x - box.left); + const right = Math.abs(point.x - box.right); + const top = Math.abs(point.y - box.top); + const bottom = Math.abs(point.y - box.bottom); return Math.min(left, right, top, bottom); } @@ -1294,6 +1345,8 @@ const BROWSER_METRIC_SOURCE = [ proseMetrics, fixedOverlapMetrics, diagramArrowMetrics, + arrowTerminalPoint, + diagramAnchorAlignment, distanceToRectEdge, diagramTextMetrics, rectsIntersect, diff --git a/plugins/visual-explainer/scripts/verify/lib/checks/browser.mjs b/plugins/visual-explainer/scripts/verify/lib/checks/browser.mjs index 7dcadf1..46fddd2 100644 --- a/plugins/visual-explainer/scripts/verify/lib/checks/browser.mjs +++ b/plugins/visual-explainer/scripts/verify/lib/checks/browser.mjs @@ -107,10 +107,11 @@ export const checks = { 'diagram-arrow-endpoint-air-gap': metricCheck('diagram-arrow-endpoint-air-gap', { profiles: ALL_RENDERED, - appliesWhen: (ctx) => /data-diagram-role=["'][^"']*(?:arrow|connector)[^"']*["']/i.test(ctx.html || ''), + runFilter: desktopRun, + appliesWhen: (ctx) => /data-diagram-role.{0,160}(?:arrow|connector)/i.test(ctx.html || ''), failWhen: (metric) => metric?.offenders?.length, evidence: (failures) => `Arrow endpoints outside 6-10px air-gap: ${summarizeOffenders(failures, 'offenders')}`, - fix_hint: 'Tagged arrow endpoints must stop 6-10px from node borders and never land inside a node bounding box.' + fix_hint: 'Tagged arrow endpoints must stop 6-10px from node borders; declared side-center anchors must hit that side and its midpoint within 3px.' }), 'diagram-text-clipping-overlap': metricCheck('diagram-text-clipping-overlap', { diff --git a/plugins/visual-explainer/scripts/verify/protocol-requirements.json b/plugins/visual-explainer/scripts/verify/protocol-requirements.json index 36dedc1..965a9fe 100644 --- a/plugins/visual-explainer/scripts/verify/protocol-requirements.json +++ b/plugins/visual-explainer/scripts/verify/protocol-requirements.json @@ -53,7 +53,7 @@ }, { "family": "F3_diagrams", - "req": "Diagram element tagging: generation templates must emit data-diagram-role on SVG elements (node/arrow/arrow-label/legend/lifeline/step/decision/merge/entity/lane/layer/ring/item) so downstream counting and classification checks can run deterministically; use the attribute, never a class named 'node' (Mermaid .node collision)." + "req": "Diagram element tagging: generation templates must emit data-diagram-role on SVG elements (node/arrow/arrow-label/legend/lifeline/step/decision/merge/entity/lane/layer/ring/item) so downstream counting and classification checks can run deterministically; use the attribute, never a class named 'node' (Mermaid .node collision). Connectors with a known target must also emit data-diagram-target matching the target node's data-diagram-id; side-center endpoints declare data-diagram-target-anchor as left-center, right-center, top-center, or bottom-center so browser verification can detect drift." }, { "family": "F3_diagrams", @@ -235,4 +235,4 @@ "family": "F7_process", "req": "SECTION RETRY LIMIT: If a sub-agent returns a fragment that violates the section-contract rejection conditions, re-dispatch it once with a corrected brief. Do not re-dispatch the same section more than once; fall back to building it yourself if the retry also fails." } -] \ No newline at end of file +]