commit e56c3ab1ea384058514b9ee43671cd4b7738f64d
parent 5221cddf6656fd323c88f4a84e94ebe224799a98
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Sun, 20 Sep 2026 19:33:02 -0400
bench: confirm a clip and the window you moved goes with it
"Do changed windows save on hitting confirm, or do I have to click save
window?" -- they did not. Nudging an edge and pressing `y` advanced to the
next clip and left the moved window unsaved, because the confirmation
patch carried the verdict alone.
The rule about what a window save writes -- the rounded edges, and
clearing a cut the new extent no longer contains -- now lives in one
`windowPatch()` helper that both `save window` and `confirmClip` call, so
there is one rule rather than two that can drift. A confirmation on a
clip whose selection differs from the stored window by more than 0.02s on
either edge sends `{verdict, start, end}` (plus the cut clearing when it
applies) as ONE patch: one write, one token, and no moment where the walk
has advanced away from a window nobody stored. A clip whose edges were
not touched confirms exactly as before, and the lock/would-revert
offering is untouched.
`y` goes through the same callback, so the keyboard walk gets it too.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Diffstat:
3 files changed, 90 insertions(+), 19 deletions(-)
diff --git a/umtool/components/projects/ClipBench.tsx b/umtool/components/projects/ClipBench.tsx
@@ -629,6 +629,32 @@ export default function ClipBench({ data }: { data: ClipBenchData }) {
[draft, clip, save, needNote],
);
+ // ---- the window patch, one rule, two callers ------------------------------
+ //
+ // `save window` writes it on demand; a confirmation carries it when the edges
+ // have actually moved. Both need the same rule about a cut the new extent no
+ // longer contains, so the rule lives here rather than in each of them.
+ const windowMoved =
+ Math.abs(round2(sel.from) - clip.start) > 0.02 || Math.abs(round2(sel.to) - clip.end) > 0.02;
+
+ const windowPatch = useCallback((): {
+ start: number;
+ end: number;
+ cutStart?: string;
+ cutEnd?: string;
+ } => {
+ const start = round2(sel.from);
+ const end = round2(sel.to);
+ const cutOutside =
+ clip.cutStart != null &&
+ clip.cutEnd != null &&
+ (clip.cutStart < start - 0.02 || clip.cutEnd > end + 0.02);
+ // The extent is the judgement being made right now; the cut was derived
+ // from a wider one and is no longer inside it. Clearing it in the SAME
+ // patch is what keeps the writer's rule and the screen agreeing.
+ return cutOutside ? { start, end, cutStart: "", cutEnd: "" } : { start, end };
+ }, [sel.from, sel.to, clip.cutStart, clip.cutEnd]);
+
// ---- the walk's verdict ---------------------------------------------------
//
// "Is this clip what the report says it is" is the question the walk exists
@@ -637,12 +663,23 @@ export default function ClipBench({ data }: { data: ClipBenchData }) {
// case; saying no stays put, because the note has to be typed -- and then
// re-read, which is why it still stays put once it is saved.
const confirmClip = useCallback(async () => {
+ // A MOVED WINDOW RIDES ALONG. Confirming a clip whose edges you just nudged
+ // is a judgement about THAT window, and the advance would otherwise walk
+ // away from it -- so the edges go in the SAME patch as the verdict rather
+ // than needing `save window` pressed first. One write, one token.
+ const win = windowMoved ? windowPatch() : null;
// A note survives a confirmation. It stops being a complaint and becomes
// what it now says it is: why this clip is here in the shape it is in.
- const ok = await save({ verdict: "confirmed" });
+ const ok = await save({ verdict: "confirmed", ...(win ?? {}) });
if (ok) setNeedNote(false);
+ if (ok && win)
+ setNote(
+ win.cutStart !== undefined
+ ? "saved — the window moved with it, and the cut no longer fitted and was cleared"
+ : "saved — the window you moved was saved with it",
+ );
if (ok && data.next) router.push(`/browse/${data.project}/clip/${data.next}`);
- }, [save, router, data.project, data.next]);
+ }, [save, router, data.project, data.next, windowMoved, windowPatch]);
const rejectClip = useCallback(() => {
setNeedNote(true);
@@ -925,23 +962,12 @@ export default function ClipBench({ data }: { data: ClipBenchData }) {
/** Save the window, and drop a cut the new extent no longer contains. */
const saveWindow = useCallback(() => {
- const start = round2(sel.from);
- const end = round2(sel.to);
- const cutOutside =
- clip.cutStart != null &&
- clip.cutEnd != null &&
- (clip.cutStart < start - 0.02 || clip.cutEnd > end + 0.02);
- if (cutOutside) {
- // The extent is the judgement being made right now; the cut was derived
- // from a wider one and is no longer inside it. Clearing it in the SAME
- // patch is what keeps the writer's rule and the screen agreeing.
- void save({ start, end, cutStart: "", cutEnd: "" }).then((ok) => {
- if (ok) setNote("saved — the cut no longer fitted this window and was cleared");
- });
- return;
- }
- void save({ start, end });
- }, [sel.from, sel.to, clip.cutStart, clip.cutEnd, save]);
+ const patch = windowPatch();
+ void save(patch).then((ok) => {
+ if (ok && patch.cutStart !== undefined)
+ setNote("saved — the cut no longer fitted this window and was cleared");
+ });
+ }, [windowPatch, save]);
// ---- the warnings --------------------------------------------------------
const endCue = cues.find((c) => sel.to >= c.start - 0.02 && sel.to <= c.end + 0.02) ?? null;
diff --git a/umtool/docs/clip-bench.md b/umtool/docs/clip-bench.md
@@ -228,6 +228,11 @@ the cursor in the `correction` box and marks it **required**, because the note
clip. That save is ONE patch, `{correction, verdict: "incorrect"}`, so the
manifest never holds a complaint with no verdict.
+An edge you nudged but never saved is confirmed WITH the clip: if the selection
+differs from the stored window, <kbd>y</kbd> carries `start`/`end` (and clears a
+cut the new extent no longer contains) in the same patch as the verdict, so
+walking on never loses the window you just chose.
+
A note on a clip that is already **confirmed** is just a note — why its window
moved, a caveat for the writers — and writing one there leaves the verdict
alone. The state line reads *not yet reviewed*, *confirmed*, *confirmed · with
diff --git a/umtool/e2e/clip-bench.spec.ts b/umtool/e2e/clip-bench.spec.ts
@@ -619,6 +619,46 @@ test("`y` confirms the clip and walks on; the manifest says so", async ({ page,
await expect.poll(() => readClip("c01").verdict).toBeUndefined();
});
+test("`y` saves a window you nudged but never saved, in the same write", async ({
+ page,
+ request,
+}) => {
+ // Known ground on both clips the walk touches, so the only thing that moves
+ // an edge here is the key press below.
+ const { token: t0 } = await token(request, "c01");
+ await request.put("/api/report/window", {
+ data: { project: PROJECT, clip: "c01", start: 3, end: 6, verdict: "", token: t0 },
+ });
+ const { token: t1 } = await token(request, "c04");
+ await request.put("/api/report/window", {
+ data: { project: PROJECT, clip: "c04", verdict: "", token: t1 },
+ });
+
+ await page.goto(bench("c01"));
+ await keyboardLive(page);
+ // 6.00 -> 6.05, unsaved: `save window` is lit and nobody has pressed it.
+ await page.locator("body").press(".");
+ await expect(page.getByRole("button", { name: "save window" })).toBeEnabled();
+
+ await page.locator("body").press("y");
+
+ // ONE write carries both. Confirming a clip whose edge you just nudged is a
+ // judgement about THAT window, and the advance would otherwise walk away
+ // from it.
+ await expect.poll(() => readClip("c01").end).toBe(6.05);
+ expect(readClip("c01").start).toBe(3);
+ expect(readClip("c01").verdict).toBe("confirmed");
+ // And it still advances, over c02 and c03, which are not fetched.
+ await expect(page.locator("[data-bench=c04]")).toBeVisible();
+
+ // Put c01 back where the rest of this file found it.
+ const { token: t2 } = await token(request, "c01");
+ await request.put("/api/report/window", {
+ data: { project: PROJECT, clip: "c01", start: 3, end: 9, verdict: "", token: t2 },
+ });
+ await expect.poll(() => readClip("c01").verdict).toBeUndefined();
+});
+
test("`x` requires the note, writes both fields at once, and stays put", async ({ page }) => {
await page.goto(bench("c03"));
// The key IS the assertion: `x` answers "no" by putting the cursor where the
