commit 6c11b0949289c61ac2f872101587bd8c075e6302
parent 688894977476c3b78f8a31837fc74cb026772873
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Wed, 23 Sep 2026 19:47:14 -0400
review fixes: a poll that hangs is abandoned, the dispatcher is text-banned, one row mapping
- `lib/usePolledPayload.ts`: each tick gets an AbortController (aborted in the
effect cleanup) combined with `AbortSignal.timeout(max(10 s, 3 × pollMs))`.
Serial polling had made a never-settling fetch stop the poll for good — new
for the operations board and the sync console, whose setInterval loops used
to recover. A timed-out tick is a failed tick and schedules the next one.
`refetch` gets the same timeout. `pollTimeoutMs` is exported and unit-tested.
- `api/view/views.test.ts`: the constructor text ban also covers
`[name]/route.ts`, so a hoisted shared constructor in the dispatcher fails.
- `widgetActionableRows` (lib/actionable/loadActionable.ts) is the one
row mapping, called by the view handler and by the numbers tool.
- `e2e/view-route.spec.ts`: also strips `disk.freeBytes` from the activeJobs
comparison (a live statfs; equal only while the fixture's disk gate is off).
- Three comments that named deleted routes now name the real file or path.
- editor/CHANGELOG.md entry; the slice record's deviation 4 names the
serial-polling + timeout trade-off.
Numbers re-checked at one instant (old code path vs this one): diff empty.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Diffstat:
12 files changed, 137 insertions(+), 44 deletions(-)
diff --git a/common/lib/operations.ts b/common/lib/operations.ts
@@ -30,7 +30,7 @@
// corpus at the time of writing: 77,106 videos, 836 with media still on disk, 1
// diarized. A single "remaining" number would therefore read 77,105 — and 91x of
// that is unreachable without re-downloading. The repo has already been burned by
-// exactly this once: editor/app/api/widget/actionable/route.ts deliberately
+// exactly this once: common/views/widgetActionable.ts deliberately
// refuses to filter on the digest work count (historically the `noDigest`
// bucket) because during the backfill that is 99.87% of the corpus and counting
// it would put every channel in the list forever. So
diff --git a/common/views/inputs.ts b/common/views/inputs.ts
@@ -7,7 +7,8 @@
// builder that constructs is a builder that cannot be called from a status
// poll, cannot be unit-tested without a real pool, and quietly re-seeds state
// that `/api/test/invalidate-cache` had just cleared between e2e specs. See
-// `editor/app/api/pulse/route.ts` for the incident that rule is written from.
+// `editor/app/api/view/pulseView.ts` (served at /api/view/pulse; /api/pulse is a
+// rewrite onto it) for the incident that rule is written from.
//
// Injecting the readers makes the rule structural rather than remembered: a
// view here CANNOT construct, because it never imports a getter. The editor
diff --git a/editor/CHANGELOG.md b/editor/CHANGELOG.md
@@ -1,6 +1,7 @@
# Changelog
## [Unreleased]
+- **Every live panel now polls one endpoint, `/api/view/<name>`, and the eight old addresses still answer.** The change token, the job head, workers, the operations board, the sync console and the widget's three strips were eight separate API routes that each did the same thing; they are one route serving eight named views (`pulse`, `activeJobs`, `workers`, `autoQueueStatus`, `schedulerStatus`, `widgetSync`, `widgetActionable`, `cleanable`), and the editor's own pages poll it there. `/api/pulse`, `/api/jobs/active`, `/api/workers`, `/api/auto-queue/status`, `/api/scheduler/status` and `/api/widget/{sync,actionable,cleanable}` are kept as **rewrites**, not redirects — same method, status, body and query string (`?rev=` included) — so a monitor widget pinned in a browser, or any script polling the old path, keeps working untouched. `/api/widget/presets` is unchanged. An unknown view name is a 404. **One behaviour change you might notice:** the operations board (every 3 s) and the sync console (every 5 s) now send their next poll only after the previous one answers, and abandon a poll that takes longer than 10–15 s — so a slow editor no longer piles requests up behind itself, and a hung request no longer stops the page updating.
- **A lane's pause is one key on the lane, and the four old pause fields are gone from `settings.json`.** Holding a lane has been `autoQueue.<lane>.held` since the runner work landed; until now the file also still carried the four flags that used to mean it — `transcriptionsPaused`, `downloadsPaused`, `digest.digestsPaused` and the backwards `backfill.enabled` (where *enabled* meant *not held*) — which were read only when a lane had no `held` yet, to carry an older file's pause across. Every lane now carries its own key, so those four are **deleted**: nothing reads them, no form writes them, and the next settings save drops them from the file. A settings.json that still spells one of them holds nothing with it, so a hand-edited file (or a very old backup restored over a newer one) can no longer resurrect a pause you had lifted, or lift one you had set. "Run the backfill lane" on the diarization page and the Hold/Pause buttons write the one key, as they already did. **UPGRADING: boot once on the release that writes `held` before taking this one.** That release is the one that moved the gate onto the lane and carried the old fields across on read; a single boot of it (any settings save, or just starting the editor and pausing/resuming anything) puts `autoQueue.<lane>.held` in your settings.json, after which **nothing you can see changes here** — the same buttons, the same labels, the same pauses. An install that jumps straight from an older release to this one has no `held` keys at all and **loses its pauses**: transcription, downloads and digests come up running, and the backfill lane comes up held. Re-set them from the dashboard, or add the keys by hand before starting.
- **A relocate job says how far it has got.** `rsync` has been printing its progress the whole time (`--info=progress2`) and every frame of it went into the job log as a carriage-return redraw of one line — so a 131 GB move and a 3 MB one looked identical from `/jobs`: a spinner. Now each frame is parsed into the **task bar** every other long job on that page already draws, reading `12.3 GB of 45.6 GB · 27 % · 110.50MB/s · ETA 5:32`, and the log gets **one line per 10 %** instead of several thousand frames of one. The percentage is against the tree the job already measured for its space check, not rsync's own — under incremental recursion that one is a percentage of what it has enumerated so far and walks backwards.
- **`/channels` is where storage is managed now.** Two new columns: **Location** (which volume this channel's media is on — *Internal* when it has not moved) and **Size** (every byte under its `data/`, from its last report, sortable biggest-first). Free space is *not* a column, because it is a fact about a disk and not about a channel: there is one read-out per **volume** in a new bar above the rack, and each chip is also a **filter** — `?location=platter` lists exactly the channels on that drive, and `/storage` links straight here with the biggest first. Beside them, **Free up N GB**: type a number, press *Select largest*, and the largest channels still on the internal disk are ticked until the target is met, ready for the Move button that was already there. Channels already on another volume are never picked (moving one frees nothing on the disk you are emptying) and channels whose report carries no size are **skipped and counted** rather than ranked as empty — which would have put the biggest thing on the disk at the bottom of the list.
diff --git a/editor/app/api/view/views.test.ts b/editor/app/api/view/views.test.ts
@@ -22,6 +22,12 @@ const HERE = path.dirname(fileURLToPath(import.meta.url));
// settings or the worker pool races the singletons the page under test is
// setting up, on a 1-second timer, forever.
//
+// The same ban covers the DISPATCHER (`[name]/route.ts`). The tempting
+// optimisation is one shared constructor hoisted above the table "so every
+// view gets its inputs for free" — which puts pulse on the constructing path
+// again. Each handler in views.ts builds its own inputs; the dispatcher builds
+// none, and this makes that a failing test rather than a comment.
+//
// THE MATCH IS CONTEXT-BLIND, deliberately: a comment naming one of these with
// its parenthesis fails too. Reword the comment; do not loosen the test.
const CONSTRUCTORS = [
@@ -48,6 +54,18 @@ test("the pulse view observes and never constructs", () => {
}
});
+test("the dispatcher constructs nothing before it picks a view", () => {
+ const src = readFileSync(path.join(HERE, "[name]", "route.ts"), "utf8");
+ // Guard the guard: this is the file that dispatches.
+ assert.ok(src.includes("VIEWS[name]("), "[name]/route.ts must dispatch");
+ for (const banned of CONSTRUCTORS) {
+ assert.ok(
+ !src.includes(banned),
+ `[name]/route.ts names ${banned} — the dispatcher may not construct`,
+ );
+ }
+});
+
// THE CLEANABLE ROW, ASSERTED AT COMPILE TIME.
//
// `common/views/cleanable.ts` re-declares the loader's row structurally rather
diff --git a/editor/app/api/view/views.ts b/editor/app/api/view/views.ts
@@ -12,10 +12,8 @@ import { buildSchedulerStatusPayload } from "../../scheduler/status";
import { widgetSyncInputs } from "../../widget/lib/syncInputs";
import { cleanableChannels } from "../../cleanup/lib/loadCleanup";
import {
- actionableDigestReachableCount,
- actionableUndownloadedCount,
- actionableUntranscribedCount,
loadActionableSummary,
+ widgetActionableRows,
} from "../../lib/actionable/loadActionable";
// THE HANDLER TABLE, AND THE COMPLETENESS PROOF.
@@ -63,20 +61,13 @@ export const VIEWS: Record<ViewName, (req: Request) => Promise<Response>> = {
widgetSync: async () =>
NextResponse.json(buildWidgetSyncPayload(await widgetSyncInputs())),
- // The widget's "Needs work" strip. The census read and the map through the
- // four count helpers (which know the availability exclusions) stay here; the
- // filter and the sort are the view's.
+ // The widget's "Needs work" strip. The census read and the per-channel
+ // counting (`widgetActionableRows`, which knows the availability exclusions)
+ // are the editor's; the filter and the sort are the view's.
widgetActionable: async () => {
const summary = await loadActionableSummary(getPaths());
return NextResponse.json(
- buildWidgetActionablePayload(
- summary.rows.map((row) => ({
- slug: row.channel.slug,
- undownloaded: actionableUndownloadedCount(row),
- untranscribed: actionableUntranscribedCount(row),
- digestReachable: actionableDigestReachableCount(row),
- })),
- ),
+ buildWidgetActionablePayload(widgetActionableRows(summary.rows)),
);
},
diff --git a/editor/app/components/dashboard/PipelineBand.tsx b/editor/app/components/dashboard/PipelineBand.tsx
@@ -137,7 +137,7 @@ export function PipelineBand({
{jobList.length > 0 && jobs ? (
// The same one list /jobs draws, with no tail: the band is the head.
- // The cockpit above already polls /api/jobs/active (rewritten to /api/view/activeJobs)
+ // The cockpit above already polls /api/view/activeJobs
// (DashboardCockpit.tsx) and this component polls it again while
// anything is live — the double poll predates this change and is out of
// scope here.
diff --git a/editor/app/lib/actionable/loadActionable.ts b/editor/app/lib/actionable/loadActionable.ts
@@ -1,6 +1,7 @@
import type { Paths } from "yt-dlp-transcript-common/lib/paths";
import { cache } from "react";
import type { ChannelBrief } from "yt-dlp-transcript-common/controller/channels";
+import type { WidgetActionableChannel } from "yt-dlp-transcript-common/views/widgetActionable";
import { getChannelBriefs } from "../requestCache";
import {
digestWorkOf,
@@ -151,6 +152,24 @@ export function actionableCleanExtraFormatsBytes(row: ActionableRow): number {
return row.snapshot?.cleanupBytes?.multipleAudioFormats ?? 0;
}
+// The census rows as the widget's "Needs work" strip counts them: one row per
+// channel, through the three count helpers above. The filter and the sort are
+// `buildWidgetActionablePayload`'s (common/views/widgetActionable.ts); this is
+// the half that knows the availability exclusions, so it stays beside them.
+// Called by the /api/view/widgetActionable handler AND by
+// plans/tools/phase3-view-numbers.ts, so the numbers tool measures the mapping
+// the endpoint actually serves rather than a copy of it.
+export function widgetActionableRows(
+ rows: readonly ActionableRow[],
+): WidgetActionableChannel[] {
+ return rows.map((row) => ({
+ slug: row.channel.slug,
+ undownloaded: actionableUndownloadedCount(row),
+ untranscribed: actionableUntranscribedCount(row),
+ digestReachable: actionableDigestReachableCount(row),
+ }));
+}
+
export async function loadActionableSummary(
paths: Paths,
): Promise<ActionableSummary> {
diff --git a/editor/app/lib/usePolledPayload.test.ts b/editor/app/lib/usePolledPayload.test.ts
@@ -0,0 +1,15 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import { pollTimeoutMs } from "./usePolledPayload";
+
+// The hook itself has no harness here (no DOM, no React renderer in the editor
+// unit suite); its hang recovery is the timeout below plus the rule that a
+// timed-out tick is a failed tick and schedules the next one. This pins the
+// number, so "serial polling can stall forever" cannot come back as a 0 or an
+// accidental per-interval timeout that misses every cold compile.
+test("a poll is abandoned after three intervals, never sooner than 10 s", () => {
+ assert.equal(pollTimeoutMs(1_000), 10_000);
+ assert.equal(pollTimeoutMs(3_000), 10_000);
+ assert.equal(pollTimeoutMs(5_000), 15_000);
+ assert.equal(pollTimeoutMs(15_000), 45_000);
+});
diff --git a/editor/app/lib/usePolledPayload.ts b/editor/app/lib/usePolledPayload.ts
@@ -36,6 +36,14 @@ export function useNow(): number | null {
// first poll would re-fetch what the server just rendered.
export type PollOptions = { immediate?: boolean };
+// How long one poll may take before it is abandoned and the next is scheduled:
+// three intervals, never under 10 s (a cold dev-server compile of a route can
+// legitimately take several seconds, and a timeout that fires on that would
+// turn the first poll of every page into a guaranteed miss).
+export function pollTimeoutMs(pollMs: number): number {
+ return Math.max(10_000, pollMs * 3);
+}
+
export function usePolledPayload<T>(
url: string,
enabled: boolean,
@@ -56,24 +64,41 @@ export function usePolledPayload<T>(
}, []);
const refetch = useCallback(async () => {
try {
- const res = await fetch(url, { cache: "no-store" });
+ const res = await fetch(url, {
+ cache: "no-store",
+ signal: AbortSignal.timeout(pollTimeoutMs(pollMs)),
+ });
if (!res.ok) return;
const next = (await res.json()) as T;
if (mounted.current) setData(next);
} catch {
// transient — ignore
}
- }, [url]);
+ }, [url, pollMs]);
useEffect(() => {
if (!enabled) return;
let cancelled = false;
let timer: ReturnType<typeof setTimeout> | null = null;
+ // One controller for this subscription: the cleanup aborts whatever tick
+ // is in flight, so an unmount or a disable never leaves a request running.
+ const ctrl = new AbortController();
async function tick() {
try {
- const res = await fetch(url, { cache: "no-store" });
+ // THE TIMEOUT IS WHAT MAKES SERIAL POLLING SAFE. The next tick is only
+ // scheduled once this one settles, so a fetch that never settled would
+ // stop the poll for good — the old setInterval loops recovered from a
+ // hang by firing again regardless. A tick that times out is a failed
+ // tick like any other: it lands in `catch` and the next is scheduled.
+ const res = await fetch(url, {
+ cache: "no-store",
+ signal: AbortSignal.any([
+ ctrl.signal,
+ AbortSignal.timeout(pollTimeoutMs(pollMs)),
+ ]),
+ });
if (res.ok && !cancelled) setData((await res.json()) as T);
} catch {
- // transient — keep polling
+ // transient, aborted or timed out — keep polling
} finally {
if (!cancelled) timer = setTimeout(tick, pollMs);
}
@@ -86,6 +111,7 @@ export function usePolledPayload<T>(
else timer = setTimeout(tick, pollMs);
return () => {
cancelled = true;
+ ctrl.abort();
if (timer) clearTimeout(timer);
};
}, [url, enabled, pollMs, immediate]);
diff --git a/editor/e2e/view-route.spec.ts b/editor/e2e/view-route.spec.ts
@@ -15,12 +15,13 @@ import { resetData } from "./helpers";
// this wants a credential, and that the query string survives the rewrite —
// which is the whole of `/api/pulse?rev=`.
-// The two fields that carry the clock. Everything else in these payloads is
-// read from disk or from in-memory state that does not move in an idle fixture,
-// so it is compared verbatim.
+// The fields that move between two back-to-back calls. Everything else in these
+// payloads is read from disk or from in-memory state that does not move in an
+// idle fixture, so it is compared verbatim. Dotted keys reach one level down.
const VOLATILE: Record<string, string[]> = {
- // `builtAt` is Date.now() at build time.
- "/api/jobs/active": ["builtAt"],
+ // `builtAt` is Date.now() at build time. `disk.freeBytes` is a live statfs:
+ // it is null only while the fixture's disk gate is off (minFreeDiskGB: 0).
+ "/api/jobs/active": ["builtAt", "disk.freeBytes"],
// `now` is stamped so the console can age its rows client-side.
"/api/scheduler/status": ["now"],
};
@@ -39,7 +40,16 @@ const PAIRS: Array<[string, string]> = [
function strip(body: unknown, keys: string[]): unknown {
if (!body || typeof body !== "object") return body;
const copy = { ...(body as Record<string, unknown>) };
- for (const key of keys) delete copy[key];
+ for (const key of keys) {
+ const [head, tail] = key.split(".");
+ if (tail === undefined) {
+ delete copy[head];
+ } else if (copy[head] && typeof copy[head] === "object") {
+ const inner = { ...(copy[head] as Record<string, unknown>) };
+ delete inner[tail];
+ copy[head] = inner;
+ }
+ }
return copy;
}
diff --git a/plans/one-core-phase-3.md b/plans/one-core-phase-3.md
@@ -534,9 +534,17 @@ Deviations:
3. **The hook gained `{ immediate?: boolean }`** (default true) and an unmount guard on
`refetch`. useOperationsStatus and SyncConsole are SSR-seeded and never fetched on
mount; they pass `immediate: false`. Both carried the unmount guard by hand.
-4. **Behaviour change: `setInterval` → serial polling** for those two. The hook schedules
- the next tick only after the previous fetch settles, so a slow response can no longer
- stack requests. Cadences unchanged (3 s, 5 s).
+4. **Behaviour change: `setInterval` → serial polling + a timeout** for those two. The
+ trade-off, named: serial polling means a slow response can no longer stack requests
+ behind it, but on its own it also meant a fetch that NEVER settled stopped the poll for
+ good — the old `setInterval` loops recovered from a hang by firing again regardless.
+ The review fix gives every tick an `AbortController` (aborted in the effect cleanup, so
+ an unmount or disable never leaves a request in flight) combined via `AbortSignal.any`
+ with `AbortSignal.timeout(pollTimeoutMs(pollMs))`, where `pollTimeoutMs = max(10 s,
+ 3 × pollMs)`; an abort or timeout is a failed tick and still schedules the next one.
+ `refetch` gets the same timeout. Cadences unchanged (3 s, 5 s). The hook has no React
+ harness in the editor unit suite, so the unit test (`lib/usePolledPayload.test.ts`) pins
+ `pollTimeoutMs` only; the hang recovery itself is not unit-tested.
5. **JobsTable's `enabled` is `polling` state**, seeded from the props and adjusted during
render with `if (polling !== anyLive) setPolling(anyLive)` — React's documented
"adjust state when a prop changes" form. It is needed because the poll's result feeds
@@ -548,7 +556,21 @@ Deviations:
never-nulled `data` are unchanged.
6. **Two comments named deleted routes**: `PipelineBand.tsx:140` and the
`widget/lib/syncInputs.ts` header now say the old path is rewritten to `/api/view/…`.
-7. **Commit trailers** on commits 2–3 were rewritten with `filter-branch` after e2e exited
+7. **Review fixes (one commit after `4288009b`)**: the hook timeout above; the
+ `CONSTRUCTORS` text ban now also covers `api/view/[name]/route.ts`, so a hoisted
+ constructor in the dispatcher fails a test; `view-route.spec.ts` also strips
+ `disk.freeBytes` from the activeJobs comparison (a live statfs, equal only while the
+ fixture's disk gate is off); the widgetActionable row mapping is one exported
+ `widgetActionableRows` in `lib/actionable/loadActionable.ts`, called by both the view
+ handler and the numbers tool; three more comments naming deleted routes fixed
+ (`common/views/inputs.ts`, `common/lib/operations.ts`, `PipelineBand.tsx`); an
+ `editor/CHANGELOG.md` entry. After the mapping moved, the numbers were re-checked:
+ the tool against a fresh read no longer matched the morning's `before` file, because
+ the live corpus had moved (videos 78,238 → 78,254, new sync timestamps — the running
+ editor syncing, not this code). So the comparison was re-run at ONE instant: the
+ pre-slice variant (route bodies copied inline) and the committed tool back to back over
+ the same corpus — **diff empty**.
+8. **Commit trailers** on commits 2–3 were rewritten with `filter-branch` after e2e exited
(they had picked up the wrong model line); the shas above are post-rewrite. Code
unchanged by the rewrite.
diff --git a/plans/tools/phase3-view-numbers.ts b/plans/tools/phase3-view-numbers.ts
@@ -53,10 +53,8 @@ import {
} from "../../common/views/widgetSync";
import { cleanableChannels } from "../../editor/app/cleanup/lib/loadCleanup";
import {
- actionableDigestReachableCount,
- actionableUndownloadedCount,
- actionableUntranscribedCount,
loadActionableSummary,
+ widgetActionableRows,
} from "../../editor/app/lib/actionable/loadActionable";
import { widgetSyncInputs } from "../../editor/app/widget/lib/syncInputs";
@@ -86,18 +84,10 @@ function sortedJson(value: unknown): string {
async function main() {
const paths = getPaths();
- // /api/view/widgetActionable — the shell's map through the four count
- // helpers, then the pure fold.
+ // /api/view/widgetActionable — the handler's own mapping, then the pure fold.
const summary = await loadActionableSummary(paths);
const widgetActionable: WidgetActionablePayload =
- buildWidgetActionablePayload(
- summary.rows.map((row) => ({
- slug: row.channel.slug,
- undownloaded: actionableUndownloadedCount(row),
- untranscribed: actionableUntranscribedCount(row),
- digestReachable: actionableDigestReachableCount(row),
- })),
- );
+ buildWidgetActionablePayload(widgetActionableRows(summary.rows));
// /api/view/cleanable — the lean per-channel snapshot read, then the sum.
const cleanable: CleanablePayload = buildCleanablePayload(
