fix(cli): snapshot rejects a negative --at and builds its sheet from its own frames (#6179)

* fix(cli): snapshot rejects a negative --at and builds its sheet from its own frames

* fix(cli): snapshot --at refuses an empty value and trailing junk too

* test(cli): the contact sheet test's fake image reads orientation like the sheet now does
This commit is contained in:
Miguel Ángel
2026-10-24 13:23:36 +00:00
committed by GitHub
parent 3cbd98890e
commit 80be6ff039
4 changed files with 70 additions and 26 deletions
+5 -12
View File
@@ -214,22 +214,15 @@ export async function createScrollContactSheet(
* Returns array of written file paths.
*/
export async function createSnapshotContactSheet(
snapshotsDir: string,
paths: string[],
outputPath: string,
budget: Pick<ContactSheetOptions, "remainingMs"> = {},
): Promise<string[]> {
if (!existsSync(snapshotsDir)) return [];
if (paths.length === 0) return [];
const snapshotFiles = readdirSync(snapshotsDir)
.filter((f) => f.startsWith("frame-") && f.endsWith(".png"))
.sort();
if (snapshotFiles.length === 0) return [];
const paths = snapshotFiles.map((f) => join(snapshotsDir, f));
const labels = snapshotFiles.map((f) => {
const m = f.match(/at-([\d.]+)s/);
return m ? `${m[1]}s` : f;
const labels = paths.map((path) => {
const m = basename(path).match(/at-([\d.]+)s/);
return m ? `${m[1]}s` : basename(path);
});
// 3 cols, 9 per page (3×3)
@@ -150,12 +150,64 @@ describe("snapshot contact sheet failures", () => {
],
});
expect(generate).toHaveBeenCalledWith(join(project, "snapshots"), sheet);
expect(generate).toHaveBeenCalledWith(
["frame-00-at-0.5s.png", "frame-01-at-1s.png"].map((name) =>
join(project, "snapshots", name),
),
sheet,
);
expect(console.log).toHaveBeenCalledWith(expect.stringContaining("contact-sheet.jpg"));
expect(console.warn).not.toHaveBeenCalled();
expect(console.error).not.toHaveBeenCalled();
});
it("builds the sheet from this run's frames, not another frame-*.png in the folder", async () => {
const output = join(project, "snapshots");
mkdirSync(output, { recursive: true });
writeFileSync(join(output, "frame-hero.png"), "user image");
const read: string[] = [];
const image = {
metadata: async () => ({ width: 640, height: 360, autoOrient: { width: 640, height: 360 } }),
autoOrient: () => image,
resize: () => image,
composite: () => image,
jpeg: () => image,
toBuffer: async () => Buffer.from("sheet"),
};
vi.doMock("../utils/optionalPackages.js", async (importOriginal) => ({
...(await importOriginal<typeof import("../utils/optionalPackages.js")>()),
loadSharp: async () => (input: unknown) => {
if (typeof input === "string") read.push(input);
return image;
},
}));
await runCommand(snapshotCommand, {
rawArgs: [project, ...["--at", "0.5,1", "--no-end", "--describe", "false", "-o", output]],
});
expect([...new Set(read)]).toEqual([
join(output, "frame-00-at-0.5s.png"),
join(output, "frame-01-at-1s.png"),
]);
expect(readFileSync(join(output, "frame-hero.png"), "utf8")).toBe("user image");
expect(console.warn).not.toHaveBeenCalled();
});
it.each(["-1", "0.5,-1", "Infinity", "NaN", "1,", "1junk", ""])(
"refuses --at %s before writing anything",
async (at) => {
const output = join(project, "snapshots");
await expect(
runCommand(snapshotCommand, {
rawArgs: [project, "--at", at, "--no-end", "--describe", "false", "-o", output],
}),
).rejects.toThrow("--at must be comma-separated times in seconds, each 0 or more");
expect(capture.screenshot).not.toHaveBeenCalled();
expect(existsSync(output)).toBe(false);
},
);
it("keeps and records the captured frames when another process holds history", async () => {
const args = {
rawArgs: [
+6 -13
View File
@@ -1,6 +1,7 @@
import { reportArtifacts } from "../utils/toolContext.js";
import { withProjectLock } from "../utils/projectLock.js";
import {
parseSnapshotAt,
parseSnapshotRange,
sampleSnapshotRange,
resolveSnapshotCompositionRange,
@@ -1280,6 +1281,8 @@ export default defineCommand({
}
const frames = parseInt(args.frames as string, 10) || 5;
const range = args.range === undefined ? undefined : parseSnapshotRange(String(args.range));
const atTimestamps = args.at === undefined ? undefined : parseSnapshotAt(String(args.at));
if (args.keep === true && atTimestamps) assertDistinctKeepNames(atTimestamps);
const composition =
args.composition === undefined ? undefined : String(args.composition).trim();
if (composition === "") throw new Error("--composition requires a composition ID");
@@ -1316,13 +1319,6 @@ export default defineCommand({
failCommand();
}
const timeout = parseInt(args.timeout as string, 10) || 5000;
const atTimestamps = args.at
? String(args.at)
.split(",")
.map((s) => parseFloat(s.trim()))
.filter((n) => !isNaN(n))
: undefined;
if (args.keep === true && atTimestamps) assertDistinctKeepNames(atTimestamps);
// Gemini frame analysis runs by default (silently skipped if
// GEMINI_API_KEY is not set). `--describe "custom question"` overrides
// the default prompt with a targeted question. `--describe false` opts
@@ -1379,11 +1375,8 @@ export default defineCommand({
log,
});
const paths = captured.paths;
reportArtifacts(
"snapshot",
"frame",
paths.map((path) => resolve(project.dir, path)),
);
const framePaths = paths.map((path) => resolve(project.dir, path));
reportArtifacts("snapshot", "frame", framePaths);
if (paths.length === 0) {
fail(`\n${c.error("✗")} Could not determine composition duration — no frames captured`);
@@ -1417,7 +1410,7 @@ export default defineCommand({
printReview(captured.review, reviewSheets, log);
} else if (args.keep !== true) {
contactSheets = await createSnapshotContactSheet(
snapshotDir,
framePaths,
join(snapshotDir, "contact-sheet.jpg"),
);
}
@@ -13,6 +13,12 @@ export function parseSnapshotRange(value: string): SnapshotRange {
throw new Error("--range must be START,END in seconds, with 0 <= START < END");
return [range[0]!, range[1]!];
}
export function parseSnapshotAt(value: string): number[] {
const times = value.split(",").map((part) => (part.trim() === "" ? NaN : Number(part)));
if (!times.every((time) => Number.isFinite(time) && time >= 0))
throw new Error("--at must be comma-separated times in seconds, each 0 or more");
return times;
}
export function sampleSnapshotRange(
[start, end]: SnapshotRange,
frames: number,