fix(cli): connector_detached now skips fully dash-hidden connector shafts (#3912)

* fix(cli): connector_detached now skips dash-hidden connector shafts

connector_detached fired on SVG connector paths that were 100% hidden
via stroke-dashoffset/stroke-dasharray draw-on entrances, evaluating
rendered/user-space geometry unconditionally. The sibling
connector_orphan check already gates on shaftDashHidden(path); add the
same gate here, in the same position.

Co-Authored-By: Miguel Angel <miguel.sierra@heygen.com>

* fix(cli): connector dash gate reads the whole stroke-dasharray; one owner for connector enumeration

`shaftDashHidden` read only the first `stroke-dasharray` entry, so any
pattern starting with 0 (`0 4` dotted) or a long first dash counted as
hidden and suppressed both connector findings on a visible stroke. It now
resolves the full computed list (comma/space separated, units,
percentages against the SVG viewport diagonal, odd lists repeated;
`none`, all-zero and negative lists render solid) and treats the shaft as
hidden only when the window `[dashoffset, dashoffset + length]` sits in a
single gap, with up to 10% of the length overlapping a neighbouring dash.
Zero-length dashes paint only under `stroke-linecap: round | square`;
under the default `butt` they render nothing, which also covers the
`0px, 999999px` state a finished draw-off tween leaves behind.

`connector_detached` and `connector_orphan` each walked SVG paths with
their own copy of the candidate/gate logic, which had already drifted.
A single `connectorShafts(root)` generator now owns enumeration, the
dash gate and endpoint resolution, and yields the `painted` verdict;
both findings consume it. The path length is queried once per shaft
(`pathUserEndpoints` returns it) instead of twice, and the attach
threshold formula has one owner.

Tests pin `0 4` (butt and round), `0, 4`, `4 0`, `0`, `none`, px units,
negative offsets, a draw-off end state, and both sides of the 10%
boundary.

Co-Authored-By: Miguel Ángel <miguel.sierra@heygen.com>

* refactor(cli): shorten the connector and dash-window comments

* test(cli): shorten the dash-gate table comment

---------

Co-authored-by: Miguel Angel <miguel.sierra@heygen.com>
This commit is contained in:
miga-heygen
2026-10-02 05:38:42 -07:00
committed by GitHub
co-authored by Miguel Ángel
parent 385e9cf337
commit efdabb758d
2 changed files with 233 additions and 144 deletions
+169 -130
View File
@@ -1436,7 +1436,7 @@
);
}
/** Raw `d`-space endpoints (no CTM) — the mapping authors use when they paste screen coords into `d`. */
// Raw `d`-space endpoints (no CTM), as authors paste screen coords into `d`, plus the path length.
function pathUserEndpoints(path) {
if (typeof path.getTotalLength !== "function" || typeof path.getPointAtLength !== "function") {
return null;
@@ -1450,7 +1450,7 @@
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 } };
return { start: { x: start.x, y: start.y }, end: { x: end.x, y: end.y }, total };
}
// Screen endpoints via getScreenCTM (viewBox, preserveAspectRatio, group transforms).
@@ -1502,86 +1502,98 @@
return { compact, painted };
}
// 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;
// Attach near-miss tolerance (screen px) for both connector findings; not the glyph chord floor.
function connectorAttachThreshold(rootRect) {
return Math.max(32, Math.min(rootRect.width, rootRect.height) * 0.02);
}
// The one connector enumeration for connector_detached and connector_orphan. Skips dash-hidden
// shafts; reports `painted` for callers to gate on, and `chord`, the rendered span in screen px.
function* connectorShafts(root) {
for (const svg of Array.from(root.querySelectorAll("svg"))) {
if (!isVisibleElement(svg)) continue;
for (const path of Array.from(svg.querySelectorAll("path"))) {
if (path.closest(CONNECTOR_SKIP_CONTAINERS)) continue;
if (!isConnectorPath(svg, path)) continue;
const user = pathUserEndpoints(path);
if (!user || shaftDashHidden(path, user.total)) continue;
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(
if (!rendered) continue;
const chord = 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;
// 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);
const pasteBug = Boolean(userStartKey && userEndKey && userStartKey !== userEndKey);
// Guessed marked shaft: both frames miss. Same-anchor grazes attach in user-space
// and must stay skipped. Name-only decorative flow/arrow paths stay skipped.
// 80px keeps short marker glyphs (chevrons, tips) out.
const markedMiss =
renderedChord >= 80 &&
!userStartKey &&
!userEndKey &&
(path.hasAttribute("marker-start") || path.hasAttribute("marker-end"));
if (!pasteBug && !markedMiss) continue;
const gap = Math.round(
Math.min(
Math.min(...anchors.compact.map((a) => distanceToRect(rendered.start, a.rect))),
Math.min(...anchors.compact.map((a) => distanceToRect(rendered.end, a.rect))),
),
);
issues.push({
code: "connector_detached",
severity: "warning",
time,
selector: selectorFor(path),
containerSelector: selectorFor(svg),
message: pasteBug
? `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.`
: `Connector path endpoints render ${gap}px from the nearest anchorable element — a marked shaft that meets no node.`,
rect: toRect({
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: pasteBug
? "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."
: "Measure the settled node boxes and write `d` in the SVG's user space (invert getScreenCTM), or grow a layout-owned shaft from the source node.",
});
yield { svg, path, user, rendered, chord, painted: shaftIsPainted(path) };
}
}
}
// 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;
const threshold = connectorAttachThreshold(rootRect);
const MIN_CONNECTOR_CHORD_PX = 8;
for (const { svg, path, user, rendered, chord } of connectorShafts(root)) {
if (chord < MIN_CONNECTOR_CHORD_PX) continue;
if (anchors === null) anchors = connectorAnchorRects(root, rootRect);
if (anchors.compact.length < 2) return issues;
// 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);
const pasteBug = Boolean(userStartKey && userEndKey && userStartKey !== userEndKey);
// Guessed marked shaft: both frames miss. Same-anchor grazes attach in user-space
// and must stay skipped. Name-only decorative flow/arrow paths stay skipped.
// 80px keeps short marker glyphs (chevrons, tips) out.
const markedMiss =
chord >= 80 &&
!userStartKey &&
!userEndKey &&
(path.hasAttribute("marker-start") || path.hasAttribute("marker-end"));
if (!pasteBug && !markedMiss) continue;
const gap = Math.round(
Math.min(
Math.min(...anchors.compact.map((a) => distanceToRect(rendered.start, a.rect))),
Math.min(...anchors.compact.map((a) => distanceToRect(rendered.end, a.rect))),
),
);
issues.push({
code: "connector_detached",
severity: "warning",
time,
selector: selectorFor(path),
containerSelector: selectorFor(svg),
message: pasteBug
? `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.`
: `Connector path endpoints render ${gap}px from the nearest anchorable element — a marked shaft that meets no node.`,
rect: toRect({
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: pasteBug
? "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."
: "Measure the settled node boxes and write `d` in the SVG's user space (invert getScreenCTM), or grow a layout-owned shaft from the source node.",
});
}
return issues;
}
@@ -1598,19 +1610,58 @@
return opacityChain(path) >= 0.2;
}
function shaftDashHidden(path) {
if (typeof path.getTotalLength !== "function") return false;
let total;
try {
total = path.getTotalLength();
} catch {
return false;
}
if (!Number.isFinite(total) || total <= 0) return false;
// True when the visible dash window [offset, offset + length] sits in one gap (an unstarted
// draw-on), allowing 10% overlap. Zero-length dashes paint only with a round or square linecap.
function shaftDashHidden(path, total) {
const style = getComputedStyle(path);
const offset = Number.parseFloat(style.strokeDashoffset || "0");
const dash = Number.parseFloat(String(style.strokeDasharray || "").split(/[\s,]+/)[0] || "0");
return offset >= total * 0.9 && dash >= total * 0.9;
const dashes = dashArrayLengths(style.strokeDasharray, path);
if (dashes === null) return false;
const dotsPaint = (style.strokeLinecap || "butt") !== "butt";
const dashPaints = (index) => index % 2 === 0 && (dashes[index] > 0 || dotsPaint);
if (!dashes.some((_, index) => dashPaints(index))) return true; // only butt-capped dots
const period = dashes.reduce((sum, length) => sum + length, 0);
if (total > period) return false; // a full period of dash paints — call it visible
const offset = dashLength(style.strokeDashoffset, path); // unparseable reads as 0
const start = Number.isFinite(offset) ? ((offset % period) + period) % period : 0;
const end = start + total;
let painted = 0;
let segmentStart = 0;
// Two periods cover any window that starts inside the first.
for (let i = 0; i < dashes.length * 2; i++) {
const segmentEnd = segmentStart + dashes[i % dashes.length];
if (dashPaints(i % dashes.length)) {
if (segmentStart >= start && segmentEnd <= end) return false;
painted += Math.max(0, Math.min(segmentEnd, end) - Math.max(segmentStart, start));
}
segmentStart = segmentEnd;
}
return painted <= total * 0.1;
}
// Computed `stroke-dasharray` as an even-length list of user-unit lengths, or null when the
// stroke is solid: `none`, an all-zero list, or any negative/unparseable entry (which the spec
// renders as `none`). Odd lists repeat, per spec.
function dashArrayLengths(value, path) {
const text = String(value || "none").trim();
if (text === "none") return null;
const lengths = text.split(/[\s,]+/).map((token) => dashLength(token, path));
if (lengths.some((length) => !Number.isFinite(length) || length < 0)) return null;
if (lengths.every((length) => length === 0)) return null;
return lengths.length % 2 === 0 ? lengths : lengths.concat(lengths);
}
// One dash length in user units. Computed lengths are already px; a percentage is relative to
// the normalised diagonal of the owning SVG viewport (viewBox when set, else the layout box).
function dashLength(token, path) {
const text = String(token).trim();
const value = Number.parseFloat(text);
if (!Number.isFinite(value) || !text.endsWith("%")) return value;
const svg = path.ownerSVGElement;
if (!svg) return NaN;
const box = svg.viewBox && svg.viewBox.baseVal;
const { width, height } =
box && box.width > 0 && box.height > 0 ? box : svg.getBoundingClientRect();
return (value / 100) * (Math.hypot(width, height) / Math.SQRT2);
}
function connectorEndpointCandidates(root, rootRect) {
@@ -1633,57 +1684,45 @@
function connectorOrphanIssues(root, rootRect, time) {
const issues = [];
let candidates = null;
const threshold = Math.max(32, Math.min(rootRect.width, rootRect.height) * 0.02);
for (const svg of Array.from(root.querySelectorAll("svg"))) {
if (!isVisibleElement(svg)) continue;
for (const path of Array.from(svg.querySelectorAll("path"))) {
if (path.closest(CONNECTOR_SKIP_CONTAINERS)) continue;
if (!isConnectorPath(svg, path)) continue;
if (!shaftIsPainted(path) || shaftDashHidden(path)) continue;
const user = pathUserEndpoints(path);
const rendered = pathScreenEndpoints(svg, path, user);
if (!user || !rendered) continue;
const renderedChord = Math.hypot(
rendered.end.x - rendered.start.x,
rendered.end.y - rendered.start.y,
);
if (renderedChord < 80) continue;
if (candidates === null) candidates = connectorEndpointCandidates(root, rootRect);
const dark = [];
for (const point of [rendered.start, rendered.end]) {
let best = null;
let attached = false;
for (const candidate of candidates) {
const gap = distanceToRect(point, candidate.rect);
if (gap > threshold) continue;
if (isVisibleElement(candidate.element)) {
attached = true;
break;
}
if (best === null || gap < best.gap) best = { gap, candidate };
const threshold = connectorAttachThreshold(rootRect);
for (const { svg, path, rendered, chord, painted } of connectorShafts(root)) {
if (!painted) continue;
if (chord < 80) continue;
if (candidates === null) candidates = connectorEndpointCandidates(root, rootRect);
const dark = [];
for (const point of [rendered.start, rendered.end]) {
let best = null;
let attached = false;
for (const candidate of candidates) {
const gap = distanceToRect(point, candidate.rect);
if (gap > threshold) continue;
if (isVisibleElement(candidate.element)) {
attached = true;
break;
}
if (!attached && best !== null) dark.push(best.candidate);
if (best === null || gap < best.gap) best = { gap, candidate };
}
if (dark.length === 0) continue;
issues.push({
code: "connector_orphan",
severity: "warning",
time,
selector: selectorFor(path),
containerSelector: selectorFor(svg),
message: `Connector shaft is visible while ${dark.length === 2 ? "both endpoints are" : `its endpoint ${selectorFor(dark[0].element)} is`} not on stage.`,
rect: toRect({
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:
"Show the shaft only after both ends are on, and hide it with the earlier exit. Do not give the line its own clock.",
});
if (!attached && best !== null) dark.push(best.candidate);
}
if (dark.length === 0) continue;
issues.push({
code: "connector_orphan",
severity: "warning",
time,
selector: selectorFor(path),
containerSelector: selectorFor(svg),
message: `Connector shaft is visible while ${dark.length === 2 ? "both endpoints are" : `its endpoint ${selectorFor(dark[0].element)} is`} not on stage.`,
rect: toRect({
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:
"Show the shaft only after both ends are on, and hide it with the earlier exit. Do not give the line its own clock.",
});
}
return issues;
}
@@ -1462,8 +1462,7 @@ describe("layout-audit.browser coordinate-frame findings", () => {
expect(issues.some((issue) => issue.code === "canvas_overflow")).toBe(true);
});
it("flags connector paths drawn in a foreign frame and passes anchored ones", () => {
document.body.innerHTML = `
const foreignFrameDom = `
<div id="root" data-composition-id="main" data-width="1920" data-height="1080">
<div id="n1"></div>
<div id="n2"></div>
@@ -1474,18 +1473,20 @@ describe("layout-audit.browser coordinate-frame findings", () => {
</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)" },
},
);
const foreignFrameRects = {
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 }),
};
const foreignFrameStyles = {
n1: { backgroundColor: "rgb(30, 40, 50)" },
n2: { backgroundColor: "rgb(30, 40, 50)" },
};
it("flags connector paths drawn in a foreign frame and passes anchored ones", () => {
document.body.innerHTML = foreignFrameDom;
installGeometry(foreignFrameRects, foreignFrameStyles);
// Screen CTM translates svg user space by the svg's offset (80, 227): the detached path's
// start (980, 580) renders at (1060, 807) — 147px below #n1's box — while the anchored
// path's start (900, 353) renders at (980, 580), inside #n1.
@@ -1500,6 +1501,55 @@ describe("layout-audit.browser coordinate-frame findings", () => {
expect(issues[0]?.fixHint).toContain("invert getScreenCTM");
});
it("does not flag a paste-bug connector still hidden behind its dash offset", () => {
document.body.innerHTML = foreignFrameDom;
// The fixture above, with #detached fully dash-hidden — draw-on entrance not yet advanced.
installGeometry(foreignFrameRects, {
...foreignFrameStyles,
detached: { strokeDasharray: "100", strokeDashoffset: "100" },
});
installConnectorGeometry({ e: 80, f: 227 });
installAuditScript();
expect(runAudit().filter((issue) => issue.code === "connector_detached")).toEqual([]);
});
// Hidden only when the visible window sits in one gap; a zero-length dash paints only with a
// round/square cap. Path length is 100, so `50 100` at offset 40 is the 10% boundary.
it.each([
{ dasharray: "0 4", offset: "0", count: 0 },
{ dasharray: "0 4", offset: "0", linecap: "round", count: 1 },
{ dasharray: "0, 4", offset: "0", linecap: "square", count: 1 },
{ dasharray: "0px, 999999px", offset: "-99.999px", count: 0 },
{ dasharray: "0 400", offset: "0", linecap: "round", count: 1 },
{ dasharray: "0 400", offset: "1", linecap: "round", count: 0 },
{ dasharray: "4 0", offset: "0", count: 1 },
{ dasharray: "0", offset: "0", count: 1 },
{ dasharray: "none", offset: "0", count: 1 },
{ dasharray: "100px", offset: "100px", count: 0 },
{ dasharray: "100", offset: "-100", count: 0 },
{ dasharray: "50 100", offset: "50", count: 0 },
{ dasharray: "50 100", offset: "40", count: 0 },
{ dasharray: "50 100", offset: "30", count: 1 },
])(
"stroke-dasharray $dasharray, dashoffset $offset, linecap $linecap → $count connector_detached",
({ dasharray, offset, linecap, count }) => {
document.body.innerHTML = foreignFrameDom;
installGeometry(foreignFrameRects, {
...foreignFrameStyles,
detached: {
strokeDasharray: dasharray,
strokeDashoffset: offset,
...(linecap ? { strokeLinecap: linecap } : {}),
},
});
installConnectorGeometry({ e: 80, f: 227 });
installAuditScript();
expect(runAudit().filter((issue) => issue.code === "connector_detached")).toHaveLength(count);
},
);
it("skips svgs and paths without connector intent", () => {
document.body.innerHTML = `
<div id="root" data-composition-id="main" data-width="1920" data-height="1080">