commit 8a955cde3e0169e4de307391227b6ec7c87e4059
parent bd4bbe8ec9fec7e33741450000c1190ed8e31a57
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Mon, 21 Sep 2026 02:02:30 -0400
deliver: prove Stop abandons the rest, and read a wrapped exclusion sentence
Two from review.
STOP HAD NO TEST, and it is the whole argument for one step per clip: the
cancel kills the running ffmpeg's process group and abandons the REST, and the
next press picks up the clips with no file rather than re-cutting the ones that
have one. Both halves are now asserted on disk -- a count after the stop, then
the MTIMES of the files the first job wrote, which must not move.
Made deterministic rather than racy. deliver-stop-fixture is six confirmed,
uncut clips of its own (stopping halfway through deliver-fixture's two would
leave the batch tests a state they do not expect), and bin/cut-from-cache.mjs
honours UMTOOL_CUT_DELAY_MS -- test-only, set by the fixture's server, two
seconds per step. The click waits for the first CUT-OK, so six seconds of work
provably cannot have happened when Stop lands.
And `sharedIdsIn` read the "already shared (…)" sentence line-scoped, so a
hand-wrapped list silently dropped its exclusions -- which is how that sentence
is actually written: the phrase ends one line and the parenthesis opens the
next. It now spans up to 120 characters, which reads a wrap and still will not
let "already shared" in one paragraph claim a parenthetical three paragraphs
down. Verified on a wrapped copy of the real sentence: all six ids come back.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Diffstat:
6 files changed, 128 insertions(+), 7 deletions(-)
diff --git a/umtool/bin/cut-from-cache.mjs b/umtool/bin/cut-from-cache.mjs
@@ -40,6 +40,14 @@ const reencode = argv.includes("--reencode")
? "never"
: "auto";
+// TEST-ONLY PACING, and the only reason it exists: a cut of a cached window is
+// about a second of ffmpeg, which is too fast for a spec to press Stop in the
+// middle of -- and "Stop abandons the REST, and the next run resumes rather
+// than re-cutting" is the whole argument for one step per clip. The e2e
+// fixture's server sets this; nothing else ever should.
+const delay = Number(process.env.UMTOOL_CUT_DELAY_MS ?? 0);
+if (delay > 0) await new Promise((r) => setTimeout(r, delay));
+
const res = await cutClipFromCache(r.project, clipId, { reencode });
if (!res.ok) {
console.error(`${clipId}: ${res.error}`);
diff --git a/umtool/docs/clip-bench.md b/umtool/docs/clip-bench.md
@@ -360,7 +360,7 @@ that is a different list, and it lives on the **project page** as `Deliver`
| | |
|---|---|
| **counts by section** | folded on the id's letter prefix, because `69 of 163 confirmed` says nothing about the section where eleven of nineteen clips were thrown out — and that is the section whose argument has to change. Section HEADINGS are read out of the report's own `content.py`, nth heading to nth letter. |
-| **cut N confirmed clips from cache** | one job, **one step per clip**, so `k of n` and Stop are the job runner's own (`stepIndex`, and a cancel that kills the step's process group). |
+| **cut N confirmed clips from cache** | one job, **one step per clip**, so `k of n` and Stop are the job runner's own (`stepIndex`, and a cancel that kills the step's process group). Stop abandons the REST; the next press is the server's list again — "confirmed, no file" — so it resumes without re-cutting. |
| **not fetched** | a confirmed clip with nothing cached that holds it. Listed, never downloaded here — the managed fetch is the editor's, on the clip page. |
| **build share batch** | `share-<name>/{orig,std,small}/<Section>/<id>_<date>_<title>.mp4` + `LIST.md`. |
| **apply rulings** | runs the project's own `apply-manifest.py`, then `umtool corrections`. |
@@ -392,7 +392,9 @@ existing `share-*/LIST.md` already shipped and every clip ruled `incorrect`. A
id counts as shipped, in order of how much they can be trusted: the file NAMES
a list gives, the `<!-- shared-ids: … -->` marker this writes, and — narrowly —
the phrase `already shared (…)`, taking only the tokens shaped like a clip id.
-The last one is not decoration: six ElfpireEva clips went out in an earlier set
+(the parenthesis has to follow the phrase within ~120 characters, so one line
+wrap is fine and a paragraph away is not). The last one is not decoration: six
+ElfpireEva clips went out in an earlier set
under DIFFERENT ids (`em01`, `ie01` — a separate cut of the same moments), and
the sentence the next batch wrote down is the only thing on disk tying the two.
Without it, the next batch re-ships all six. A batch this writes phrases it the
diff --git a/umtool/e2e/deliver.spec.ts b/umtool/e2e/deliver.spec.ts
@@ -1,6 +1,6 @@
import { test, expect } from "@playwright/test";
import { execFileSync } from "node:child_process";
-import { existsSync, readFileSync } from "node:fs";
+import { existsSync, readdirSync, readFileSync, statSync } from "node:fs";
import path from "node:path";
import { fileURLToPath } from "node:url";
@@ -24,6 +24,7 @@ const HERE = path.dirname(fileURLToPath(import.meta.url));
const FIXTURE = path.join(HERE, "..", ".e2e-song");
const PROJECT = "reports/deliver-fixture";
const DIR = path.join(FIXTURE, "reports", "deliver-fixture");
+const STOP_DIR = path.join(FIXTURE, "reports", "deliver-stop-fixture");
const seconds = (file: string): number =>
Number(
@@ -224,3 +225,70 @@ test("rebuild runs build.py once per content variant present", async ({ page })
"built from content_lawyer",
);
});
+
+
+// ---------------------------------------------------------------------------
+// STOP, and what "resume" means.
+//
+// One step per clip is not a presentation choice: it is what makes Stop kill
+// the running ffmpeg's process group and abandon the REST, and what makes the
+// next press pick up the clips that have no file rather than re-cutting the
+// ones that do. Both halves are asserted on DISK -- a count, and then the
+// mtimes of the files the first job wrote, which must not have moved.
+//
+// Deterministic rather than racy: the fixture's server sets
+// UMTOOL_CUT_DELAY_MS=2000 (playwright.config.ts), the job is six clips long,
+// and the click waits for the first CUT-OK. Three clips of work -- six
+// seconds -- therefore cannot have happened by the time Stop lands.
+// ---------------------------------------------------------------------------
+
+const stopCuts = () =>
+ readdirSync(path.join(STOP_DIR, "clips"))
+ .filter((n) => /^s\d+\.mp4$/.test(n))
+ .sort();
+
+test("Stop abandons the rest of the cut, and the next one resumes without re-cutting", async ({
+ page,
+}) => {
+ test.setTimeout(240_000);
+ await page.goto("/browse/reports/deliver-stop-fixture");
+ await expect(page.locator("[data-deliver]")).toHaveAttribute("data-deliver-need-cut", "6");
+ expect(stopCuts()).toHaveLength(0);
+
+ await page.locator('[data-action="deliver-cut"]').click();
+ await expect(page.locator("[data-deliver-progress]")).toContainText(/of 6/);
+ // WAIT FOR REAL WORK. Stopping before anything finished would prove only
+ // that a job can be cancelled, not that what it had already done survives.
+ await expect(page.locator("[data-deliver-log]")).toContainText("CUT-OK", { timeout: 90_000 });
+ await page.locator('[data-action="deliver-stop"]').click();
+
+ const stopped = await waitForJob(page);
+ // A cancelled step is a failed one, and the log says who asked.
+ expect(stopped.state, stopped.log).toBe("failed");
+ expect(stopped.log).toContain("cancel");
+
+ const done = stopCuts();
+ expect(done.length).toBeGreaterThanOrEqual(1);
+ expect(done.length).toBeLessThan(6);
+ const stamps = new Map(
+ done.map((n) => [n, statSync(path.join(STOP_DIR, "clips", n)).mtimeMs]),
+ );
+
+ // THE SERVER'S LIST IS THE RESUME. It is "confirmed, no file", so the second
+ // job is exactly the remainder -- nothing in the browser had to remember
+ // where the first one stopped.
+ await page.reload();
+ await expect(page.locator("[data-deliver]")).toHaveAttribute(
+ "data-deliver-need-cut",
+ String(6 - done.length),
+ );
+ await page.locator('[data-action="deliver-cut"]').click();
+ const second = await waitForJob(page);
+ expect(second.state, second.log).toBe("done");
+ expect(stopCuts()).toHaveLength(6);
+
+ for (const [name, ms] of stamps) {
+ expect(statSync(path.join(STOP_DIR, "clips", name)).mtimeMs, `${name} was re-cut`).toBe(ms);
+ expect(second.log, `${name} was re-cut`).not.toContain(`CUT-OK ${name.replace(".mp4", "")}`);
+ }
+});
diff --git a/umtool/e2e/fixtures/make-fixture.mjs b/umtool/e2e/fixtures/make-fixture.mjs
@@ -1126,6 +1126,38 @@ ff([
path.join(DELIVER, "clips", "b01.mp4"),
]);
+// A SECOND deliver project, for STOP.
+//
+// Its own, because the cut is one job over every clip that needs one: a spec
+// that stopped halfway through deliver-fixture's two would leave that project
+// in a state the batch tests do not expect, and a six-clip job is what makes
+// "stopped after k of n" a measurement rather than a race.
+//
+// Six confirmed clips, none of them cut, all inside the same cached window.
+// With UMTOOL_CUT_DELAY_MS set (playwright.config.ts) each step takes about
+// two seconds, so the spec can see the first CUT-OK, press Stop, and know that
+// at least three clips could not possibly have been reached.
+const STOP = writeProject(
+ "deliver-stop-fixture",
+ manifest("deliver-stop-fixture", "The Deliver Stop Fixture", { siteOrigin: "https://archive.example" }, [
+ { type: "clip", id: "s01", video: "vid6", start: 0.0, end: 2.0, cite: 0, section: 0, lock: true, verdict: "confirmed", date: "2025-02-01", title: "A Fixture Stream", quote: "The deliver fixture opens." },
+ { type: "clip", id: "s02", video: "vid6", start: 2.0, end: 4.0, cite: 2, section: 0, lock: true, verdict: "confirmed", date: "2025-02-02", title: "A Fixture Stream", quote: "The first clip is confirmed." },
+ { type: "clip", id: "s03", video: "vid6", start: 4.0, end: 6.0, cite: 4, section: 0, lock: true, verdict: "confirmed", date: "2025-02-03", title: "A Fixture Stream", quote: "And so is the second." },
+ { type: "clip", id: "s04", video: "vid6", start: 6.0, end: 8.0, cite: 6, section: 0, lock: true, verdict: "confirmed", date: "2025-02-04", title: "A Fixture Stream", quote: "And so is the third." },
+ { type: "clip", id: "s05", video: "vid6", start: 8.0, end: 10.0, cite: 8, section: 0, lock: true, verdict: "confirmed", date: "2025-02-05", title: "A Fixture Stream", quote: "Which does not start on a keyframe." },
+ { type: "clip", id: "s06", video: "vid6", start: 10.0, end: 12.0, cite: 10, section: 0, lock: true, verdict: "confirmed", date: "2025-02-06", title: "A Fixture Stream", quote: "Nor does this one." },
+ ]),
+);
+mkdirSync(path.join(STOP, "out", "clips-raw"), { recursive: true });
+copyFileSync(
+ path.join(DELIVER, "out", "clips-raw", "vid6_0.00-24.00.mp4"),
+ path.join(STOP, "out", "clips-raw", "vid6_0.00-24.00.mp4"),
+);
+// An EMPTY clips/, so the spec can count files without first asking whether
+// the directory exists -- and so "nothing has been cut yet" is a state the
+// fixture states rather than one it leaves to chance.
+mkdirSync(path.join(STOP, "clips"), { recursive: true });
+
// -- STUB BINARIES, so a build is offline and deterministic --------------------
//
// The pipeline shells out to yt-dlp for the availability preflight and for every
@@ -1409,4 +1441,5 @@ console.log(` walk-fixture (read-only: w01/w04 walkable, w02 unfetche
console.log(` editor-fetch-{,many-,reuse-}fixture (nothing cached — the editor fetch's subjects),`);
console.log(` longform-fixture (cue gap, legacy .bak, ffmeta), longform-edit-fixture, dash-fixture`);
console.log(` deliver-fixture (writable: a01/a02 to cut, a03 unfetched, b01 shared, b02 incorrect, b03 unjudged)`);
+console.log(` deliver-stop-fixture (writable: six confirmed clips to cut, for Stop and resume)`);
console.log(` ${taken} candidate files copied, 2 mix tracks synthesised`);
diff --git a/umtool/lib/report/deliver.mjs b/umtool/lib/report/deliver.mjs
@@ -107,10 +107,16 @@ export async function sharedIdsIn(dir) {
// h02)". Without this the next batch re-ships all six.
//
// Prose, and read as narrowly as prose can be: the phrase, then a
- // parenthesis, then only the tokens that are shaped like a clip id. The
- // batches this writes phrase it the same way, so a generated list round
- // trips through here unchanged.
- for (const m of list.matchAll(/already shared[^(\n]*\(([^)]*)\)/gi)) {
+ // parenthesis within the next 120 characters, then only the tokens that
+ // are shaped like a clip id. The batches this writes phrase it the same
+ // way, so a generated list round trips through here unchanged.
+ //
+ // The 120 is what lets a HAND-WRAPPED list still be read -- the phrase and
+ // its parenthesis routinely end up on two lines -- while stopping "already
+ // shared" in one paragraph from claiming a parenthetical three paragraphs
+ // down. Ids inside the parenthesis are still filtered by shape, so a
+ // wrongly-claimed one contributes nothing unless it reads like a clip id.
+ for (const m of list.matchAll(/already shared[^(]{0,120}\(([^)]*)\)/gi)) {
for (const tok of m[1].split(/[\s,]+/)) {
if (/^[A-Za-z]{1,3}\d{1,3}$/.test(tok)) ids.add(tok);
}
diff --git a/umtool/playwright.config.ts b/umtool/playwright.config.ts
@@ -71,6 +71,10 @@ export default defineConfig({
// with. Set here rather than in a step's env: jobView() echoes a step's
// env back to the browser, and this is a token.
`ARCHILYZER_EDITOR_URL=http://127.0.0.1:${STUB_PORT} WORKER_TOKEN=umtool-e2e-token ` +
+ // Two seconds of pacing per CUT, so the deliver spec can press Stop in
+ // the middle of a job and prove that cancelling abandons the rest while
+ // the next run resumes. Read only by bin/cut-from-cache.mjs.
+ `UMTOOL_CUT_DELAY_MS=2000 ` +
`NEXT_DIST_DIR=.next-e2e pnpm exec next dev --port ${PORT}`,
port: PORT,
reuseExistingServer: false,
