feat(lint): add connector_motion_detached layout check

[xuanruli]

What it catches

A half-attached connector during motion: one endpoint stays anchored to a node (<= 24px) while the other dangles > 80px from every node on >= 80% of held frames (t >= 0.45 * dur). The line/spoke visually detaches from what it should connect while the diagram animates.

The existing connector check only scans named <path> elements, requires both ends far, and judges a single frame — so it misses (a) <line> spokes, (b) one-end detachment, and (c) transient motion detachment. This check samples both <line> and <path> and evaluates persistence across the held-frame window.

How it works

• Per-frame sampler records both endpoints of each <line>/<path> connector and the node-candidate set.
• Half-attached signature: one end near a node, other end far from all nodes, sustained across the held window.
• Node-candidate set includes HTML boxes and sizable SVG <circle>/<rect>/<ellipse>/ring-<path> shapes (so a connector anchored to an SVG-drawn hub/dot reads as attached, not dangling).
• Excludes gauge/needle/pointer/tick/indicator elements (name + geometry: anchored end near arc-center & dangling end radially outward) — those are the off_pivot_rotation check's domain, keeping the architectural boundary clean.

Corpus evidence (autonomous geometry-fuzz run, 81 fuzzed diagrams)

• 1 true positive (fuzz011), 0 false positives across all 81 samples after tuning (100% precision).
• fuzz011 = 4 ring spokes half-attached (~185–190px dangle) — invisible to the existing connector check.
• Tuning path that reached 0 FP:
  • Initial sweep: 4/81 fires, 3 FP (fuzz033/038/074) — all the same cause: connector anchored to an SVG-drawn hub while the node set was HTML-only.
  • Added sizable SVG shapes to node candidates → 3 FP → 1 FP; new FP fuzz080 = a correct gauge needle mistaken for a half-attached connector.
  • Added the gauge-indicator exclusion → 0 FP / 1 TP.
• The VLM judge itself produced false positives on this cluster (fuzz051, fuzz014 render fully attached) — deterministic code inspection was the arbiter.

Validation

• Autonomous Gemini-3.6 video-judge fuzz run to surface candidates, then a deterministic FP sweep across all 81 rendered compositions.
• 13 unit tests (checkPipeline.connectorMotionDetached.test.ts) + full check suite pass; bun run build green.

Note for reviewers

This branch is the 3 connector commits cleanly rebased onto main — the dense-motion re-sampling change they were originally developed alongside ships separately (its own PR), so this branch contains only the connector detector.

🤖 Generated with Claude Code

[xuanruli]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• feat(lint): add connector_motion_detached layout check #2745 👈 (View in Graphite)
• feat(lint): dense motion re-sampling for content_overlap #2746
• feat(lint): add off_pivot_rotation hub-referenced layout check #2744
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[miguel-heygen]

The rule is well-scoped relative to connector_detached: sampling both endpoints across the held window and dropping aliased selectors is a sensible way to target the half-attached motion failure without widening the existing per-frame detector.

I found two blocking gaps in the rule’s stated contract:

1. The “anchored” endpoint does not have to stay anchored. endpointHeldGaps records only minGap, and isAnchored returns true if that minimum is within 24px (packages/cli/src/utils/checkPipeline.ts:990-1025). One momentary contact on any held frame therefore marks the endpoint anchored for the entire window. I reproduced a finding where endpoint A is detached on every held frame except one, while B remains dangling. Please track an attached-frame fraction and require sustained attachment (consistent with the existing ~80% held criterion), with a regression for one-frame transient contact.

2. The PR promises SVG circle/rect/ellipse/ring-path node candidates, but connectorNodeBoxes excludes every element inside SVG from the HTML pass and the SVG pass queries only circle, ellipse, rect (packages/cli/src/commands/layout-audit.browser.js:1781-1817). A hub/ring drawn as <path> is invisible to nearest-node matching, creating false detach findings for a common shape this PR explicitly claims to support. Please include validated ring paths, ideally by reusing the existing arc/hub path geometry rather than creating another path classifier, and add a path-ring fixture.

Requesting changes for these false-positive paths.

— Magi

[xuanruli]

Thanks Magi — both are real. Fix in progress:

1. Sustained attachment: replacing the minGap/isAnchored single-frame contact with an attached-frame fraction requiring sustained attachment (consistent with the existing ~80% held criterion). One momentary contact no longer marks an endpoint anchored for the whole window; your repro (A detached on every held frame but one, B dangling) will fire. Adding the one-frame-transient-contact regression.
2. Ring <path> nodes: extending the SVG node pass (currently circle, ellipse, rect only) to include validated ring/hub paths, reusing the existing arc/hub path geometry rather than a new classifier, so path-drawn hubs are visible to nearest-node matching and don't produce false detach findings. Adding a path-ring fixture.

[james-russo-rames-d-jusso]

Reviewed at f17174307080ef7f57eae3b7b15f4d52eba40320.

Well-executed extension of the connector-detection surface. The additive shape reads coherently: <line> and <path> both sampled per frame (previously only static <path>); half-attached signature computed against a broader node set (HTML boxes + sizable SVG shapes vs HTML-only before); persistence across the held-frame window (previously single-frame); gauge/needle exclusion drawing an architectural boundary with #2744's off_pivot_rotation. SVG API usage checks out (SVGLineElement.x1.baseVal.value, SVGPathElement.getPointAtLength(0), getScreenCTM, createSVGPoint/matrixTransform, getTotalLength all real and matching W3C SVG DOM), root discovery matches the pre-existing sibling __hyperframesRotationSample (3-tier [data-composition-id][data-width][data-height] chain — good), pointToNodeGap bbox-perimeter approximation errs toward reporting anchor (safer FP direction), and this new cross-sample check doesn't double-report against the pre-existing connector_detached per-sample check (the pre-existing one requires BOTH endpoints detached; this one requires exactly one dangling — non-overlapping predicates).

Blockers: none.

The two 🟠 concerns are about the gauge-exclusion boundary being narrower than the PR body advertises and a defensive gap in pathScreenEndpoints that this PR amplifies by sample count. Neither prevents merge but both are worth folding in.

Cross-cutting themes

1. "By name AND geometry" gauge exclusion is technically two independent OR filters, and both prongs have holes. The geometry branch (isGaugeIndicator at checkPipeline.ts:1035) requires node.ring === true on a ConnectorNodeBox, but connectorNodeBoxes (browser.js:1802) only registers circle, ellipse, rect — the very common arc-path dial pattern (<path d="M... A..." fill="none"/>) never becomes a ConnectorNodeBox at all, so the geometry prong cannot fire for it. That leaves only the name-based isIndicatorConnector regex added in the tip commit — a /needle|pointer|gauge|tick|indicator/i walk up the ancestor chain, with no unit-test coverage. Both prongs individually have gaps; the safety net has holes on both sides. Two levers to close the gap: (a) extend connectorNodeBoxes to register <path fill="none"> when the path is closed / near-circular (mirror the arc-hub fit that #2744 already implements), and (b) add a positive data-hf-gauge / data-hf-indicator opt-in data attribute that both checks honor as an explicit-author-intent signal. Inline anchors below.

2. Defensive gaps in pathScreenEndpoints (browser.js:1237-1238) amplify by sample count. getTotalLength() is try/catch-guarded at :1221-1225 but the two getPointAtLength() calls that use its return value are not. In Chrome the pair is usually safe once total > 0, but the API is documented to throw for malformed paths. If it does throw, the exception propagates out of __hyperframesConnectorSample → page.evaluate rejects → driver.collectConnectorSample(time) rejects → the whole runBrowserCheck rejects → every layout/contrast/motion finding for the run is dropped over one malformed path. Pre-existing shape, but this PR moves the sampler from being called once (connector_detached) to being called per-sample — the throw budget multiplies by sample count.

3. Framework consistency with #2744 (which this stacks over).

• Wire shape: collectConnectorSample(time) returns ConnectorFrame = { time, connectors, nodes } — the frame-envelope shape with time hoisted. #2744's collectOffPivotRotationSample(time) returns OffPivotRotationSample[] with time on each sample. Same "one seek's data" concept, two different shapes. The frame envelope here is objectively better (per-frame nodes shared across all connectors, no duplication) — this PR is on the right side of the seam. Worth mentioning here so the eventual reconciliation lands as "align #2744 to #2745's shape", not the other way.

4. Doc drift: PR body cites a sibling check that doesn't exist under that name.

Your PR body says: "those are the needle_pivot_offset check's domain, keeping the architectural boundary clean". Grep of packages/cli/: needle_pivot_offset → 0 hits; off_pivot_rotation → 9 hits (canonical name from #2744). The stack branch is also named xuanru/needle-pivot-offset-check but the actual check is off_pivot_rotation. Doc-only; please update the PR body when you touch it next. A future reader searching for needle_pivot_offset (e.g. to --exclude-check needle_pivot_offset) finds nothing — the CLI would silently ignore the unknown code and the check keeps firing.

🟢 Verified-safe

• All SVG DOM APIs are real and match W3C signatures — no hallucinated APIs.
• Root discovery matches the pre-existing sibling __hyperframesRotationSample (3-tier chain). #2744's __hyperframesOffPivotRotationSample skips the primary selector — flagged separately on that PR.
• groupConnectorsBySelector aliasing behavior — a selector observed twice in one frame is dropped globally across all frames — matches the test at test:166. Correct defensive posture (false-association risk outweighs recall loss).
• pointToNodeGap ring perimeter math is correct — bbox-perimeter approximation for <circle>/<ellipse> matches the outer stroke edge; for corner-of-bbox points the approximation errs toward reporting anchor (safer FP direction).
• No duplication of the earlier connectorDetachmentIssues — the pre-existing per-sample check requires both endpoints detached; this new cross-sample check requires exactly one dangling. Non-overlapping predicates; cannot double-report.
• connector_motion_detached correctly excluded from PERSISTENCE_TIERED_CODES (layoutAudit.ts:212). Held-frame semantics already baked in via CONNECTOR_HELD_DETACH_FRAC = 0.8; adding to the tier set would double-apply the persistence gate.
• Runtime path layoutSet.has(time) gate at :386-394 ensures collectConnectorSample runs only on layout samples, not motion/transition times.
• fakeDriver in check.test.ts stubs the new collectConnectorSample returning { time, connectors: [], nodes: [] } — existing integration tests won't crash.
• CheckAuditDriver.collectConnectorSample typing round-trips cleanly through parseConnectorLine / parseConnectorNode in checkBrowser.ts:586-606.
• isVisibleElement(svg, 0.05) opacity gate at :1829 correctly excludes hidden / entrance-pre-reveal SVGs.
• Tip commit's isIndicatorConnector ancestor walk terminates at svg.parentElement (:1752) — bounded, can't escape composition root.
• ConnectorFrame.nodes populated once per frame and shared across every connector for that frame — no O(N_conn × N_node × frames) recomputation.
• 11 unit tests present as claimed; line-endpoint detection, ring anchoring, gauge exclusion (geometry branch), inward-vs-outward radial distinction, held-frame gating, min-frame threshold, and selector aliasing all covered.
• Cross-PR boundary with #2744 verifiable via fuzz080 cross-reference: PR body says "TP there, FP-cleared here via gauge exclusion" — matches #2744's PR body saying the same about fuzz080 from the other side. Coherent architectural partition.

Solid extension — the mid-development gauge-exclusion pivot (visible in the tip commit fix(lint): exclude gauge needles/pointers) reads as tightening rather than papering over, which is good. Please close the two prongs of the exclusion gap or acknowledge them explicitly as scoped-known blind spots before merge.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

🟠 The geometry prong of isGaugeIndicator silently skips arc-path dials — a very common gauge pattern.

function isGaugeIndicator(
  anchored: { x: number; y: number },
  dangling: { x: number; y: number },
  nodes: ConnectorNodeBox[],
): boolean {
  for (const node of nodes) {
    if (!node.ring) continue;
    // ...
    if (dAnchored <= GAUGE_HUB_CENTRE_FRACTION * size && dDangling > dAnchored) return true;
  }
  return false;
}

The PR body promises the gauge exclusion works by name AND geometry. This function is the geometry prong. It iterates nodes and skips anything with !node.ring, so the only exclusion carriers are <circle> / <ellipse> / <rect> shapes registered by connectorNodeBoxes (layout-audit.browser.js:1802: for (const shape of Array.from(root.querySelectorAll("circle, ellipse, rect")))).

A very common gauge/dial pattern draws the arc as <path d="M... A..." fill="none"/> — matches the fallback arcHubForSvg at browser.js:1627 that #2744 already implements for its own hub-recovery. Those <path> arcs are never registered as ConnectorNodeBox at all, so isGaugeIndicator cannot fire for them.

The remaining safety net is the name-based isIndicatorConnector regex added in the tip commit. But an anonymous unlabeled arc-path gauge → no name hit + no ring hit → false positive connector_motion_detached on the needle.

Not exercised by any test — the gauge test at test:108 hardcodes an <rect>-shaped arc with ring: true; no test constructs a needle over an arc-path gauge.

Fix options:

• (a) Extend connectorNodeBoxes to also register path[fill="none"] as ring: true when the path is closed / near-circular. Mirror the arcHubForSvg fit that already lives in the same file — you get parity with feat(lint): add off_pivot_rotation hub-referenced layout check #2744's hub-recovery for free.
• (b) Piggy-back on any OffPivotRotationSample.hx/hy the sampler already recovers for the same frame — if feat(lint): add off_pivot_rotation hub-referenced layout check #2744's off-pivot check found a hub for this SVG at this time, treat it as a ring-equivalent for this frame.
• (c) Softer: acknowledge the arc-path miss as a known blind spot in the pipeline comment at :1028-1032 so future PRs don't inherit the wrong invariant.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

🟠 This new call site amplifies a defensive gap in the pre-existing pathScreenEndpoints helper — one bad diagram path can now abort the whole check.

const ends =
  line.tagName.toLowerCase() === "line"
    ? lineScreenEndpoints(svg, line)
    : pathScreenEndpoints(svg, line);

Look at pathScreenEndpoints at :1211-1239: getTotalLength() is defensively try/catch-guarded at :1221-1225 but the two getPointAtLength(0) / getPointAtLength(total) calls at :1237-1238 are NOT:

return {
  start: toScreen(path.getPointAtLength(0)),
  end: toScreen(path.getPointAtLength(total)),
};

In Chrome the pair is usually safe once total > 0, but the API is documented to throw for malformed paths (arcs with degenerate radii, invalid path data that survives getTotalLength numerically).

Failure mode: if either getPointAtLength throws, the exception propagates out of __hyperframesConnectorSample → page.evaluate rejects → driver.collectConnectorSample(time) rejects → collectGridSamples/runAuditGrid have no per-sample guard → the whole runBrowserCheck rejects. runCheckPipeline catches and replaces the entire browser result with emptyBrowserResult() + a single runtime finding — every layout/contrast/motion finding for the run is dropped over one malformed path.

The pre-existing per-sample connector_detached code path has the same shape (called once via connectorDetachmentIssues at :1287), so this isn't a net-new hazard. But this PR extends the surface — the sampler now runs on EVERY layout sample instead of once — multiplying the throw budget by the sample count.

Fix: wrap the two getPointAtLength calls at :1237-1238 in the same try block that already guards getTotalLength, returning null on throw. Two-line defensive change; matches the surrounding style. Or wrap the pathScreenEndpoints(svg, line) call HERE in a try/catch returning null, which is more localized to your PR.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

🟡 CONNECTOR_ATTACH_PX = 24 is a flat pixel constant but its detach counterpart scales to viewport.

const CONNECTOR_ATTACH_PX = 24;
// ...
const CONNECTOR_DETACH_FLOOR_PX = 80;
const CONNECTOR_DETACH_VIEWPORT_FRACTION = 0.06;

The detach side scales up on 4K (Math.max(80, 0.06 * min(w, h)) = 129px on 3840×2160) but the attach side stays at a flat 24px on every canvas.

Failure mode (safer direction): on very-high-DPR compositions a genuinely-anchored endpoint (say 32px away from a node — visually adjacent, well within the node's own painted size) reads as !isAnchored, danglingEndpoint returns null (neither endpoint anchored), and the finding never fires. This is a false-NEGATIVE (safer than the FP direction) but breaks the symmetry the comment at :927-929 asserts ("measured against a dense node-candidate set").

Fix: either mirror the detach ladder (Math.max(CONNECTOR_ATTACH_PX, CONNECTOR_ATTACH_VIEWPORT_FRACTION * min(w, h)) with fraction ~0.018 to preserve the 24/1000×1000 anchor point), or leave the constant flat and document at :922 that anchor tolerance intentionally does NOT scale — trading recall on 4K for FP suppression on 1080p.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

🟡 Effective holdStart is ~0.425·D under default sampling, not the 0.45·D the comment / PR body promise.

const duration = frames.reduce((max, frame) => Math.max(max, frame.time), 0);
if (duration <= 0) return findings;
const holdStart = CONNECTOR_HELD_START_FRAC * duration;

duration is derived from max(frame.time), but buildLayoutSampleTimes (layoutAudit.ts:82-95) places samples at ((index + 0.5) / count) * D — so with the default 9 samples the max sampled time is 8.5/9 · D ≈ 0.944 · D, and holdStart = 0.45 · 0.944 · D ≈ 0.425 · D.

So the effective held-window starts ~5% of duration earlier than the design comment at :919-920 states. Purely cosmetic on the current corpus (test at test:74 doesn't exhibit the drift because its frames array happens to include time=0 and time=duration exactly), but a future author reading CONNECTOR_HELD_START_FRAC = 0.45 and inferring contract intent gets the wrong number.

Fix: pass the true composition duration in from the caller (grid.duration is available at the call site in runAuditGrid) rather than reducing over sample times. Or update the comment at :919-920 to note that the effective threshold is 0.45 · max_sample_time, not 0.45 · duration.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

🟡 isIndicatorConnector (the name-based prong of gauge exclusion added in the tip commit) has no unit test.

const CONNECTOR_INDICATOR_NAME = /needle|pointer|gauge|tick|indicator/i;

The filter walks the connector element's ancestry (id + className) up to the SVG parent and skips the connector before endpoint extraction. It's ONE of the two prongs the PR body advertises as by name AND geometry, and — per the sibling comment on checkPipeline.ts:1035 — it's the ONLY prong that catches arc-path gauges.

Shipped without unit coverage. The pipeline test file operates on ConnectorLineSample after the browser strip, so the regex ancestry walk is exercisable only in an integration/e2e run.

Regression pathway: someone refactors connectorNameFor (which handles className.baseVal for SVG elements) or edits the ancestor-loop termination (node !== svg.parentElement) — the filter silently regresses and CI stays green because the fuzz corpus doesn't run in CI.

Also: the regex is a small taxonomy. A connector inside <g id="chart-ticker-line"> matches (tick substring) and gets silently skipped even though it's a real dangling connector defect. A legitimate gauge needle authored as class="reading-arrow" doesn't match and slips through to the geometry prong (which has its own gap per :1035 comment) → false-positive.

Fix: extract isIndicatorConnector into a helper that can be exercised without JSDOM — pass in a minimal { id, className } chain and assert the regex disposition. Or add a corpus-e2e test wired to fuzz080 so a regression on the name filter surfaces in the same PR CI that motivated the tip-commit fix.

Stronger fix (couples with the sibling comment on checkPipeline.ts:1035): add a positive data-hf-gauge / data-hf-indicator opt-in data attribute that both checks honor as an explicit author-intent signal — removes reliance on both the fuzzy regex and the arc-path geometry gap.

— Review by Rames D Jusso

[xuanruli]

@miguel-heygen pushed the fix in 59ae513ec — ready for another look.

FP path 1 — sustained attachment:

• Replaced the single-frame minGap/isAnchored contact with an attached-frame fraction requiring sustained attachment (consistent with the existing ~80% held criterion). One momentary contact no longer anchors an endpoint for the whole window; the repro (A detached every held frame but one, B dangling) now fires. Added the one-frame-transient-contact regression.

FP path 2 — ring <path> nodes:

• The SVG node pass now registers validated ring/arc <path> hubs (reusing the existing arc/hub path geometry), so path-drawn hubs are visible to nearest-node matching. This also flows into isGaugeIndicator (node.ring), closing the geometry prong of the gauge exclusion for arc-path dials. Added a path-ring fixture, and a unit test for the name-based isIndicatorConnector regex.
• Guarded the getPointAtLength pair in pathScreenEndpoints so a single malformed path degrades gracefully instead of aborting the whole check (matters now that the sampler runs once-per-sample).

PR body updated: off_pivot_rotation (was the stale needle_pivot_offset).

CI: green on the stack's #2744 SHA run; Windows re-run finishing.