mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-10-30 09:16:55 +00:00
feat(cli): check-review findingFrames saves one plain frame per finding time (#6401)
* feat(cli): skipContrastInMotion moves to HF_TOOL_CONTEXT check-review The --skip-contrast-in-motion flag and HYPERFRAMES_CHECK_SKIP_CONTRAST_IN_MOTION env give way to a check-review capability in HF_TOOL_CONTEXT, so only a harness turns it on. The skill text and manifest go back to main. * feat(cli): check-review findingFrames saves one plain frame per finding time With HF_TOOL_CONTEXT check-review.findingFrames, every error/warning finding in layout or contrast with a real bbox gets frameTime (the middle sample of its longest held run, by the heldMs gap rule) and frame (the absolute path of an unannotated full frame shared by every finding at that time). One seek and screenshot per distinct time, capped at 60 times with errors first, into the run's snapshot folder; not listed in snapshots, history or events. Without the capability, output is unchanged. * docs(cli): finding-frame comments name the check-review capability, not a flag * fix(cli): validate finding frame times and preserve cancellation * fix(cli): count finding frame progress toward the check deadline
This commit is contained in:
@@ -80,7 +80,7 @@ the harness killing the call.
|
||||
## `check-review`: review switches for `check`
|
||||
|
||||
```json
|
||||
{ "skipContrastInMotion": true }
|
||||
{ "skipContrastInMotion": true, "findingFrames": true }
|
||||
```
|
||||
|
||||
Each field is a boolean and defaults to `false`; a value that is not `true` is
|
||||
@@ -90,6 +90,7 @@ the same as in a terminal.
|
||||
| Field | What it does |
|
||||
| --- | --- |
|
||||
| `skipContrastInMotion` | Leaves out contrast failures measured while a GSAP tween moves the text or an ancestor, or while the text is partly transparent (see [check](/packages/cli#check)). Counts, `ok` and persistence leave them out too. |
|
||||
| `findingFrames` | Saves one plain, unannotated full frame for each distinct `frameTime` of the error and warning findings in `layout` and `contrast` that have a bbox (not `motion` or `craft`), and adds two fields to each of those findings: `frameTime`, the middle sample of the longest held run in its `times` (the same gap rule as `heldMs`), and `frame`, the absolute path of that PNG, shared by every finding at that time. The `bbox` is measured at the finding's `time`, so an element that moves can sit elsewhere on the frame. Only finite, nonnegative sample times are eligible; a finding with no eligible time gets no frame. At most 60 distinct times are captured, errors first. The frames go in the run's snapshot folder but are not listed in `snapshots`, not recorded in project history and not reported as `events`. |
|
||||
|
||||
## Hooks that run code
|
||||
|
||||
|
||||
@@ -12,7 +12,7 @@ vi.mock("../telemetry/events.js", () => ({
|
||||
|
||||
import { contrastRatio, parseColorRGBA } from "./contrast-bg.js";
|
||||
import { createCheckCommand, parseCheckRange } from "./check.js";
|
||||
import { CheckDeadlineError } from "../utils/checkDeadline.js";
|
||||
import { CheckDeadlineError, CheckStoppedError } from "../utils/checkDeadline.js";
|
||||
import { trackCommandFailures } from "../utils/command-failure-tracking.js";
|
||||
import type { CommandDef } from "citty";
|
||||
|
||||
@@ -24,6 +24,7 @@ async function runViaCli(
|
||||
const wrapped = await trackCommandFailures(() => Promise.resolve(command))();
|
||||
return runCommand(wrapped, opts);
|
||||
}
|
||||
import { middleOfLongestRun, withFindingFrameFields } from "../utils/checkFindingFrames.js";
|
||||
import {
|
||||
DEFAULT_CHECK_OPTIONS,
|
||||
checkExitCode,
|
||||
@@ -32,6 +33,7 @@ import {
|
||||
runCheckPipeline,
|
||||
selectContrastTimes,
|
||||
selectFindingCropRequests,
|
||||
selectFindingFrames,
|
||||
type AnchoredLayoutIssue,
|
||||
type CheckAnchor,
|
||||
type CheckAuditDriver,
|
||||
@@ -39,6 +41,7 @@ import {
|
||||
type CheckDependencies,
|
||||
type CheckFinding,
|
||||
type CheckFindingCropRequest,
|
||||
type CheckFindingFrameRequest,
|
||||
type CheckOptions,
|
||||
type CheckReport,
|
||||
type ContrastAuditEntry,
|
||||
@@ -307,6 +310,7 @@ function dependencies(
|
||||
runtime?: CheckFinding[];
|
||||
writeSnapshot?: CheckDependencies["writeSnapshot"];
|
||||
captureFindingCrops?: CheckDependencies["captureFindingCrops"];
|
||||
captureFindingFrames?: CheckDependencies["captureFindingFrames"];
|
||||
inspectHdrAutoPromotion?: NonNullable<CheckDependencies["inspectHdrAutoPromotion"]>;
|
||||
} = {},
|
||||
): { deps: CheckDependencies; runBrowserCheck: ReturnType<typeof vi.fn> } {
|
||||
@@ -332,6 +336,7 @@ function dependencies(
|
||||
),
|
||||
),
|
||||
captureFindingCrops: options.captureFindingCrops ?? vi.fn(async () => []),
|
||||
captureFindingFrames: options.captureFindingFrames ?? vi.fn(async () => []),
|
||||
inspectHdrAutoPromotion: options.inspectHdrAutoPromotion ?? vi.fn(async () => null),
|
||||
};
|
||||
return { deps, runBrowserCheck };
|
||||
@@ -581,6 +586,32 @@ it("preserves --json after bare --frame-check", async () => {
|
||||
expect(log).toHaveBeenCalledWith(expect.stringContaining('"ok"'));
|
||||
});
|
||||
|
||||
it("turns finding frames on only from HF_TOOL_CONTEXT's check-review capability", async () => {
|
||||
const { report } = await runScenario(fakeDriver());
|
||||
const runPipeline = vi.fn(async (_project: ProjectDir, _options: CheckOptions) => report);
|
||||
vi.spyOn(console, "log").mockImplementation(() => undefined);
|
||||
const command = createCheckCommand({
|
||||
resolveProject: () => PROJECT,
|
||||
runPipeline,
|
||||
prepareMedia: vi.fn(async () => []),
|
||||
withMeta: (value) => value,
|
||||
});
|
||||
try {
|
||||
vi.stubEnv("HF_TOOL_CONTEXT", "");
|
||||
await runViaCli(command, { rawArgs: ["--json"] });
|
||||
vi.stubEnv(
|
||||
"HF_TOOL_CONTEXT",
|
||||
JSON.stringify({ version: 1, capabilities: { "check-review": { findingFrames: true } } }),
|
||||
);
|
||||
await runViaCli(command, { rawArgs: ["--json"] });
|
||||
} finally {
|
||||
vi.unstubAllEnvs();
|
||||
}
|
||||
expect(runPipeline.mock.calls[0]?.[1]).not.toHaveProperty("findingFrames");
|
||||
expect(runPipeline.mock.calls[1]?.[1]).toMatchObject({ findingFrames: true });
|
||||
expect(runPipeline.mock.calls[1]?.[1]).not.toHaveProperty("skipContrastInMotion");
|
||||
});
|
||||
|
||||
it("prints a dash-prefixed --frame-check value's parse failure exactly once, not doubled", async () => {
|
||||
// run()'s own try/catch prints parseFrameCheck's error, so the throw site must not print too.
|
||||
const { report } = await runScenario(fakeDriver());
|
||||
@@ -1275,6 +1306,101 @@ describe("selectFindingCropRequests", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("selectFindingFrames", () => {
|
||||
const BOX = { x: 10, y: 20, width: 100, height: 50 };
|
||||
const ZERO = { x: 0, y: 0, width: 0, height: 0 };
|
||||
|
||||
it("frames errors and warnings with a real bbox in layout and contrast, not motion or craft; one frame per time", () => {
|
||||
const report = reportWithFindings({
|
||||
layout: {
|
||||
...reportWithFindings().layout,
|
||||
findings: [
|
||||
layoutFindingOf("clipped_text", "warning", BOX, 2),
|
||||
layoutFindingOf("content_overlap", "info", BOX, 3),
|
||||
layoutFindingOf("text_box_overflow", "error", ZERO, 4),
|
||||
],
|
||||
},
|
||||
contrast: {
|
||||
...reportWithFindings().contrast,
|
||||
findings: [
|
||||
checkFindingOf("contrast_aa_failure", "error", BOX, 2),
|
||||
] as CheckReport["contrast"]["findings"],
|
||||
},
|
||||
craft: {
|
||||
...reportWithFindings().craft,
|
||||
findings: [checkFindingOf("line_unread", "warning", BOX, 5)],
|
||||
},
|
||||
motion: {
|
||||
...reportWithFindings().motion,
|
||||
findings: [checkFindingOf("motion_entrance_missing", "error", BOX, 7)],
|
||||
},
|
||||
runtime: { ...emptySection([checkFindingOf("console_error", "error", BOX, 6)]) },
|
||||
});
|
||||
|
||||
expect(selectFindingFrames(report)).toEqual({
|
||||
requests: [{ time: 2, filename: "finding-frame-00-at-2.000s.png" }],
|
||||
findings: [
|
||||
{ section: "contrast", index: 0, time: 2 },
|
||||
{ section: "layout", index: 0, time: 2 },
|
||||
],
|
||||
skipped: 0,
|
||||
});
|
||||
});
|
||||
|
||||
it("ignores invalid sample times instead of capturing a fabricated zero", () => {
|
||||
const findings = [
|
||||
{ ...layoutFindingOf("clipped_text", "error", BOX, 4), times: [] },
|
||||
{ ...layoutFindingOf("clipped_text", "error", BOX, 4), times: [NaN, Infinity, -1] },
|
||||
{ ...layoutFindingOf("clipped_text", "error", BOX, 4), times: [NaN, 2, 2.5, Infinity] },
|
||||
];
|
||||
const plan = selectFindingFrames(
|
||||
reportWithFindings({ layout: { ...reportWithFindings().layout, findings } }),
|
||||
);
|
||||
expect(plan.requests.map(({ time }) => time)).toEqual([2]);
|
||||
expect(plan.findings).toEqual([{ section: "layout", index: 2, time: 2 }]);
|
||||
});
|
||||
|
||||
it("caps distinct times, keeping errors before warnings", () => {
|
||||
const findings = Array.from({ length: 64 }, (_, index) =>
|
||||
layoutFindingOf("clipped_text", index === 63 ? "error" : "warning", BOX, index),
|
||||
);
|
||||
const plan = selectFindingFrames(
|
||||
reportWithFindings({ layout: { ...reportWithFindings().layout, findings } }),
|
||||
);
|
||||
expect(plan.requests).toHaveLength(60);
|
||||
expect(plan.skipped).toBe(4);
|
||||
expect(plan.findings[0]).toEqual({ section: "layout", index: 63, time: 63 });
|
||||
});
|
||||
});
|
||||
|
||||
it("adds frame metadata only for paths returned by successful captures", () => {
|
||||
const report = reportWithFindings({
|
||||
layout: {
|
||||
...reportWithFindings().layout,
|
||||
findings: [
|
||||
layoutFindingOf("clipped_text", "error", { x: 1, y: 2, width: 30, height: 40 }, 1),
|
||||
layoutFindingOf("clipped_text", "error", { x: 1, y: 2, width: 30, height: 40 }, 2),
|
||||
],
|
||||
},
|
||||
});
|
||||
const plan = selectFindingFrames(report);
|
||||
const framed = withFindingFrameFields(report, plan, ["/run/" + plan.requests[0]?.filename]);
|
||||
expect(framed.layout.findings[0]?.frameTime).toBe(1);
|
||||
expect(framed.layout.findings[1]).toEqual(report.layout.findings[1]);
|
||||
expect(report.layout.findings[0]).not.toHaveProperty("frame");
|
||||
});
|
||||
|
||||
describe("middleOfLongestRun", () => {
|
||||
it("picks the middle sample of the longest held run", () => {
|
||||
expect(middleOfLongestRun([1, 3, 3.25, 3.5, 3.75, 4, 9])).toBe(3.5);
|
||||
expect(middleOfLongestRun([0.125, 0.25, 0.375, 0.5])).toBe(0.25);
|
||||
expect(middleOfLongestRun([2, 5, 8])).toBe(2);
|
||||
expect(middleOfLongestRun([7])).toBe(7);
|
||||
expect(middleOfLongestRun([5, 1, 6, 2, 2])).toBe(1);
|
||||
expect(middleOfLongestRun([1, 1.1, 1.2, 2])).toBe(1.1);
|
||||
});
|
||||
});
|
||||
|
||||
describe("check pipeline", () => {
|
||||
afterEach(() => {
|
||||
consumeCommandResult();
|
||||
@@ -1733,6 +1859,111 @@ describe("check pipeline", () => {
|
||||
expect(noErrors).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not capture finding frames when no finding qualifies", async () => {
|
||||
const capture = vi.fn(async () => []);
|
||||
await runScenario(fakeDriver(), { findingFrames: true }, { captureFindingFrames: capture });
|
||||
expect(capture).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each([new CheckDeadlineError(), new Error("screenshot failed")])(
|
||||
"handles a finding frame failure without fabricated metadata: %s",
|
||||
async (error) => {
|
||||
const { report } = await runScenario(
|
||||
fakeDriver({
|
||||
collectLayout: vi.fn(async (time: number) => [layoutIssue("error", { time })]),
|
||||
}),
|
||||
{ findingFrames: true },
|
||||
{
|
||||
captureFindingFrames: async () => {
|
||||
throw error;
|
||||
},
|
||||
},
|
||||
);
|
||||
expect(report.layout.findings[0]).not.toHaveProperty("frame");
|
||||
expect(report.runtime.findings.length).toBe(error instanceof CheckDeadlineError ? 1 : 0);
|
||||
},
|
||||
);
|
||||
|
||||
it.each([false, true])(
|
||||
"does not duplicate an earlier stop when frames are enabled (snapshots=%s)",
|
||||
async (snapshots) => {
|
||||
const driver = fakeDriver({
|
||||
collectLayout: vi.fn(async (time: number) => [
|
||||
layoutIssue("warning", { time, code: "container_overflow" }),
|
||||
]),
|
||||
});
|
||||
const { deps } = dependencies(driver);
|
||||
deps.runBrowserCheck = async (_project, options, motion, deadline) => {
|
||||
const partial = await runAuditGrid(driver, options, motion);
|
||||
deadline?.keepPartial(() => partial);
|
||||
deadline?.stop("SIGINT");
|
||||
deadline?.signal.throwIfAborted();
|
||||
return partial;
|
||||
};
|
||||
const report = await runCheckPipeline(
|
||||
PROJECT,
|
||||
{ ...DEFAULT_CHECK_OPTIONS, findingFrames: true, snapshots },
|
||||
deps,
|
||||
);
|
||||
expect(
|
||||
report.runtime.findings.filter((finding) => finding.message.includes("SIGINT")),
|
||||
).toHaveLength(1);
|
||||
expect(deps.captureFindingFrames).not.toHaveBeenCalled();
|
||||
},
|
||||
);
|
||||
|
||||
it("reports cancellation during finding frames", async () => {
|
||||
const { report } = await runScenario(
|
||||
fakeDriver({
|
||||
collectLayout: vi.fn(async (time: number) => [
|
||||
layoutIssue("warning", { time, code: "container_overflow" }),
|
||||
]),
|
||||
}),
|
||||
{ findingFrames: true },
|
||||
{
|
||||
captureFindingFrames: async () => {
|
||||
throw new CheckStoppedError("SIGINT");
|
||||
},
|
||||
},
|
||||
);
|
||||
expect(report.ok).toBe(false);
|
||||
expect(report.runtime.findings.some((finding) => finding.message.includes("SIGINT"))).toBe(
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
it("adds frame and frameTime only with finding frames on, leaving snapshots untouched", async () => {
|
||||
const driver = () =>
|
||||
fakeDriver({
|
||||
collectLayout: vi.fn(async (time: number) => [layoutIssue("error", { time })]),
|
||||
});
|
||||
const capture = vi.fn(
|
||||
async (_project: ProjectDir, _options: CheckOptions, requests: CheckFindingFrameRequest[]) =>
|
||||
requests.map((request) => `/run/${request.filename}`),
|
||||
);
|
||||
const plain = await runScenario(driver(), {}, { captureFindingFrames: capture });
|
||||
expect(capture).not.toHaveBeenCalled();
|
||||
|
||||
const { report } = await runScenario(
|
||||
driver(),
|
||||
{ findingFrames: true },
|
||||
{ captureFindingFrames: capture },
|
||||
);
|
||||
const [requests] = capture.mock.calls.map((call) => call[2]);
|
||||
expect(requests).toHaveLength(1);
|
||||
const frameTime = requests?.[0]?.time;
|
||||
expect(report.layout.findings[0]).toEqual({
|
||||
...plain.report.layout.findings[0],
|
||||
frame: `/run/${requests?.[0]?.filename}`,
|
||||
frameTime,
|
||||
});
|
||||
expect(report.snapshots).toEqual(plain.report.snapshots);
|
||||
const { layout, ...rest } = report;
|
||||
const { layout: plainLayout, ...plainRest } = plain.report;
|
||||
expect(rest).toEqual(plainRest);
|
||||
expect({ ...layout, findings: [] }).toEqual({ ...plainLayout, findings: [] });
|
||||
});
|
||||
|
||||
it("reports the total deadline during crops while retaining the audit failure", async () => {
|
||||
const { report } = await runScenario(
|
||||
fakeDriver({
|
||||
|
||||
@@ -396,6 +396,7 @@ function parseCheckOptions(args: Record<string, unknown>): CheckOptions {
|
||||
...(checkReview()?.skipContrastInMotion ? { skipContrastInMotion: true } : {}),
|
||||
strict: args.strict === true,
|
||||
snapshots: args.snapshots === true,
|
||||
...(checkReview()?.findingFrames ? { findingFrames: true } : {}),
|
||||
captionZone: parseCaptionZone(args["caption-zone"]),
|
||||
frameCheck: parseFrameCheck(args["frame-check"]),
|
||||
layout: parseLayout(args.layout),
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
// @vitest-environment happy-dom
|
||||
// fallow-ignore-file code-duplication
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
|
||||
import os, { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import {
|
||||
@@ -12,6 +12,7 @@ import {
|
||||
} from "../capture/captureCompositionFrame.js";
|
||||
import { DEFAULT_CHECK_OPTIONS, runAuditGrid } from "./checkPipeline.js";
|
||||
import {
|
||||
captureFindingFrames,
|
||||
matchComponentFindings,
|
||||
captureOverviewShot,
|
||||
keepBrokenImageAborts,
|
||||
@@ -22,6 +23,8 @@ import {
|
||||
mergeHelperDrafts,
|
||||
samplingBrowserCount,
|
||||
} from "./checkBrowser.js";
|
||||
import { CheckDeadline } from "./checkDeadline.js";
|
||||
import { closeManagedBrowser } from "../browser/launch.js";
|
||||
import type { ProjectDir } from "./project.js";
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
@@ -67,6 +70,9 @@ const mocks = vi.hoisted(() => ({
|
||||
|
||||
vi.mock("../browser/launch.js", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("../browser/launch.js")>()),
|
||||
closeManagedBrowser: vi.fn(
|
||||
(await importOriginal<typeof import("../browser/launch.js")>()).closeManagedBrowser,
|
||||
),
|
||||
onStopSignal: (listener: () => void) => {
|
||||
mocks.stopListeners.add(listener);
|
||||
return () => mocks.stopListeners.delete(listener);
|
||||
@@ -1902,3 +1908,90 @@ describe("motion blur findings in the check output (T6)", () => {
|
||||
expect(blurCodes(result.runtimeFindings)).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("finding frame capture", () => {
|
||||
it("keeps a progressing frame batch alive past 120 seconds", async () => {
|
||||
vi.useFakeTimers();
|
||||
const dir = mkdtempSync(join(tmpdir(), "finding-frames-progress-"));
|
||||
const deadline = new CheckDeadline();
|
||||
const page = Object.assign(fakePage(), { evaluateOnNewDocument: vi.fn(async () => undefined) });
|
||||
page.screenshot.mockImplementation(async () => {
|
||||
await vi.advanceTimersByTimeAsync(4_000);
|
||||
deadline.signal.throwIfAborted();
|
||||
return "frame";
|
||||
});
|
||||
installSessionMock(page);
|
||||
const requests = Array.from({ length: 40 }, (_, index) => ({
|
||||
time: index,
|
||||
filename: `frame-${index}.png`,
|
||||
}));
|
||||
try {
|
||||
const result = await deadline
|
||||
.run(() =>
|
||||
captureFindingFrames({ ...PROJECT, dir }, DEFAULT_CHECK_OPTIONS, requests, deadline),
|
||||
)
|
||||
.catch((error: unknown) => error);
|
||||
expect(result).toHaveLength(40);
|
||||
expect(deadline.signal.aborted).toBe(false);
|
||||
} finally {
|
||||
await deadline.close();
|
||||
vi.useRealTimers();
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("seeks each requested time, writes its screenshot and closes the session", async () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), "finding-frames-"));
|
||||
const page = fakePage();
|
||||
installSessionMock(page);
|
||||
const closeCalls = mocks.serverClose.mock.calls.length;
|
||||
const requests = [
|
||||
{ time: 1, filename: "finding-frame-00.png" },
|
||||
{ time: 2, filename: "finding-frame-01.png" },
|
||||
];
|
||||
try {
|
||||
const paths = await captureFindingFrames(
|
||||
{ ...PROJECT, dir },
|
||||
DEFAULT_CHECK_OPTIONS,
|
||||
requests,
|
||||
);
|
||||
expect(paths).toHaveLength(2);
|
||||
for (const [index, path] of paths.entries()) {
|
||||
expect(readFileSync(path, "utf8")).toBe("c3R1Yg==");
|
||||
expect(seekCompositionTimeline).toHaveBeenCalledWith(
|
||||
page,
|
||||
requests[index]?.time,
|
||||
SCREENSHOT_SEEK_OPTIONS,
|
||||
);
|
||||
}
|
||||
expect(mocks.serverClose.mock.calls.length).toBe(closeCalls + 1);
|
||||
const session = await vi.mocked(openSettledCompositionPage).mock.results.at(-1)?.value;
|
||||
expect(closeManagedBrowser).toHaveBeenCalledWith(session.browser);
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("closes the session when a later screenshot fails", async () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), "finding-frames-failure-"));
|
||||
const page = fakePage();
|
||||
page.screenshot
|
||||
.mockResolvedValueOnce("first")
|
||||
.mockRejectedValueOnce(new Error("capture failed"));
|
||||
installSessionMock(page);
|
||||
const closeCalls = mocks.serverClose.mock.calls.length;
|
||||
try {
|
||||
await expect(
|
||||
captureFindingFrames({ ...PROJECT, dir }, DEFAULT_CHECK_OPTIONS, [
|
||||
{ time: 1, filename: "first.png" },
|
||||
{ time: 2, filename: "second.png" },
|
||||
]),
|
||||
).rejects.toThrow("capture failed");
|
||||
expect(mocks.serverClose.mock.calls.length).toBe(closeCalls + 1);
|
||||
const session = await vi.mocked(openSettledCompositionPage).mock.results.at(-1)?.value;
|
||||
expect(closeManagedBrowser).toHaveBeenCalledWith(session.browser);
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -67,6 +67,7 @@ import type {
|
||||
CheckBrowserResult,
|
||||
CheckFinding,
|
||||
CheckFindingCropRequest,
|
||||
CheckFindingFrameRequest,
|
||||
CheckGeometryCandidate,
|
||||
CheckOptions,
|
||||
CheckSeverity,
|
||||
@@ -565,34 +566,8 @@ export async function captureFindingCrops(
|
||||
deadline?: CheckDeadline,
|
||||
): Promise<string[]> {
|
||||
if (requests.length === 0) return [];
|
||||
const html = await loadCheckHtmlWithDeferredMedia(project.dir);
|
||||
const server = await serveStaticProjectHtml(
|
||||
project.dir,
|
||||
html,
|
||||
"Failed to bind check server",
|
||||
[],
|
||||
options.autoProxy,
|
||||
);
|
||||
let chromeBrowser: import("puppeteer-core").Browser | undefined;
|
||||
const written: string[] = [];
|
||||
let failure: { kind: "none" } | { kind: "failed"; error: unknown } = { kind: "none" };
|
||||
try {
|
||||
const session = await openSettledCompositionPage(html, server.url, {
|
||||
navigationTimeoutMs: options.timeout,
|
||||
renderReadyTimeoutMs: options.timeout,
|
||||
renderReadyWarningSuffix: "capturing finding crops",
|
||||
signal: deadline?.signal,
|
||||
onBrowser: deadline?.ownBrowser,
|
||||
onBrowserReady: deadline?.start,
|
||||
beforeNavigate: (page) => deadline?.preparePage(page),
|
||||
browserGpuMode: options.browserGpuMode ?? resolveCliChromeGpuMode(),
|
||||
});
|
||||
chromeBrowser = session.browser;
|
||||
const page = session.page;
|
||||
await waitForPreferredSeekTarget(page);
|
||||
|
||||
const snapshotDir = checkSnapshotDirectory(project.dir);
|
||||
mkdirSync(snapshotDir, { recursive: true });
|
||||
return withFindingCapturePage(project, options, deadline, "crops", async (page, snapshotDir) => {
|
||||
const written: string[] = [];
|
||||
for (const request of requests) {
|
||||
await seekCompositionTimeline(page, request.time, SCREENSHOT_SEEK_OPTIONS);
|
||||
const canvas = await page.evaluate(() => ({
|
||||
@@ -605,6 +580,62 @@ export async function captureFindingCrops(
|
||||
written.push(join(snapshotDir, request.filename));
|
||||
}
|
||||
return written;
|
||||
});
|
||||
}
|
||||
|
||||
export async function captureFindingFrames(
|
||||
project: ProjectDir,
|
||||
options: CheckOptions,
|
||||
requests: CheckFindingFrameRequest[],
|
||||
deadline?: CheckDeadline,
|
||||
): Promise<string[]> {
|
||||
if (requests.length === 0) return [];
|
||||
return withFindingCapturePage(project, options, deadline, "frames", async (page, snapshotDir) => {
|
||||
const written: string[] = [];
|
||||
for (const request of requests) {
|
||||
deadline?.enter("frames");
|
||||
await seekCompositionTimeline(page, request.time, SCREENSHOT_SEEK_OPTIONS);
|
||||
const buffer = await page.screenshot({ type: "png", optimizeForSpeed: true });
|
||||
writeFileSync(join(snapshotDir, request.filename), buffer);
|
||||
written.push(join(snapshotDir, request.filename));
|
||||
}
|
||||
return written;
|
||||
});
|
||||
}
|
||||
|
||||
async function withFindingCapturePage(
|
||||
project: ProjectDir,
|
||||
options: CheckOptions,
|
||||
deadline: CheckDeadline | undefined,
|
||||
what: "crops" | "frames",
|
||||
capture: (page: Page, snapshotDir: string) => Promise<string[]>,
|
||||
): Promise<string[]> {
|
||||
const html = await loadCheckHtmlWithDeferredMedia(project.dir);
|
||||
const server = await serveStaticProjectHtml(
|
||||
project.dir,
|
||||
html,
|
||||
"Failed to bind check server",
|
||||
[],
|
||||
options.autoProxy,
|
||||
);
|
||||
let chromeBrowser: import("puppeteer-core").Browser | undefined;
|
||||
let failure: { kind: "none" } | { kind: "failed"; error: unknown } = { kind: "none" };
|
||||
try {
|
||||
const session = await openSettledCompositionPage(html, server.url, {
|
||||
navigationTimeoutMs: options.timeout,
|
||||
renderReadyTimeoutMs: options.timeout,
|
||||
renderReadyWarningSuffix: `capturing finding ${what}`,
|
||||
signal: deadline?.signal,
|
||||
onBrowser: deadline?.ownBrowser,
|
||||
onBrowserReady: deadline?.start,
|
||||
beforeNavigate: (page) => deadline?.preparePage(page),
|
||||
browserGpuMode: options.browserGpuMode ?? resolveCliChromeGpuMode(),
|
||||
});
|
||||
chromeBrowser = session.browser;
|
||||
await waitForPreferredSeekTarget(session.page);
|
||||
const snapshotDir = checkSnapshotDirectory(project.dir);
|
||||
mkdirSync(snapshotDir, { recursive: true });
|
||||
return await capture(session.page, snapshotDir);
|
||||
} catch (error) {
|
||||
failure = { kind: "failed", error };
|
||||
throw error;
|
||||
|
||||
@@ -16,6 +16,7 @@ const PHASE_LABELS = {
|
||||
samples: "checking samples",
|
||||
overlap: "running the overlap pass",
|
||||
crops: "capturing finding crops",
|
||||
frames: "capturing finding frames",
|
||||
} as const;
|
||||
export type CheckPhase = keyof typeof PHASE_LABELS;
|
||||
const BEFORE_DEADLINE: ReadonlySet<CheckPhase> = new Set(["copies", "launch"]);
|
||||
|
||||
@@ -0,0 +1,94 @@
|
||||
import { basename } from "node:path";
|
||||
import { longestContiguousRun } from "./layoutAudit.js";
|
||||
import type {
|
||||
CheckBbox,
|
||||
CheckFinding,
|
||||
CheckFindingFrameRequest,
|
||||
CheckReport,
|
||||
} from "./checkTypes.js";
|
||||
|
||||
export const MAX_FINDING_FRAME_TIMES = 60;
|
||||
|
||||
type FramedSection = "layout" | "contrast";
|
||||
const FRAMED_SECTIONS: readonly FramedSection[] = ["layout", "contrast"];
|
||||
|
||||
export interface FramedFinding {
|
||||
section: FramedSection;
|
||||
index: number;
|
||||
time: number;
|
||||
}
|
||||
|
||||
export interface FindingFramePlan {
|
||||
requests: CheckFindingFrameRequest[];
|
||||
findings: FramedFinding[];
|
||||
skipped: number;
|
||||
}
|
||||
|
||||
/** Errors then warnings with a real bbox; one frame per distinct time, at most MAX_FINDING_FRAME_TIMES. */
|
||||
export function selectFindingFrames(report: CheckReport): FindingFramePlan {
|
||||
const candidates = FRAMED_SECTIONS.flatMap((section) =>
|
||||
report[section].findings.flatMap((finding, index) =>
|
||||
(finding.severity === "error" || finding.severity === "warning") && hasRealBbox(finding.bbox)
|
||||
? [{ section, index, finding }]
|
||||
: [],
|
||||
),
|
||||
).sort((a, b) => severityRank(a.finding) - severityRank(b.finding));
|
||||
const times = new Set<number>();
|
||||
const findings: FramedFinding[] = [];
|
||||
let skipped = 0;
|
||||
for (const { section, index, finding } of candidates) {
|
||||
const time = middleOfLongestRun(finding.times ?? [finding.time]);
|
||||
if (time === undefined) continue;
|
||||
if (!times.has(time) && times.size >= MAX_FINDING_FRAME_TIMES) {
|
||||
skipped += 1;
|
||||
continue;
|
||||
}
|
||||
times.add(time);
|
||||
findings.push({ section, index, time });
|
||||
}
|
||||
const requests = [...times]
|
||||
.sort((a, b) => a - b)
|
||||
.map((time, order) => ({ time, filename: findingFrameFilename(order, time) }));
|
||||
return { requests, findings, skipped };
|
||||
}
|
||||
|
||||
function severityRank(finding: CheckFinding): number {
|
||||
return finding.severity === "error" ? 0 : 1;
|
||||
}
|
||||
|
||||
function hasRealBbox(bbox: CheckBbox): boolean {
|
||||
return bbox.width > 0 && bbox.height > 0;
|
||||
}
|
||||
|
||||
/** The middle sample of the longest held run (the `heldMs` gap rule); the earlier of two middles. */
|
||||
export function middleOfLongestRun(times: number[]): number | undefined {
|
||||
const run = longestContiguousRun(times.filter((time) => Number.isFinite(time) && time >= 0));
|
||||
return run[Math.floor((run.length - 1) / 2)];
|
||||
}
|
||||
|
||||
function findingFrameFilename(order: number, time: number): string {
|
||||
return `finding-frame-${String(order).padStart(2, "0")}-at-${time.toFixed(3)}s.png`;
|
||||
}
|
||||
|
||||
/** Copies each framed finding with its frame path and time; every other field is untouched. */
|
||||
export function withFindingFrameFields(
|
||||
report: CheckReport,
|
||||
plan: FindingFramePlan,
|
||||
written: string[],
|
||||
): CheckReport {
|
||||
const pathOf = new Map(written.map((path) => [basename(path), path]));
|
||||
const filenameOf = new Map(plan.requests.map((request) => [request.time, request.filename]));
|
||||
const next: CheckReport = { ...report };
|
||||
for (const section of FRAMED_SECTIONS) {
|
||||
const framed = plan.findings.filter((entry) => entry.section === section);
|
||||
if (framed.length === 0) continue;
|
||||
const findings: CheckFinding[] = [...report[section].findings];
|
||||
for (const entry of framed) {
|
||||
const frame = pathOf.get(filenameOf.get(entry.time) ?? "");
|
||||
const finding = findings[entry.index];
|
||||
if (frame && finding) findings[entry.index] = { ...finding, frame, frameTime: entry.time };
|
||||
}
|
||||
Object.assign(next, { [section]: { ...report[section], findings } });
|
||||
}
|
||||
return next;
|
||||
}
|
||||
@@ -1,12 +1,23 @@
|
||||
import { BrowserCleanupError, onStopSignal } from "../browser/launch.js";
|
||||
import type { CheckProgress } from "./checkProgress.js";
|
||||
import { CheckDeadline, CheckDeadlineError, type CheckPhase } from "./checkDeadline.js";
|
||||
import {
|
||||
CheckDeadline,
|
||||
CheckDeadlineError,
|
||||
CheckStoppedError,
|
||||
type CheckPhase,
|
||||
} from "./checkDeadline.js";
|
||||
import {
|
||||
checkSnapshotDirectory,
|
||||
beginCheckSnapshots,
|
||||
finishCheckSnapshots,
|
||||
} from "./checkSnapshotDirectory.js";
|
||||
import { findingElementKey, groupSampledFindings } from "./checkFindings.js";
|
||||
import {
|
||||
MAX_FINDING_FRAME_TIMES,
|
||||
selectFindingFrames,
|
||||
withFindingFrameFields,
|
||||
} from "./checkFindingFrames.js";
|
||||
export { selectFindingFrames } from "./checkFindingFrames.js";
|
||||
import { mkdirSync, writeFileSync } from "node:fs";
|
||||
import { readFile } from "node:fs/promises";
|
||||
import type { PrelaunchedBrowser } from "../capture/captureCompositionFrame.js";
|
||||
@@ -58,6 +69,7 @@ import type {
|
||||
CheckDependencies,
|
||||
CheckFinding,
|
||||
CheckFindingCropRequest,
|
||||
CheckFindingFrameRequest,
|
||||
CheckGeometryCandidate,
|
||||
CheckOptions,
|
||||
CheckReport,
|
||||
@@ -84,6 +96,7 @@ export type {
|
||||
CheckDependencies,
|
||||
CheckFinding,
|
||||
CheckFindingCropRequest,
|
||||
CheckFindingFrameRequest,
|
||||
CheckOptions,
|
||||
CheckReport,
|
||||
CheckSection,
|
||||
@@ -1688,9 +1701,15 @@ export async function runCheckPipeline(
|
||||
);
|
||||
outcome = {
|
||||
kind: "report",
|
||||
report: options.snapshots
|
||||
? await withFindingCrops(dependencies, project, options, report, deadline)
|
||||
: report,
|
||||
report: await withOptionalFindingFrames(
|
||||
dependencies,
|
||||
project,
|
||||
options,
|
||||
options.snapshots
|
||||
? await withFindingCrops(dependencies, project, options, report, deadline)
|
||||
: report,
|
||||
deadline,
|
||||
),
|
||||
};
|
||||
} catch (error) {
|
||||
outcome = { kind: "failed", error };
|
||||
@@ -1775,6 +1794,37 @@ async function withFindingCrops(
|
||||
}
|
||||
}
|
||||
|
||||
async function withOptionalFindingFrames(
|
||||
dependencies: CheckDependencies,
|
||||
project: ProjectDir,
|
||||
options: CheckOptions,
|
||||
report: CheckReport,
|
||||
deadline: CheckDeadline,
|
||||
): Promise<CheckReport> {
|
||||
if (!options.findingFrames || deadline.signal.aborted) return report;
|
||||
const plan = selectFindingFrames(report);
|
||||
if (plan.requests.length === 0) return report;
|
||||
if (plan.skipped > 0) {
|
||||
console.error(
|
||||
` finding frames: ${plan.skipped} finding(s) left out past ${MAX_FINDING_FRAME_TIMES} distinct times`,
|
||||
);
|
||||
}
|
||||
const capture = dependencies.captureFindingFrames ?? captureFindingFrames;
|
||||
try {
|
||||
deadline.enter("frames");
|
||||
const written = await deadline.run(() => capture(project, options, plan.requests, deadline));
|
||||
return withFindingFrameFields(report, plan, written);
|
||||
} catch (error) {
|
||||
if (error instanceof CheckDeadlineError || error instanceof CheckStoppedError) {
|
||||
const finding = runtimeFailure(error);
|
||||
return { ...report, ok: false, runtime: section([...report.runtime.findings, finding]) };
|
||||
}
|
||||
console.error(" finding frames skipped: " + normalizeErrorMessage(error));
|
||||
trackCommandFailure("check-finding-frames", error);
|
||||
return report;
|
||||
}
|
||||
}
|
||||
|
||||
const MAX_FINDING_CROPS = 12;
|
||||
|
||||
/** Which error findings get a `finding-NN-<code>.png` crop for `check --snapshots`:
|
||||
@@ -2232,6 +2282,16 @@ async function writeSnapshot(
|
||||
return relative(projectDir, path);
|
||||
}
|
||||
|
||||
async function captureFindingFrames(
|
||||
project: ProjectDir,
|
||||
options: CheckOptions,
|
||||
requests: CheckFindingFrameRequest[],
|
||||
deadline?: CheckDeadline,
|
||||
): Promise<string[]> {
|
||||
const module = await import("./checkBrowser.js");
|
||||
return module.captureFindingFrames(project, options, requests, deadline);
|
||||
}
|
||||
|
||||
async function captureFindingCrops(
|
||||
project: ProjectDir,
|
||||
options: CheckOptions,
|
||||
@@ -2250,5 +2310,6 @@ const DEFAULT_DEPENDENCIES: CheckDependencies = {
|
||||
prelaunchBrowser,
|
||||
writeSnapshot,
|
||||
captureFindingCrops,
|
||||
captureFindingFrames,
|
||||
inspectHdrAutoPromotion,
|
||||
};
|
||||
|
||||
@@ -25,6 +25,7 @@ export interface CheckOptions {
|
||||
skipContrastInMotion?: boolean;
|
||||
strict: boolean;
|
||||
snapshots: boolean;
|
||||
findingFrames?: boolean;
|
||||
captionZone?: CaptionZoneOptions;
|
||||
frameCheck?: FrameCheckOptions;
|
||||
/** Opt-in layout-audit knobs (`--layout "proseCoverageFloor=0.05"`). Defaults stay in the browser audit. */
|
||||
@@ -78,6 +79,8 @@ export interface CheckAnchor {
|
||||
time: number;
|
||||
times?: number[];
|
||||
componentMatch?: boolean;
|
||||
frame?: string;
|
||||
frameTime?: number;
|
||||
}
|
||||
|
||||
export interface CheckFinding extends CheckAnchor {
|
||||
@@ -138,6 +141,11 @@ export interface CheckFindingCropRequest {
|
||||
bbox: CheckBbox;
|
||||
}
|
||||
|
||||
export interface CheckFindingFrameRequest {
|
||||
time: number;
|
||||
filename: string;
|
||||
}
|
||||
|
||||
export interface CheckGeometryCandidate extends CheckAnchor {
|
||||
kind: "text" | "media";
|
||||
tag: string;
|
||||
@@ -401,6 +409,12 @@ export interface CheckDependencies {
|
||||
requests: CheckFindingCropRequest[],
|
||||
deadline?: CheckDeadline,
|
||||
): Promise<string[]>;
|
||||
captureFindingFrames?(
|
||||
project: ProjectDir,
|
||||
options: CheckOptions,
|
||||
requests: CheckFindingFrameRequest[],
|
||||
deadline?: CheckDeadline,
|
||||
): Promise<string[]>;
|
||||
inspectHdrAutoPromotion?(project: ProjectDir): Promise<CheckReport["hdr"]["autoPromotion"]>;
|
||||
}
|
||||
|
||||
|
||||
@@ -278,21 +278,24 @@ export function collapseStaticLayoutIssues(
|
||||
}
|
||||
|
||||
export function longestContiguousRunMs(times: number[]): number {
|
||||
const run = longestContiguousRun(times);
|
||||
return ((run.at(-1) ?? 0) - (run[0] ?? 0)) * 1000;
|
||||
}
|
||||
|
||||
export function longestContiguousRun(times: number[]): number[] {
|
||||
const sorted = [...new Set(times)].sort((a, b) => a - b);
|
||||
const first = sorted[0];
|
||||
const last = sorted.at(-1);
|
||||
if (first === undefined || last === undefined) return 0;
|
||||
let longest = 0;
|
||||
let runStart = first;
|
||||
let previous = first;
|
||||
let longest: number[] = [];
|
||||
let run: number[] = [];
|
||||
const span = (values: number[]) => (values.at(-1) ?? 0) - (values[0] ?? 0);
|
||||
for (const current of sorted) {
|
||||
if ((current - previous) * 1000 > CONTIGUOUS_SAMPLE_GAP_MS) {
|
||||
longest = Math.max(longest, previous - runStart);
|
||||
runStart = current;
|
||||
const previous = run.at(-1);
|
||||
if (previous !== undefined && (current - previous) * 1000 > CONTIGUOUS_SAMPLE_GAP_MS) {
|
||||
if (longest.length === 0 || span(run) > span(longest)) longest = run;
|
||||
run = [];
|
||||
}
|
||||
previous = current;
|
||||
run.push(current);
|
||||
}
|
||||
return Math.max(longest, last - runStart) * 1000;
|
||||
return longest.length === 0 || span(run) > span(longest) ? run : longest;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -51,10 +51,12 @@ describe("readToolContext", () => {
|
||||
it("reads check-review switches as booleans that default to false", () => {
|
||||
expect(
|
||||
readToolContext(context({ "check-review": { skipContrastInMotion: true, other: 1 } })),
|
||||
).toEqual({ checkReview: { skipContrastInMotion: true } });
|
||||
expect(readToolContext(context({ "check-review": { skipContrastInMotion: 1 } }))).toEqual({
|
||||
checkReview: { skipContrastInMotion: false },
|
||||
});
|
||||
).toEqual({ checkReview: { skipContrastInMotion: true, findingFrames: false } });
|
||||
expect(
|
||||
readToolContext(
|
||||
context({ "check-review": { skipContrastInMotion: 1, findingFrames: true } }),
|
||||
),
|
||||
).toEqual({ checkReview: { skipContrastInMotion: false, findingFrames: true } });
|
||||
expect(readToolContext(context({ "check-review": true }))).toEqual({});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -6,7 +6,7 @@ const TOOL_CONTEXT_VERSION = 1;
|
||||
export interface ToolContext {
|
||||
events?: { path: string };
|
||||
projectLock?: { waitMs: number };
|
||||
checkReview?: { skipContrastInMotion: boolean };
|
||||
checkReview?: { skipContrastInMotion: boolean; findingFrames: boolean };
|
||||
}
|
||||
|
||||
export type ArtifactRole = "contact-sheet" | "frame" | "finding" | "check-report";
|
||||
@@ -37,7 +37,10 @@ function parseProjectLock(value: unknown): ToolContext["projectLock"] {
|
||||
|
||||
function parseCheckReview(value: unknown): ToolContext["checkReview"] {
|
||||
if (!isRecord(value)) return undefined;
|
||||
return { skipContrastInMotion: value.skipContrastInMotion === true };
|
||||
return {
|
||||
skipContrastInMotion: value.skipContrastInMotion === true,
|
||||
findingFrames: value.findingFrames === true,
|
||||
};
|
||||
}
|
||||
|
||||
export function readToolContext(raw = process.env.HF_TOOL_CONTEXT): ToolContext {
|
||||
|
||||
Reference in New Issue
Block a user