diff --git a/packages/cli/src/commands/check.test.ts b/packages/cli/src/commands/check.test.ts index 5718465db..18fd3568e 100644 --- a/packages/cli/src/commands/check.test.ts +++ b/packages/cli/src/commands/check.test.ts @@ -108,10 +108,14 @@ function anchor(selector: string, time: number): CheckAnchor { }; } -function layoutIssue(severity: "error" | "warning" | "info" = "error"): AnchoredLayoutIssue { +function layoutIssue( + severity: "error" | "warning" | "info" = "error", + overrides: { time?: number; code?: AnchoredLayoutIssue["code"] } = {}, +): AnchoredLayoutIssue { + const time = overrides.time ?? 0.5; return { - ...anchor("#hero", 0.5), - code: severity === "warning" ? "content_overlap" : "clipped_text", + ...anchor("#hero", time), + code: overrides.code ?? (severity === "warning" ? "content_overlap" : "clipped_text"), severity, text: "Hero", message: severity === "warning" ? "Text may overlap." : "Text is clipped.", @@ -133,6 +137,10 @@ function contrastEntry(overrides: Partial = {}): ContrastAud } function fakeDriver(overrides: Partial = {}): CheckAuditDriver { + // A distinct string per call so the frozen-sweep guard (#U10) never fires + // by accident in unrelated scenarios — tests that want it force a constant + // via `collectLayoutGeometry: vi.fn(async () => "same")`. + let geometryCallCount = 0; return { initialize: vi.fn(async (_contrast: boolean) => undefined), getDuration: vi.fn(async () => 9), @@ -141,6 +149,7 @@ function fakeDriver(overrides: Partial = {}): CheckAuditDriver findAmbiguousSelectors: vi.fn(async (_selectors: string[]) => []), seek: vi.fn(async (_time: number) => undefined), collectLayout: vi.fn(async (_time: number, _tolerance: number) => []), + collectLayoutGeometry: vi.fn(async () => `geometry-${geometryCallCount++}`), collectGeometryCandidates: vi.fn(async () => []), collectMotionFrame: vi.fn(async (time: number) => ({ time, data: {}, liveness: {} })), anchorMotionIssues: vi.fn(async (issues: LayoutIssue[]) => @@ -883,7 +892,9 @@ describe("check pipeline", () => { it("preserves a resolving selector, source file, identity, bbox, and sample time", async () => { const { report } = await runScenario( - fakeDriver({ collectLayout: vi.fn(async () => [layoutIssue()]) }), + fakeDriver({ + collectLayout: vi.fn(async (time: number) => [layoutIssue("error", { time })]), + }), ); expect(report.layout.findings[0]).toMatchObject({ selector: "#hero", @@ -896,7 +907,9 @@ describe("check pipeline", () => { it("reports layout and runtime errors from one browser session", async () => { const { report, browser } = await runScenario( - fakeDriver({ collectLayout: vi.fn(async () => [layoutIssue()]) }), + fakeDriver({ + collectLayout: vi.fn(async (time: number) => [layoutIssue("error", { time })]), + }), {}, { runtime: [runtimeError()] }, ); @@ -961,7 +974,9 @@ describe("check pipeline", () => { ) => ["snapshots/finding-00-clipped_text.png"], ); const { report } = await runScenario( - fakeDriver({ collectLayout: vi.fn(async () => [layoutIssue()]) }), + fakeDriver({ + collectLayout: vi.fn(async (time: number) => [layoutIssue("error", { time })]), + }), { snapshots: true }, { captureFindingCrops: capture }, ); @@ -978,7 +993,9 @@ describe("check pipeline", () => { const withoutSnapshots = vi.fn(async () => ["unused.png"]); await runScenario( - fakeDriver({ collectLayout: vi.fn(async () => [layoutIssue()]) }), + fakeDriver({ + collectLayout: vi.fn(async (time: number) => [layoutIssue("error", { time })]), + }), { snapshots: false }, { captureFindingCrops: withoutSnapshots }, ); @@ -986,7 +1003,15 @@ describe("check pipeline", () => { const noErrors = vi.fn(async () => ["unused.png"]); await runScenario( - fakeDriver({ collectLayout: vi.fn(async () => [layoutIssue("warning")]) }), + fakeDriver({ + collectLayout: vi.fn( + async (time: number) => + // container_overflow, not content_overlap: this fixture wants a plain + // warning-severity finding held across the whole run, unaffected by + // content_overlap's #U10 held-duration re-promotion to error. + [layoutIssue("warning", { time, code: "container_overflow" })], + ), + }), { snapshots: true }, { captureFindingCrops: noErrors }, ); @@ -995,7 +1020,15 @@ describe("check pipeline", () => { it("--strict flips a warnings-only result from exit 0 to exit 1", async () => { const warningDriver = () => - fakeDriver({ collectLayout: vi.fn(async () => [layoutIssue("warning")]) }); + fakeDriver({ + collectLayout: vi.fn( + async (time: number) => + // container_overflow, not content_overlap: this fixture wants a plain + // warning-severity finding held across the whole run, unaffected by + // content_overlap's #U10 held-duration re-promotion to error. + [layoutIssue("warning", { time, code: "container_overflow" })], + ), + }); const normal = await runScenario(warningDriver(), { strict: false }); const strict = await runScenario(warningDriver(), { strict: true }); @@ -1026,6 +1059,57 @@ describe("check pipeline", () => { ); expect(checkExitCode(report)).toBe(1); }); + + describe("frozen-sweep guard (#U10)", () => { + it("fails with sweep_static when a 6s composition's geometry never changes across samples", async () => { + const driver = fakeDriver({ + getDuration: vi.fn(async () => 6), + collectLayoutGeometry: vi.fn(async () => "frozen"), + }); + const { report } = await runScenario(driver); + + expect(report.ok).toBe(false); + expect( + report.layout.findings.some( + (finding) => + finding.code === "sweep_static" && + finding.severity === "error" && + finding.message.includes("did not advance"), + ), + ).toBe(true); + }); + + it("does not flag a 1.5s static title card — too short for the guard to apply", async () => { + const driver = fakeDriver({ + getDuration: vi.fn(async () => 1.5), + collectLayoutGeometry: vi.fn(async () => "frozen"), + }); + const { report } = await runScenario(driver); + + expect(report.layout.findings.some((finding) => finding.code === "sweep_static")).toBe(false); + }); + + it("does not double-report when a motion_frozen finding already covers the same symptom", async () => { + const motion: MotionSpecResolution = { + kind: "valid", + path: "/project/index.motion.json", + spec: { assertions: [{ kind: "keepsMoving" }] }, + }; + const driver = fakeDriver({ + getDuration: vi.fn(async () => 6), + collectLayoutGeometry: vi.fn(async () => "frozen"), + collectMotionFrame: vi.fn(async (time: number) => ({ + time, + data: {}, + liveness: { "*": "unchanging" }, + })), + }); + const { report } = await runScenario(driver, {}, { motion }); + + expect(report.motion.findings.some((finding) => finding.code === "motion_frozen")).toBe(true); + expect(report.layout.findings.some((finding) => finding.code === "sweep_static")).toBe(false); + }); + }); }); describe("check report telemetry", () => { diff --git a/packages/cli/src/commands/deprecationTestHarness.ts b/packages/cli/src/commands/deprecationTestHarness.ts new file mode 100644 index 000000000..bce26290d --- /dev/null +++ b/packages/cli/src/commands/deprecationTestHarness.ts @@ -0,0 +1,115 @@ +// Shared scaffolding for the U5 deprecation tests in inspect.test.ts, +// layout.test.ts, and validate.test.ts: those commands all fail fast (via a +// mocked dynamic import) so the tests can assert the shared deprecation +// envelope (stderr notice, JSON `_meta.deprecated`) without needing a real +// project or headless Chrome. +// +// vi.mock factories are hoisted above imports, so each test file keeps its +// own thin `vi.mock("", () => someFactory())` call (mocking a module +// path can't itself be shared across files) but delegates the factory body +// here. +import type { ArgsDef, CommandDef } from "citty"; +import { runCommand } from "citty"; +import { expect, vi } from "vitest"; + +const FAKE_PROJECT = { + dir: "/fake-project", + name: "fake-project", + indexPath: "/fake-project/index.html", +}; + +export function resolveProjectMock() { + return { resolveProject: vi.fn(() => FAKE_PROJECT) }; +} + +export function bundleToSingleHtmlFailureMock() { + return { + bundleToSingleHtml: vi.fn(async () => { + throw new Error("bundling failed (test double)"); + }), + }; +} + +export function lintProjectFailureMock() { + return { + lintProject: vi.fn(async () => { + throw new Error("lint failed (test double)"); + }), + }; +} + +/** + * citty's `meta` is `Resolvable` (object | promise | thunk). + * These test files always define it as a synchronous object literal, so + * narrow to that shape instead of asserting it with `as`. + */ +export function metaDescription(command: CommandDef): string { + const meta = command.meta; + if (meta && typeof meta === "object" && "description" in meta) { + return String(meta.description ?? ""); + } + throw new Error("expected a synchronous meta object"); +} + +/** + * Run a command with stdout/stderr writes captured (and process.exit / + * console.log stubbed so the run stays silent and non-terminating), and + * return the captured text for the caller to assert on. + */ +export async function runAndCaptureStdio( + command: CommandDef, + rawArgs: string[] = ["--json"], +): Promise<{ stderrText: string; stdoutText: string }> { + const stderrWrites: string[] = []; + const stdoutWrites: string[] = []; + vi.spyOn(process.stderr, "write").mockImplementation((chunk: unknown) => { + stderrWrites.push(String(chunk)); + return true; + }); + vi.spyOn(process.stdout, "write").mockImplementation((chunk: unknown) => { + stdoutWrites.push(String(chunk)); + return true; + }); + vi.spyOn(process, "exit").mockImplementation(() => undefined as never); + vi.spyOn(console, "log").mockImplementation(() => {}); + + await runCommand(command, { rawArgs }); + + return { stderrText: stderrWrites.join(""), stdoutText: stdoutWrites.join("") }; +} + +/** + * Run a command with process.exit stubbed and console.log spied, returning + * the first console.log call that looks like a JSON object (the `--json` + * failure envelope). Callers assert on definedness/shape themselves, since + * that differs slightly per call site. + */ +export async function runAndFindJsonLogCall( + command: CommandDef, + rawArgs: string[] = ["--json"], +): Promise { + vi.spyOn(process.stderr, "write").mockImplementation(() => true); + vi.spyOn(process, "exit").mockImplementation(() => undefined as never); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + await runCommand(command, { rawArgs }); + + return logSpy.mock.calls.find(([arg]) => typeof arg === "string" && arg.trim().startsWith("{")); +} + +/** + * Convenience wrapper: parse the JSON envelope found by runAndFindJsonLogCall. + * `parsed` is intentionally left as JSON.parse's inferred `any` (matching + * every call site's prior inline `JSON.parse(...)` usage) rather than + * annotated `unknown`, since callers assert directly into its shape + * (`.ok`, `._meta.deprecated`) the same way the original inline tests did. + */ +export async function runAndParseJsonEnvelope( + command: CommandDef, + rawArgs: string[] = ["--json"], +) { + const jsonCall = await runAndFindJsonLogCall(command, rawArgs); + expect(jsonCall).toBeDefined(); + const parsed = JSON.parse(String(jsonCall?.[0])); + return { jsonCall, parsed }; +} diff --git a/packages/cli/src/commands/inspect.test.ts b/packages/cli/src/commands/inspect.test.ts index 59288c7eb..3c378363a 100644 --- a/packages/cli/src/commands/inspect.test.ts +++ b/packages/cli/src/commands/inspect.test.ts @@ -1,37 +1,21 @@ -import type { CommandDef } from "citty"; -import { runCommand } from "citty"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { + bundleToSingleHtmlFailureMock, + metaDescription, + resolveProjectMock, + runAndCaptureStdio, + runAndParseJsonEnvelope, +} from "./deprecationTestHarness.js"; // See layout.test.ts for why these two dynamic-import targets are mocked: // resolveProject skips real filesystem resolution, and bundleToSingleHtml // gives a fast, deterministic failure that exercises run()'s outer catch // (the JSON failure envelope) without needing headless Chrome. -const FAKE_PROJECT = { - dir: "/fake-project", - name: "fake-project", - indexPath: "/fake-project/index.html", -}; - -vi.mock("../utils/project.js", () => ({ - resolveProject: vi.fn(() => FAKE_PROJECT), -})); - -vi.mock("@hyperframes/core/compiler", () => ({ - bundleToSingleHtml: vi.fn(async () => { - throw new Error("bundling failed (test double)"); - }), -})); +vi.mock("../utils/project.js", () => resolveProjectMock()); +vi.mock("@hyperframes/core/compiler", () => bundleToSingleHtmlFailureMock()); import inspectCommand from "./inspect.js"; -function metaDescription(command: CommandDef): string { - const meta = command.meta; - if (meta && typeof meta === "object" && "description" in meta) { - return String(meta.description ?? ""); - } - throw new Error("expected a synchronous meta object"); -} - afterEach(() => { vi.restoreAllMocks(); }); @@ -42,39 +26,14 @@ describe("inspect command deprecation (U5)", () => { }); it("prints a one-line deprecation notice naming 'inspect' on stderr, never stdout", async () => { - const stderrWrites: string[] = []; - const stdoutWrites: string[] = []; - vi.spyOn(process.stderr, "write").mockImplementation((chunk: unknown) => { - stderrWrites.push(String(chunk)); - return true; - }); - vi.spyOn(process.stdout, "write").mockImplementation((chunk: unknown) => { - stdoutWrites.push(String(chunk)); - return true; - }); - vi.spyOn(process, "exit").mockImplementation(() => undefined as never); - vi.spyOn(console, "log").mockImplementation(() => {}); - - await runCommand(inspectCommand, { rawArgs: ["--json"] }); - - const stderrText = stderrWrites.join(""); + const { stderrText, stdoutText } = await runAndCaptureStdio(inspectCommand); expect(stderrText).toContain("hyperframes inspect"); expect(stderrText).toContain("hyperframes check"); - expect(stdoutWrites.join("")).toBe(""); + expect(stdoutText).toBe(""); }); it("--json output is valid JSON with _meta.deprecated === true on failure", async () => { - vi.spyOn(process.stderr, "write").mockImplementation(() => true); - vi.spyOn(process, "exit").mockImplementation(() => undefined as never); - const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); - - await runCommand(inspectCommand, { rawArgs: ["--json"] }); - - const jsonCall = logSpy.mock.calls.find( - ([arg]) => typeof arg === "string" && arg.trim().startsWith("{"), - ); - expect(jsonCall).toBeDefined(); - const parsed = JSON.parse(String(jsonCall?.[0])); + const { parsed } = await runAndParseJsonEnvelope(inspectCommand); expect(parsed.ok).toBe(false); expect(parsed._meta.deprecated).toBe(true); }); diff --git a/packages/cli/src/commands/layout-audit.browser.js b/packages/cli/src/commands/layout-audit.browser.js index 568ccfbae..f939a9dce 100644 --- a/packages/cli/src/commands/layout-audit.browser.js +++ b/packages/cli/src/commands/layout-audit.browser.js @@ -627,9 +627,12 @@ const area = intersectionArea(a.rect, b.rect); if (area <= Math.min(rectArea(a.rect), rectArea(b.rect)) * 0.2) return null; return { - // Warning, not error: must not fail the exit code (ok = errorCount === 0) - // for compositions that intentionally layer text. Re-promote once the - // data-layout-allow-overlap opt-out is widely adopted. + // Warning at the per-sample level: a single-sample overlap is usually an + // entrance/exit transient (two blocks crossing mid-animation), not a real + // collision. `collapseStaticLayoutIssues` (utils/layoutAudit.ts) re-promotes + // this to error once the SAME overlap is held across >= 2 adjacent samples + // (or ~500ms of timeline) — a persistence-tiered replacement for the old + // "re-promote once data-layout-allow-overlap is widely adopted" plan (#U10). code: "content_overlap", severity: "warning", time, @@ -728,37 +731,66 @@ return hit; } + const OCCLUSION_PROBE_Y_FRACTIONS = [0.25, 0.5, 0.75]; + const OCCLUSION_PROBE_X_FRACTIONS = [0.03, 0.1, 0.2, 0.35, 0.5, 0.65, 0.8, 0.9, 0.97]; + const OCCLUSION_GRID_POINTS = + OCCLUSION_PROBE_Y_FRACTIONS.length * OCCLUSION_PROBE_X_FRACTIONS.length; + + // Short, atomic text (a label/button/word, no whitespace) reads as a single + // unit — ANY covered probe point changes what it says, so flag at any hit + // (the pre-#U10 behaviour). Longer prose survives a nibbled edge; only flag + // once a real share of it is covered — see `occludedTextIssue`. + const ATOMIC_LABEL_MAX_CHARS = 16; + const PROSE_COVERAGE_FLOOR = 0.15; + + function isAtomicLabel(text) { + return text.length > 0 && text.length <= ATOMIC_LABEL_MAX_CHARS && !/\s/.test(text); + } + // Sweep a grid across the text box (three rows, not just the mid-line, so - // overlays covering only part of a multi-line block are caught) and return - // the first opaque element painted over any sample point. - function firstOccluder(element, textRect) { - for (const yFraction of [0.25, 0.5, 0.75]) { + // overlays covering only part of a multi-line block are caught). Unlike a + // first-hit scan, this keeps sampling every point so it can report what + // fraction of the box is actually covered — a corner nibble on a paragraph + // reads very differently from a label buried under an overlay. Still + // returns the first opaque element found, for `containerSelector`. + function occlusionCoverage(element, textRect) { + let occluder = null; + let hits = 0; + for (const yFraction of OCCLUSION_PROBE_Y_FRACTIONS) { const y = textRect.top + textRect.height * yFraction; - for (const xFraction of [0.03, 0.1, 0.2, 0.35, 0.5, 0.65, 0.8, 0.9, 0.97]) { - const occluder = occluderAt(element, textRect.left + textRect.width * xFraction, y); - if (occluder) return occluder; + for (const xFraction of OCCLUSION_PROBE_X_FRACTIONS) { + const hit = occluderAt(element, textRect.left + textRect.width * xFraction, y); + if (!hit) continue; + hits += 1; + if (!occluder) occluder = hit; } } - return null; + return { occluder, coveredFraction: round(hits / OCCLUSION_GRID_POINTS) }; } // Catches the blind spot the overflow checks miss: text that fits its box - // perfectly but is covered by a later sibling/overlay. + // perfectly but is covered by a later sibling/overlay. An atomic label + // (short, no whitespace) flags at any coverage; ordinary prose only flags + // once coveredFraction clears PROSE_COVERAGE_FLOOR, since a sliver of edge + // cover on a paragraph is usually a styling artifact, not a reading defect. function occludedTextIssue(element, time) { if (hasAllowOcclusionFlag(element)) return null; const textRect = textRectFor(element); if (!textRect) return null; - const occluder = firstOccluder(element, textRect); + const text = textContentFor(element); + const { occluder, coveredFraction } = occlusionCoverage(element, textRect); if (!occluder) return null; + if (!isAtomicLabel(text) && coveredFraction < PROSE_COVERAGE_FLOOR) return null; return { code: "text_occluded", severity: "error", time, selector: selectorFor(element), containerSelector: selectorFor(occluder), - text: textContentFor(element), + text, message: "Text is hidden beneath an opaque element.", rect: textRect, + coveredFraction, fixHint: "Give the text its own zone, raise its stacking order above the covering element, or mark intentional layering with data-layout-allow-occlusion.", }; @@ -855,4 +887,28 @@ issues.push(...contentOverlapIssues(root, time)); return issues; }; + + // Frozen-sweep guard (#U10, checkPipeline.ts): a compact per-sample + // fingerprint of every visible element's box + opacity, in DOM order. Node + // calls this once per seeked grid point and compares the strings across the + // whole run — if every sample produces the identical string, the seek never + // actually moved anything and the whole audit run is unreliable. Deliberately + // a single opaque string (not a structured array) since Node only ever needs + // equality, not per-element diffing. + window.__hyperframesLayoutGeometry = function collectLayoutGeometry() { + const root = + document.querySelector("[data-composition-id][data-width][data-height]") || + document.querySelector("[data-composition-id]") || + document.body; + const elements = Array.from(root.querySelectorAll("*")).filter((element) => + isVisibleElement(element), + ); + return elements + .map((element) => { + const rect = toRect(element.getBoundingClientRect()); + const opacity = round(opacityChain(element)); + return `${rect.left},${rect.top},${rect.width},${rect.height},${opacity}`; + }) + .join("|"); + }; })(); diff --git a/packages/cli/src/commands/layout-audit.browser.test.ts b/packages/cli/src/commands/layout-audit.browser.test.ts index 0f05e8077..dbd6d00e4 100644 --- a/packages/cli/src/commands/layout-audit.browser.test.ts +++ b/packages/cli/src/commands/layout-audit.browser.test.ts @@ -651,8 +651,92 @@ describe("layout-audit.browser occlusion", () => { }); expect(issues.some((issue) => issue.code === "text_occluded")).toBe(false); }); + + it("carries the fully-covered fraction when the occluder hits every probe point", () => { + const occluded = auditOcclusionScene({ + overlayStyle: { backgroundColor: "rgb(10, 10, 10)" }, + topmostId: "overlay", + }).find((issue) => issue.code === "text_occluded"); + expect(occluded?.coveredFraction).toBe(1); + }); + + // #U10: a 2-point hit on the 27-point probe grid (3 rows x 9 columns) is a + // sliver of edge cover — reports ~0.07 coverage either way, but only GATES + // (produces a finding) for short atomic labels; ordinary prose survives it. + it("reports ~0.07 coverage for a 2-of-27 grid hit and flags an atomic label at that coverage", () => { + const issues = auditCoverageScene({ text: "SUBSCRIBE", hitCount: 2 }); + const occluded = issues.find((issue) => issue.code === "text_occluded"); + expect(occluded).toBeDefined(); + expect(occluded?.coveredFraction).toBe(0.07); + }); + + it("does not flag ordinary prose at the same ~0.07 coverage a label would flag at", () => { + const issues = auditCoverageScene({ + text: "This paragraph is long enough to read as ordinary prose, not a label.", + hitCount: 2, + }); + expect(issues.some((issue) => issue.code === "text_occluded")).toBe(false); + }); + + it("flags prose once coverage clears the 0.15 floor", () => { + // 5/27 ≈ 0.185, comfortably over the ~0.15 prose floor. + const issues = auditCoverageScene({ + text: "This paragraph is long enough to read as ordinary prose, not a label.", + hitCount: 5, + }); + expect(issues.some((issue) => issue.code === "text_occluded")).toBe(true); + }); }); +// Mirrors OCCLUSION_PROBE_Y_FRACTIONS / OCCLUSION_PROBE_X_FRACTIONS in +// layout-audit.browser.js, so a test can force an exact number of grid hits +// against the same probe coordinates the audit itself sweeps. +const OCCLUSION_PROBE_Y_FRACTIONS = [0.25, 0.5, 0.75]; +const OCCLUSION_PROBE_X_FRACTIONS = [0.03, 0.1, 0.2, 0.35, 0.5, 0.65, 0.8, 0.9, 0.97]; + +function occlusionProbePoints(textRect: RectInput): Array<{ x: number; y: number }> { + const points: Array<{ x: number; y: number }> = []; + for (const yFraction of OCCLUSION_PROBE_Y_FRACTIONS) { + const y = textRect.top + textRect.height * yFraction; + for (const xFraction of OCCLUSION_PROBE_X_FRACTIONS) { + points.push({ x: textRect.left + textRect.width * xFraction, y }); + } + } + return points; +} + +// Builds an occlusion scene where exactly `hitCount` of the 27 probe points +// are covered by an opaque overlay and the rest hit the headline itself +// (self-hit — not foreign, so not counted as occluded). +function auditCoverageScene(options: { + text: string; + hitCount: number; +}): ReturnType { + const textRect = { left: 200, top: 500, width: 600, height: 80 }; + document.body.innerHTML = ` +
+
${options.text}
+
+
+ `; + installOcclusionGeometry({ + styleOverrides: { overlay: { backgroundColor: "rgb(10, 10, 10)" } }, + headlineTextRect: rect(textRect), + topmostId: "headline", + }); + const hitPoints = occlusionProbePoints(textRect).slice(0, options.hitCount); + ( + document as unknown as { elementFromPoint: (x: number, y: number) => Element | null } + ).elementFromPoint = (x, y) => { + const isHit = hitPoints.some( + (point) => Math.abs(point.x - x) < 0.01 && Math.abs(point.y - y) < 0.01, + ); + return document.getElementById(isHit ? "overlay" : "headline"); + }; + installAuditScript(); + return runAudit(); +} + function auditOcclusionScene(options: { headlineAttrs?: string; overlayStyle: Partial>; @@ -812,22 +896,19 @@ async function runContrastAudit(): Promise>> { return w.__contrastAuditFinish("stub", 0, candidates); } -function runAudit(): Array<{ +interface AuditIssue { code: string; selector: string; containerSelector?: string; overflow?: Record; message?: string; -}> { + coveredFraction?: number; +} + +function runAudit(): AuditIssue[] { const audit = ( window as unknown as { - __hyperframesLayoutAudit: (options: { time: number; tolerance: number }) => Array<{ - code: string; - selector: string; - containerSelector?: string; - overflow?: Record; - message?: string; - }>; + __hyperframesLayoutAudit: (options: { time: number; tolerance: number }) => AuditIssue[]; } ).__hyperframesLayoutAudit; return audit({ time: 1, tolerance: 2 }); diff --git a/packages/cli/src/commands/layout.test.ts b/packages/cli/src/commands/layout.test.ts index bd7cdb4d7..ab63315c5 100644 --- a/packages/cli/src/commands/layout.test.ts +++ b/packages/cli/src/commands/layout.test.ts @@ -1,6 +1,12 @@ -import type { CommandDef } from "citty"; -import { runCommand } from "citty"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { + bundleToSingleHtmlFailureMock, + metaDescription, + resolveProjectMock, + runAndCaptureStdio, + runAndFindJsonLogCall, + runAndParseJsonEnvelope, +} from "./deprecationTestHarness.js"; // resolveProject and bundleToSingleHtml are both reached via a dynamic // `await import(...)` inside layout.ts's run() / runLayoutAudit(), so @@ -9,37 +15,11 @@ import { afterEach, describe, expect, it, vi } from "vitest"; // bundleToSingleHtml gives a deterministic, fast failure well before any // real browser or network work — exercising run()'s outer catch (the JSON // failure envelope) without needing headless Chrome. -const FAKE_PROJECT = { - dir: "/fake-project", - name: "fake-project", - indexPath: "/fake-project/index.html", -}; - -vi.mock("../utils/project.js", () => ({ - resolveProject: vi.fn(() => FAKE_PROJECT), -})); - -vi.mock("@hyperframes/core/compiler", () => ({ - bundleToSingleHtml: vi.fn(async () => { - throw new Error("bundling failed (test double)"); - }), -})); +vi.mock("../utils/project.js", () => resolveProjectMock()); +vi.mock("@hyperframes/core/compiler", () => bundleToSingleHtmlFailureMock()); import { createInspectCommand } from "./layout.js"; -/** - * citty's `meta` is `Resolvable` (object | promise | thunk). - * This file's commands always define it as a synchronous object literal, so - * narrow to that shape instead of asserting it with `as`. - */ -function metaDescription(command: CommandDef): string { - const meta = command.meta; - if (meta && typeof meta === "object" && "description" in meta) { - return String(meta.description ?? ""); - } - throw new Error("expected a synchronous meta object"); -} - afterEach(() => { vi.restoreAllMocks(); }); @@ -51,53 +31,20 @@ describe("layout command deprecation (U5)", () => { }); it("prints a one-line deprecation notice to stderr and never to stdout", async () => { - const stderrWrites: string[] = []; - const stdoutWrites: string[] = []; - vi.spyOn(process.stderr, "write").mockImplementation((chunk: unknown) => { - stderrWrites.push(String(chunk)); - return true; - }); - vi.spyOn(process.stdout, "write").mockImplementation((chunk: unknown) => { - stdoutWrites.push(String(chunk)); - return true; - }); - vi.spyOn(process, "exit").mockImplementation(() => undefined as never); - vi.spyOn(console, "log").mockImplementation(() => {}); - - await runCommand(createInspectCommand("layout"), { rawArgs: ["--json"] }); - - const stderrText = stderrWrites.join(""); + const { stderrText, stdoutText } = await runAndCaptureStdio(createInspectCommand("layout")); expect(stderrText).toContain("hyperframes layout"); expect(stderrText).toContain("hyperframes check"); - expect(stdoutWrites.join("")).toBe(""); + expect(stdoutText).toBe(""); }); it("--json output is valid JSON with _meta.deprecated === true on failure", async () => { - vi.spyOn(process.stderr, "write").mockImplementation(() => true); - vi.spyOn(process, "exit").mockImplementation(() => undefined as never); - const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); - - await runCommand(createInspectCommand("layout"), { rawArgs: ["--json"] }); - - const jsonCall = logSpy.mock.calls.find( - ([arg]) => typeof arg === "string" && arg.trim().startsWith("{"), - ); - expect(jsonCall).toBeDefined(); - const parsed = JSON.parse(String(jsonCall?.[0])); + const { parsed } = await runAndParseJsonEnvelope(createInspectCommand("layout")); expect(parsed.ok).toBe(false); expect(parsed._meta.deprecated).toBe(true); }); it("the inspect command name produces the same _meta.deprecated === true envelope", async () => { - vi.spyOn(process.stderr, "write").mockImplementation(() => true); - vi.spyOn(process, "exit").mockImplementation(() => undefined as never); - const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); - - await runCommand(createInspectCommand("inspect"), { rawArgs: ["--json"] }); - - const jsonCall = logSpy.mock.calls.find( - ([arg]) => typeof arg === "string" && arg.trim().startsWith("{"), - ); + const jsonCall = await runAndFindJsonLogCall(createInspectCommand("inspect")); const parsed = JSON.parse(String(jsonCall?.[0])); expect(parsed._meta.deprecated).toBe(true); }); diff --git a/packages/cli/src/commands/layout.ts b/packages/cli/src/commands/layout.ts index 1fb0f3a28..82bea5377 100644 --- a/packages/cli/src/commands/layout.ts +++ b/packages/cli/src/commands/layout.ts @@ -529,7 +529,7 @@ export function createInspectCommand(commandName: "inspect" | "layout") { ); } const allIssues = collapseStatic - ? collapseStaticLayoutIssues(result.rawIssues) + ? collapseStaticLayoutIssues(result.rawIssues, result.samples.length) : result.rawIssues; const limited = limitLayoutIssues(allIssues, maxIssues); const summary = summarizeLayoutIssues(allIssues); diff --git a/packages/cli/src/commands/validate.test.ts b/packages/cli/src/commands/validate.test.ts index f35f3ffd8..7b5aebaa0 100644 --- a/packages/cli/src/commands/validate.test.ts +++ b/packages/cli/src/commands/validate.test.ts @@ -1,6 +1,15 @@ -import type { CommandDef } from "citty"; -import { runCommand } from "citty"; import { afterEach, describe, expect, it, vi } from "vitest"; +// Imported before "./validate.js" below: validate.js's own static import of +// ../utils/project.js triggers that mocked module's factory as soon as +// validate.js loads, so resolveProjectMock/lintProjectFailureMock must +// already be bound by then (see the vi.mock calls a few lines down). +import { + lintProjectFailureMock, + metaDescription, + resolveProjectMock, + runAndCaptureStdio, + runAndParseJsonEnvelope, +} from "./deprecationTestHarness.js"; import { extractCompositionErrorsFromLint, navigationTimeoutHint, @@ -37,21 +46,8 @@ vi.mock("../utils/producer.js", () => ({ // (the first await inside validateInBrowser) gives a fast, deterministic // failure well before any real browser or network work — exercising run()'s // outer catch (the JSON failure envelope) without needing headless Chrome. -const FAKE_PROJECT = { - dir: "/fake-project", - name: "fake-project", - indexPath: "/fake-project/index.html", -}; - -vi.mock("../utils/project.js", () => ({ - resolveProject: vi.fn(() => FAKE_PROJECT), -})); - -vi.mock("../utils/lintProject.js", () => ({ - lintProject: vi.fn(async () => { - throw new Error("lint failed (test double)"); - }), -})); +vi.mock("../utils/project.js", () => resolveProjectMock()); +vi.mock("../utils/lintProject.js", () => lintProjectFailureMock()); // Regression for the validate audio-duration-probe timeout: a slow-loading // media element's duration was snapshotted once, at a fixed point in time, @@ -308,14 +304,6 @@ describe("navigationTimeoutHint", () => { }); }); -function metaDescription(command: CommandDef): string { - const meta = command.meta; - if (meta && typeof meta === "object" && "description" in meta) { - return String(meta.description ?? ""); - } - throw new Error("expected a synchronous meta object"); -} - describe("validate command deprecation (U5)", () => { afterEach(() => { vi.restoreAllMocks(); @@ -327,41 +315,16 @@ describe("validate command deprecation (U5)", () => { }); it("prints a one-line deprecation notice to stderr and never to stdout", async () => { - const stderrWrites: string[] = []; - const stdoutWrites: string[] = []; - vi.spyOn(process.stderr, "write").mockImplementation((chunk: unknown) => { - stderrWrites.push(String(chunk)); - return true; - }); - vi.spyOn(process.stdout, "write").mockImplementation((chunk: unknown) => { - stdoutWrites.push(String(chunk)); - return true; - }); - vi.spyOn(process, "exit").mockImplementation(() => undefined as never); - vi.spyOn(console, "log").mockImplementation(() => {}); - const { default: validateCommand } = await import("./validate.js"); - await runCommand(validateCommand, { rawArgs: ["--json"] }); - - const stderrText = stderrWrites.join(""); + const { stderrText, stdoutText } = await runAndCaptureStdio(validateCommand); expect(stderrText).toContain("hyperframes validate"); expect(stderrText).toContain("hyperframes check"); - expect(stdoutWrites.join("")).toBe(""); + expect(stdoutText).toBe(""); }); it("--json output is valid JSON with _meta.deprecated === true on failure", async () => { - vi.spyOn(process.stderr, "write").mockImplementation(() => true); - vi.spyOn(process, "exit").mockImplementation(() => undefined as never); - const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); - const { default: validateCommand } = await import("./validate.js"); - await runCommand(validateCommand, { rawArgs: ["--json"] }); - - const jsonCall = logSpy.mock.calls.find( - ([arg]) => typeof arg === "string" && arg.trim().startsWith("{"), - ); - expect(jsonCall).toBeDefined(); - const parsed = JSON.parse(String(jsonCall?.[0])); + const { parsed } = await runAndParseJsonEnvelope(validateCommand); expect(parsed.ok).toBe(false); expect(parsed._meta.deprecated).toBe(true); }); diff --git a/packages/cli/src/utils/checkBrowser.ts b/packages/cli/src/utils/checkBrowser.ts index da777d59f..8978e2cb8 100644 --- a/packages/cli/src/utils/checkBrowser.ts +++ b/packages/cli/src/utils/checkBrowser.ts @@ -256,6 +256,7 @@ function createPageDriver(page: Page, setTime: (time: number) => void): CheckAud await seekCompositionTimeline(page, time, SEEK_OPTIONS); }, collectLayout: (time, tolerance) => collectLayout(page, time, tolerance), + collectLayoutGeometry: () => collectLayoutGeometry(page), collectGeometryCandidates: (time, request) => collectGeometryCandidates(page, time, request), collectMotionFrame: (time, selectors, scopes) => collectMotionFrame(page, time, selectors, scopes), @@ -368,6 +369,15 @@ async function collectLayout( return anchorLayoutIssues(page, raw.flatMap(parseLayoutIssue)); } +async function collectLayoutGeometry(page: Page): Promise { + return page.evaluate(() => { + const geometry = Reflect.get(window, "__hyperframesLayoutGeometry"); + if (typeof geometry !== "function") return ""; + const result = Reflect.apply(geometry, window, []); + return typeof result === "string" ? result : ""; + }); +} + async function collectGeometryCandidates( page: Page, time: number, @@ -814,6 +824,8 @@ function assignOptionalLayoutFields(issue: LayoutIssue, value: Record | null { diff --git a/packages/cli/src/utils/checkPipeline.ts b/packages/cli/src/utils/checkPipeline.ts index 0f1b08684..0a6ae3008 100644 --- a/packages/cli/src/utils/checkPipeline.ts +++ b/packages/cli/src/utils/checkPipeline.ts @@ -186,6 +186,8 @@ interface GridSamples { contrastEntries: ContrastAuditEntry[]; screenshots: CheckScreenshot[]; contrastMs: number; + /** One geometry+opacity fingerprint per layout sample (#U10 frozen-sweep guard). */ + geometrySignatures: string[]; } interface GeometrySeen { @@ -356,6 +358,7 @@ async function collectGridSamples( contrastEntries: [], screenshots: [], contrastMs: 0, + geometrySignatures: [], }; for (const time of mergeSampleTimes(grid.layoutSamples, motion.times)) { await driver.seek(time); @@ -367,6 +370,7 @@ async function collectGridSamples( const layoutIssues = await driver.collectLayout(time, options.tolerance); collected.layoutIssues.push(...layoutIssues); issuesAtTime.push(...layoutIssues); + collected.geometrySignatures.push(await driver.collectLayoutGeometry()); } if (canvas) { const geometryIssues = await collectGeometryAt( @@ -401,6 +405,55 @@ async function collectGridSamples( return collected; } +// Frozen-sweep guard (#U10): compositions this short can legitimately hold a +// single static frame the whole time (a title card) — never flag those. +const SWEEP_STATIC_MIN_DURATION_SEC = 3; +const ZERO_LAYOUT_RECT: LayoutRect = { + left: 0, + top: 0, + right: 0, + bottom: 0, + width: 0, + height: 0, +}; + +/** + * Frozen-sweep guard (#U10): if every layout-grid sample produced the exact + * same geometry+opacity fingerprint (see layout-audit.browser.js), the seek + * never actually advanced the composition's timeline — every other green + * verdict from this run is meaningless, not just a missed defect. Skips + * short (<3s) compositions, single-sample runs (nothing to compare), and + * runs where a `motion_frozen` finding already reported the same underlying + * symptom (no double-reporting the one thing that's wrong). + */ +function detectSweepStatic( + duration: number, + geometrySignatures: string[], + motionIssues: AnchoredLayoutIssue[], +): AnchoredLayoutIssue[] { + if (duration < SWEEP_STATIC_MIN_DURATION_SEC) return []; + if (geometrySignatures.length < 2) return []; + if (motionIssues.some((issue) => issue.code === "motion_frozen")) return []; + const [first, ...rest] = geometrySignatures; + if (!first || rest.some((signature) => signature !== first)) return []; + return [ + { + code: "sweep_static", + severity: "error", + time: 0, + selector: "[data-composition-id]", + dataAttributes: {}, + sourceFile: "index.html", + bbox: ZERO_BBOX, + rect: ZERO_LAYOUT_RECT, + message: + "Timeline did not advance under seek; every green verdict on this run is unreliable.", + fixHint: + "Confirm the composition seeks a paused GSAP/CSS timeline under `data-*` timing attributes rather than only autoplaying.", + }, + ]; +} + /** 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. */ @@ -431,6 +484,11 @@ export async function runAuditGrid( ); motionIssues = await driver.anchorMotionIssues(evaluated); } + const sweepFindings = detectSweepStatic( + grid.duration, + collected.geometrySignatures, + motionIssues, + ); const contrast = buildContrastResults(collected.contrastEntries); return { duration: grid.duration, @@ -438,7 +496,7 @@ export async function runAuditGrid( transitionSamples: grid.transitionSamples, transitionSamplesDropped: grid.transitionSamplesDropped, runtimeFindings: [], - layoutIssues: collected.layoutIssues, + layoutIssues: [...collected.layoutIssues, ...sweepFindings], motionIssues, motionSampleCount: collected.motionFrames.length, contrastSamples: grid.contrastSamples, @@ -719,7 +777,7 @@ function shapeLayoutSection( browser: CheckBrowserResult, options: CheckOptions, ): CheckReport["layout"] { - const shaped = shapeLayoutFindings(issues, options); + const shaped = shapeLayoutFindings(issues, options, browser.layoutSamples.length); return { ...section(shaped.findings), duration: browser.duration, @@ -735,9 +793,12 @@ function shapeLayoutSection( function shapeLayoutFindings( issues: AnchoredLayoutIssue[], options: CheckOptions, + totalSampleCount?: number, ): { findings: AnchoredLayoutIssue[]; totalIssueCount: number; truncated: boolean } { const deduped = dedupeLayoutIssues(issues); - const all = options.collapseStatic ? collapseStaticLayoutIssues(deduped) : deduped; + const all = options.collapseStatic + ? collapseStaticLayoutIssues(deduped, totalSampleCount) + : deduped; const limited = limitLayoutIssues(all, options.maxIssues); return { findings: limited.issues.map(ensureAnchoredLayoutIssue), diff --git a/packages/cli/src/utils/checkTypes.ts b/packages/cli/src/utils/checkTypes.ts index 63d97a3a9..8625c61a2 100644 --- a/packages/cli/src/utils/checkTypes.ts +++ b/packages/cli/src/utils/checkTypes.ts @@ -131,6 +131,10 @@ export interface CheckAuditDriver { findAmbiguousSelectors(selectors: string[]): Promise; seek(time: number): Promise; collectLayout(time: number, tolerance: number): Promise; + /** Frozen-sweep guard (#U10): an opaque per-sample geometry+opacity + * fingerprint of the current seeked state, for detecting a timeline that + * never advances under seek. See layout-audit.browser.js. */ + collectLayoutGeometry(): Promise; collectGeometryCandidates( time: number, request: GeometryCandidateRequest, diff --git a/packages/cli/src/utils/layoutAudit.test.ts b/packages/cli/src/utils/layoutAudit.test.ts index 6c4ffc970..ba5717103 100644 --- a/packages/cli/src/utils/layoutAudit.test.ts +++ b/packages/cli/src/utils/layoutAudit.test.ts @@ -199,6 +199,82 @@ describe("layoutAudit helpers", () => { }); }); +// #U10: held-duration severity tiering on top of the existing collapse step. +// Sample counts below (9) mirror the CLI's default grid so the "1 sample = +// entrance/exit transient, 2+ adjacent samples = held" framing in the +// approach doc lines up with the numbers used here. +describe("persistence-tiered severity (#U10)", () => { + it("demotes a content_overlap seen at only one sample among several to info", () => { + const collapsed = collapseStaticLayoutIssues( + [{ ...issue("content_overlap", "warning"), time: 3 }], + 9, + ); + + expect(collapsed).toHaveLength(1); + expect(collapsed[0]).toMatchObject({ severity: "info", occurrences: 1 }); + }); + + it("promotes content_overlap held across >= 2 adjacent samples to error", () => { + const collapsed = collapseStaticLayoutIssues( + [ + { ...issue("content_overlap", "warning"), time: 3 }, + { ...issue("content_overlap", "warning"), time: 3.6 }, + ], + 9, + ); + + expect(collapsed).toHaveLength(1); + expect(collapsed[0]).toMatchObject({ severity: "error", occurrences: 2 }); + }); + + it("does not demote a finding held at every sample — persistence, not a single hit", () => { + const collapsed = collapseStaticLayoutIssues( + [ + { ...issue("text_box_overflow", "error"), time: 1 }, + { ...issue("text_box_overflow", "error"), time: 3 }, + { ...issue("text_box_overflow", "error"), time: 5 }, + ], + 9, + ); + + expect(collapsed).toHaveLength(1); + expect(collapsed[0]).toMatchObject({ severity: "error", occurrences: 3 }); + }); + + it("only re-promotes content_overlap — other held codes keep their original severity", () => { + const collapsed = collapseStaticLayoutIssues( + [ + { ...issue("container_overflow", "warning"), time: 3 }, + { ...issue("container_overflow", "warning"), time: 3.6 }, + ], + 9, + ); + + expect(collapsed[0]).toMatchObject({ severity: "warning" }); + }); + + it("skips tiering entirely on a single-sample run — nothing to compare a transient against", () => { + const collapsed = collapseStaticLayoutIssues( + [{ ...issue("content_overlap", "warning"), time: 3 }], + 1, + ); + + expect(collapsed[0]).toMatchObject({ severity: "warning" }); + }); + + it("infers the sample count from distinct issue times when none is given", () => { + // Two distinct times among the raw issues imply a multi-sample run even + // without an explicit count, so the single-occurrence group still demotes. + const collapsed = collapseStaticLayoutIssues([ + { ...issue("content_overlap", "warning"), time: 3 }, + { ...issue("text_box_overflow", "error"), time: 5 }, + ]); + + const overlap = collapsed.find((found) => found.code === "content_overlap"); + expect(overlap).toMatchObject({ severity: "info" }); + }); +}); + function issue(code: LayoutIssue["code"], severity: LayoutIssue["severity"]): LayoutIssue { return { code, diff --git a/packages/cli/src/utils/layoutAudit.ts b/packages/cli/src/utils/layoutAudit.ts index e006bf673..6ba63ccb9 100644 --- a/packages/cli/src/utils/layoutAudit.ts +++ b/packages/cli/src/utils/layoutAudit.ts @@ -18,6 +18,9 @@ export type LayoutIssueCode = | "text_occluded" | "caption_zone_collision" | "frame_out_of_frame" + // Frozen-sweep guard (#U10) — a whole-run meta-finding, not a per-sample + // geometry observation; never persistence-tiered (see `applyPersistenceTier`). + | "sweep_static" // Motion-verification findings (#1437) — evaluated against the seeked timeline. | "motion_appears_late" | "motion_out_of_order" @@ -42,6 +45,9 @@ export interface LayoutIssue { rect: LayoutRect; containerRect?: LayoutRect; overflow?: LayoutOverflow; + /** `text_occluded` only: approximate fraction (0-1) of the occlusion probe + * grid that hit an opaque occluder — see layout-audit.browser.js. */ + coveredFraction?: number; fixHint?: string; } @@ -164,7 +170,44 @@ export function dedupeLayoutIssues(issues: LayoutIssue[]): LayoutIssue[] { return result; } -export function collapseStaticLayoutIssues(issues: LayoutIssue[]): LayoutIssue[] { +// Persistence-tier thresholds (#U10, adapted from Adam Rosler's visual-linter +// design). The approach doc frames these as held-duration floors — ignore +// under ~250ms, re-promote content_overlap at >= ~500ms — measured against +// the SAME firstSeen/lastSeen span this collapse step already tracks. At the +// default 9-sample grid over a multi-second composition, a single collapsed +// occurrence is held 0ms (one entrance/exit transient sample) and two +// collapsed occurrences are already >= one sample-to-sample gap, which is +// well past 500ms — so "held under 250ms" reduces to `occurrences <= 1` and +// "held >= 500ms" reduces to `occurrences >= 2`. Tiering below is written in +// those sample-count terms (the mapping the approach doc asks to document), +// with the literal ms span (CONTENT_OVERLAP_HELD_ERROR_MS) kept as a fallback +// for callers whose samples really are spaced close enough together for the +// ms floor to matter on its own (dense `--at`/`--at-transitions` runs). The +// ~250ms ignore floor needs no separate constant — see the occurrences <= 1 +// branch below. +const CONTENT_OVERLAP_HELD_ERROR_MS = 500; +const HELD_ACROSS_SAMPLES_MIN_OCCURRENCES = 2; + +// Tiering only applies to layout-audit.browser.js's own per-sample seek-grid +// findings — the ones this collapse step's firstSeen/lastSeen span was built +// to describe. `caption_zone_collision`/`frame_out_of_frame` (a different +// script, U3) and the `motion_*`/`sweep_static` codes (evaluated once over +// the whole run, not per grid sample) already carry their own singular +// dedupe/severity semantics; re-interpreting their occurrence count as a +// held-duration signal would misread it. +const PERSISTENCE_TIERED_CODES: ReadonlySet = new Set([ + "text_box_overflow", + "clipped_text", + "canvas_overflow", + "container_overflow", + "content_overlap", + "text_occluded", +]); + +export function collapseStaticLayoutIssues( + issues: LayoutIssue[], + totalSampleCount?: number, +): LayoutIssue[] { const groups = new Map< string, { @@ -193,13 +236,57 @@ export function collapseStaticLayoutIssues(issues: LayoutIssue[]): LayoutIssue[] existing.occurrences += 1; } - return [...groups.values()].map(({ issue, firstSeen, lastSeen, occurrences }) => ({ - ...issue, - time: firstSeen, - firstSeen, - lastSeen, - occurrences, - })); + // A run that only ever sampled one point in time can't distinguish a + // transient from a persistent finding — skip tiering entirely rather than + // guess (see `applyPersistenceTier`). + const sampleCount = totalSampleCount ?? new Set(issues.map((issue) => issue.time)).size; + const multiSampleRun = sampleCount > 1; + + return [...groups.values()].map(({ issue, firstSeen, lastSeen, occurrences }) => + applyPersistenceTier( + { ...issue, time: firstSeen, firstSeen, lastSeen, occurrences }, + multiSampleRun, + ), + ); +} + +/** + * Held-duration severity tiering (#U10). A finding observed at only one + * sample among several (held 0ms) is an entrance/exit transient, not a held + * defect — demote to info so it stays in the data (verbose/--json output) + * without gating the run. `content_overlap` specifically re-promotes from + * warning to error once it's held long enough to be a real, sustained + * collision rather than a crossfade/transition blip (resolves the TODO in + * layout-audit.browser.js's `overlapIssue`). A finding held at every sample + * (a genuinely static defect) is well past both thresholds and is left + * untouched either way — persistence, not the code, decides the tier. + */ +function applyPersistenceTier(issue: LayoutIssue, multiSampleRun: boolean): LayoutIssue { + if (!multiSampleRun) return issue; + if (!PERSISTENCE_TIERED_CODES.has(issue.code)) return issue; + + const occurrences = issue.occurrences ?? 1; + // A single collapsed occurrence is held 0ms by construction (firstSeen === + // lastSeen) — always under the ignore floor, so occurrences <= 1 is a + // complete (not approximate) test for "held under 250ms". + if (occurrences <= 1) { + return { ...issue, severity: "info" }; + } + if (issue.code === "content_overlap" && isContentOverlapHeldLongEnough(issue, occurrences)) { + return { ...issue, severity: "error" }; + } + return issue; +} + +// Split out of applyPersistenceTier so the two independent "held long enough" +// signals (sample count vs. wall-clock span) read as one boolean question +// instead of adding a third compound branch to the tiering ladder above. +function isContentOverlapHeldLongEnough(issue: LayoutIssue, occurrences: number): boolean { + if (occurrences >= HELD_ACROSS_SAMPLES_MIN_OCCURRENCES) return true; + const firstSeen = issue.firstSeen ?? issue.time; + const lastSeen = issue.lastSeen ?? issue.time; + const heldMs = (lastSeen - firstSeen) * 1000; + return heldMs >= CONTENT_OVERLAP_HELD_ERROR_MS; } export function limitLayoutIssues( diff --git a/packages/producer/src/logger.test.ts b/packages/producer/src/logger.test.ts index 8e767d36f..90b6797a2 100644 --- a/packages/producer/src/logger.test.ts +++ b/packages/producer/src/logger.test.ts @@ -2,6 +2,52 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { createConsoleLogger, defaultLogger } from "./logger.js"; import type { LogLevel, ProducerLogger } from "./logger.js"; +// `isLevelEnabled` is optional on ProducerLogger, so every call site guards +// it with `?.`; pulled out once so the loops below stay single-branch. +function isLevelEnabledSafe( + log: Pick, + level: LogLevel, +): boolean | undefined { + return log.isLevelEnabled?.(level); +} + +// Shared by the isLevelEnabled matrix cases below: assert a threshold's +// enabled levels report true and its disabled levels report false. +function assertLevelEnabledMatrix( + log: Pick, + enabled: ReadonlyArray, + disabled: ReadonlyArray, +): void { + for (const lvl of enabled) { + expect(isLevelEnabledSafe(log, lvl)).toBe(true); + } + for (const lvl of disabled) { + expect(isLevelEnabledSafe(log, lvl)).toBe(false); + } +} + +// The `isLevelEnabled?.("debug") ?? true` call-site gate pattern itself, +// isolated so runGatedDebugLoop's own branch count stays at "loop + if". +function isDebugGated(log: Pick): boolean { + return log.isLevelEnabled?.("debug") ?? true; +} + +// Shared by the call-site gate cases below: run the `isLevelEnabled?.("debug") +// ?? true` pattern callers use to skip expensive meta construction, the +// exact number of times the test needs, so each test asserts only the +// pattern's outcome (buildCount / logged calls) and not the loop mechanics. +function runGatedDebugLoop( + log: Pick, + iterations: number, + buildMeta: () => Record, +): void { + for (let i = 0; i < iterations; i++) { + if (isDebugGated(log)) { + log.debug("evt", buildMeta()); + } + } +} + describe("createConsoleLogger", () => { // We capture calls to console.{log,warn,error} via `vi.fn` so we can // assert what would have been printed without polluting test output. @@ -194,12 +240,7 @@ describe("createConsoleLogger", () => { for (const { threshold, enabled, disabled } of cases) { it(`level=${threshold} reports enabled levels correctly`, () => { const log = createConsoleLogger(threshold); - for (const lvl of enabled) { - expect(log.isLevelEnabled?.(lvl)).toBe(true); - } - for (const lvl of disabled) { - expect(log.isLevelEnabled?.(lvl)).toBe(false); - } + assertLevelEnabledMatrix(log, enabled, disabled); }); } @@ -214,11 +255,7 @@ describe("createConsoleLogger", () => { return { expensive: true }; }; - for (let i = 0; i < 100; i++) { - if (log.isLevelEnabled?.("debug") ?? true) { - log.debug("hot-loop", buildMeta()); - } - } + runGatedDebugLoop(log, 100, buildMeta); expect(buildCount).toBe(0); expect(errorSpy.mock.calls.length).toBe(0); @@ -232,11 +269,7 @@ describe("createConsoleLogger", () => { return { iter: buildCount }; }; - for (let i = 0; i < 5; i++) { - if (log.isLevelEnabled?.("debug") ?? true) { - log.debug("loop", buildMeta()); - } - } + runGatedDebugLoop(log, 5, buildMeta); expect(buildCount).toBe(5); expect(errorSpy.mock.calls.length).toBe(5); @@ -260,11 +293,7 @@ describe("createConsoleLogger", () => { return { i: buildCount }; }; - for (let i = 0; i < 3; i++) { - if (customLog.isLevelEnabled?.("debug") ?? true) { - customLog.debug("evt", buildMeta()); - } - } + runGatedDebugLoop(customLog, 3, buildMeta); expect(buildCount).toBe(3); expect(calls).toHaveLength(3);