From 399196e2de626187e88a8284f32ada1e50e950d0 Mon Sep 17 00:00:00 2001 From: xuanru Date: Thu, 23 Jul 2026 10:57:50 +0000 Subject: [PATCH 1/4] feat(lint): add off_pivot_rotation hub-referenced layout check --- packages/cli/src/commands/check.test.ts | 1 + .../cli/src/commands/layout-audit.browser.js | 184 ++++++++++++++ packages/cli/src/utils/checkBrowser.ts | 56 +++++ .../checkPipeline.offPivotRotation.test.ts | 123 ++++++++++ packages/cli/src/utils/checkPipeline.ts | 227 +++++++++++++++++- packages/cli/src/utils/checkTypes.ts | 23 ++ packages/cli/src/utils/layoutAudit.ts | 3 + 7 files changed, 616 insertions(+), 1 deletion(-) create mode 100644 packages/cli/src/utils/checkPipeline.offPivotRotation.test.ts diff --git a/packages/cli/src/commands/check.test.ts b/packages/cli/src/commands/check.test.ts index 09b214aa34..a9b2758700 100644 --- a/packages/cli/src/commands/check.test.ts +++ b/packages/cli/src/commands/check.test.ts @@ -150,6 +150,7 @@ function fakeDriver(overrides: Partial = {}): CheckAuditDriver collectLayout: vi.fn(async (_time: number, _tolerance: number) => []), collectLayoutGeometry: vi.fn(async () => `geometry-${geometryCallCount++}`), collectRotationSample: vi.fn(async (_time: number) => []), + collectOffPivotRotationSample: vi.fn(async (_time: number) => []), collectGeometryCandidates: vi.fn(async () => []), collectMotionFrame: vi.fn(async (time: number) => ({ time, data: {}, liveness: {} })), anchorMotionIssues: vi.fn(async (issues: LayoutIssue[]) => diff --git a/packages/cli/src/commands/layout-audit.browser.js b/packages/cli/src/commands/layout-audit.browser.js index 78b9dd8da8..20c7d9c493 100644 --- a/packages/cli/src/commands/layout-audit.browser.js +++ b/packages/cli/src/commands/layout-audit.browser.js @@ -1531,4 +1531,188 @@ } return samples; }; + + // Needle-pivot sampling (off_pivot_rotation). A gauge/clock/radar pointer + // whose center-of-rotation sits far from the dial hub. bbox-intrinsic measures + // can't tell a correct sweep from a broken one (a base-pivoted needle's bbox + // center orbits either way), so this records two MATERIAL points on each + // elongated rotating SVG figure — mapped through getScreenCTM so the actual + // rendered transform is honored regardless of svgOrigin/transform-origin — and + // the dial's static hub (the point shared by the most non-rotating circles). + // The pipeline fits a rotation to the material-point trajectories to recover + // the real center-of-rotation and flags it when it drifts off that hub. + function ctmRotationDeg(ctm) { + if (!ctm) return null; + return (Math.atan2(ctm.b, ctm.a) * 180) / Math.PI; + } + + function ctmScale(ctm) { + return Math.hypot(ctm.a, ctm.b); + } + + function mapPoint(svg, ctm, x, y) { + const point = svg.createSVGPoint(); + point.x = x; + point.y = y; + const mapped = point.matrixTransform(ctm); + return { x: mapped.x, y: mapped.y }; + } + + // Walks up to (and including) the composition root, NOT just the owner : + // an element spun by a div ancestor above its svg must not be mistaken for a + // static hub anchor (else a lone rotating arc becomes its own dial center). + function hasRotatedAncestor(element, root) { + let node = element; + while (node) { + const angle = rotationAngleDeg(getComputedStyle(node).transform); + if (angle !== null && Math.abs(angle) > 1) return true; + if (node === root) break; + node = node.parentElement; + } + return false; + } + + function fitCirclePoints(points) { + const count = points.length; + if (count < 3) return null; + const meanX = points.reduce((sum, p) => sum + p.x, 0) / count; + const meanY = points.reduce((sum, p) => sum + p.y, 0) / count; + let suu = 0, + svv = 0, + suv = 0, + suuu = 0, + svvv = 0, + suvv = 0, + svuu = 0; + for (const point of points) { + const u = point.x - meanX; + const v = point.y - meanY; + suu += u * u; + svv += v * v; + suv += u * v; + suuu += u * u * u; + svvv += v * v * v; + suvv += u * v * v; + svuu += v * u * u; + } + const det = suu * svv - suv * suv; + if (Math.abs(det) < 1e-6) return null; + const uc = (((suuu + suvv) / 2) * svv - ((svvv + svuu) / 2) * suv) / det; + const vc = (((svvv + svuu) / 2) * suu - ((suuu + suvv) / 2) * suv) / det; + const cx = uc + meanX; + const cy = vc + meanY; + const radius = Math.sqrt(uc * uc + vc * vc + (suu + svv) / count); + let residual = 0; + for (const point of points) + residual += Math.abs(Math.hypot(point.x - cx, point.y - cy) - radius); + return { cx, cy, radius, residual: residual / count }; + } + + // Fallback for dials drawn as arc rather than rings: sample + // the largest static, near-circular path and recover its arc center. + function arcHubForSvg(svg, root) { + let best = null; + for (const path of Array.from(svg.querySelectorAll("path"))) { + if (hasRotatedAncestor(path, root)) continue; + if (typeof path.getTotalLength !== "function") continue; + const total = path.getTotalLength(); + if (total < 200) continue; + const ctm = path.getScreenCTM(); + if (!ctm) continue; + const points = []; + for (let i = 0; i <= 16; i++) { + const local = path.getPointAtLength((total * i) / 16); + points.push(mapPoint(svg, ctm, local.x, local.y)); + } + const fit = fitCirclePoints(points); + if (!fit || fit.radius < 40) continue; + if (fit.residual > 0.05 * fit.radius) continue; + if (!best || fit.radius > best.radius) best = fit; + } + return best ? { hx: best.cx, hy: best.cy, hr: best.radius, count: 2 } : null; + } + + function dialHubForSvg(svg, root) { + const centers = []; + for (const circle of Array.from(svg.querySelectorAll("circle"))) { + if (hasRotatedAncestor(circle, root)) continue; + const ctm = circle.getScreenCTM(); + if (!ctm) continue; + const cx = Number.parseFloat(circle.getAttribute("cx") || "0"); + const cy = Number.parseFloat(circle.getAttribute("cy") || "0"); + const center = mapPoint(svg, ctm, cx, cy); + const radius = Number.parseFloat(circle.getAttribute("r") || "0") * ctmScale(ctm); + centers.push({ x: center.x, y: center.y, radius }); + } + let best = null; + for (const anchor of centers) { + const cluster = centers.filter( + (other) => Math.hypot(other.x - anchor.x, other.y - anchor.y) <= 8, + ); + if (!best || cluster.length > best.cluster.length) best = { anchor, cluster }; + } + if (best && best.cluster.length >= 2) { + const count = best.cluster.length; + const hx = best.cluster.reduce((sum, item) => sum + item.x, 0) / count; + const hy = best.cluster.reduce((sum, item) => sum + item.y, 0) / count; + const hr = best.cluster.reduce((max, item) => Math.max(max, item.radius), 0); + return { hx, hy, hr, count }; + } + return arcHubForSvg(svg, root); + } + + window.__hyperframesOffPivotRotationSample = function collectOffPivotRotationSample() { + const root = document.querySelector("[data-composition-id]") || document.body; + const samples = []; + const hubCache = new Map(); + const CANDIDATE_CAP = 60; + for (const element of Array.from( + root.querySelectorAll("path, polygon, line, rect, polyline, g"), + )) { + if (samples.length >= CANDIDATE_CAP) break; + const svg = element.ownerSVGElement; + if (!svg || typeof element.getBBox !== "function") continue; + if (element.closest("[data-layout-allow-orbit]")) continue; + if (!isVisibleElement(element, 0.05)) continue; + const ctm = element.getScreenCTM(); + const angle = ctmRotationDeg(ctm); + if (ctm === null || angle === null) continue; + let bbox; + try { + bbox = element.getBBox(); + } catch { + continue; + } + const long = Math.max(bbox.width, bbox.height); + const short = Math.min(bbox.width, bbox.height); + if (short <= 0 || long / short < 3 || long < 40) continue; + const vertical = bbox.height >= bbox.width; + const midMajor = vertical ? bbox.x + bbox.width / 2 : bbox.y + bbox.height / 2; + const a = vertical + ? mapPoint(svg, ctm, midMajor, bbox.y) + : mapPoint(svg, ctm, bbox.x, midMajor); + const b = vertical + ? mapPoint(svg, ctm, midMajor, bbox.y + bbox.height) + : mapPoint(svg, ctm, bbox.x + bbox.width, midMajor); + let hub = hubCache.get(svg); + if (hub === undefined) { + hub = dialHubForSvg(svg, root); + hubCache.set(svg, hub); + } + samples.push({ + selector: selectorFor(element), + ax: round(a.x), + ay: round(a.y), + bx: round(b.x), + by: round(b.y), + len: round(Math.hypot(b.x - a.x, b.y - a.y)), + angle: round(angle), + hx: hub ? round(hub.hx) : null, + hy: hub ? round(hub.hy) : null, + hr: hub ? round(hub.hr) : null, + hubCount: hub ? hub.count : 0, + }); + } + return samples; + }; })(); diff --git a/packages/cli/src/utils/checkBrowser.ts b/packages/cli/src/utils/checkBrowser.ts index 2da9fc1918..a4f7984677 100644 --- a/packages/cli/src/utils/checkBrowser.ts +++ b/packages/cli/src/utils/checkBrowser.ts @@ -42,6 +42,7 @@ import type { ContrastCapture, GeometryCandidateRequest, MotionSpecResolution, + OffPivotRotationSample, RotationSample, RunAuditGrid, } from "./checkTypes.js"; @@ -349,6 +350,7 @@ function createPageDriver(page: Page, setTime: (time: number) => void): CheckAud collectLayout: (time, tolerance) => collectLayout(page, time, tolerance), collectLayoutGeometry: () => collectLayoutGeometry(page), collectRotationSample: (time) => collectRotationSample(page, time), + collectOffPivotRotationSample: (time) => collectOffPivotRotationSample(page, time), collectGeometryCandidates: (time, request) => collectGeometryCandidates(page, time, request), collectMotionFrame: (time, selectors, scopes) => collectMotionFrame(page, time, selectors, scopes), @@ -494,6 +496,59 @@ function parseRotationSample(value: unknown, time: number): RotationSample[] { return [{ time, selector, cx, cy, w, h, angle }]; } +async function collectOffPivotRotationSample( + page: Page, + time: number, +): Promise { + const raw = await page.evaluate(() => { + const sample = Reflect.get(window, "__hyperframesOffPivotRotationSample"); + if (typeof sample !== "function") return []; + const result = Reflect.apply(sample, window, []); + return Array.isArray(result) ? result : []; + }); + return raw.flatMap((value) => parseOffPivotRotationSample(value, time)); +} + +function parseOffPivotRotationSample(value: unknown, time: number): OffPivotRotationSample[] { + if (!isRecord(value)) return []; + const selector = stringValue(value, "selector"); + const ax = numberValue(value, "ax"); + const ay = numberValue(value, "ay"); + const bx = numberValue(value, "bx"); + const by = numberValue(value, "by"); + const len = numberValue(value, "len"); + const angle = numberValue(value, "angle"); + const hubCount = numberValue(value, "hubCount"); + if ( + !selector || + ax === null || + ay === null || + bx === null || + by === null || + len === null || + angle === null || + hubCount === null + ) { + return []; + } + return [ + { + time, + selector, + ax, + ay, + bx, + by, + len, + angle, + hx: numberValue(value, "hx"), + hy: numberValue(value, "hy"), + hr: numberValue(value, "hr"), + hubCount, + }, + ]; +} + async function collectGeometryCandidates( page: Page, time: number, @@ -1078,6 +1133,7 @@ const LAYOUT_ISSUE_CODES: readonly LayoutIssueCode[] = [ "panel_out_of_canvas", "connector_detached", "rotation_pivot_drift", + "off_pivot_rotation", "motion_appears_late", "motion_out_of_order", "motion_off_frame", diff --git a/packages/cli/src/utils/checkPipeline.offPivotRotation.test.ts b/packages/cli/src/utils/checkPipeline.offPivotRotation.test.ts new file mode 100644 index 0000000000..c891b7e37f --- /dev/null +++ b/packages/cli/src/utils/checkPipeline.offPivotRotation.test.ts @@ -0,0 +1,123 @@ +import { describe, expect, it } from "vitest"; + +import { detectOffPivotRotation } from "./checkPipeline.js"; +import type { OffPivotRotationSample } from "./checkTypes.js"; + +/** One needle sample; defaults describe a sizable pointer on a hub at (500,500). */ +function sample(overrides: Partial = {}): OffPivotRotationSample { + return { + time: 0, + selector: "#needle", + ax: 0, + ay: 0, + bx: 0, + by: 0, + len: 200, + angle: 0, + hx: 500, + hy: 500, + hr: 220, + hubCount: 3, + ...overrides, + }; +} + +/** Endpoint positions on a circle of `radius` about `(cx,cy)` at 0/90/180°. */ +function orbit(cx: number, cy: number, radius: number): Array<[number, number]> { + return [ + [cx + radius, cy], + [cx, cy + radius], + [cx - radius, cy], + ]; +} + +/** A group that SHOULD fire: the pointer sweeps (0→90→180) about a recovered + * center at (500,300) — 200px above the dial hub at (500,500). Two material + * endpoints trace concentric circles about that wrong center. */ +function offPivotIndicator(selector = "#needle"): OffPivotRotationSample[] { + const a = orbit(500, 300, 100); + const b = orbit(500, 300, 40); + return [0, 1, 2].map((i) => + sample({ + selector, + time: i, + angle: i * 90, + ax: a[i]?.[0] ?? 0, + ay: a[i]?.[1] ?? 0, + bx: b[i]?.[0] ?? 0, + by: b[i]?.[1] ?? 0, + }), + ); +} + +describe("detectOffPivotRotation", () => { + it("fires when the recovered center-of-rotation is far from the dial hub", () => { + const findings = detectOffPivotRotation(offPivotIndicator()); + expect(findings).toHaveLength(1); + const [f] = findings; + expect(f?.code).toBe("off_pivot_rotation"); + expect(f?.severity).toBe("warning"); + expect(f?.selector).toBe("#needle"); + expect(f?.message).toContain("200px"); + expect(f?.fixHint).toContain("hub"); + }); + + it("does not fire with fewer than the minimum samples", () => { + expect(detectOffPivotRotation(offPivotIndicator().slice(0, 2))).toHaveLength(0); + }); + + it("does not fire when the pointer barely sweeps (fixed tilt)", () => { + const group = offPivotIndicator().map((s, i) => ({ ...s, angle: i * 3 })); + expect(detectOffPivotRotation(group)).toHaveLength(0); + }); + + it("does not fire when no dial hub is resolvable", () => { + const group = offPivotIndicator().map((s) => ({ ...s, hx: null, hy: null, hubCount: 0 })); + expect(detectOffPivotRotation(group)).toHaveLength(0); + }); + + it("does not fire when the hub has too few concentric circles", () => { + const group = offPivotIndicator().map((s) => ({ ...s, hubCount: 1 })); + expect(detectOffPivotRotation(group)).toHaveLength(0); + }); + + it("does not fire on a correctly-hubbed needle (center-of-rotation on the hub)", () => { + const a = orbit(500, 500, 100); + const b = orbit(500, 500, 40); + const group = [0, 1, 2].map((i) => + sample({ + time: i, + angle: i * 90, + ax: a[i]?.[0] ?? 0, + ay: a[i]?.[1] ?? 0, + bx: b[i]?.[0] ?? 0, + by: b[i]?.[1] ?? 0, + }), + ); + expect(detectOffPivotRotation(group)).toHaveLength(0); + }); + + it("does not fire when the endpoint trajectory is not a clean circle", () => { + const group = [ + sample({ time: 0, angle: 0, ax: 600, ay: 300, bx: 540, by: 300 }), + sample({ time: 1, angle: 90, ax: 505, ay: 402, bx: 500, by: 341 }), + sample({ time: 2, angle: 180, ax: 402, ay: 305, bx: 461, by: 299 }), + ].map((s) => ({ ...s, ax: s.ax + (s.time === 1 ? 180 : 0) })); + expect(detectOffPivotRotation(group)).toHaveLength(0); + }); + + it("collapses nested candidates on the same hub to a single finding", () => { + const group = offPivotIndicator().map((s) => ({ ...s, len: 200 })); + const child = offPivotIndicator("#needle > path:nth-of-type(1)").map((s) => ({ + ...s, + len: 150, + })); + const findings = detectOffPivotRotation([...group, ...child]); + expect(findings).toHaveLength(1); + expect(findings[0]?.selector).toBe("#needle"); + }); + + it("returns nothing for an empty sample set", () => { + expect(detectOffPivotRotation([])).toHaveLength(0); + }); +}); diff --git a/packages/cli/src/utils/checkPipeline.ts b/packages/cli/src/utils/checkPipeline.ts index cebcf4c4a2..13eeb7ed26 100644 --- a/packages/cli/src/utils/checkPipeline.ts +++ b/packages/cli/src/utils/checkPipeline.ts @@ -48,6 +48,7 @@ import type { ContrastAuditEntry, GeometryCandidateRequest, MotionSpecResolution, + OffPivotRotationSample, RotationSample, } from "./checkTypes.js"; @@ -193,6 +194,9 @@ interface GridSamples { /** Every rotatable element's geometry at each layout sample; grouped by * selector after the run to detect rotation_pivot_drift. */ rotationSamples: RotationSample[]; + /** Every elongated rotating SVG figure's material-point geometry + dial hub at + * each layout sample; grouped by selector to detect off_pivot_rotation. */ + indicatorSamples: OffPivotRotationSample[]; } interface GeometrySeen { @@ -365,6 +369,7 @@ async function collectGridSamples( contrastMs: 0, geometrySignatures: [], rotationSamples: [], + indicatorSamples: [], }; for (const time of mergeSampleTimes(grid.layoutSamples, motion.times)) { await driver.seek(time); @@ -378,6 +383,7 @@ async function collectGridSamples( issuesAtTime.push(...layoutIssues); collected.geometrySignatures.push(await driver.collectLayoutGeometry()); collected.rotationSamples.push(...(await driver.collectRotationSample(time))); + collected.indicatorSamples.push(...(await driver.collectOffPivotRotationSample(time))); } if (canvas) { const geometryIssues = await collectGeometryAt( @@ -640,6 +646,219 @@ export function detectRotationPivotDrift( return findings; } +// off_pivot_rotation thresholds. A pointer must actually sweep, sit on a +// resolvable dial hub, and its recovered center-of-rotation must land far from +// that hub relative to the pointer's own length. +const INDICATOR_MIN_SAMPLES = 3; +const INDICATOR_MIN_ANGLE_SPREAD_DEG = 20; +const INDICATOR_MIN_HUB_CIRCLES = 2; +// A circle fit whose residual exceeds this fraction of the orbit radius is not +// a clean rotation (jitter/scatter) — skip rather than risk a bad center. +const INDICATOR_MAX_FIT_RESIDUAL_FRACTION = 0.25; +// Drift beyond this fraction of the pointer length is a wrong pivot. A correctly +// hubbed needle recovers a center within a few px of the hub (≈0); a base/edge +// pivot mistake puts the recovered center a large fraction of the needle away. +const INDICATOR_DRIFT_LENGTH_FRACTION = 0.35; + +interface CircleFit { + cx: number; + cy: number; + radius: number; + residual: number; +} + +/** Kåsa algebraic circle fit; null when the points are near-collinear/degenerate + * (no stable center recoverable). */ +function fitCircle(points: Array<{ x: number; y: number }>): CircleFit | null { + const count = points.length; + if (count < 3) return null; + const meanX = points.reduce((sum, p) => sum + p.x, 0) / count; + const meanY = points.reduce((sum, p) => sum + p.y, 0) / count; + let suu = 0; + let svv = 0; + let suv = 0; + let suuu = 0; + let svvv = 0; + let suvv = 0; + let svuu = 0; + for (const point of points) { + const u = point.x - meanX; + const v = point.y - meanY; + suu += u * u; + svv += v * v; + suv += u * v; + suuu += u * u * u; + svvv += v * v * v; + suvv += u * v * v; + svuu += v * u * u; + } + const determinant = suu * svv - suv * suv; + if (Math.abs(determinant) < 1e-6) return null; + const rhsU = (suuu + suvv) / 2; + const rhsV = (svvv + svuu) / 2; + const uc = (rhsU * svv - rhsV * suv) / determinant; + const vc = (rhsV * suu - rhsU * suv) / determinant; + const cx = uc + meanX; + const cy = vc + meanY; + const radius = Math.sqrt(uc * uc + vc * vc + (suu + svv) / count); + let residual = 0; + for (const point of points) { + residual += Math.abs(Math.hypot(point.x - cx, point.y - cy) - radius); + } + return { cx, cy, radius, residual: residual / count }; +} + +function groupIndicatorSamplesBySelector( + samples: OffPivotRotationSample[], +): Map { + const bySelector = new Map(); + for (const sample of samples) { + const group = bySelector.get(sample.selector); + if (group) group.push(sample); + else bySelector.set(sample.selector, [sample]); + } + return bySelector; +} + +/** Recover the center-of-rotation from whichever major-axis endpoint traces the + * larger orbit (more movement → more numerically stable), rejecting scattered + * fits. */ +function recoverPivot(group: OffPivotRotationSample[]): CircleFit | null { + const endpointA = group.map((s) => ({ x: s.ax, y: s.ay })); + const endpointB = group.map((s) => ({ x: s.bx, y: s.by })); + const fits = [fitCircle(endpointA), fitCircle(endpointB)].filter( + (fit): fit is CircleFit => fit !== null, + ); + if (fits.length === 0) return null; + const best = fits.reduce((widest, fit) => (fit.radius > widest.radius ? fit : widest)); + if (best.radius <= 0) return null; + if (best.residual > INDICATOR_MAX_FIT_RESIDUAL_FRACTION * best.radius) return null; + return best; +} + +function offPivotRotationFinding( + group: OffPivotRotationSample[], + drift: number, +): AnchoredLayoutIssue | null { + const last = group[group.length - 1]; + if (!last) return null; + const rect: LayoutRect = { + left: Math.min(last.ax, last.bx) - 4, + top: Math.min(last.ay, last.by) - 4, + right: Math.max(last.ax, last.bx) + 4, + bottom: Math.max(last.ay, last.by) + 4, + width: Math.abs(last.bx - last.ax) + 8, + height: Math.abs(last.by - last.ay) + 8, + }; + return { + code: "off_pivot_rotation", + severity: "warning", + time: last.time, + selector: last.selector, + dataAttributes: {}, + sourceFile: "index.html", + bbox: rectToBbox(rect), + rect, + message: `Gauge/dial pointer rotates about a point ${Math.round(drift)}px from the dial hub — its center-of-rotation is off the hub, so it points wrong or overshoots the track (check the rotating group's pivot: translate to the hub before rotate, or set svgOrigin/transform-origin to the hub).`, + fixHint: + "The pointer's center-of-rotation must coincide with the dial hub. Rotate the group about the hub (e.g. transform-origin/svgOrigin set to the hub center, or translate the group so its rotation pivot lands on the hub) rather than about the element's own bbox center.", + }; +} + +/** + * off_pivot_rotation: a gauge needle / clock hand / radar sweep / dial pointer + * that rotates about the WRONG pivot — its center-of-rotation is far from the + * dial's hub, so the pointer aims wrong, runs off the track, or leaves the canvas. + * + * This is a HUB-REFERENCED check by necessity: a correctly-swept needle's bbox + * center orbits identically to a broken one (the pivot sits at the needle's + * base/edge either way), so no element-intrinsic measure separates them. The + * browser sampler records two fixed MATERIAL points on the pointer per frame; a + * circle fit recovers the true center-of-rotation, which is then compared to the + * dial's static hub (the screen point shared by the most non-rotating circles). + * + * FP guards are strict: requires real sweep (angle varies), a resolvable hub + * (≥2 concentric static circles), a clean circle fit (low residual), and drift + * exceeding a fraction of the pointer's own length. When no hub reference can be + * resolved it does not fire, so a lone decorative rotator can't trip it. + */ +interface IndicatorCandidate { + hubKey: string; + angleAboutHub: number; + drift: number; + length: number; + finding: AnchoredLayoutIssue | null; +} + +/** Distinct angular positions about the hub among a set of candidates (nested + * blade parts share one angle; orbiting bodies spread to several). */ +function countAngularBodies(angles: number[]): number { + const clusters: number[] = []; + for (const angle of angles) { + if ( + !clusters.some((rep) => Math.min(Math.abs(angle - rep), 360 - Math.abs(angle - rep)) <= 25) + ) { + clusters.push(angle); + } + } + return clusters.length; +} + +export function detectOffPivotRotation(samples: OffPivotRotationSample[]): AnchoredLayoutIssue[] { + const candidates: IndicatorCandidate[] = []; + for (const group of groupIndicatorSamplesBySelector(samples).values()) { + if (group.length < INDICATOR_MIN_SAMPLES) continue; + if (maxAngleSpread(group.map((s) => s.angle)) <= INDICATOR_MIN_ANGLE_SPREAD_DEG) continue; + const last = group[group.length - 1]; + if ( + !last || + last.hx === null || + last.hy === null || + last.hubCount < INDICATOR_MIN_HUB_CIRCLES + ) { + continue; + } + const pivot = recoverPivot(group); + if (!pivot) continue; + const drift = Math.hypot(pivot.cx - last.hx, pivot.cy - last.hy); + const length = median(group.map((s) => s.len)); + const angleAboutHub = + (Math.atan2((last.ay + last.by) / 2 - last.hy, (last.ax + last.bx) / 2 - last.hx) * 180) / + Math.PI; + const hubKey = `${Math.round(last.hx / 5)}:${Math.round(last.hy / 5)}`; + const eligible = drift > INDICATOR_DRIFT_LENGTH_FRACTION * length; + candidates.push({ + hubKey, + angleAboutHub, + drift, + length, + finding: eligible ? offPivotRotationFinding(group, drift) : null, + }); + } + // Per hub: multiple bodies at distinct angular positions = an orbit/atom/radial + // system, not a single dial pointer — suppress. A real pointer (group + nested + // blades) collapses to one angular cluster. Otherwise emit one finding per hub, + // keeping the longest pointer. + const byHub = new Map(); + for (const candidate of candidates) { + const group = byHub.get(candidate.hubKey); + if (group) group.push(candidate); + else byHub.set(candidate.hubKey, [candidate]); + } + const findings: AnchoredLayoutIssue[] = []; + for (const group of byHub.values()) { + if (countAngularBodies(group.map((c) => c.angleAboutHub)) >= 2) continue; + const best = group + .filter((c) => c.finding !== null) + .reduce( + (max, c) => (!max || c.length > max.length ? c : max), + null, + ); + if (best?.finding) findings.push(best.finding); + } + return findings; +} + /** Error-severity findings with real geometry become labeled overview boxes. * Contrast failures are annotated separately by the driver itself, since * they're only known once contrast measurement for this sample completes. */ @@ -679,6 +898,7 @@ export async function runAuditGrid( collected.rotationSamples, await driver.getCanvas(), ); + const offPivotFindings = detectOffPivotRotation(collected.indicatorSamples); const contrast = buildContrastResults(collected.contrastEntries); return { duration: grid.duration, @@ -686,7 +906,12 @@ export async function runAuditGrid( transitionSamples: grid.transitionSamples, transitionSamplesDropped: grid.transitionSamplesDropped, runtimeFindings: [], - layoutIssues: [...collected.layoutIssues, ...sweepFindings, ...rotationFindings], + layoutIssues: [ + ...collected.layoutIssues, + ...sweepFindings, + ...rotationFindings, + ...offPivotFindings, + ], motionIssues, motionSampleCount: collected.motionFrames.length, contrastSamples: grid.contrastSamples, diff --git a/packages/cli/src/utils/checkTypes.ts b/packages/cli/src/utils/checkTypes.ts index d04554d7d2..d0f1dbd986 100644 --- a/packages/cli/src/utils/checkTypes.ts +++ b/packages/cli/src/utils/checkTypes.ts @@ -133,6 +133,26 @@ export interface RotationSample { angle: number; } +/** One elongated rotating SVG figure's material geometry at a single seeked + * sample, produced by `__hyperframesOffPivotRotationSample`. `(ax,ay)`/`(bx,by)` are + * two fixed material points on the figure (major-axis endpoints) mapped to + * screen space, so their cross-frame trajectory recovers the true center of + * rotation; `(hx,hy)` is the dial's static hub and `hr` its radius. */ +export interface OffPivotRotationSample { + time: number; + selector: string; + ax: number; + ay: number; + bx: number; + by: number; + len: number; + angle: number; + hx: number | null; + hy: number | null; + hr: number | null; + hubCount: number; +} + export type MotionSpecResolution = | { kind: "none" } | { kind: "valid"; path: string; spec: MotionSpec } @@ -153,6 +173,9 @@ export interface CheckAuditDriver { /** rotation_pivot_drift: every rotatable element's bbox center/size/angle at * the current seeked state. Accumulated across the grid — see checkPipeline. */ collectRotationSample(time: number): Promise; + /** off_pivot_rotation: every elongated rotating SVG figure's material-point + * geometry + resolved dial hub at the current seeked state. */ + collectOffPivotRotationSample(time: number): Promise; collectGeometryCandidates( time: number, request: GeometryCandidateRequest, diff --git a/packages/cli/src/utils/layoutAudit.ts b/packages/cli/src/utils/layoutAudit.ts index e72df4f87d..678e1210c6 100644 --- a/packages/cli/src/utils/layoutAudit.ts +++ b/packages/cli/src/utils/layoutAudit.ts @@ -26,6 +26,9 @@ export type LayoutIssueCode = // Cross-sample rotation finding — a spinning element whose bbox center drifts // because it pivots about the wrong point (bad transformOrigin/svgOrigin). | "rotation_pivot_drift" + // Hub-referenced rotation finding — a gauge/clock/radar pointer whose recovered + // center-of-rotation sits far from the dial's static hub (wrong pivot point). + | "off_pivot_rotation" // Frozen-sweep guard (#U10) — a whole-run meta-finding, not a per-sample // geometry observation; never persistence-tiered (see `applyPersistenceTier`). | "sweep_static" From bf8fd6de99d88a856d7c63bb168706a430d1936e Mon Sep 17 00:00:00 2001 From: xuanru Date: Fri, 24 Jul 2026 08:51:48 +0000 Subject: [PATCH 2/4] fix(lint): overdetermined + rigid-body guards for off_pivot_rotation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-1 blocker: the "clean circle" FP guard was vacuous — with 3 samples any non-collinear points fit a circle at ~0 residual, so arbitrary endpoint deformation was misread as off-hub rotation. Now: - Require an OVERDETERMINED fit: >=5 distinct time-samples, RMS fit residual normalized by radius under a tight tolerance (0.05); non-circular motion is rejected. - Add a RIGID-BODY guard: the two tracked material points must keep a ~constant separation (<=15% of median) across the window; deformation is not flagged. Round-2 follow-ups: - Structural FP guards: hub-validity now requires a sustained majority (>=60%) of frames to carry a hub, resolved as the median center; angle-about-hub is averaged across the window — no more last-frame-only reads. - Align the sampler root selector to the sibling samplers ([data-composition-id][data-width][data-height] || ...). - KEEP IN SYNC headers on both Kåsa circle-fit copies (TS + browser JS). - Wire-shape: collectOffPivotRotationSample returns a time-hoisted OffPivotFrame envelope, matching connector_motion_detached's ConnectorFrame. Tests: rewritten with a rigid-sweep builder; new regressions for the 3-point repro, non-rigid deformation, high-residual scatter, sustained-hub, multi-body suppression, plus direct countAngularBodies unit tests. Fallow: extracted a shared groupBy, a generic sampler-evaluate helper, and a requiredNumbers parser — clearing the complexity + duplication findings; the cross-language Kåsa clone and pre-existing test-scaffold clone are exempted in .fallowrc.jsonc with justification. Co-Authored-By: Claude Opus 4.8 --- .fallowrc.jsonc | 17 ++ packages/cli/src/commands/check.test.ts | 2 +- .../cli/src/commands/layout-audit.browser.js | 19 +- packages/cli/src/utils/checkBrowser.ts | 86 +++---- .../checkPipeline.offPivotRotation.test.ts | 233 ++++++++++++----- packages/cli/src/utils/checkPipeline.ts | 235 ++++++++++++------ packages/cli/src/utils/checkTypes.ts | 17 +- 7 files changed, 416 insertions(+), 193 deletions(-) diff --git a/.fallowrc.jsonc b/.fallowrc.jsonc index 2e47f6d433..1bcd56387b 100644 --- a/.fallowrc.jsonc +++ b/.fallowrc.jsonc @@ -593,6 +593,23 @@ // makes non-trivial. "packages/studio-server/src/helpers/screenshotClip.ts", "packages/studio/vite.browser.ts", + // off_pivot_rotation Kåsa circle fit (feat/needle-pivot-offset-check): + // fitCirclePoints in layout-audit.browser.js and fitCircle in + // checkPipeline.ts are the same least-squares circle fit, but the browser + // copy is injected as a raw string via page.addScriptTag and cannot import + // the Node-side module across puppeteer's serialization boundary. The two + // copies carry matching "KEEP IN SYNC" headers; the duplication is + // intentional and per-language, so it's exempted here rather than faked + // away with cosmetic divergence. + "packages/cli/src/commands/layout-audit.browser.js", + "packages/cli/src/utils/checkPipeline.ts", + // check.test.ts: the fakeDriver-based command tests share a pre-existing + // arrange/act/assert scaffold (runScenario + vi.fn runPipeline + spy + + // createCheckCommand). Adding the required collectOffPivotRotationSample + // stub to the CheckAuditDriver fake shifts line numbers and re-flags that + // inherited clone; consistent with the norm above of leaving parallel + // command-test cases unabstracted. + "packages/cli/src/commands/check.test.ts", ], }, "health": { diff --git a/packages/cli/src/commands/check.test.ts b/packages/cli/src/commands/check.test.ts index a9b2758700..cdac2483ae 100644 --- a/packages/cli/src/commands/check.test.ts +++ b/packages/cli/src/commands/check.test.ts @@ -150,7 +150,7 @@ function fakeDriver(overrides: Partial = {}): CheckAuditDriver collectLayout: vi.fn(async (_time: number, _tolerance: number) => []), collectLayoutGeometry: vi.fn(async () => `geometry-${geometryCallCount++}`), collectRotationSample: vi.fn(async (_time: number) => []), - collectOffPivotRotationSample: vi.fn(async (_time: number) => []), + collectOffPivotRotationSample: vi.fn(async (time: number) => ({ time, samples: [] })), collectGeometryCandidates: vi.fn(async () => []), collectMotionFrame: vi.fn(async (time: number) => ({ time, data: {}, liveness: {} })), anchorMotionIssues: vi.fn(async (issues: LayoutIssue[]) => diff --git a/packages/cli/src/commands/layout-audit.browser.js b/packages/cli/src/commands/layout-audit.browser.js index 20c7d9c493..1ee9085dd4 100644 --- a/packages/cli/src/commands/layout-audit.browser.js +++ b/packages/cli/src/commands/layout-audit.browser.js @@ -1572,6 +1572,10 @@ return false; } + // KEEP IN SYNC with `fitCircle` in packages/cli/src/utils/checkPipeline.ts — + // this browser copy resolves arc-drawn dial hubs and is injected as a raw + // string (no import across the puppeteer boundary), so the Kåsa math is + // intentionally duplicated per-language. Any change must land in both copies. function fitCirclePoints(points) { const count = points.length; if (count < 3) return null; @@ -1602,10 +1606,12 @@ const cx = uc + meanX; const cy = vc + meanY; const radius = Math.sqrt(uc * uc + vc * vc + (suu + svv) / count); - let residual = 0; - for (const point of points) - residual += Math.abs(Math.hypot(point.x - cx, point.y - cy) - radius); - return { cx, cy, radius, residual: residual / count }; + let squaredError = 0; + for (const point of points) { + const delta = Math.hypot(point.x - cx, point.y - cy) - radius; + squaredError += delta * delta; + } + return { cx, cy, radius, residual: Math.sqrt(squaredError / count) }; } // Fallback for dials drawn as arc rather than rings: sample @@ -1662,7 +1668,10 @@ } window.__hyperframesOffPivotRotationSample = function collectOffPivotRotationSample() { - const root = document.querySelector("[data-composition-id]") || document.body; + const root = + document.querySelector("[data-composition-id][data-width][data-height]") || + document.querySelector("[data-composition-id]") || + document.body; const samples = []; const hubCache = new Map(); const CANDIDATE_CAP = 60; diff --git a/packages/cli/src/utils/checkBrowser.ts b/packages/cli/src/utils/checkBrowser.ts index a4f7984677..d392e82677 100644 --- a/packages/cli/src/utils/checkBrowser.ts +++ b/packages/cli/src/utils/checkBrowser.ts @@ -42,6 +42,7 @@ import type { ContrastCapture, GeometryCandidateRequest, MotionSpecResolution, + OffPivotFrame, OffPivotRotationSample, RotationSample, RunAuditGrid, @@ -472,13 +473,20 @@ async function collectLayoutGeometry(page: Page): Promise { }); } -async function collectRotationSample(page: Page, time: number): Promise { - const raw = await page.evaluate(() => { - const sample = Reflect.get(window, "__hyperframesRotationSample"); +/** Invoke a `window.__hyperframes*` sampler injected by layout-audit.browser.js + * and return its array result (or [] when absent / non-array). Shared by the + * per-frame sample collectors so the page.evaluate boilerplate lives once. */ +async function evaluateSampler(page: Page, globalName: string): Promise { + return page.evaluate((name) => { + const sample = Reflect.get(window, name); if (typeof sample !== "function") return []; const result = Reflect.apply(sample, window, []); return Array.isArray(result) ? result : []; - }); + }, globalName); +} + +async function collectRotationSample(page: Page, time: number): Promise { + const raw = await evaluateSampler(page, "__hyperframesRotationSample"); return raw.flatMap((value) => parseRotationSample(value, time)); } @@ -496,55 +504,47 @@ function parseRotationSample(value: unknown, time: number): RotationSample[] { return [{ time, selector, cx, cy, w, h, angle }]; } -async function collectOffPivotRotationSample( - page: Page, - time: number, -): Promise { - const raw = await page.evaluate(() => { - const sample = Reflect.get(window, "__hyperframesOffPivotRotationSample"); - if (typeof sample !== "function") return []; - const result = Reflect.apply(sample, window, []); - return Array.isArray(result) ? result : []; - }); - return raw.flatMap((value) => parseOffPivotRotationSample(value, time)); +async function collectOffPivotRotationSample(page: Page, time: number): Promise { + const raw = await evaluateSampler(page, "__hyperframesOffPivotRotationSample"); + return { time, samples: raw.flatMap(parseOffPivotRotationSample) }; +} + +/** Read every named key as a finite number; null if ANY is missing/non-finite. + * The mapped return type keeps each field a plain `number` (not `number | + * undefined`) so callers read `nums.ax` without re-narrowing. */ +function requiredNumbers( + value: Record, + keys: readonly K[], +): { [P in K]: number } | null { + const out = {} as { [P in K]: number }; + for (const key of keys) { + const num = numberValue(value, key); + if (num === null) return null; + out[key] = num; + } + return out; } -function parseOffPivotRotationSample(value: unknown, time: number): OffPivotRotationSample[] { +const OFF_PIVOT_REQUIRED_NUMBERS = ["ax", "ay", "bx", "by", "len", "angle", "hubCount"] as const; + +function parseOffPivotRotationSample(value: unknown): OffPivotRotationSample[] { if (!isRecord(value)) return []; const selector = stringValue(value, "selector"); - const ax = numberValue(value, "ax"); - const ay = numberValue(value, "ay"); - const bx = numberValue(value, "bx"); - const by = numberValue(value, "by"); - const len = numberValue(value, "len"); - const angle = numberValue(value, "angle"); - const hubCount = numberValue(value, "hubCount"); - if ( - !selector || - ax === null || - ay === null || - bx === null || - by === null || - len === null || - angle === null || - hubCount === null - ) { - return []; - } + const nums = requiredNumbers(value, OFF_PIVOT_REQUIRED_NUMBERS); + if (!selector || !nums) return []; return [ { - time, selector, - ax, - ay, - bx, - by, - len, - angle, + ax: nums.ax, + ay: nums.ay, + bx: nums.bx, + by: nums.by, + len: nums.len, + angle: nums.angle, hx: numberValue(value, "hx"), hy: numberValue(value, "hy"), hr: numberValue(value, "hr"), - hubCount, + hubCount: nums.hubCount, }, ]; } diff --git a/packages/cli/src/utils/checkPipeline.offPivotRotation.test.ts b/packages/cli/src/utils/checkPipeline.offPivotRotation.test.ts index c891b7e37f..7c07fe03a9 100644 --- a/packages/cli/src/utils/checkPipeline.offPivotRotation.test.ts +++ b/packages/cli/src/utils/checkPipeline.offPivotRotation.test.ts @@ -1,10 +1,14 @@ import { describe, expect, it } from "vitest"; -import { detectOffPivotRotation } from "./checkPipeline.js"; -import type { OffPivotRotationSample } from "./checkTypes.js"; +import { countAngularBodies, detectOffPivotRotation } from "./checkPipeline.js"; +import type { OffPivotFrame, OffPivotRotationSample } from "./checkTypes.js"; + +/** A sample carrying its frame time, for test ergonomics; {@link toFrames} + * re-hoists the time into the on-the-wire {@link OffPivotFrame} envelope. */ +type TimedSample = OffPivotRotationSample & { time: number }; /** One needle sample; defaults describe a sizable pointer on a hub at (500,500). */ -function sample(overrides: Partial = {}): OffPivotRotationSample { +function sample(overrides: Partial = {}): TimedSample { return { time: 0, selector: "#needle", @@ -22,37 +26,83 @@ function sample(overrides: Partial = {}): OffPivotRotati }; } -/** Endpoint positions on a circle of `radius` about `(cx,cy)` at 0/90/180°. */ -function orbit(cx: number, cy: number, radius: number): Array<[number, number]> { - return [ - [cx + radius, cy], - [cx, cy + radius], - [cx - radius, cy], - ]; +/** Merge per-selector timed samples into the time-hoisted frame envelope the + * detector consumes (all samples sharing a time land in one frame). */ +function toFrames(...groups: TimedSample[][]): OffPivotFrame[] { + const byTime = new Map(); + for (const { time, ...rest } of groups.flat()) { + const frame = byTime.get(time); + if (frame) frame.push(rest); + else byTime.set(time, [rest]); + } + return [...byTime.entries()] + .sort(([a], [b]) => a - b) + .map(([time, samples]) => ({ time, samples })); +} + +interface SweepOptions { + /** Center the endpoints actually orbit (the recovered center-of-rotation). */ + cx: number; + cy: number; + /** Screen hub the check compares the recovered center against. */ + hx?: number; + hy?: number; + selector?: string; + frames?: number; + spreadDeg?: number; + /** Outer/inner material-point radii; both share one angle so the pointer is + * rigid (constant separation |a-b| = rOuter - rInner). */ + rOuter?: number; + rInner?: number; + /** Per-frame inner-radius growth — breaks rigidity (endpoint deformation). */ + innerGrowth?: number; + hubCount?: number; } -/** A group that SHOULD fire: the pointer sweeps (0→90→180) about a recovered - * center at (500,300) — 200px above the dial hub at (500,500). Two material - * endpoints trace concentric circles about that wrong center. */ -function offPivotIndicator(selector = "#needle"): OffPivotRotationSample[] { - const a = orbit(500, 300, 100); - const b = orbit(500, 300, 40); - return [0, 1, 2].map((i) => - sample({ +/** A rigid pointer swept about `(cx,cy)`: two material points on one radial line + * orbit that center, so their separation is constant (rigid body). */ +function sweep(options: SweepOptions): TimedSample[] { + const { + cx, + cy, + hx = 500, + hy = 500, + selector = "#needle", + frames = 6, + spreadDeg = 180, + rOuter = 100, + rInner = 40, + innerGrowth = 0, + hubCount = 3, + } = options; + return Array.from({ length: frames }, (_, i) => { + const deg = frames <= 1 ? 0 : (spreadDeg * i) / (frames - 1); + const rad = (deg * Math.PI) / 180; + const inner = rInner + innerGrowth * i; + const ax = cx + rOuter * Math.cos(rad); + const ay = cy + rOuter * Math.sin(rad); + const bx = cx + inner * Math.cos(rad); + const by = cy + inner * Math.sin(rad); + return sample({ selector, time: i, - angle: i * 90, - ax: a[i]?.[0] ?? 0, - ay: a[i]?.[1] ?? 0, - bx: b[i]?.[0] ?? 0, - by: b[i]?.[1] ?? 0, - }), - ); + angle: deg, + ax, + ay, + bx, + by, + len: Math.hypot(ax - bx, ay - by), + hx, + hy, + hubCount, + }); + }); } describe("detectOffPivotRotation", () => { it("fires when the recovered center-of-rotation is far from the dial hub", () => { - const findings = detectOffPivotRotation(offPivotIndicator()); + // Rigid pointer orbits (500,300) — 200px above the hub at (500,500). + const findings = detectOffPivotRotation(toFrames(sweep({ cx: 500, cy: 300 }))); expect(findings).toHaveLength(1); const [f] = findings; expect(f?.code).toBe("off_pivot_rotation"); @@ -62,62 +112,127 @@ describe("detectOffPivotRotation", () => { expect(f?.fixHint).toContain("hub"); }); - it("does not fire with fewer than the minimum samples", () => { - expect(detectOffPivotRotation(offPivotIndicator().slice(0, 2))).toHaveLength(0); + it("does not fire on a correctly-hubbed needle (center-of-rotation on the hub)", () => { + expect(detectOffPivotRotation(toFrames(sweep({ cx: 500, cy: 500 })))).toHaveLength(0); }); - it("does not fire when the pointer barely sweeps (fixed tilt)", () => { - const group = offPivotIndicator().map((s, i) => ({ ...s, angle: i * 3 })); - expect(detectOffPivotRotation(group)).toHaveLength(0); + it("does not fire with fewer than the minimum (overdetermined) samples", () => { + // Blocker-1 core: any 3 non-collinear points fit a circle with ~0 residual, + // so a 3-sample "clean circle" guard is vacuous. Below 5 samples we refuse. + expect(detectOffPivotRotation(toFrames(sweep({ cx: 500, cy: 300, frames: 4 })))).toHaveLength( + 0, + ); + expect(detectOffPivotRotation(toFrames(sweep({ cx: 500, cy: 300, frames: 5 })))).toHaveLength( + 1, + ); }); - it("does not fire when no dial hub is resolvable", () => { - const group = offPivotIndicator().map((s) => ({ ...s, hx: null, hy: null, hubCount: 0 })); - expect(detectOffPivotRotation(group)).toHaveLength(0); + it("does not fire on arbitrary 3-point endpoint deformation (the reviewer repro)", () => { + // Three arbitrary non-collinear endpoint positions: geometrically they fit a + // circle exactly, but there are too few samples to be an overdetermined fit. + const group = [ + sample({ time: 0, angle: 0, ax: 600, ay: 300, bx: 540, by: 300 }), + sample({ time: 1, angle: 90, ax: 505, ay: 402, bx: 500, by: 341 }), + sample({ time: 2, angle: 180, ax: 402, ay: 305, bx: 461, by: 299 }), + ]; + expect(detectOffPivotRotation(toFrames(group))).toHaveLength(0); }); - it("does not fire when the hub has too few concentric circles", () => { - const group = offPivotIndicator().map((s) => ({ ...s, hubCount: 1 })); - expect(detectOffPivotRotation(group)).toHaveLength(0); + it("does not fire when the tracked points deform (non-rigid) even with >=5 samples", () => { + // Endpoint A traces a clean wide circle about (500,300) — a naive fit would + // recover a 200px-off center and flag it — but the inner point drifts outward + // each frame, so the figure is deforming, not rotating rigidly. + const group = sweep({ cx: 500, cy: 300, frames: 6, innerGrowth: 8 }); + expect(detectOffPivotRotation(toFrames(group))).toHaveLength(0); }); - it("does not fire on a correctly-hubbed needle (center-of-rotation on the hub)", () => { - const a = orbit(500, 500, 100); - const b = orbit(500, 500, 40); - const group = [0, 1, 2].map((i) => + it("does not fire when the >=5-sample trajectory is not a clean circle (high residual)", () => { + // Rigid separation (len constant) but the points jitter along a rough path, + // not one circle — the overdetermined RMS-residual guard rejects it. + const scatter = [ + [600, 300], + [520, 420], + [470, 260], + [610, 350], + [430, 400], + [560, 250], + ]; + const group = scatter.map(([ax, ay], i) => sample({ time: i, - angle: i * 90, - ax: a[i]?.[0] ?? 0, - ay: a[i]?.[1] ?? 0, - bx: b[i]?.[0] ?? 0, - by: b[i]?.[1] ?? 0, + angle: i * 40, + ax: ax ?? 0, + ay: ay ?? 0, + bx: (ax ?? 0) - 60, + by: ay ?? 0, + len: 60, }), ); - expect(detectOffPivotRotation(group)).toHaveLength(0); + expect(detectOffPivotRotation(toFrames(group))).toHaveLength(0); }); - it("does not fire when the endpoint trajectory is not a clean circle", () => { - const group = [ - sample({ time: 0, angle: 0, ax: 600, ay: 300, bx: 540, by: 300 }), - sample({ time: 1, angle: 90, ax: 505, ay: 402, bx: 500, by: 341 }), - sample({ time: 2, angle: 180, ax: 402, ay: 305, bx: 461, by: 299 }), - ].map((s) => ({ ...s, ax: s.ax + (s.time === 1 ? 180 : 0) })); - expect(detectOffPivotRotation(group)).toHaveLength(0); + it("does not fire when the pointer barely sweeps (fixed tilt)", () => { + expect( + detectOffPivotRotation(toFrames(sweep({ cx: 500, cy: 300, spreadDeg: 10 }))), + ).toHaveLength(0); }); - it("collapses nested candidates on the same hub to a single finding", () => { - const group = offPivotIndicator().map((s) => ({ ...s, len: 200 })); - const child = offPivotIndicator("#needle > path:nth-of-type(1)").map((s) => ({ + it("does not fire when no dial hub is resolvable", () => { + const group = sweep({ cx: 500, cy: 300 }).map((s) => ({ ...s, - len: 150, + hx: null, + hy: null, + hubCount: 0, })); - const findings = detectOffPivotRotation([...group, ...child]); + expect(detectOffPivotRotation(toFrames(group))).toHaveLength(0); + }); + + it("does not fire when the hub has too few concentric circles", () => { + const group = sweep({ cx: 500, cy: 300 }).map((s) => ({ ...s, hubCount: 1 })); + expect(detectOffPivotRotation(toFrames(group))).toHaveLength(0); + }); + + it("requires the hub to be present for a sustained majority, not just the last frame", () => { + // Hub resolves only on the final frame; every earlier frame lacks it. A + // single-frame hub is not a reliable anchor — suppression is structural. + const group = sweep({ cx: 500, cy: 300 }).map((s, i, all) => + i === all.length - 1 ? s : { ...s, hx: null, hy: null, hubCount: 0 }, + ); + expect(detectOffPivotRotation(toFrames(group))).toHaveLength(0); + }); + + it("collapses nested candidates on the same hub to a single finding", () => { + const group = sweep({ cx: 500, cy: 300 }); + const child = sweep({ cx: 500, cy: 300, selector: "#needle > path:nth-of-type(1)" }); + const findings = detectOffPivotRotation(toFrames(group, child)); expect(findings).toHaveLength(1); expect(findings[0]?.selector).toBe("#needle"); }); + it("suppresses multiple distinct bodies orbiting one hub (orbit/atom system)", () => { + // Two rigid bodies at DIFFERENT angular positions about the same hub — an + // orbit/radial system, not a single dial pointer. + const bodyA = sweep({ cx: 700, cy: 500, selector: "#electron-a" }); + const bodyB = sweep({ cx: 300, cy: 500, selector: "#electron-b" }); + expect(detectOffPivotRotation(toFrames(bodyA, bodyB))).toHaveLength(0); + }); + it("returns nothing for an empty sample set", () => { expect(detectOffPivotRotation([])).toHaveLength(0); }); }); + +describe("countAngularBodies (multi-body / orbit-atom suppression guard)", () => { + it("collapses tightly-clustered angles to a single body", () => { + expect(countAngularBodies([10, 12, 13, -8])).toBe(1); + }); + + it("counts well-separated angular positions as distinct bodies", () => { + expect(countAngularBodies([0, 90, 180])).toBe(3); + }); + + it("wraps around 360 so 350 and 5 are the same body", () => { + expect(countAngularBodies([350, 5])).toBe(1); + expect(countAngularBodies([])).toBe(0); + }); +}); diff --git a/packages/cli/src/utils/checkPipeline.ts b/packages/cli/src/utils/checkPipeline.ts index 13eeb7ed26..367fdf1d4f 100644 --- a/packages/cli/src/utils/checkPipeline.ts +++ b/packages/cli/src/utils/checkPipeline.ts @@ -48,6 +48,7 @@ import type { ContrastAuditEntry, GeometryCandidateRequest, MotionSpecResolution, + OffPivotFrame, OffPivotRotationSample, RotationSample, } from "./checkTypes.js"; @@ -194,9 +195,10 @@ interface GridSamples { /** Every rotatable element's geometry at each layout sample; grouped by * selector after the run to detect rotation_pivot_drift. */ rotationSamples: RotationSample[]; - /** Every elongated rotating SVG figure's material-point geometry + dial hub at - * each layout sample; grouped by selector to detect off_pivot_rotation. */ - indicatorSamples: OffPivotRotationSample[]; + /** One time-hoisted frame of every elongated rotating SVG figure's + * material-point geometry + dial hub per layout sample; flattened by selector + * to detect off_pivot_rotation. */ + indicatorFrames: OffPivotFrame[]; } interface GeometrySeen { @@ -369,7 +371,7 @@ async function collectGridSamples( contrastMs: 0, geometrySignatures: [], rotationSamples: [], - indicatorSamples: [], + indicatorFrames: [], }; for (const time of mergeSampleTimes(grid.layoutSamples, motion.times)) { await driver.seek(time); @@ -383,7 +385,7 @@ async function collectGridSamples( issuesAtTime.push(...layoutIssues); collected.geometrySignatures.push(await driver.collectLayoutGeometry()); collected.rotationSamples.push(...(await driver.collectRotationSample(time))); - collected.indicatorSamples.push(...(await driver.collectOffPivotRotationSample(time))); + collected.indicatorFrames.push(await driver.collectOffPivotRotationSample(time)); } if (canvas) { const geometryIssues = await collectGeometryAt( @@ -554,14 +556,21 @@ function rotationDriftFinding( }; } -function groupRotationSamplesBySelector(samples: RotationSample[]): Map { - const bySelector = new Map(); - for (const sample of samples) { - const group = bySelector.get(sample.selector); - if (group) group.push(sample); - else bySelector.set(sample.selector, [sample]); +/** Bucket items by a derived key, preserving insertion order within each bucket. */ +function groupBy(items: T[], keyOf: (item: T) => string): Map { + const byKey = new Map(); + for (const item of items) { + const group = byKey.get(keyOf(item)); + if (group) group.push(item); + else byKey.set(keyOf(item), [item]); } - return bySelector; + return byKey; +} + +/** Bucket time-samples by their CSS selector so each element's trajectory can be + * analyzed independently. Shared by rotation_pivot_drift and off_pivot_rotation. */ +function groupBySelector(samples: T[]): Map { + return groupBy(samples, (sample) => sample.selector); } /** Enough samples to establish a spin trajectory (one frame can't). */ @@ -634,7 +643,7 @@ export function detectRotationPivotDrift( ): AnchoredLayoutIssue[] { const findings: AnchoredLayoutIssue[] = []; const viewportFloor = ROTATION_DRIFT_VIEWPORT_FRACTION * Math.min(canvas.width, canvas.height); - for (const group of groupRotationSamplesBySelector(samples).values()) { + for (const group of groupBySelector(samples).values()) { if (!isRotationDriftCandidate(group)) continue; const medianSize = median(group.map((s) => Math.max(s.w, s.h))); const threshold = Math.max(ROTATION_DRIFT_SIZE_FRACTION * medianSize, viewportFloor); @@ -649,12 +658,25 @@ export function detectRotationPivotDrift( // off_pivot_rotation thresholds. A pointer must actually sweep, sit on a // resolvable dial hub, and its recovered center-of-rotation must land far from // that hub relative to the pointer's own length. -const INDICATOR_MIN_SAMPLES = 3; +// +// The fit MUST be OVERDETERMINED: any 3 non-collinear points fit a circle with +// ~zero residual, so a 3-sample "clean circle" guard is vacuous — arbitrary +// endpoint deformation or translation would read as a perfect orbit. Require +// >=5 distinct time-samples so the least-squares residual actually measures +// whether the trajectory lies on one circle. +const INDICATOR_MIN_SAMPLES = 5; const INDICATOR_MIN_ANGLE_SPREAD_DEG = 20; const INDICATOR_MIN_HUB_CIRCLES = 2; -// A circle fit whose residual exceeds this fraction of the orbit radius is not -// a clean rotation (jitter/scatter) — skip rather than risk a bad center. -const INDICATOR_MAX_FIT_RESIDUAL_FRACTION = 0.25; +// The Kåsa fit's RMS residual normalized by the fitted radius. With >=5 samples +// a genuine rotation lands well under this; non-circular motion (deformation, +// translation, skew, jitter/scatter) blows past it, so we skip rather than +// misread it as an off-hub pivot. +const INDICATOR_MAX_FIT_RESIDUAL_FRACTION = 0.05; +// Rigid-body guard: rotation preserves shape, so the pairwise distance between +// the two tracked material points must stay ~constant. If it swings more than +// this fraction of its median across the window the element is deforming, not +// rotating rigidly — do not flag. +const INDICATOR_MAX_RIGID_LEN_VARIATION = 0.15; // Drift beyond this fraction of the pointer length is a wrong pivot. A correctly // hubbed needle recovers a center within a few px of the hub (≈0); a base/edge // pivot mistake puts the recovered center a large fraction of the needle away. @@ -668,7 +690,13 @@ interface CircleFit { } /** Kåsa algebraic circle fit; null when the points are near-collinear/degenerate - * (no stable center recoverable). */ + * (no stable center recoverable). + * + * KEEP IN SYNC with `fitCirclePoints` in + * packages/cli/src/commands/layout-audit.browser.js — the browser sampler needs + * the same fit to resolve arc-drawn dial hubs, but it is injected as a raw + * string (no import across the puppeteer boundary), so the math is intentionally + * duplicated per-language. Any change to the algorithm must land in both. */ function fitCircle(points: Array<{ x: number; y: number }>): CircleFit | null { const count = points.length; if (count < 3) return null; @@ -701,29 +729,57 @@ function fitCircle(points: Array<{ x: number; y: number }>): CircleFit | null { const cx = uc + meanX; const cy = vc + meanY; const radius = Math.sqrt(uc * uc + vc * vc + (suu + svv) / count); - let residual = 0; + // RMS radial residual: penalizes any point straying off the fitted circle + // more sharply than a mean-absolute error, so an overdetermined fit exposes + // non-circular trajectories instead of averaging them away. + let squaredError = 0; for (const point of points) { - residual += Math.abs(Math.hypot(point.x - cx, point.y - cy) - radius); + const delta = Math.hypot(point.x - cx, point.y - cy) - radius; + squaredError += delta * delta; } - return { cx, cy, radius, residual: residual / count }; + return { cx, cy, radius, residual: Math.sqrt(squaredError / count) }; } -function groupIndicatorSamplesBySelector( - samples: OffPivotRotationSample[], -): Map { - const bySelector = new Map(); - for (const sample of samples) { - const group = bySelector.get(sample.selector); - if (group) group.push(sample); - else bySelector.set(sample.selector, [sample]); - } - return bySelector; +/** A dial hub that is present for a SUSTAINED majority of the sampled window, + * not just the final frame — a transient single-frame hub is not a reliable + * anchor. The representative center is the median across the frames that carry + * one, so a stray frame can't yank it. */ +const INDICATOR_MIN_HUB_PRESENCE_FRACTION = 0.6; + +interface ResolvedHub { + hx: number; + hy: number; +} + +function resolveHub(group: OffPivotRotationSample[]): ResolvedHub | null { + const present = group.filter( + (s) => s.hx !== null && s.hy !== null && s.hubCount >= INDICATOR_MIN_HUB_CIRCLES, + ); + if (present.length < INDICATOR_MIN_HUB_PRESENCE_FRACTION * group.length) return null; + const hx = median(present.map((s) => s.hx as number)); + const hy = median(present.map((s) => s.hy as number)); + return { hx, hy }; +} + +/** Rigid-body guard: a rotation preserves the figure's shape, so the distance + * between the two tracked material points stays ~constant. Arbitrary endpoint + * deformation (morphing, stretching) changes it — reject so it isn't misread as + * an off-hub orbit. */ +function isRigidBody(group: OffPivotRotationSample[]): boolean { + const lengths = group.map((s) => s.len).filter((len) => len > 0); + if (lengths.length < group.length) return false; + const mid = median(lengths); + if (mid <= 0) return false; + const spread = Math.max(...lengths) - Math.min(...lengths); + return spread <= INDICATOR_MAX_RIGID_LEN_VARIATION * mid; } /** Recover the center-of-rotation from whichever major-axis endpoint traces the * larger orbit (more movement → more numerically stable), rejecting scattered - * fits. */ + * fits. The fit is OVERDETERMINED (>=INDICATOR_MIN_SAMPLES points) so a high RMS + * residual genuinely means the trajectory is not one circle. */ function recoverPivot(group: OffPivotRotationSample[]): CircleFit | null { + if (group.length < INDICATOR_MIN_SAMPLES) return null; const endpointA = group.map((s) => ({ x: s.ax, y: s.ay })); const endpointB = group.map((s) => ({ x: s.bx, y: s.by })); const fits = [fitCircle(endpointA), fitCircle(endpointB)].filter( @@ -736,8 +792,13 @@ function recoverPivot(group: OffPivotRotationSample[]): CircleFit | null { return best; } +/** An off-pivot sample with its frame time reattached for grouping/annotation. + * The wire shape ({@link OffPivotFrame}) hoists time to the frame; the detector + * flattens back to timed samples so the last frame's time anchors the finding. */ +type TimedIndicatorSample = OffPivotRotationSample & { time: number }; + function offPivotRotationFinding( - group: OffPivotRotationSample[], + group: TimedIndicatorSample[], drift: number, ): AnchoredLayoutIssue | null { const last = group[group.length - 1]; @@ -777,10 +838,13 @@ function offPivotRotationFinding( * circle fit recovers the true center-of-rotation, which is then compared to the * dial's static hub (the screen point shared by the most non-rotating circles). * - * FP guards are strict: requires real sweep (angle varies), a resolvable hub - * (≥2 concentric static circles), a clean circle fit (low residual), and drift - * exceeding a fraction of the pointer's own length. When no hub reference can be - * resolved it does not fire, so a lone decorative rotator can't trip it. + * FP guards are strict and STRUCTURAL (evaluated across the whole sampled + * window, not a single frame): requires real sweep (angle varies), a hub present + * for a sustained majority of frames (≥2 concentric static circles), a + * rigid-body trajectory (the two tracked points keep a constant separation), an + * OVERDETERMINED circle fit (≥5 samples, low RMS residual), and drift exceeding a + * fraction of the pointer's own length. When no sustained hub reference resolves + * it does not fire, so a lone decorative rotator can't trip it. */ interface IndicatorCandidate { hubKey: string; @@ -791,8 +855,9 @@ interface IndicatorCandidate { } /** Distinct angular positions about the hub among a set of candidates (nested - * blade parts share one angle; orbiting bodies spread to several). */ -function countAngularBodies(angles: number[]): number { + * blade parts share one angle; orbiting bodies spread to several). Exported for + * direct unit coverage of the multi-body/orbit-atom suppression guard. */ +export function countAngularBodies(angles: number[]): number { const clusters: number[] = []; for (const angle of angles) { if ( @@ -804,47 +869,43 @@ function countAngularBodies(angles: number[]): number { return clusters.length; } -export function detectOffPivotRotation(samples: OffPivotRotationSample[]): AnchoredLayoutIssue[] { - const candidates: IndicatorCandidate[] = []; - for (const group of groupIndicatorSamplesBySelector(samples).values()) { - if (group.length < INDICATOR_MIN_SAMPLES) continue; - if (maxAngleSpread(group.map((s) => s.angle)) <= INDICATOR_MIN_ANGLE_SPREAD_DEG) continue; - const last = group[group.length - 1]; - if ( - !last || - last.hx === null || - last.hy === null || - last.hubCount < INDICATOR_MIN_HUB_CIRCLES - ) { - continue; - } - const pivot = recoverPivot(group); - if (!pivot) continue; - const drift = Math.hypot(pivot.cx - last.hx, pivot.cy - last.hy); - const length = median(group.map((s) => s.len)); - const angleAboutHub = - (Math.atan2((last.ay + last.by) / 2 - last.hy, (last.ax + last.bx) / 2 - last.hx) * 180) / - Math.PI; - const hubKey = `${Math.round(last.hx / 5)}:${Math.round(last.hy / 5)}`; - const eligible = drift > INDICATOR_DRIFT_LENGTH_FRACTION * length; - candidates.push({ - hubKey, - angleAboutHub, - drift, - length, - finding: eligible ? offPivotRotationFinding(group, drift) : null, - }); - } - // Per hub: multiple bodies at distinct angular positions = an orbit/atom/radial - // system, not a single dial pointer — suppress. A real pointer (group + nested - // blades) collapses to one angular cluster. Otherwise emit one finding per hub, - // keeping the longest pointer. - const byHub = new Map(); - for (const candidate of candidates) { - const group = byHub.get(candidate.hubKey); - if (group) group.push(candidate); - else byHub.set(candidate.hubKey, [candidate]); - } +/** The pointer's angular position about the resolved hub, averaged across the + * window so the multi-body clustering compares a stable per-body angle rather + * than a single-frame snapshot. */ +function angleAboutHub(group: OffPivotRotationSample[], hub: ResolvedHub): number { + const midX = median(group.map((s) => (s.ax + s.bx) / 2)); + const midY = median(group.map((s) => (s.ay + s.by) / 2)); + return (Math.atan2(midY - hub.hy, midX - hub.hx) * 180) / Math.PI; +} + +/** Gate one element's samples into a candidate, applying every structural FP + * guard. Returns null when the element is not an off-pivot-able dial pointer. */ +function buildIndicatorCandidate(group: TimedIndicatorSample[]): IndicatorCandidate | null { + if (group.length < INDICATOR_MIN_SAMPLES) return null; + if (maxAngleSpread(group.map((s) => s.angle)) <= INDICATOR_MIN_ANGLE_SPREAD_DEG) return null; + const hub = resolveHub(group); + if (!hub) return null; + if (!isRigidBody(group)) return null; + const pivot = recoverPivot(group); + if (!pivot) return null; + const drift = Math.hypot(pivot.cx - hub.hx, pivot.cy - hub.hy); + const length = median(group.map((s) => s.len)); + const eligible = drift > INDICATOR_DRIFT_LENGTH_FRACTION * length; + return { + hubKey: `${Math.round(hub.hx / 5)}:${Math.round(hub.hy / 5)}`, + angleAboutHub: angleAboutHub(group, hub), + drift, + length, + finding: eligible ? offPivotRotationFinding(group, drift) : null, + }; +} + +/** Per hub: multiple bodies at distinct angular positions = an orbit/atom/radial + * system, not a single dial pointer — suppress. A real pointer (group + nested + * blades) collapses to one angular cluster. Otherwise emit one finding per hub, + * keeping the longest pointer. */ +function selectHubFindings(candidates: IndicatorCandidate[]): AnchoredLayoutIssue[] { + const byHub = groupBy(candidates, (candidate) => candidate.hubKey); const findings: AnchoredLayoutIssue[] = []; for (const group of byHub.values()) { if (countAngularBodies(group.map((c) => c.angleAboutHub)) >= 2) continue; @@ -859,6 +920,18 @@ export function detectOffPivotRotation(samples: OffPivotRotationSample[]): Ancho return findings; } +export function detectOffPivotRotation(frames: OffPivotFrame[]): AnchoredLayoutIssue[] { + const timed: TimedIndicatorSample[] = frames.flatMap((frame) => + frame.samples.map((sample) => ({ ...sample, time: frame.time })), + ); + const candidates: IndicatorCandidate[] = []; + for (const group of groupBySelector(timed).values()) { + const candidate = buildIndicatorCandidate(group); + if (candidate) candidates.push(candidate); + } + return selectHubFindings(candidates); +} + /** Error-severity findings with real geometry become labeled overview boxes. * Contrast failures are annotated separately by the driver itself, since * they're only known once contrast measurement for this sample completes. */ @@ -898,7 +971,7 @@ export async function runAuditGrid( collected.rotationSamples, await driver.getCanvas(), ); - const offPivotFindings = detectOffPivotRotation(collected.indicatorSamples); + const offPivotFindings = detectOffPivotRotation(collected.indicatorFrames); const contrast = buildContrastResults(collected.contrastEntries); return { duration: grid.duration, diff --git a/packages/cli/src/utils/checkTypes.ts b/packages/cli/src/utils/checkTypes.ts index d0f1dbd986..569051724a 100644 --- a/packages/cli/src/utils/checkTypes.ts +++ b/packages/cli/src/utils/checkTypes.ts @@ -137,9 +137,9 @@ export interface RotationSample { * sample, produced by `__hyperframesOffPivotRotationSample`. `(ax,ay)`/`(bx,by)` are * two fixed material points on the figure (major-axis endpoints) mapped to * screen space, so their cross-frame trajectory recovers the true center of - * rotation; `(hx,hy)` is the dial's static hub and `hr` its radius. */ + * rotation; `(hx,hy)` is the dial's static hub and `hr` its radius. The sample + * time is hoisted to the enclosing {@link OffPivotFrame}, not repeated here. */ export interface OffPivotRotationSample { - time: number; selector: string; ax: number; ay: number; @@ -153,6 +153,14 @@ export interface OffPivotRotationSample { hubCount: number; } +/** All elongated rotating figures at one seeked sample. Time is hoisted to the + * frame (matching {@link ConnectorFrame}) so the per-figure samples don't repeat + * it; frames are accumulated across the grid to detect off_pivot_rotation. */ +export interface OffPivotFrame { + time: number; + samples: OffPivotRotationSample[]; +} + export type MotionSpecResolution = | { kind: "none" } | { kind: "valid"; path: string; spec: MotionSpec } @@ -174,8 +182,9 @@ export interface CheckAuditDriver { * the current seeked state. Accumulated across the grid — see checkPipeline. */ collectRotationSample(time: number): Promise; /** off_pivot_rotation: every elongated rotating SVG figure's material-point - * geometry + resolved dial hub at the current seeked state. */ - collectOffPivotRotationSample(time: number): Promise; + * geometry + resolved dial hub at the current seeked state, as a time-hoisted + * frame envelope. */ + collectOffPivotRotationSample(time: number): Promise; collectGeometryCandidates( time: number, request: GeometryCandidateRequest, From a4f0497a906689dc41e3866eea69b4fddfcc852b Mon Sep 17 00:00:00 2001 From: xuanru Date: Fri, 24 Jul 2026 22:06:27 +0000 Subject: [PATCH 3/4] docs(lint): note off_pivot_rotation scoped-known blind spots Acknowledge two reviewer-flagged edge cases as documented, intentional limitations (per review): recoverPivot keeps the wider-radius fit and misses rather than falling back to the narrower fit; hubKey buckets by rounded screen coords so two dials at ~identical screen position merge. Comment-only; no behavior change. Co-Authored-By: Claude Opus 4.8 --- packages/cli/src/utils/checkPipeline.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/cli/src/utils/checkPipeline.ts b/packages/cli/src/utils/checkPipeline.ts index 367fdf1d4f..88931c3696 100644 --- a/packages/cli/src/utils/checkPipeline.ts +++ b/packages/cli/src/utils/checkPipeline.ts @@ -786,6 +786,10 @@ function recoverPivot(group: OffPivotRotationSample[]): CircleFit | null { (fit): fit is CircleFit => fit !== null, ); if (fits.length === 0) return null; + // Scoped-known blind spot: we keep the wider-radius endpoint fit and, if it + // fails the residual gate below, return null rather than falling back to the + // narrower fit. Deliberately conservative — a missed pointer (false negative) + // over threading both fits and risking a spurious pivot on a noisy arc. const best = fits.reduce((widest, fit) => (fit.radius > widest.radius ? fit : widest)); if (best.radius <= 0) return null; if (best.residual > INDICATOR_MAX_FIT_RESIDUAL_FRACTION * best.radius) return null; @@ -892,6 +896,10 @@ function buildIndicatorCandidate(group: TimedIndicatorSample[]): IndicatorCandid const length = median(group.map((s) => s.len)); const eligible = drift > INDICATOR_DRIFT_LENGTH_FRACTION * length; return { + // Scoped-known blind spot: hubs bucketed by rounded screen coords, so two + // separate dials whose screen hubs land in the same 5px bucket merge into + // one multi-body group (rare — distinct dials stacked at ~identical screen + // position). Key by owning identity if that pattern shows up. hubKey: `${Math.round(hub.hx / 5)}:${Math.round(hub.hy / 5)}`, angleAboutHub: angleAboutHub(group, hub), drift, From 962f73781b869d4fdd4d4f42c0615a7c0202a360 Mon Sep 17 00:00:00 2001 From: xuanru Date: Fri, 24 Jul 2026 22:16:48 +0000 Subject: [PATCH 4/4] fix(lint): count only eligible candidates in off_pivot multi-body suppression A lone off-pivot hand on a multi-hand clock/dial was silently suppressed when its sibling hands were correctly hubbed, because the multi-body guard counted all candidates including finding===null (correctly-hubbed) ones. Count angular bodies over eligible findings only. Co-Authored-By: Claude Opus 4.8 --- .../utils/checkPipeline.offPivotRotation.test.ts | 14 ++++++++++++++ packages/cli/src/utils/checkPipeline.ts | 7 ++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/utils/checkPipeline.offPivotRotation.test.ts b/packages/cli/src/utils/checkPipeline.offPivotRotation.test.ts index 7c07fe03a9..cba3a0eeb9 100644 --- a/packages/cli/src/utils/checkPipeline.offPivotRotation.test.ts +++ b/packages/cli/src/utils/checkPipeline.offPivotRotation.test.ts @@ -217,6 +217,20 @@ describe("detectOffPivotRotation", () => { expect(detectOffPivotRotation(toFrames(bodyA, bodyB))).toHaveLength(0); }); + it("flags one off-pivot hand on a multi-hand clock whose sibling hands are correctly hubbed", () => { + // Analog clock: three hands on one hub at (500,500). Two are correctly + // hubbed (center-of-rotation on the hub → not eligible findings); one is + // off-pivot (orbits (500,300), 200px above the hub). The multi-body + // suppression must count only ELIGIBLE candidates — the two hubbed hands + // are not orbiting bodies, so the lone off-pivot hand is NOT suppressed. + const offPivot = sweep({ cx: 500, cy: 300, selector: "#hand-second" }); + const hubbedA = sweep({ cx: 500, cy: 500, selector: "#hand-hour" }); + const hubbedB = sweep({ cx: 500, cy: 500, selector: "#hand-minute" }); + const findings = detectOffPivotRotation(toFrames(offPivot, hubbedA, hubbedB)); + expect(findings).toHaveLength(1); + expect(findings[0]?.selector).toBe("#hand-second"); + }); + it("returns nothing for an empty sample set", () => { expect(detectOffPivotRotation([])).toHaveLength(0); }); diff --git a/packages/cli/src/utils/checkPipeline.ts b/packages/cli/src/utils/checkPipeline.ts index 88931c3696..6f5145c8ff 100644 --- a/packages/cli/src/utils/checkPipeline.ts +++ b/packages/cli/src/utils/checkPipeline.ts @@ -916,7 +916,12 @@ function selectHubFindings(candidates: IndicatorCandidate[]): AnchoredLayoutIssu const byHub = groupBy(candidates, (candidate) => candidate.hubKey); const findings: AnchoredLayoutIssue[] = []; for (const group of byHub.values()) { - if (countAngularBodies(group.map((c) => c.angleAboutHub)) >= 2) continue; + // Count angular bodies only over ELIGIBLE (off-pivot) candidates. Sibling + // hands that are correctly hubbed (finding === null) are not orbiting + // bodies, so counting them would falsely suppress a lone off-pivot hand on + // a multi-hand clock/dial whose other hands pivot correctly. + const eligible = group.filter((c) => c.finding !== null); + if (countAngularBodies(eligible.map((c) => c.angleAboutHub)) >= 2) continue; const best = group .filter((c) => c.finding !== null) .reduce(