fix(cli): address review — clip-duration audit in check, failure classing, crop observability

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.
This commit is contained in:
Miguel Angel Simon Sierra
2026-07-10 17:53:04 -04:00
parent 96cb5e39dd
commit 67cdf0fdb7
4 changed files with 61 additions and 5 deletions
+1 -1
View File
@@ -132,7 +132,7 @@ export function raceMediaReady(
* the live page to read each element's intrinsic `.duration`, which static lint * the live page to read each element's intrinsic `.duration`, which static lint
* can't see. * can't see.
*/ */
async function auditClipDurations( export async function auditClipDurations(
page: import("puppeteer-core").Page, page: import("puppeteer-core").Page,
analyzeClipMediaFit: typeof import("@hyperframes/engine").analyzeClipMediaFit, analyzeClipMediaFit: typeof import("@hyperframes/engine").analyzeClipMediaFit,
extraWaitMs: number, extraWaitMs: number,
@@ -26,6 +26,13 @@ vi.mock("../capture/captureCompositionFrame.js", async (importOriginal) => ({
waitForPreferredSeekTarget: vi.fn(async () => undefined), 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<typeof import("../commands/validate.js")>()),
auditClipDurations: vi.fn(async () => [] as Array<{ level: "error" | "warning"; text: string }>),
}));
vi.mock("./staticProjectServer.js", () => ({ vi.mock("./staticProjectServer.js", () => ({
serveStaticProjectHtml: vi.fn(async () => ({ serveStaticProjectHtml: vi.fn(async () => ({
url: "http://127.0.0.1:3000", 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 = `
<div data-composition-id="main" data-duration="10" data-width="640" data-height="360"></div>
`;
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", () => { describe("captureOverviewShot", () => {
it("injects the annotation overlay before the overview shot and removes it right after", async () => { it("injects the annotation overlay before the overview shot and removes it right after", async () => {
const calls: string[] = []; const calls: string[] = [];
+10 -1
View File
@@ -12,7 +12,7 @@ import {
seekCompositionTimeline, seekCompositionTimeline,
waitForPreferredSeekTarget, waitForPreferredSeekTarget,
} from "../capture/captureCompositionFrame.js"; } from "../capture/captureCompositionFrame.js";
import { shouldIgnoreRequestFailure } from "../commands/validate.js"; import { auditClipDurations, shouldIgnoreRequestFailure } from "../commands/validate.js";
import { loadBrowserScript } from "../commands/layout.js"; import { loadBrowserScript } from "../commands/layout.js";
import { normalizeErrorMessage } from "./errorMessage.js"; import { normalizeErrorMessage } from "./errorMessage.js";
import { ambiguousIssue, type MotionFrame } from "./motionAudit.js"; import { ambiguousIssue, type MotionFrame } from "./motionAudit.js";
@@ -109,6 +109,15 @@ export async function runBrowserCheck(
const rootAnchor = await resolveRootAnchor(page); const rootAnchor = await resolveRootAnchor(page);
const launchSettleMs = Date.now() - launchSettleStart; 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) => { const driver = createPageDriver(page, (time) => {
currentTime = time; currentTime = time;
}); });
+10 -3
View File
@@ -1,6 +1,6 @@
import { mkdirSync, writeFileSync } from "node:fs"; import { mkdirSync, writeFileSync } from "node:fs";
import { join, relative } from "node:path"; 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 { getRunId } from "../telemetry/runId.js";
import type { ProjectDir } from "./project.js"; import type { ProjectDir } from "./project.js";
import { lintProject, shouldBlockRender, type ProjectLintResult } from "./lintProject.js"; import { lintProject, shouldBlockRender, type ProjectLintResult } from "./lintProject.js";
@@ -518,7 +518,10 @@ export async function runCheckPipeline(
try { try {
lintResult = await dependencies.lintProject(project.dir); lintResult = await dependencies.lintProject(project.dir);
} catch (error) { } 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); const lint = buildLintSection(lintResult);
@@ -589,7 +592,11 @@ async function withFindingCrops(
try { try {
const findingFiles = await dependencies.captureFindingCrops(project, options, cropRequests); const findingFiles = await dependencies.captureFindingCrops(project, options, cropRequests);
return { ...report, snapshots: { ...report.snapshots, findingFiles } }; 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; return report;
} }
} }