From 67cdf0fdb795984addb0cf3588a1678db107bd08 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Fri, 10 Jul 2026 17:53:04 -0400 Subject: [PATCH] =?UTF-8?q?fix(cli):=20address=20review=20=E2=80=94=20clip?= =?UTF-8?q?-duration=20audit=20in=20check,=20failure=20classing,=20crop=20?= =?UTF-8?q?observability?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Port validate's per-media-element clip audit into check's session (clip_media_fit findings): an intrinsic duration meaningfully shorter than the data-duration slot silently shortens the slot at render time, and neither lint nor the runtime listeners can see it. A linter crash now reports as check_lint_failure instead of masquerading as a runtime failure. Finding-crop capture failures stay non-gating but emit a stderr note and a telemetry error event so rollouts can measure the second-session failure rate. --- packages/cli/src/commands/validate.ts | 2 +- packages/cli/src/utils/checkBrowser.test.ts | 40 +++++++++++++++++++++ packages/cli/src/utils/checkBrowser.ts | 11 +++++- packages/cli/src/utils/checkPipeline.ts | 13 +++++-- 4 files changed, 61 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts index a3b99ae40..962461571 100644 --- a/packages/cli/src/commands/validate.ts +++ b/packages/cli/src/commands/validate.ts @@ -132,7 +132,7 @@ export function raceMediaReady( * the live page to read each element's intrinsic `.duration`, which static lint * can't see. */ -async function auditClipDurations( +export async function auditClipDurations( page: import("puppeteer-core").Page, analyzeClipMediaFit: typeof import("@hyperframes/engine").analyzeClipMediaFit, extraWaitMs: number, diff --git a/packages/cli/src/utils/checkBrowser.test.ts b/packages/cli/src/utils/checkBrowser.test.ts index 1d986c4fe..1e1933166 100644 --- a/packages/cli/src/utils/checkBrowser.test.ts +++ b/packages/cli/src/utils/checkBrowser.test.ts @@ -26,6 +26,13 @@ vi.mock("../capture/captureCompositionFrame.js", async (importOriginal) => ({ waitForPreferredSeekTarget: vi.fn(async () => undefined), })); +vi.mock("../commands/validate.js", async (importOriginal) => ({ + // Partial mock: shouldIgnoreRequestFailure stays real; the clip audit is + // faked so tests control its findings without loading real media. + ...(await importOriginal()), + auditClipDurations: vi.fn(async () => [] as Array<{ level: "error" | "warning"; text: string }>), +})); + vi.mock("./staticProjectServer.js", () => ({ serveStaticProjectHtml: vi.fn(async () => ({ url: "http://127.0.0.1:3000", @@ -206,6 +213,39 @@ it("round-trips the browser script's raw contrast candidates back into finish", } }); +it("carries validate's clip-duration audit into the runtime findings", async () => { + vi.spyOn(Date, "now").mockReturnValue(100); + document.body.innerHTML = ` +
+ `; + Object.defineProperty(window, "innerWidth", { configurable: true, value: 640 }); + Object.defineProperty(window, "innerHeight", { configurable: true, value: 360 }); + const validateModule = await import("../commands/validate.js"); + vi.mocked(validateModule.auditClipDurations).mockResolvedValue([ + { + level: "warning", + text: "Audio is 22.10s but its slot (data-duration) is 30.00s — the slot is shortened to the media length when rendered.", + }, + ]); + const page = fakePage(); + installSessionMock(page); + + const result = await runBrowserCheck( + PROJECT, + { ...DEFAULT_CHECK_OPTIONS, samples: 1, contrast: false }, + { kind: "none" }, + runAuditGrid, + ); + + expect(result.runtimeFindings).toEqual([ + expect.objectContaining({ + code: "clip_media_fit", + severity: "warning", + message: expect.stringContaining("slot is shortened"), + }), + ]); +}); + describe("captureOverviewShot", () => { it("injects the annotation overlay before the overview shot and removes it right after", async () => { const calls: string[] = []; diff --git a/packages/cli/src/utils/checkBrowser.ts b/packages/cli/src/utils/checkBrowser.ts index f44828ae1..d889e41b7 100644 --- a/packages/cli/src/utils/checkBrowser.ts +++ b/packages/cli/src/utils/checkBrowser.ts @@ -12,7 +12,7 @@ import { seekCompositionTimeline, waitForPreferredSeekTarget, } from "../capture/captureCompositionFrame.js"; -import { shouldIgnoreRequestFailure } from "../commands/validate.js"; +import { auditClipDurations, shouldIgnoreRequestFailure } from "../commands/validate.js"; import { loadBrowserScript } from "../commands/layout.js"; import { normalizeErrorMessage } from "./errorMessage.js"; import { ambiguousIssue, type MotionFrame } from "./motionAudit.js"; @@ -109,6 +109,15 @@ export async function runBrowserCheck( const rootAnchor = await resolveRootAnchor(page); const launchSettleMs = Date.now() - launchSettleStart; + // validate's per-media-element audit, kept in the consolidation: a clip + // whose intrinsic duration is meaningfully shorter than its data-duration + // slot silently shortens the slot at render time — invisible to lint (no + // intrinsic durations statically) and to the runtime listeners (nothing + // errors). The session is already open, so this is one extra evaluate. + const { analyzeClipMediaFit } = await import("@hyperframes/engine"); + for (const entry of await auditClipDurations(page, analyzeClipMediaFit, options.timeout)) { + drafts.push({ code: "clip_media_fit", severity: entry.level, message: entry.text, time: 0 }); + } const driver = createPageDriver(page, (time) => { currentTime = time; }); diff --git a/packages/cli/src/utils/checkPipeline.ts b/packages/cli/src/utils/checkPipeline.ts index 7ecb196ee..8148654b2 100644 --- a/packages/cli/src/utils/checkPipeline.ts +++ b/packages/cli/src/utils/checkPipeline.ts @@ -1,6 +1,6 @@ import { mkdirSync, writeFileSync } from "node:fs"; import { join, relative } from "node:path"; -import { trackCheckReport } from "../telemetry/events.js"; +import { trackCheckReport, trackCommandFailure } from "../telemetry/events.js"; import { getRunId } from "../telemetry/runId.js"; import type { ProjectDir } from "./project.js"; import { lintProject, shouldBlockRender, type ProjectLintResult } from "./lintProject.js"; @@ -518,7 +518,10 @@ export async function runCheckPipeline( try { lintResult = await dependencies.lintProject(project.dir); } catch (error) { - return failureReport(options, runtimeFailure(error)); + // The linter itself crashed (unreadable file, internal error) — distinct + // from lint findings; a runtime-failure code would send the agent hunting + // for a script problem that doesn't exist. + return failureReport(options, runtimeFailure(error, "check_lint_failure")); } const lint = buildLintSection(lintResult); @@ -589,7 +592,11 @@ async function withFindingCrops( try { const findingFiles = await dependencies.captureFindingCrops(project, options, cropRequests); return { ...report, snapshots: { ...report.snapshots, findingFiles } }; - } catch { + } catch (error) { + // Still non-gating, but observable: rollouts need the crop-failure rate + // (a second Chrome launch failing/timing out) without failing the run. + console.error(" finding crops skipped: " + normalizeErrorMessage(error)); + trackCommandFailure("check-finding-crops", error); return report; } }