From d9972db407cb83d6cf2078ba7ab5ebc6c1a9bbf1 Mon Sep 17 00:00:00 2001
From: Miguel Angel Simon Sierra <miguel.sierra@heygen.com>
Date: Sat, 19 Sep 2026 05:33:12 -0400
Subject: [PATCH] fix(scripts): the capture checker exits 2 when it cannot
 check

A bad option, a missing binary or file, or an undecodable video used to
read as clean or crash with exit 1, which the caller cannot tell from a
flagged capture. Options are now validated, the frame height is passed
to ffmpeg explicitly so it always matches the size the check assumes,
and the tests pin the thresholds, window boundary, ordering and mean.
---
 CONTRIBUTING.md                          |  2 +-
 scripts/check-capture-reversion.mjs      | 74 +++++++++++++++---------
 scripts/check-capture-reversion.test.mjs | 48 ++++++++++++++-
 3 files changed, 95 insertions(+), 29 deletions(-)

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 53ed8631ba..69d8752311 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -149,7 +149,7 @@ The script wipes `docs/catalog/` before regenerating, so deleted items are autom
 
 ### Checking a capture
 
-A screen recording attached to a PR must not show a region briefly returning to an older picture (a clip that reappears for one frame, a panel that flashes its previous state). Run `node scripts/check-capture-reversion.mjs [--crop=W:H:X:Y] <video>...` once per region that matters; it needs `ffmpeg` and `ffprobe`, exits 1 on a flagged video, and names the frames that differ and the two matching frames around them. Continuous motion is not flagged, but a toast or cursor inside the crop can be, so crop it out or check the named frames by eye.
+A screen recording attached to a PR must not show a region briefly returning to an older picture (a clip that reappears for one frame, a panel that flashes its previous state). Run `node scripts/check-capture-reversion.mjs [--crop=W:H:X:Y] <video>...` once per region that matters; it needs `ffmpeg` (and `ffprobe` when no `--crop` is given), takes `--window`, `--change` and `--same` to tune the match, exits 0 when clean, 1 when a video is flagged and 2 when it could not check (bad option, missing binary or file), and names the frames that differ and the two matching frames around them. Continuous motion is not flagged, but a toast or cursor inside the crop can be, so crop it out or check the named frames by eye.
 
 ## Packages
 
diff --git a/scripts/check-capture-reversion.mjs b/scripts/check-capture-reversion.mjs
index 6abe2fdd8a..1b2e39c80b 100644
--- a/scripts/check-capture-reversion.mjs
+++ b/scripts/check-capture-reversion.mjs
@@ -1,17 +1,23 @@
 #!/usr/bin/env node
 // Flags a capture region that briefly returns to an older picture (A, B, A frames).
 // node scripts/check-capture-reversion.mjs [--crop=W:H:X:Y] [--window=N] [--change=D] [--same=D] <video>...
-// Frames are cropped, 96px wide, gray; D is mean abs pixel diff (0-255). Exit 1 if flagged.
+// Frames are cropped, 96px wide, gray; D is mean abs pixel diff (0-255). Exit 0 clean, 1 flagged, 2 could not check.
 
 import { spawn } from "node:child_process";
 import { fileURLToPath } from "node:url";
 
 const WIDTH = 96;
-const args = { crop: undefined, window: 20, change: 1.0, same: 0.25 };
+const USAGE =
+  "usage: node scripts/check-capture-reversion.mjs [--crop=W:H:X:Y] [--window=N] [--change=D] [--same=D] <video> ...";
 
-function readFrames(path, crop) {
+/** Output frame height for a source of cw x ch scaled to WIDTH: even, at least 2. */
+export function frameHeight(cw, ch) {
+  return Math.max(2, 2 * Math.round((WIDTH * (ch / cw)) / 2));
+}
+
+function readFrames(path, crop, height) {
   return new Promise((resolve, reject) => {
-    const scale = `scale=${WIDTH}:-2:flags=area,format=gray`;
+    const scale = `scale=${WIDTH}:${height}:flags=area,format=gray`;
     const vf = crop ? `crop=${crop},${scale}` : scale;
     const child = spawn("ffmpeg", ["-v", "error", "-i", path, "-vf", vf, "-f", "rawvideo", "-"]);
     const chunks = [];
@@ -43,9 +49,10 @@ function probeSize(path) {
     let out = "";
     child.stdout.on("data", (c) => (out += c));
     child.on("error", reject);
-    child.on("close", () => {
+    child.on("close", (code) => {
       const [w, h] = out.trim().split(",").map(Number);
-      resolve({ w, h });
+      if (code !== 0 || !(w > 0) || !(h > 0)) return reject(new Error(`ffprobe found no video size in ${path}`));
+      resolve([w, h]);
     });
   });
 }
@@ -88,26 +95,39 @@ export function findReversions(buf, frameSize, { window, change, same }) {
   return [...byPeak.values()].sort((x, y) => x.changed - y.changed);
 }
 
-function parseArgs(argv) {
+/** Options and video paths from argv; throws on a malformed flag so a typo cannot read as "clean". */
+export function parseArgs(argv) {
+  const options = { crop: undefined, window: 20, change: 1.0, same: 0.25 };
   const paths = [];
   for (const arg of argv) {
-    const m = /^--(crop|window|change|same)=(.*)$/.exec(arg);
-    if (m) args[m[1]] = m[1] === "crop" ? m[2] : Number(m[2]);
-    else paths.push(arg);
+    const m = /^--([a-z]+)=(.*)$/.exec(arg);
+    if (!m) {
+      if (arg.startsWith("--")) throw new Error(`unknown option ${arg}`);
+      paths.push(arg);
+    } else if (m[1] === "crop") {
+      if (!/^[1-9]\d*:[1-9]\d*:\d+:\d+$/.test(m[2])) throw new Error(`--crop needs W:H:X:Y, got ${m[2]}`);
+      options.crop = m[2];
+    } else if (m[1] in options) {
+      const n = Number(m[2]);
+      if (m[2] === "" || !Number.isFinite(n) || n < 0) throw new Error(`--${m[1]} needs a number, got ${m[2]}`);
+      options[m[1]] = n;
+    } else {
+      throw new Error(`unknown option ${arg}`);
+    }
   }
-  return paths;
+  return { options, paths };
 }
 
 // fallow-ignore-next-line complexity
-async function checkVideo(path) {
-  const [cw, ch] = args.crop
-    ? args.crop.split(":").map(Number)
-    : Object.values(await probeSize(path));
-  const frameSize = WIDTH * 2 * Math.round((WIDTH * (ch / cw)) / 2);
-  const buf = await readFrames(path, args.crop);
+async function checkVideo(path, options) {
+  const [cw, ch] = options.crop ? options.crop.split(":").map(Number) : await probeSize(path);
+  const height = frameHeight(cw, ch);
+  const frameSize = WIDTH * height;
+  const buf = await readFrames(path, options.crop, height);
   const frames = Math.floor(buf.length / frameSize);
-  const hits = findReversions(buf, frameSize, args);
-  const crop = args.crop ? `  crop=${args.crop}` : "";
+  if (frames === 0) throw new Error(`no frames decoded from ${path}`);
+  const hits = findReversions(buf, frameSize, options);
+  const crop = options.crop ? `  crop=${options.crop}` : "";
   console.log(
     `${hits.length ? "FLAGGED" : "clean"}  ${path}${crop}  (${frames} frames checked, 0-${frames - 1})`,
   );
@@ -121,16 +141,16 @@ async function checkVideo(path) {
 
 // fallow-ignore-next-line complexity
 async function main(argv) {
-  const paths = parseArgs(argv);
-  if (paths.length === 0) {
-    console.error(
-      "usage: node scripts/check-capture-reversion.mjs [--crop=W:H:X:Y] [--window=N] [--change=D] [--same=D] <video> ...",
-    );
+  try {
+    const { options, paths } = parseArgs(argv);
+    if (paths.length === 0) throw new Error(USAGE);
+    let flagged = false;
+    for (const path of paths) flagged = (await checkVideo(path, options)) || flagged;
+    process.exit(flagged ? 1 : 0);
+  } catch (error) {
+    console.error(`check-capture-reversion: ${error.message}`);
     process.exit(2);
   }
-  let flagged = false;
-  for (const path of paths) flagged = (await checkVideo(path)) || flagged;
-  process.exit(flagged ? 1 : 0);
 }
 
 if (process.argv[1] === fileURLToPath(import.meta.url)) main(process.argv.slice(2));
diff --git a/scripts/check-capture-reversion.test.mjs b/scripts/check-capture-reversion.test.mjs
index 5c8b5e0090..5d01f60fd1 100644
--- a/scripts/check-capture-reversion.test.mjs
+++ b/scripts/check-capture-reversion.test.mjs
@@ -1,7 +1,7 @@
 import { strict as assert } from "node:assert";
 import { test } from "node:test";
 
-import { findReversions } from "./check-capture-reversion.mjs";
+import { findReversions, frameHeight, parseArgs } from "./check-capture-reversion.mjs";
 
 const OPTS = { window: 20, change: 1, same: 0.25 };
 const SIZE = 4;
@@ -30,3 +30,49 @@ test("a return further away than the window is ignored", () => {
   const far = frames(10, 200, 10);
   assert.deepEqual(findReversions(far, SIZE, { ...OPTS, window: 1 }), []);
 });
+
+test("the window boundary is inclusive", () => {
+  const hits = findReversions(frames(10, 200, 10), SIZE, { ...OPTS, window: 2 });
+  assert.equal(hits.length, 1);
+});
+
+test("a change below the threshold is not flagged, one above it is", () => {
+  assert.deepEqual(findReversions(frames(10, 11, 10), SIZE, { ...OPTS, change: 1 }), []);
+  assert.equal(findReversions(frames(10, 12, 10), SIZE, { ...OPTS, change: 1 }).length, 1);
+});
+
+test("a middle frame that matches only one side is not a reversion", () => {
+  assert.deepEqual(findReversions(frames(10, 10, 10, 200), SIZE, OPTS), []);
+});
+
+test("hits come back ordered by the changed frame", () => {
+  const hits = findReversions(frames(10, 200, 10, 10, 90, 10), SIZE, OPTS);
+  assert.deepEqual(
+    hits.map((h) => h.changed),
+    [1, 4],
+  );
+});
+
+test("the reported delta is the mean difference, not the sum", () => {
+  const [hit] = findReversions(frames(10, 110, 10), SIZE, OPTS);
+  assert.equal(hit.delta, 100);
+});
+
+test("frame height is even and never below 2", () => {
+  assert.equal(frameHeight(1280, 200), 16);
+  assert.equal(frameHeight(1280, 800), 60);
+  assert.equal(frameHeight(1000, 2), 2);
+});
+
+test("a malformed option throws instead of reading as clean", () => {
+  for (const bad of ["--crop=iw:200:0:0", "--window=abc", "--change=", "--same=-1", "--crp=1:1:0:0"]) {
+    assert.throws(() => parseArgs([bad, "v.webm"]), bad);
+  }
+});
+
+test("options and paths are separated", () => {
+  const { options, paths } = parseArgs(["--crop=1280:200:0:600", "--window=5", "a.webm", "b.webm"]);
+  assert.deepEqual(paths, ["a.webm", "b.webm"]);
+  assert.equal(options.crop, "1280:200:0:600");
+  assert.equal(options.window, 5);
+});