refactor(cli): single-source-of-truth pass over the check branch

Every duplicated decision gets one owner: rectToBbox lives in checkTypes
(was verbatim in pipeline and browser layers); the audit seek tuning is
one exported AUDIT_SEEK_OPTIONS consumed by check and the deprecated
inspect path; zoom padding/scale defaults export from the capture module
instead of re-literalized in three files; the optional run_id property
is built by one helper across all three telemetry events; check's
--max-transition-samples parsing reuses its own positiveInteger helper;
validate drops a leftover re-export and redundant explicit-default args
go away.

Tests: the contrast candidate round-trip gains a real integration
anchor (the actual browser script eval'd in-page, a wrapper asserting
finish receives the page-script bbox shape) replacing regex-over-source
as the primary guard; the redundant geometry source-golden and a
duplicated deprecation-envelope assertion are dropped.
This commit is contained in:
Miguel Angel Simon Sierra
2026-07-10 13:52:31 -04:00
parent cf7c1d7609
commit cea3458016
13 changed files with 171 additions and 66 deletions
@@ -7,6 +7,15 @@ const SHADER_TRANSITIONS_TIMEOUT_MS = 90_000;
const CAPTURE_SETTLE_MS = 1500;
const PREFERRED_SEEK_TARGET_WAIT_MS = 500;
// The audit-grade seek tuning shared by check and the deprecated inspect/layout:
// bridge+timeline fallback, ordered double-rAF settle, bounded font wait, sleep.
export const AUDIT_SEEK_OPTIONS = {
fallbackToBridgeAndTimelines: true,
animationFrameSettle: "double",
waitForFontsMs: 500,
settleMs: 120,
} as const;
export interface SeekCompositionTimelineOptions {
fallbackToBridgeAndTimelines?: boolean;
waitForPreferredSeekTargetMs?: number;
@@ -305,7 +314,10 @@ export type ZoomTarget =
// Four bare comma-separated numbers is unambiguous — no valid CSS selector
// parses as that shape — so it always means "exact pixel region".
const ZOOM_REGION_PATTERN = /^-?\d+(?:\.\d+)?(?:,-?\d+(?:\.\d+)?){3}$/;
const DEFAULT_ZOOM_PADDING_PX = 24;
export const DEFAULT_ZOOM_PADDING_PX = 24;
// One knob for every zoom/crop consumer (snapshot --zoom-scale default, check's
// finding crops): density of the captured pixels relative to CSS pixels.
export const DEFAULT_ZOOM_SCALE = 3;
/** Parse `snapshot --zoom` into either a CSS selector or an exact pixel region "x,y,w,h". */
export function parseZoomTarget(value: string): ZoomTarget {
-11
View File
@@ -1267,14 +1267,3 @@ describe("contrast candidate round-trip", () => {
expect(source).not.toMatch(/prepared\.map\(\(entry\) => entry\.candidate\)/);
});
});
describe("geometry candidate plumbing", () => {
it("wires the opt-in browser primitive without round-tripping normalized candidates", () => {
const source = checkBrowserSource();
expect(source).toMatch(/collectGeometryCandidates: \(time, request\) =>/);
expect(source).toMatch(/__hyperframesGeometryCandidates/);
expect(source).toMatch(/raw\.flatMap\(\(value\) => parseGeometryCandidate\(value, time\)\)/);
expect(source).not.toMatch(/resolveAnchors\(page, raw/);
});
});
+2 -5
View File
@@ -147,15 +147,12 @@ export function createCheckCommand(
}
function parseCheckOptions(args: Record<string, unknown>): CheckOptions {
const maxTransitionSamplesRaw = parseInt(String(args["max-transition-samples"] ?? ""), 10);
const maxTransitionSamples = positiveInteger(args["max-transition-samples"], 0);
return {
samples: positiveInteger(args.samples, DEFAULT_CHECK_OPTIONS.samples),
at: parseAt(args.at),
atTransitions: args["at-transitions"] === true,
maxTransitionSamples:
Number.isFinite(maxTransitionSamplesRaw) && maxTransitionSamplesRaw > 0
? maxTransitionSamplesRaw
: undefined,
maxTransitionSamples: maxTransitionSamples > 0 ? maxTransitionSamples : undefined,
maxIssues: positiveInteger(args["max-issues"], DEFAULT_CHECK_OPTIONS.maxIssues),
collapseStatic: args["collapse-static"] !== false,
tolerance: nonNegativeNumber(args.tolerance, DEFAULT_CHECK_OPTIONS.tolerance),
@@ -4,7 +4,6 @@ import {
metaDescription,
resolveProjectMock,
runAndCaptureStdio,
runAndParseJsonEnvelope,
} from "./deprecationTestHarness.js";
// See layout.test.ts for why these two dynamic-import targets are mocked:
@@ -31,10 +30,4 @@ describe("inspect command deprecation (U5)", () => {
expect(stderrText).toContain("hyperframes check");
expect(stdoutText).toBe("");
});
it("--json output is valid JSON with _meta.deprecated === true on failure", async () => {
const { parsed } = await runAndParseJsonEnvelope(inspectCommand);
expect(parsed.ok).toBe(false);
expect(parsed._meta.deprecated).toBe(true);
});
});
+2 -7
View File
@@ -27,6 +27,7 @@ import {
} from "../utils/motionAudit.js";
import { findMotionSpec, readMotionSpec, type MotionSpec } from "../utils/motionSpec.js";
import {
AUDIT_SEEK_OPTIONS,
seekCompositionTimeline,
waitForCompositionFonts,
type SeekCompositionTimelineOptions,
@@ -34,13 +35,7 @@ import {
const __filename = fileURLToPath(import.meta.url);
const __dirname = dirname(__filename);
const SEEK_SETTLE_MS = 120;
const LAYOUT_SEEK_OPTIONS: SeekCompositionTimelineOptions = {
fallbackToBridgeAndTimelines: true,
animationFrameSettle: "double",
waitForFontsMs: 500,
settleMs: SEEK_SETTLE_MS,
};
const LAYOUT_SEEK_OPTIONS: SeekCompositionTimelineOptions = AUDIT_SEEK_OPTIONS;
// All new envelope fields are optional (?); additive changes don't bump this.
const INSPECT_SCHEMA_VERSION = 1;
// Motion verification (#1437): dense sampling grid for the seeked-timeline checks.
+7 -2
View File
@@ -4,6 +4,7 @@ import { existsSync, mkdtempSync, readFileSync, mkdirSync, rmSync, writeFileSync
import { tmpdir } from "node:os";
import { resolve, join, relative, isAbsolute, basename } from "node:path";
import {
DEFAULT_ZOOM_SCALE,
captureRegionCrop,
openSettledCompositionPage,
parseZoomTarget,
@@ -119,7 +120,7 @@ export const examples: Example[] = [
* falls back to the default for anything that doesn't parse as a positive number. */
export function parseZoomScale(value: unknown): number {
const parsed = parseFloat(String(value ?? ""));
return Number.isFinite(parsed) && parsed > 0 ? parsed : 3;
return Number.isFinite(parsed) && parsed > 0 ? parsed : DEFAULT_ZOOM_SCALE;
}
/**
@@ -457,7 +458,11 @@ async function captureSnapshots(
);
continue;
}
const buffer = await captureRegionCrop(page, region, opts.zoomScale ?? 3);
const buffer = await captureRegionCrop(
page,
region,
opts.zoomScale ?? DEFAULT_ZOOM_SCALE,
);
writeFileSync(framePath, buffer);
} else {
await page.screenshot({ path: framePath, type: "png" });
+1 -1
View File
@@ -16,8 +16,8 @@ import {
raceMediaReady,
resolveNavigationTimeoutMs,
shouldIgnoreRequestFailure,
waitForPreferredSeekTarget,
} from "./validate.js";
import { waitForPreferredSeekTarget } from "../capture/captureCompositionFrame.js";
import type { ProjectLintResult } from "../utils/lintProject.js";
// validateInBrowser lazy-loads the producer localize helpers via loadProducer;
-2
View File
@@ -19,8 +19,6 @@ import {
seekCompositionTimeline,
} from "../capture/captureCompositionFrame.js";
export { waitForPreferredSeekTarget } from "../capture/captureCompositionFrame.js";
const __filename = fileURLToPath(import.meta.url);
const __dirname = dirname(__filename);
+9 -3
View File
@@ -3,6 +3,12 @@ import type { SubTimelineWaitOutcome } from "@hyperframes/engine";
import { trackEvent } from "./client.js";
import { readConfig } from "./config.js";
// run_id is attached only when the orchestrator set HYPERFRAMES_RUN_ID — an
// absent property, never null/"" (PostHog treats those as real values).
function runIdField(runId: string | undefined): { run_id?: string } {
return runId !== undefined ? { run_id: runId } : {};
}
export interface RenderObservabilityTelemetryPayload {
/** Worst sub-composition timeline wait outcome across sessions. */
subTimelineWait?: SubTimelineWaitOutcome;
@@ -101,7 +107,7 @@ function redactTelemetryMessage(value: string): string {
export function trackCommand(command: string, runId?: string): void {
trackEvent("cli_command", {
command,
...(runId !== undefined ? { run_id: runId } : {}),
...runIdField(runId),
});
}
@@ -554,7 +560,7 @@ export function trackCommandResult(props: {
success: props.success,
exit_code: props.exitCode,
duration_ms: props.durationMs,
...(props.runId !== undefined ? { run_id: props.runId } : {}),
...runIdField(props.runId),
});
}
@@ -606,6 +612,6 @@ export function trackCheckReport(props: {
contrast_points: props.contrastPoints,
ok: props.ok,
exit_code: props.exitCode,
...(props.runId !== undefined ? { run_id: props.runId } : {}),
...runIdField(props.runId),
});
}
+117 -1
View File
@@ -16,7 +16,10 @@ vi.mock("@hyperframes/core/compiler", () => ({
bundleToSingleHtml: vi.fn(async () => "<html></html>"),
}));
vi.mock("../capture/captureCompositionFrame.js", () => ({
vi.mock("../capture/captureCompositionFrame.js", async (importOriginal) => ({
// Partial mock: constants (AUDIT_SEEK_OPTIONS, DEFAULT_ZOOM_*) stay real so
// they remain single-sourced; only the browser-touching functions are faked.
...(await importOriginal<typeof import("../capture/captureCompositionFrame.js")>()),
openSettledCompositionPage: vi.fn(),
resolveCliChromeGpuMode: vi.fn(() => "hardware"),
seekCompositionTimeline: vi.fn(async () => undefined),
@@ -38,9 +41,14 @@ const PROJECT: ProjectDir = {
afterEach(() => {
vi.restoreAllMocks();
vi.unstubAllGlobals();
document.body.innerHTML = "";
Reflect.deleteProperty(window, "__hyperframesGeometryCandidates");
Reflect.deleteProperty(window, "__hyperframesLayoutAudit");
Reflect.deleteProperty(window, "__contrastAuditPrepare");
Reflect.deleteProperty(window, "__contrastAuditFinish");
Reflect.deleteProperty(window, "__contrastAuditRestores");
Reflect.deleteProperty(window, "__contrastAuditRestoreIfPending");
});
it("carries raw browser geometry through the page driver and pipeline", async () => {
@@ -94,6 +102,114 @@ it("carries raw browser geometry through the page driver and pipeline", async ()
expect(mocks.serverClose).toHaveBeenCalledOnce();
});
it("round-trips the browser script's raw contrast candidates back into finish", async () => {
// The U2 regression class: Node parses prepare's candidates for reporting,
// but must hand the UNTOUCHED objects back to __contrastAuditFinish — the
// page script samples pixels via its own bbox shape (w/h). A normalized
// candidate (width/height) makes every sample rect NaN and the audit
// silently reports zero checked elements as green.
vi.spyOn(Date, "now").mockReturnValue(100);
document.body.innerHTML = `
<div data-composition-id="main" data-duration="10" data-width="640" data-height="360">
<div id="headline">Readable copy</div>
</div>
`;
Object.defineProperty(window, "innerWidth", { configurable: true, value: 640 });
Object.defineProperty(window, "innerHeight", { configurable: true, value: 360 });
const root = document.querySelector("[data-composition-id]");
const headline = document.querySelector("#headline");
if (!root || !headline) throw new Error("Contrast fixture failed to mount");
vi.spyOn(root, "getBoundingClientRect").mockReturnValue(new DOMRect(0, 0, 640, 360));
vi.spyOn(headline, "getBoundingClientRect").mockReturnValue(new DOMRect(50, 50, 300, 40));
vi.spyOn(window, "getComputedStyle").mockImplementation(
() =>
({
display: "block",
visibility: "visible",
opacity: "1",
color: "rgb(255,255,255)",
fill: "",
backgroundColor: "rgba(0,0,0,0)",
backgroundImage: "none",
fontSize: "32px",
fontWeight: "700",
}) as unknown as CSSStyleDeclaration,
);
// happy-dom can't decode PNGs: stub Image (sync onload) and the canvas 2D
// context the way layout-audit.browser.test.ts's contrast harness does, so
// the REAL __contrastAuditFinish runs its sampling path end to end.
class MockImage {
onload: (() => void) | null = null;
onerror: (() => void) | null = null;
naturalWidth = 640;
naturalHeight = 360;
set src(_value: string) {
this.onload?.();
}
}
vi.stubGlobal("Image", MockImage);
const getContextSpy = vi.spyOn(HTMLCanvasElement.prototype, "getContext") as unknown as {
mockReturnValue(value: CanvasRenderingContext2D): void;
};
getContextSpy.mockReturnValue({
drawImage() {},
getImageData() {
return { data: new Uint8ClampedArray(640 * 360 * 4).fill(255) };
},
} as unknown as CanvasRenderingContext2D);
const received: Array<Record<string, unknown>> = [];
const page = fakePage();
const injectScript = page.addScriptTag;
page.addScriptTag = vi.fn(async (arg: { content: string }) => {
await injectScript(arg);
const w = window as unknown as {
__contrastAuditFinish?: ((...args: unknown[]) => Promise<unknown>) & { wrapped?: boolean };
};
const finish = w.__contrastAuditFinish;
if (finish && !finish.wrapped) {
const wrapper = Object.assign(
async (...args: unknown[]) => {
const candidates = args[2];
if (Array.isArray(candidates)) {
received.push(...(candidates as Array<Record<string, unknown>>));
}
return finish(...args);
},
{ wrapped: true },
);
w.__contrastAuditFinish = wrapper;
}
});
page.screenshot = vi.fn(async () => "c3R1Yg==");
const browser = Object.assign(Object.create(null), {
close: vi.fn(async () => undefined),
});
vi.mocked(openSettledCompositionPage).mockImplementation(
async (_html: string, _url: string, options: OpenSettledCompositionPageOptions) => {
await options.beforeNavigate?.(page);
return { page, browser, renderReadyTimedOut: false };
},
);
await runBrowserCheck(
PROJECT,
{ ...DEFAULT_CHECK_OPTIONS, samples: 1, contrast: true },
{ kind: "none" },
runAuditGrid,
);
expect(received.length).toBeGreaterThan(0);
for (const candidate of received) {
const bbox = candidate.bbox as Record<string, unknown>;
// The page script's own shape (w/h), not Node's envelope shape (width/height):
expect(typeof bbox.w).toBe("number");
expect(typeof bbox.h).toBe("number");
}
});
describe("captureOverviewShot", () => {
it("injects the annotation overlay before the overview shot and removes it right after", async () => {
const calls: string[] = [];
+10 -22
View File
@@ -2,6 +2,9 @@ import { mkdirSync, writeFileSync } from "node:fs";
import { join } from "node:path";
import type { Page } from "puppeteer-core";
import {
AUDIT_SEEK_OPTIONS,
DEFAULT_ZOOM_PADDING_PX,
DEFAULT_ZOOM_SCALE,
captureRegionCrop,
openSettledCompositionPage,
padCropRegion,
@@ -15,6 +18,7 @@ import { normalizeErrorMessage } from "./errorMessage.js";
import { ambiguousIssue, type MotionFrame } from "./motionAudit.js";
import type { LayoutIssue, LayoutIssueCode, LayoutRect } from "./layoutAudit.js";
import { serveStaticProjectHtml } from "./staticProjectServer.js";
import { rectToBbox } from "./checkTypes.js";
import type {
AnchoredLayoutIssue,
CheckAnchor,
@@ -35,18 +39,6 @@ import type {
} from "./checkTypes.js";
import type { ProjectDir } from "./project.js";
const SEEK_OPTIONS = {
fallbackToBridgeAndTimelines: true,
animationFrameSettle: "double" as const,
waitForFontsMs: 500,
settleMs: 120,
};
// --zoom's default padding/scale, reused here so finding crops carry the same
// bit of surrounding context an agent gets from `snapshot --zoom`.
const FINDING_CROP_PADDING_PX = 24;
const FINDING_CROP_SCALE = 3;
interface RuntimeDraft {
code: string;
severity: CheckSeverity;
@@ -113,7 +105,7 @@ export async function runBrowserCheck(
});
chromeBrowser = session.browser;
const page = session.page;
await waitForPreferredSeekTarget(page, 500);
await waitForPreferredSeekTarget(page);
const rootAnchor = await resolveRootAnchor(page);
const launchSettleMs = Date.now() - launchSettleStart;
@@ -157,18 +149,18 @@ export async function captureFindingCrops(
});
chromeBrowser = session.browser;
const page = session.page;
await waitForPreferredSeekTarget(page, 500);
await waitForPreferredSeekTarget(page);
const snapshotDir = join(project.dir, "snapshots");
mkdirSync(snapshotDir, { recursive: true });
for (const request of requests) {
await seekCompositionTimeline(page, request.time, SEEK_OPTIONS);
await seekCompositionTimeline(page, request.time, AUDIT_SEEK_OPTIONS);
const canvas = await page.evaluate(() => ({
width: window.innerWidth,
height: window.innerHeight,
}));
const region = padCropRegion(request.bbox, canvas, FINDING_CROP_PADDING_PX);
const buffer = await captureRegionCrop(page, region, FINDING_CROP_SCALE);
const region = padCropRegion(request.bbox, canvas, DEFAULT_ZOOM_PADDING_PX);
const buffer = await captureRegionCrop(page, region, DEFAULT_ZOOM_SCALE);
writeFileSync(join(snapshotDir, request.filename), buffer);
written.push(join("snapshots", request.filename));
}
@@ -253,7 +245,7 @@ function createPageDriver(page: Page, setTime: (time: number) => void): CheckAud
findAmbiguousSelectors: (selectors) => findAmbiguousSelectors(page, selectors),
seek: async (time) => {
setTime(time);
await seekCompositionTimeline(page, time, SEEK_OPTIONS);
await seekCompositionTimeline(page, time, AUDIT_SEEK_OPTIONS);
},
collectLayout: (time, tolerance) => collectLayout(page, time, tolerance),
collectLayoutGeometry: () => collectLayoutGeometry(page),
@@ -899,10 +891,6 @@ function fallbackAnchor(request: AnchorRequest | undefined): CheckAnchor {
};
}
function rectToBbox(rect: LayoutRect): CheckBbox {
return { x: rect.left, y: rect.top, width: rect.width, height: rect.height };
}
function parseBbox(value: unknown): CheckBbox | null {
if (!isRecord(value)) return null;
const x = numberValue(value, "x");
+1 -4
View File
@@ -28,6 +28,7 @@ import {
suggestCompliantForegroundColor,
type Rgb,
} from "../commands/contrast-bg.js";
import { rectToBbox } from "./checkTypes.js";
import type {
AnchoredLayoutIssue,
CheckAnnotationBox,
@@ -877,10 +878,6 @@ function failureReport(options: CheckOptions, finding: CheckFinding): CheckRepor
return buildReport(options, lint, browser, { kind: "none" }, [], []);
}
function rectToBbox(rect: LayoutRect): CheckBbox {
return { x: rect.left, y: rect.top, width: rect.width, height: rect.height };
}
function isBbox(value: unknown): value is CheckBbox {
if (typeof value !== "object" || value === null) return false;
return ["x", "y", "width", "height"].every((key) => typeof Reflect.get(value, key) === "number");
+9
View File
@@ -235,3 +235,12 @@ export interface CheckDependencies {
requests: CheckFindingCropRequest[],
): Promise<string[]>;
}
export function rectToBbox(rect: {
left: number;
top: number;
width: number;
height: number;
}): CheckBbox {
return { x: rect.left, y: rect.top, width: rect.width, height: rect.height };
}