commit af39d6ccf573b389477c73660efef5c3401fa268
parent 66f21c97b8e5f542173d185c99d5b2a6320956ae
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Mon, 7 Sep 2026 21:45:21 -0400
plans: slice 1.2 is shipped, and the record says what it cost
The `### 1.2, as shipped` section for `plans/one-core-phase-1.md`: the seven
commits, what was deleted, the measurements, every divergence from the plan's
bullets with the reason, the bug the new e2e spec found, Operator gate A as a
checklist an operator can follow without re-reading the slice, and the traps for
whoever writes 1.3.
The measurements worth having in one place:
* numbers — the before/after diff of `phase1-numbers.ts` over the live corpus
is EMPTY, and stays empty because the live settings.json carries no
`autoQueue.digest` or `.backfill` block. The two lanes are live in code and
disabled on disk;
* the status poll, warm — 86-102 ms before, 386-413 ms with the lanes live
and no parse memo, 256 ms with it. The lanes going live cost ~150 ms a poll
and `readChannelSnapshotShared` gives ~130-155 ms of that back. The cold
first pass is 4.8 s: the duration index filling for the `cheapest` order,
paid once per process;
* the `cheapest` composition, which is DATE primary and duration as the
tiebreak — the opposite of the order the two sort() calls it replaces are
written in, and the half a 1.3 migration would otherwise drop;
* 515 e2e passed / 0 failed in 23.0 min, against 511 before.
Gate A names the exact settings block to add, the channel to scope it to and
why that one (`ObviousRises-rumble`, 6 reachable digests at the last snapshot),
what to watch, and the two steps — resume after a restart, and hold without
stopping — that 1.3 does not land without.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Diffstat:
1 file changed, 323 insertions(+), 0 deletions(-)
diff --git a/plans/one-core-phase-1.md b/plans/one-core-phase-1.md
@@ -327,3 +327,326 @@ three-second poll, so widen the projection in `run()`'s loop rather than in
`computeLeafPending` if the two need to differ. `AutoQueueStatusPayload` is
`Record<AutoQueueKind, AutoQueueKindStatus> & { lanes }`, so `data.digest` /
`data.backfill` are already on the wire.
+
+## 1.2, as shipped
+
+`83954f1` (one executor) → `596ddb6` (the runner runs operations) → `8c5a49d`
+(the console per lane) → `ddbdb30` (the e2e spec) → `1f531fb` (dead half-rules
+removed) → `24a2069` (three fixes to the long-lived half) → `9a16d65` (two
+runners, one state object — the bug the e2e spec found), on `one-core/phase-1`
+off `3ced7dd`. **Numbers: the before/after diff of `plans/tools/phase1-numbers.ts`
+over the live corpus is EMPTY.** It stays empty because the live `settings.json`
+carries no `autoQueue.digest` or `.backfill` block, and the script's lane
+sections come from the KEYS PRESENT IN THE FILE — which is exactly why it was
+written that way. The two lanes are live in code and disabled on disk.
+
+**Deleted:** `controller/digestBatch.ts` (647 lines) and
+`controller/backfillBatch.ts` (937), replaced by `controller/operationBatch.ts`.
+`countMissingDigests` and `countBackfillWork` are one `countOperationWork(lane,
+paths, channel, opts)`. `backfillBatch.test.ts` is `operationBatch.test.ts`, and
+`lib/operations.test.ts -> controller/backfillBatch` is off
+`architecture.test.ts`'s allow-list — the test that needed it moved to the
+dispatch layer where it belongs. **No entry was added.**
+
+Verification: `tsc --noEmit` clean in common, editor, export, mcp, homepage and
+umtool; `yt-dlp-transcript-common` **896** tests (890 before, +6 `laneLimit`
+cases; the moved `candidateAction` test is a relocation, not an addition),
+`yt-dlp-transcript-mcp` **205**; `next build` clean; full editor e2e from a
+`p12-e2e` worktree of `9a16d65` — **515 passed, 0 failed, 23.0 min**, one worker
+behind the queue lock. 511 before, +4 from `lane-runner.spec.ts`.
+
+### What it measures
+
+**The status poll**, over the live corpus, `plans/tools/phase1-poll-timing.ts`
+(new; `ROUNDS=` and `NOMEMO=1`). It folds `computeLeafPending` for all four
+lanes exactly as `buildAutoQueueStatusPayload` does:
+
+| | warm round | what the two new lanes fold |
+|---|---|---|
+| before (1.1) | 86–102 ms | nothing — both projections were empty |
+| after, no memo | 386–413 ms | digest 56,223 · backfill 80,193 pending ids |
+| after, with memo | **256 ms** | same |
+
+So the lanes going live costs ~150 ms a poll and the `(mtime, size)` parse memo
+in `readChannelSnapshotShared` gives ~130–155 ms of that back. The first pass of
+a process is 4.8 s: that is the duration index filling for the `cheapest` order,
+55,956 head-reads at concurrency 64, and a duration never changes, so it is paid
+once per process and never again.
+
+### Decisions the plan asked for
+
+**The `cheapest` composition, preserved exactly.** `digestBatch.ts:242-261`
+sorted by DURATION and then sorted AGAIN, stably, by upload date. Array#sort is
+stable and the recency comparator returns 0 for two videos sharing a YYYYMMDD
+key, so the live meaning is **date primary, duration as the tiebreak within a
+day** — "newest day first, shortest video within a day" — which is the opposite
+of the order the two sort() calls are written in. That is now one comparator,
+`cheapestComparator`, and the date half's DIRECTION is a parameter, today
+`settings.digest.recencyOrder` (live value `newest`). Unknown durations sort
+LAST in either direction: the batch dropped them from candidacy entirely, and
+the runner cannot (its candidates are the snapshot's reachable ids), so it puts
+what it cannot price behind what it can rather than letting `null` sort first.
+**1.3 fixes the direction at newest-first** when `recencyOrder` retires into the
+tree; a digest lane wanting pure recency then sets `order: newest` and gets no
+duration term at all.
+
+`cheapest` on a lane other than digest is accepted by the sanitizer (one enum,
+one sanitizer) and **falls back to LISTED** — `recencyOrdering` returns a null
+comparator, which everywhere else in the runner means "do not sort". Not id
+order and not a date order nobody asked for: both would be a real reordering
+dressed up as a fallback. The console does not offer it there either
+(`LANE_ORDERS`).
+
+**The status poll memo** is above. It is deliberately NOT on `readChannelSnapshot`
+itself: that one is called by the snapshot GENERATOR and by half a dozen editor
+render paths, and handing out a shared parsed object is a claim about
+read-only-ness that code has never had to honour.
+
+### What review found afterwards, and `24a2069` fixed
+
+Three things that only bite a runner, which is the half of this slice with no
+precedent — every previous caller of this machinery was a job that ended.
+
+- **The metered spend cap was resettable by a clock.** `openOperationRun` starts
+ a context with `costUsd: 0`, and the runner rebuilds its context on a
+ sixty-second TTL, so a `$5` per-run cap would have become `$5 a minute` with
+ the log still printing a cap it was no longer enforcing. The accumulator and
+ the disk-floor latch survive the refresh now.
+- **A refresh could swap the context out from under an in-flight unit**, whose
+ llm and unit-executor leases decrement counters on the object it was
+ dispatched with — a permanently leaked slot from `laneLimit`'s point of view.
+ The refresh waits for the lane to be idle.
+- **A dead engine was re-probed every three seconds**, because the TTL covered a
+ successful open and not a failed one.
+
+Plus two smaller ones: the batch's resume line said "Transcription finished"
+whatever the hold had been, so lifting an operator pause reported something that
+never happened; and `countOperationWork` resolved each operation's freshness
+target inside the per-video loop rather than once per operation.
+
+### The bug the e2e spec found, and `9a16d65` fixed
+
+**Every runner held its own copy of `.auto-queue/state.json`, and
+`writeAutoQueueState` serializes the WHOLE file.** So each persist wrote back
+that copy's idea of the other three lanes: whichever runner dispatched most
+recently erased the others' pick log and fairness memory. It is a pre-existing
+defect that only this slice could expose — auto-transcribe persists minutes
+apart and auto-download rarely faster, so with one slow lane it is invisible.
+With two fast lanes it is immediate: the digest lane wrote a pick and the
+backfill runner overwrote it a few hundred milliseconds later, and
+`/api/auto-queue/status` reads the FILE, so the console showed a lane with zero
+picks and `no-pending` while its `ai-digest.json` files were appearing on disk.
+
+The runners share one object now, on the auto-runner singleton — whose lifetime
+is already right, since the e2e harness clears it between specs.
+`computeLeafPending` and the download lane's cooldown merge still read the file
+on purpose: the first wants a value it can clone without touching live
+fairness, the second exists precisely to pick up what a manual sync wrote from
+outside the runner.
+
+This is what the concurrency spec was for, and it is worth saying how it was
+nearly missed: the first version of that spec sampled for "both lanes in flight
+at one instant", which is the same property measured by a coin toss — it failed
+on a loaded machine and passed in isolation, which reads exactly like flake. The
+version that shipped compares the two timestamped pick logs AFTER the work, and
+it failed the same way in isolation, which is what turned "flaky test" into
+"empty pick log for a lane that is demonstrably working".
+
+### Divergences from the 1.2 bullets, and why
+
+- **One batch function, not two.** The plan kept `runDigestBatch` /
+ `runBackfillBatch` and had them "drive operationBatch". They are one
+ `runOperationBatch({lane, …})` with one `OperationBatchResult`, because the
+ two result types were the drift: the card's summary printed `deferred` and
+ `blocked`, the sweep's printed `skipped`, and neither printed the other's.
+ `runDigestChannelJob` / `runBackfillChannelJob` keep their job kinds, their
+ queue keys and their own summary lines, which is what the bullet was
+ protecting.
+- **`runOperationUnit(run, unit, opts)` takes a prepared run context**, not
+ `{op, videoId, channel, settings, signal, force}`. The engine, the
+ duplicate-cluster plan and the fan-out envelope are per-RUN state — resolving
+ them per video is precisely how a counter and a runner end up disagreeing
+ about what is stale — so `openOperationRun` resolves them once and both
+ callers (the batch, and the runner's loop) hold one.
+- **`laneLimit(settings, live)`**, with the lane on a tagged `live` union rather
+ than as a separate first argument, so TypeScript narrows the shape instead of
+ the function casting.
+- **The digest unit calls `digestVideo`, not `op.run()`.** `OperationRunOutcome`
+ is five strings and digest's registry entry discards `engineCalls`, `costUsd`
+ and `warningCount`. Routing it through `run()` would have silently disabled
+ the SPEND CAP — `laneLimit` reads a per-run cost that would then never leave
+ zero — and emptied the metered accounting line. It is one place, named in a
+ comment, and the fix is to widen `OperationRunOutcome` to carry a cost, not to
+ grow a second executor.
+- **`buildPendingByLeaf` gained `defaultOperations`.** The plan said a
+ digest/backfill leaf naming no operation draws `operationsForLane(lane)`; the
+ pure function had no way to express that, since `defaultBuckets` only reaches
+ `ch.buckets`. It is the operation half of the same parameter and is `[]` for
+ the two bucket lanes, so every tree written before operations existed is
+ byte-identical.
+- **A pick is a VIDEO; the operation is chosen at dispatch.** `pending[leaf]` is
+ a list of ids and a union-drawing leaf holds a video that may need diarization
+ OR attribution, so `run()` asks the lane's operations in dependency order and
+ runs the first with work. That forced the session's completed-set to be keyed
+ by **(operation, video)**: retiring the whole video after one unit would break
+ the dependency chain the lane exists to walk. A video leaves a leaf's list
+ only once every operation THAT LEAF draws is done for it.
+- **The sweep panel's `data-lane` became `data-sweep-lane`.** The plan has the
+ runner console render "beside the existing sweep section"; both carrying
+ `data-lane` on one page fails every scoped lookup under Playwright's strict
+ mode. `<section data-lane={kind}>` is the contract all four lanes share, so
+ the attribute stayed with the runner and the panel being retired moved. Eight
+ selectors across `auto-queue`, `backfill` and `attribution` specs follow it,
+ and all eight are sweep-panel assertions that retire in 1.3.
+- **`/api/test/resume-lane?lane=` exists now**, not in 1.3. The plan called it
+ "the existing resume test route"; there wasn't one for a runner —
+ `/api/test/resume-backfill-sweep` resumes the SWEEP. This is the same shape,
+ and 1.3 deletes that one rather than renaming it.
+- **Three new idle reasons**: `lane-blocked` (the lane's sweep is armed),
+ `lane-held` (its own gate is shut — distinct from `downloads-paused`, which
+ names one flag, and from `capped`, which is contention), `engine-unreachable`.
+ The runner's engine preflight is an IDLE, not a throw: an unreachable ollama
+ is a configuration problem an operator fixes without restarting anything, and
+ it is re-probed on the run context's 60 s TTL.
+- **Two new job kinds**, `auto-digest` and `auto-backfill`, registered and
+ pinned in `jobKinds.test.ts`. There is deliberately no `auto-digest-unit` to
+ match `auto-download-unit`: a runner-dispatched digest runs IN-PROCESS and
+ makes no job record, which is what lets a lane work per video where the manual
+ verbs must work per channel.
+- **The concurrency spec measures overlapping pick-log INTERVALS**, not
+ simultaneous in-flight counts. See the bug note above for why.
+- **`backfill.spec.ts`'s concurrency test is kept as well as re-asserted.** The
+ plan says the `backfill.spec.ts:615` invariant "moves, does not vanish". At
+ the runner the mechanism is different — two loops on queueKey `""` dispatching
+ in-process units, no queue key involved — so `lane-runner.spec.ts` proves the
+ property there, and the original still covers the per-channel jobs, which
+ still exist and still need distinct keys. 1.3 must keep at least one.
+- **One real behaviour change, and it is a convergence.** The digest lane's
+ eligibility now goes through the registry's `state()` instead of an inline
+ `allFresh`, so a video whose `transcript.cues.json` is superseded classifies
+ `deferred` and is no longer dispatched-and-then-skipped by the engine, and
+ `countOperationWork("digest")` counts the reachable set where
+ `countMissingDigests` counted "not fresh". Corpus-wide `digest.deferred` is
+ **15**, so a per-channel progress target moves by at most that. The registry
+ entry's own header asked for exactly this: "If this entry and the lane's
+ executor ever disagree about whether a video is digested, that is a bug."
+- **`plans/tools/phase1-poll-timing.ts`** is new, and `phase1-numbers.ts` was
+ NOT extended: 1.2 moves no lane-visible number, for the reason at the top.
+
+### The `laneFor` trap is closed
+
+The GPU idle-only rule keyed off `k.laneFor &&` — the field EXISTING — as a
+proxy for "this one's resource depends on settings". It asks the DECLARATION
+now: `laneYieldsToTranscription(laneForOperation(op.id) ?? op.lane)`, on
+`contendsFor`. Identical on today's registry (no BACKFILL_QUEUE operation
+declares a static GPU lane), and an operation with a fixed GPU lane and no
+`laneFor` is now caught rather than missed. `operationBatch.test.ts` ("the GPU
+carve-out is keyed on contendsFor, not on laneFor existing") pins it, and the
+`Operation.laneFor` comment and the `plans/FACTS.md` entry both say the presence
+is no longer load-bearing.
+
+### Operator gate A — the production pass
+
+**Do this before 1.3 lands.** It is the "has run in production once" precondition
+every retirement in 1.3 stands on. ~20 minutes of attention, most of it waiting.
+
+Preconditions to check first, all read-only:
+
+1. `settings.json` (repo root) has **no `autoQueue.digest` key** and
+ `digest.sweepEnabled: false`. If the sweep is armed the runner refuses to
+ start and says so — that is the intended answer, not a bug.
+2. `digest.digestsPaused` is `false` and `digest.localAppId` is `ollama-direct`
+ with `ollama serve` up. A dead engine idles the runner at
+ `engine-unreachable` rather than failing 55,956 times.
+3. `transcription` is where the GPU contention lives: `digest.yieldToTranscription`
+ is on, so the lane stands aside while whisper works. Leave it on.
+
+**The edit.** Add ONE block to `settings.json` — everything else untouched:
+
+```json
+"autoQueue": {
+ "transcription": { …unchanged… },
+ "download": { …unchanged… },
+ "digest": {
+ "enabled": true,
+ "maxWorkers": 1,
+ "order": "cheapest",
+ "root": {
+ "id": "digest-root",
+ "mode": "strict",
+ "children": [
+ { "id": "digest-obviousrises", "match": { "type": "channel", "value": "ObviousRises-rumble" }, "weight": 1, "maxWorkers": null }
+ ]
+ }
+ }
+}
+```
+
+`ObviousRises-rumble` is chosen because it held **6** reachable digests and 0
+blocked at the last snapshot — enough to watch several units, small enough to
+finish. Any channel works; note its `backfill.digest` counts before you start.
+**Do NOT add an `autoQueue.backfill` block:** `backfill.allowRedownload` is
+`true` with `reach: "corpus"` on this settings file, and that is gate B's
+decision, not gate A's.
+
+**Then, on `/operations/digest`:**
+
+1. Restart the editor (or POST `{"kind":"digest","action":"start"}` to
+ `/api/auto-queue/control`). The **Auto-digest** section — the one with
+ `data-lane="digest"` — should read *Runner running* with a job-log link.
+2. Watch the claim ladder's rule count fall and *Recent picks* fill. The pending
+ figure beside it is this lane's own; it is not the sweep panel's.
+3. Let it run a pass. Record: **units dispatched** (Recent picks, or the job
+ log's `Auto-digest: <channel>/<id>` lines), the channel's
+ `backfill.digest.missing` **before and after** (read `snapshot.json`; the
+ runner requests a regen after each unit), and any `Idle —` line and its
+ reason.
+4. If a transcription starts beside it, record the **observed yield**: the
+ runner's idle reason goes to `capped` and the job log carries the
+ `Yielding the GPU to transcription …` line ONCE per contention window (it is
+ edge-triggered on purpose). Note whether digest throughput actually stopped.
+5. **Restart the editor.** With `autoQueue.digest.enabled` still true, the boot
+ hook must bring it back by itself — `startAutoRunnersIfEnabled` iterates
+ LANES now. Confirm *Runner running* without touching a button, and that
+ Recent picks continues rather than starting over.
+6. Hold and resume once from the page's **Hold the lane** button: the runner
+ must stay *running* with `Idle — the lane is held`, not stop. That is the
+ invariant the whole design rests on and it costs ten seconds to see.
+7. When you are done, set `"enabled": false` in the block (or leave it running —
+ it is the intended end state).
+
+Record the numbers in this file under a `Gate A` heading. If step 5 or step 6
+fails, 1.3 does not land.
+
+### Traps for the 1.3 implementer
+
+- **`data-sweep-lane` goes with the panel.** Deleting `SweepOperationView`
+ deletes the attribute; the eight specs pointing at it are all sweep tests and
+ go with them. Do not "restore" `data-lane` to anything — the runner section
+ owns it on all four lanes now.
+- **`laneBlockedReason` is the only thing left in `lib/pauseGates.ts` that reads
+ a sweep flag.** It goes when the sweeps do, along with the `lane-blocked`
+ idle reason and its two copies of the wording (`dispatch.ts`,
+ `buildActiveJobs.ts`). `arbiter.ts`'s `arbiterBlockedReason` is now a
+ two-line delegate to it and dies with the arbiter.
+- **`buildActiveJobs.buildLanes` still draws the digest and backfill rows as
+ SWEEP lanes** (`sweepLane({kind: "digest-sweep" …})`) while the runner rows
+ above them are named. 1.3's job is to make all four rows runner rows; the
+ `getAutoRunnerStatus(lane)` they need already answers for four.
+- **`settings.digest.recencyOrder` is read in two places now**: the batch's own
+ ordering and `recencyOrdering`'s `cheapest` composition in `autoRunner.ts`.
+ The migration must carry BOTH halves of the digest order (duration and date)
+ or it silently drops one — see the composition above.
+- **`.auto-queue/state.json` is written whole by whoever persists.** The
+ runners share one in-memory object (`sharedAutoQueueState`) so that is safe;
+ anything else that read-modify-writes it — `jobs/downloadBackoff.ts` does —
+ can still lose picks recorded between its read and its write. If 1.3 adds
+ another writer, give it the shared object rather than a fresh read.
+- **`OperationRun.settings` is a snapshot for IDENTITY only.** Guards re-read
+ live settings at dispatch (`laneLimit` takes `getSettings()` from its caller).
+ The runner rebuilds the whole context when the digest/backfill/diarization/
+ attribution blocks change or on a 60 s TTL; the duplicate-cluster plan is
+ carried across those rebuilds on purpose.
+- **`countOperationWork` opens a run with `useClusters: false`.** Counting must
+ not read the corpus-wide duplicates report, and a mirror is `present` or
+ `missing` on its own merits either way — which is what the snapshot counts.
