fix(check): elongated pivot drift + counterfactual connector_detached (#2819)

This commit is contained in:
Xuanru Li
2026-07-26 21:43:42 -07:00
committed by GitHub
parent 45b458c007
commit 75ed99e1d4
4 changed files with 531 additions and 121 deletions
@@ -1198,6 +1198,7 @@
return issues;
}
// Soft prior only — the counterfactual attach test (below) is what makes detachment a finding.
const CONNECTOR_NAME = /\b(conn(ector)?|arrow|edge|link|flow|wire)\b/i;
const CONNECTOR_SKIP_CONTAINERS = "defs, marker, clipPath, mask, symbol, pattern";
@@ -1207,14 +1208,16 @@
return `${element.id || ""} ${className}`;
}
// Screen-space endpoints via the browser: getScreenCTM covers viewBox, preserveAspectRatio and group transforms.
function pathScreenEndpoints(svg, path) {
if (
typeof path.getTotalLength !== "function" ||
typeof path.getPointAtLength !== "function" ||
typeof path.getScreenCTM !== "function" ||
typeof svg.createSVGPoint !== "function"
) {
function isConnectorPath(svg, path) {
if (path.hasAttribute("marker-start") || path.hasAttribute("marker-end")) return true;
return (
CONNECTOR_NAME.test(connectorNameFor(svg)) || CONNECTOR_NAME.test(connectorNameFor(path))
);
}
/** Raw `d`-space endpoints (no CTM) — the mapping authors use when they paste screen coords into `d`. */
function pathUserEndpoints(path) {
if (typeof path.getTotalLength !== "function" || typeof path.getPointAtLength !== "function") {
return null;
}
let total;
@@ -1224,6 +1227,20 @@
return null;
}
if (!Number.isFinite(total) || total <= 0) return null;
const start = path.getPointAtLength(0);
const end = path.getPointAtLength(total);
return { start: { x: start.x, y: start.y }, end: { x: end.x, y: end.y } };
}
// Screen endpoints via getScreenCTM (viewBox, preserveAspectRatio, group transforms).
function pathScreenEndpoints(svg, path, user) {
if (
!user ||
typeof path.getScreenCTM !== "function" ||
typeof svg.createSVGPoint !== "function"
) {
return null;
}
const matrix = path.getScreenCTM();
if (!matrix) return null;
const toScreen = (local) => {
@@ -1233,10 +1250,7 @@
const mapped = point.matrixTransform(matrix);
return { x: mapped.x, y: mapped.y };
};
return {
start: toScreen(path.getPointAtLength(0)),
end: toScreen(path.getPointAtLength(total)),
};
return { start: toScreen(user.start), end: toScreen(user.end) };
}
function distanceToRect(point, rect) {
@@ -1246,6 +1260,7 @@
}
// Solid, compact elements a connector could plausibly anchor to.
// Both tiers keep `element` so attachment identity is stable across containment vs near-miss.
function connectorAnchorRects(root, rootRect) {
const compact = [];
const painted = [];
@@ -1261,42 +1276,57 @@
if (area < 400) continue;
// Containment tier: large opaque targets only — a text-bearing wrapper contains its own diagram's endpoints.
if (opaque && area <= rootArea * 0.6) painted.push({ rect, element });
if (area <= rootArea * 0.15) compact.push(rect);
if (area <= rootArea * 0.15) compact.push({ rect, element });
}
return { compact, painted };
}
function isConnectorPath(svg, path) {
if (path.hasAttribute("marker-start") || path.hasAttribute("marker-end")) return true;
return (
CONNECTOR_NAME.test(connectorNameFor(svg)) || CONNECTOR_NAME.test(connectorNameFor(path))
);
}
// A connector whose BOTH endpoints land far from every anchorable element was drawn in the wrong frame.
// min over the two endpoints is intentional: a half-attached connector is a design choice, not frame drift.
// Flag only the documented bug: rendered endpoints miss, but user-space-as-screen would attach.
function connectorDetachmentIssues(root, rootRect, time) {
const issues = [];
let anchors = null;
// Attach near-miss tolerance (screen px). Separate from the closed-glyph chord floor.
const threshold = Math.max(32, Math.min(rootRect.width, rootRect.height) * 0.02);
const MIN_CONNECTOR_CHORD_PX = 8;
for (const svg of Array.from(root.querySelectorAll("svg"))) {
if (!isVisibleElement(svg) || hasAllowOverflowFlag(svg)) continue;
for (const path of Array.from(svg.querySelectorAll("path"))) {
if (path.closest(CONNECTOR_SKIP_CONTAINERS)) continue;
if (!isConnectorPath(svg, path)) continue;
const endpoints = pathScreenEndpoints(svg, path);
if (!endpoints) continue;
const user = pathUserEndpoints(path);
const rendered = pathScreenEndpoints(svg, path, user);
if (!user || !rendered) continue;
// Closed/glyph paths collapse to one point — compare in screen px (not user units).
const renderedChord = Math.hypot(
rendered.end.x - rendered.start.x,
rendered.end.y - rendered.start.y,
);
if (renderedChord < MIN_CONNECTOR_CHORD_PX) continue;
if (anchors === null) anchors = connectorAnchorRects(root, rootRect);
if (anchors.compact.length < 2) return issues;
const attached = (point) =>
anchors.painted.some(
(anchor) => !anchor.element.contains(svg) && distanceToRect(point, anchor.rect) === 0,
) || anchors.compact.some((rect) => distanceToRect(point, rect) <= threshold);
if (attached(endpoints.start) || attached(endpoints.end)) continue;
// Stable DOM identity across painted (inside) and compact (near-miss) tiers.
const attachmentKey = (point) => {
for (const anchor of anchors.painted) {
if (!anchor.element.contains(svg) && distanceToRect(point, anchor.rect) === 0) {
return anchor.element;
}
}
for (const anchor of anchors.compact) {
if (distanceToRect(point, anchor.rect) <= threshold) return anchor.element;
}
return null;
};
const attached = (point) => attachmentKey(point) !== null;
// Half-attached as drawn is allowed; only full render-miss proceeds.
if (attached(rendered.start) || attached(rendered.end)) continue;
// Paste-into-`d` bug: both raw endpoints land on distinct anchors as screen pixels.
const userStartKey = attachmentKey(user.start);
const userEndKey = attachmentKey(user.end);
if (!userStartKey || !userEndKey || userStartKey === userEndKey) continue;
const gap = Math.round(
Math.min(
Math.min(...anchors.compact.map((rect) => distanceToRect(endpoints.start, rect))),
Math.min(...anchors.compact.map((rect) => distanceToRect(endpoints.end, rect))),
Math.min(...anchors.compact.map((a) => distanceToRect(rendered.start, a.rect))),
Math.min(...anchors.compact.map((a) => distanceToRect(rendered.end, a.rect))),
),
);
issues.push({
@@ -1305,17 +1335,17 @@
time,
selector: selectorFor(path),
containerSelector: selectorFor(svg),
message: `Connector path endpoints are ${gap}px from the nearest anchorable element — measured coordinates were likely drawn into an SVG with a different origin.`,
message: `Connector path endpoints render ${gap}px from the nearest anchorable element, but the path's user-space coordinates would attach if read as screen pixels — screen/viewport numbers were likely written into SVG \`d\` without inverting the CTM.`,
rect: toRect({
left: Math.min(endpoints.start.x, endpoints.end.x),
top: Math.min(endpoints.start.y, endpoints.end.y),
right: Math.max(endpoints.start.x, endpoints.end.x),
bottom: Math.max(endpoints.start.y, endpoints.end.y),
width: Math.abs(endpoints.end.x - endpoints.start.x),
height: Math.abs(endpoints.end.y - endpoints.start.y),
left: Math.min(rendered.start.x, rendered.end.x),
top: Math.min(rendered.start.y, rendered.end.y),
right: Math.max(rendered.start.x, rendered.end.x),
bottom: Math.max(rendered.start.y, rendered.end.y),
width: Math.abs(rendered.end.x - rendered.start.x),
height: Math.abs(rendered.end.y - rendered.start.y),
}),
fixHint:
"Subtract the SVG's own rect when converting measured coordinates, and keep the SVG a direct child of the stage.",
"Convert measured screen coordinates into the SVG's user space (subtract the SVG rect / invert getScreenCTM) before writing path `d`, and keep the SVG a direct child of the stage.",
});
}
}
@@ -847,8 +847,8 @@ describe("layout-audit.browser coordinate-frame findings", () => {
// The marker tip path is skipped outright; only the detached line reports.
expect(issues).toHaveLength(1);
expect(issues[0]).toMatchObject({ severity: "warning", selector: "#detached" });
expect(issues[0]?.message).toContain("drawn into an SVG with a different origin");
expect(issues[0]?.fixHint).toContain("Subtract the SVG's own rect");
expect(issues[0]?.message).toContain("user-space coordinates would attach");
expect(issues[0]?.fixHint).toContain("invert getScreenCTM");
});
it("skips svgs and paths without connector intent", () => {
@@ -877,6 +877,254 @@ describe("layout-audit.browser coordinate-frame findings", () => {
// "knowledge-overflow" contains conn-family substrings only across word boundaries — no match.
expect(runAudit().filter((issue) => issue.code === "connector_detached")).toEqual([]);
});
// Counterfactual: decorative paths miss anchors both as rendered and as user-as-screen → not the frame bug.
it("skips decorative arrow/flow paths whose user-space coords would not attach either", () => {
document.body.innerHTML = `
<div id="root" data-composition-id="main" data-width="1920" data-height="1080">
<div id="n1"></div>
<div id="n2"></div>
<svg id="arrow-l" class="arrow">
<path id="arrow-glyph" d="M70 20 L10 20" marker-end="url(#tip)" />
</svg>
<svg id="decor"><path id="flow-line" class="flow-line" d="M-100 200 L2020 880" /></svg>
</div>
`;
installGeometry(
{
root: rect({ left: 0, top: 0, width: 1920, height: 1080 }),
n1: rect({ left: 900, top: 500, width: 160, height: 160 }),
n2: rect({ left: 300, top: 200, width: 160, height: 160 }),
"arrow-l": rect({ left: 100, top: 500, width: 80, height: 40 }),
decor: rect({ left: 0, top: 0, width: 1920, height: 1080 }),
},
{
n1: { backgroundColor: "rgb(30, 40, 50)" },
n2: { backgroundColor: "rgb(30, 40, 50)" },
},
);
installConnectorGeometry({ e: 100, f: 500 });
// Full-bleed decor SVG uses identity translate so user-as-screen == rendered (still off-canvas).
for (const path of Array.from(document.querySelectorAll("#decor path"))) {
Object.defineProperty(path, "getScreenCTM", {
value: () => ({ a: 1, b: 0, c: 0, d: 1, e: 0, f: 0 }),
});
Object.defineProperty(path, "getTotalLength", { value: () => 100 });
Object.defineProperty(path, "getPointAtLength", {
value: (length: number) => (length === 0 ? { x: -100, y: 200 } : { x: 2020, y: 880 }),
});
}
const decorSvg = document.getElementById("decor");
if (decorSvg) {
Object.defineProperty(decorSvg, "createSVGPoint", {
value: () => ({
x: 0,
y: 0,
matrixTransform(m: { a: number; b: number; c: number; d: number; e: number; f: number }) {
return { x: this.x * m.a + this.y * m.c + m.e, y: this.x * m.b + this.y * m.d + m.f };
},
}),
});
}
installAuditScript();
expect(runAudit().filter((issue) => issue.code === "connector_detached")).toEqual([]);
});
// Same DOM node via painted-inside + compact-near-miss must share one identity (not p0 vs c0).
it("skips same-anchor cross-tier arrows that only graze one node", () => {
document.body.innerHTML = `
<div id="root" data-composition-id="main" data-width="1920" data-height="1080">
<div id="n1"></div>
<div id="n2"></div>
<svg id="arrow-svg" class="arrow">
<path id="cross-tier" d="M 980 580 L 1080 580" marker-end="url(#tip)" />
</svg>
</div>
`;
installGeometry(
{
root: rect({ left: 0, top: 0, width: 1920, height: 1080 }),
n1: rect({ left: 900, top: 500, width: 160, height: 160 }),
n2: rect({ left: 300, top: 200, width: 160, height: 160 }),
"arrow-svg": rect({ left: 80, top: 227, width: 1740, height: 830 }),
},
{
n1: { backgroundColor: "rgb(30, 40, 50)" },
n2: { backgroundColor: "rgb(30, 40, 50)" },
},
);
// Raw start inside #n1; raw end just outside #n1 but within attach tolerance — one element.
installConnectorGeometry({ e: 80, f: 227 });
installAuditScript();
expect(runAudit().filter((issue) => issue.code === "connector_detached")).toEqual([]);
});
// One raw endpoint on a node is not the paste-into-`d` bug (decorative arrow / partial aim).
it("skips one-ended decorative arrows when only one user endpoint attaches", () => {
document.body.innerHTML = `
<div id="root" data-composition-id="main" data-width="1920" data-height="1080">
<div id="n1"></div>
<div id="n2"></div>
<svg id="arrow-svg" class="arrow">
<path id="one-ended" d="M 980 580 L 200 100" marker-end="url(#tip)" />
</svg>
</div>
`;
installGeometry(
{
root: rect({ left: 0, top: 0, width: 1920, height: 1080 }),
n1: rect({ left: 900, top: 500, width: 160, height: 160 }),
n2: rect({ left: 300, top: 200, width: 160, height: 160 }),
"arrow-svg": rect({ left: 80, top: 227, width: 1740, height: 830 }),
},
{
n1: { backgroundColor: "rgb(30, 40, 50)" },
n2: { backgroundColor: "rgb(30, 40, 50)" },
},
);
// CTM offset moves both rendered ends off anchors; raw start sits in #n1, raw end in empty space.
installConnectorGeometry({ e: 80, f: 227 });
installAuditScript();
expect(runAudit().filter((issue) => issue.code === "connector_detached")).toEqual([]);
});
// Scaled viewBox: user chord can be <32 while screen chord is hundreds of px — must not skip.
it("flags foreign-frame connectors when user-space chord is short but screen chord is long", () => {
document.body.innerHTML = `
<div id="root" data-composition-id="main" data-width="1920" data-height="1080">
<div id="n1"></div>
<div id="n2"></div>
<svg id="scaled-svg" viewBox="0 0 192 108">
<path id="short-user" class="connector" d="M 100 58 L 140 58" />
</svg>
</div>
`;
installGeometry(
{
root: rect({ left: 0, top: 0, width: 1920, height: 1080 }),
// Non-overlapping anchors so both user endpoints hit distinct keys.
n1: rect({ left: 70, top: 40, width: 50, height: 40 }),
n2: rect({ left: 125, top: 40, width: 50, height: 40 }),
"scaled-svg": rect({ left: 0, top: 0, width: 1920, height: 1080 }),
},
{
n1: { backgroundColor: "rgb(30, 40, 50)" },
n2: { backgroundColor: "rgb(30, 40, 50)" },
},
);
// 10× viewBox scale: user chord 30 (< old 32px gate) → screen chord 300.
const path = document.getElementById("short-user");
const svg = document.getElementById("scaled-svg");
const matrix = { a: 10, b: 0, c: 0, d: 10, e: 0, f: 0 };
const prop = { configurable: true, writable: true };
if (path) {
Object.defineProperty(path, "getTotalLength", { ...prop, value: () => 30 });
Object.defineProperty(path, "getPointAtLength", {
...prop,
value: (length: number) => (length === 0 ? { x: 100, y: 58 } : { x: 140, y: 58 }),
});
Object.defineProperty(path, "getScreenCTM", { ...prop, value: () => matrix });
}
if (svg) {
Object.defineProperty(svg, "createSVGPoint", {
...prop,
value: () => ({
x: 0,
y: 0,
matrixTransform(m: typeof matrix) {
return { x: this.x * m.a + this.y * m.c + m.e, y: this.x * m.b + this.y * m.d + m.f };
},
}),
});
}
installAuditScript();
const issues = runAudit().filter((issue) => issue.code === "connector_detached");
expect(issues).toHaveLength(1);
expect(issues[0]).toMatchObject({ selector: "#short-user" });
});
// Closed glyph: rendered chord ~0 — not a two-ended frame bug even if the point sits on a node.
it("skips closed filled glyphs whose user-space endpoints collapse", () => {
document.body.innerHTML = `
<div id="root" data-composition-id="main" data-width="1920" data-height="1080">
<div id="n1"></div>
<div id="n2"></div>
<svg id="arrow-svg" class="arrow">
<path id="main-arrow" d="M10 10 L90 10 L50 90 Z" />
</svg>
</div>
`;
installGeometry(
{
root: rect({ left: 0, top: 0, width: 1920, height: 1080 }),
n1: rect({ left: 900, top: 500, width: 160, height: 160 }),
n2: rect({ left: 300, top: 200, width: 160, height: 160 }),
"arrow-svg": rect({ left: 0, top: 0, width: 1920, height: 1080 }),
},
{
n1: { backgroundColor: "rgb(30, 40, 50)" },
n2: { backgroundColor: "rgb(30, 40, 50)" },
},
);
// Closed path: start≈end in user space (and after CTM).
for (const path of Array.from(document.querySelectorAll("#main-arrow"))) {
Object.defineProperty(path, "getTotalLength", { value: () => 100 });
Object.defineProperty(path, "getPointAtLength", {
value: () => ({ x: 980, y: 580 }),
});
Object.defineProperty(path, "getScreenCTM", {
value: () => ({ a: 1, b: 0, c: 0, d: 1, e: 0, f: 0 }),
});
}
const svg = document.getElementById("arrow-svg");
if (svg) {
Object.defineProperty(svg, "createSVGPoint", {
value: () => ({
x: 0,
y: 0,
matrixTransform(m: { a: number; b: number; c: number; d: number; e: number; f: number }) {
return { x: this.x * m.a + this.y * m.c + m.e, y: this.x * m.b + this.y * m.d + m.f };
},
}),
});
}
installAuditScript();
expect(runAudit().filter((issue) => issue.code === "connector_detached")).toEqual([]);
});
// Correct inverse-CTM authoring: rendered attaches → never flag, even with an offset SVG.
it("skips connectors whose rendered endpoints already attach", () => {
document.body.innerHTML = `
<div id="root" data-composition-id="main" data-width="1920" data-height="1080">
<div id="n1"></div>
<div id="n2"></div>
<svg id="connector-svg">
<path id="anchored-only" class="connector-line" d="M 900 353 L 300 53" />
</svg>
</div>
`;
installGeometry(
{
root: rect({ left: 0, top: 0, width: 1920, height: 1080 }),
n1: rect({ left: 900, top: 500, width: 160, height: 160 }),
n2: rect({ left: 300, top: 200, width: 160, height: 160 }),
"connector-svg": rect({ left: 80, top: 227, width: 1740, height: 830 }),
},
{
n1: { backgroundColor: "rgb(30, 40, 50)" },
n2: { backgroundColor: "rgb(30, 40, 50)" },
},
);
installConnectorGeometry({ e: 80, f: 227 });
installAuditScript();
expect(runAudit().filter((issue) => issue.code === "connector_detached")).toEqual([]);
});
});
describe("layout-audit.browser content overlap", () => {
@@ -1954,10 +2202,12 @@ interface CtmTranslate {
}
// happy-dom has no SVG geometry APIs; endpoints come from the path's `d`, the CTM is a pure translate.
function installConnectorGeometry(translate: CtmTranslate): void {
function installConnectorGeometry(translate: CtmTranslate, root: ParentNode = document): void {
const matrix = { a: 1, b: 0, c: 0, d: 1, e: translate.e, f: translate.f };
for (const svg of Array.from(document.querySelectorAll("svg"))) {
const prop = { configurable: true, writable: true };
for (const svg of Array.from(root.querySelectorAll("svg"))) {
Object.defineProperty(svg, "createSVGPoint", {
...prop,
value: () => ({
x: 0,
y: 0,
@@ -1970,11 +2220,12 @@ function installConnectorGeometry(translate: CtmTranslate): void {
const numbers = (path.getAttribute("d")?.match(/-?\d*\.?\d+/g) || []).map(Number);
const start = { x: numbers[0] ?? 0, y: numbers[1] ?? 0 };
const end = { x: numbers[numbers.length - 2] ?? 0, y: numbers[numbers.length - 1] ?? 0 };
Object.defineProperty(path, "getTotalLength", { value: () => 100 });
Object.defineProperty(path, "getTotalLength", { ...prop, value: () => 100 });
Object.defineProperty(path, "getPointAtLength", {
...prop,
value: (length: number) => (length === 0 ? start : end),
});
Object.defineProperty(path, "getScreenCTM", { value: () => matrix });
Object.defineProperty(path, "getScreenCTM", { ...prop, value: () => matrix });
}
}
}