Archilyzer · Source

archilyzer

Archilyzer
git clone https://archilyzer.pages.dev/source/archilyzer.git
Log | Files | Refs | README | LICENSE

commit 47870ae59c13ab7c4bf47f14e5196b4e93f1e9df
parent d75e71202184e49c7aff6f031c0e19f3f8f3d186
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date:   Mon, 21 Sep 2026 02:29:15 -0400

storage: the eight nits the plan named

Each is small; four of them are a number that could lie.

- **`volumeFreeBytes` believed an unmounted mountpoint.** The `stat` proved the
  directory was there, which is not the drive being there: an unmounted
  mountpoint is a real, empty directory on its PARENT's filesystem, so
  `getFreeBytes` succeeded and reported the parent volume's free space — on this
  machine, the disk the operator is trying to empty. Comparing `st.dev` with the
  parent's is what a mount IS. Asked only when the location has a learned
  `volume.uuid`, because that field is the assertion that the root is supposed
  to be its own volume; a location on a plain directory shares its parent's
  device legitimately.
- **"Free up N GB" proposed channels the move would refuse.** A channel
  mid-relocation is a guaranteed skip in the bulk move, so selecting one counted
  its bytes toward a total the Move could never deliver. Excluded and COUNTED
  (`moving`), with its own clause in the note — and it is not folded into
  `unmeasured`, because it has a size; it is unavailable.
- **`bytesOnLocation` travelled without its caveat.** The rows carry
  `unknownBytes` beside `bytes` precisely because a sum that omits an unmeasured
  400 GB channel is worse than no sum, and a caller lifting the map out to draw
  its own meter was getting the number without it. `unknownBytesOnLocation` /
  `unknownBytesInPlace` now pair with the two.
- **"internal" was an available location id.** `LOCATION_ID_RE` admits it in
  both the form and the sanitizer, and it is the id /storage assembles the
  synthetic corpus-volume row under — a stored entry would build the row twice
  and make `locationOfDataDir` start matching unrelocated channels.
  `INTERNAL_LOCATION_ID` moved to `lib/storageLocations.ts` (re-exported from
  the controller, where callers name it) because `lib/settings.ts` has to refuse
  it and lib may not import controller. Refused in the action as well, since a
  write that silently drops the entry reads as a button that did nothing.
- **`measureTree` ran per render on a `force-dynamic` page the AutoRefresh
  re-renders on a timer.** The saved-video store grows on every pin, every
  `keepSourceVideo` and every full-source fetch, so the walk got slower exactly
  as the thing it measures got bigger. `lib/measureStore.ts` caches for 60 s,
  keyed by directory, capped at 32 entries (a re-point changes the key). Only
  the SIZE is cached; location, status and marker stay fresh, because those are
  the safety facts. `measureTree` joins BANNED in
  `noCorpusWalkInRenderPaths.test.ts` with an allow map naming the one wrapper
  file — and the match is now word-bounded so `measureTreeCached` is not
  indistinguishable from it, which is how an allow map stops meaning anything.
- **AGENTS.md** gains the saved-videos marker, the `autoPaused` invariant, the
  no-materialising-a-mount rule and the by-age eviction rule (three → six).
- **`measure-nav.mjs`** was still listing eleven routes; `/storage` is the
  twelfth, and `nav.ts` already records it as the exception.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Diffstat:
MAGENTS.md | 26++++++++++++++++++++++++--
Mcommon/controller/noCorpusWalkInRenderPaths.test.ts | 35+++++++++++++++++++++++++++++++----
Mcommon/controller/storageLocations.ts | 36++++++++++++++++++++++++++++++------
Mcommon/lib/settings.ts | 16+++++++++++++++-
Mcommon/lib/storageLocations.ts | 13+++++++++++++
Mcommon/views/freeUpSelection.test.ts | 28++++++++++++++++++++++++++++
Mcommon/views/freeUpSelection.ts | 41+++++++++++++++++++++++++++++++++--------
Mcommon/views/storage.ts | 18+++++++++++++++++-
Meditor/app/channels/components/ChannelsTable.tsx | 5+++++
Meditor/app/storage/actions.ts | 12++++++++++++
Meditor/app/storage/buildStorage.ts | 11+++++++++--
Aeditor/app/storage/lib/measureStore.ts | 68++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Meditor/scripts/measure-nav.mjs | 3++-
13 files changed, 287 insertions(+), 25 deletions(-)

diff --git a/AGENTS.md b/AGENTS.md @@ -212,7 +212,7 @@ which is why re-pointing a location rewrites only links and `dataDir`. The on-di cwd-relative writes, the LMDB index (it stores mtimes, and `rsync -a` preserves them) and the export build all keep working with no call-site changes. -Three things that are not optional: +Six things that are not optional: - **Never symlink a whole channel dir.** Channel listing filters `isDirectory()` on `channelsDir` entries (`channels.ts:246,363`), so a symlinked `<slug>/` vanishes from @@ -225,7 +225,29 @@ Three things that are not optional: in `runManagedFunction`. If you add a path that reads `data/`, guard it there. - **`channels/<slug>/.relocating.json`** is the in-flight marker. Its presence means "media is in transition" to every guard and lets an interrupted move resume from its - `phase`. `deleteChannel` and `renameChannel` refuse while it exists. + `phase`. `deleteChannel` and `renameChannel` refuse while it exists. The saved-video + store has its own, one level up: **`transcripts/.relocating-saved-videos.json`**, the + same `{target, direction, startedAt, phase}` shape. Its reader and + `assertSavedVideosStoreWritable` live in `common/lib/savedVideoStore.ts` — in *lib* + because `savedVideo-server.ts` is what persists a container and lib may not import + controller. +- **An auto-paused channel is the machine's decision, and the operator's word beats it.** + `settings.channelPriority.channels[slug].autoPaused` (`{reason, since, previousTier}`) + is written by the storage watch when a drive stops answering and cleared when it comes + back. Every manual tier change clears it (`clearAutoPause`, called by the one priority + writer), so a drive returning can never un-pause a channel a person paused. `/review` + lists them with `autoPauseReasonOf`'s sentence — the same one the rack and the channel + page say — and Resume posts the `previousTier`, never "normal". +- **A move never materialises its destination.** Every absolute `mkdir` in the two movers + is `{recursive: true}`, so a relocation aimed at an unmounted root would build it on the + root filesystem and fill it. `assertRelocationRootPresent` + (`controller/relocateChannelMedia.ts`) stats the root and, when it belongs to a location + carrying a `volume.uuid`, requires the probe's identity to match. It runs from the + preview AND immediately before each copy phase's mkdir. +- **`data/<id>/clips/` is a cache, and `evictClipWindows` is the only thing that prunes + it — BY AGE.** Nothing in the editor can know whether a umtool report still cites a + window (the manifests are in a umtool project), so there is no reference count and every + surface says so. An evicted window is re-fetchable: the cost is a fetch, not data. ## The live instances diff --git a/common/controller/noCorpusWalkInRenderPaths.test.ts b/common/controller/noCorpusWalkInRenderPaths.test.ts @@ -51,7 +51,33 @@ const VIEWS = path.resolve(REPO, "common", "views"); // it too. That is deliberate and not worth softening: a guard that skipped // comments and strings is a guard an offending call can hide behind, and the // cost of the false positive is one reworded comment. -const BANNED = ["listChannelStatsFromDisk", "buildDigestSweepPlan"]; +// +// It IS word-bounded, which is a different thing from context: `measureTree` +// must not match `measureTreeCached`, the bounded wrapper that exists so the +// raw walk has one caller. A substring match would make the safe name +// indistinguishable from the banned one and force every mention of the wrapper +// into the allow map — which is how an allow map stops meaning anything. +// +// `measureTree` is here for a different reason from the other two: it is not a +// corpus walk, it is a walk of ONE directory — but the directory is the +// saved-video store, which grows on every pin, every `keepSourceVideo` and +// every full-source fetch, and /storage is `force-dynamic` with the global +// AutoRefresh re-rendering it on a timer. An uncapped walk there gets slower +// exactly as the thing it is measuring gets bigger. +const BANNED = [ + "listChannelStatsFromDisk", + "buildDigestSweepPlan", + "measureTree", +]; + +// THE ONE FILE ALLOWED TO NAME EACH BANNED IDENTIFIER, and why it is a map +// rather than a blanket exception: the exemption is a claim about a specific +// file's specific mitigation, and the next call site added elsewhere must +// still fail. `lib/measureStore.ts` wraps `measureTree` in a 60 s, +// bounded-size cache; that wrapper is what `buildStorage.ts` calls. +const ALLOWED: Record<string, string[]> = { + "editor/app/storage/lib/measureStore.ts": ["measureTree"], +}; async function walk(dir: string): Promise<string[]> { const out: string[] = []; @@ -88,10 +114,11 @@ test("the corpus walk never reaches a render path", async () => { await Promise.all( files.map(async (file) => { const source = await readFile(file, "utf8"); + const rel = path.relative(REPO, file); for (const banned of BANNED) { - if (source.includes(banned)) { - offenders.push(`${banned} in ${path.relative(REPO, file)}`); - } + if (!new RegExp(`\\b${banned}\\b`).test(source)) continue; + if (ALLOWED[rel]?.includes(banned)) continue; + offenders.push(`${banned} in ${rel}`); } }), ); diff --git a/common/controller/storageLocations.ts b/common/controller/storageLocations.ts @@ -8,6 +8,8 @@ import { type SiteSettings, } from "../lib/settings"; import { + INTERNAL_LOCATION_ID, + INTERNAL_LOCATION_LABEL, locationOfDataDir, type StorageLocation, type StorageVolume, @@ -147,8 +149,11 @@ function emptyRollup(locationId: string): LocationRollup { // // So it is assembled where it is rendered, out of the same three facts every // other row carries, and it is the one row with no actions. -export const INTERNAL_LOCATION_ID = "internal"; -export const INTERNAL_LOCATION_LABEL = "Internal (in place)"; +// THE TWO CONSTANTS LIVE IN `lib/storageLocations.ts` and are re-exported +// here, where every caller names them. They had to move: `lib/settings.ts` +// must REFUSE "internal" as a stored location id (see `LOCATION_ID_RE` there), +// and lib may not import controller. +export { INTERNAL_LOCATION_ID, INTERNAL_LOCATION_LABEL }; // One pass over the corpus for EVERY location, not one pass per location: the // page draws a row per location and the channel list is the same list for all @@ -242,13 +247,32 @@ export async function volumeFreeBytes(opts: { out[INTERNAL_LOCATION_ID] = Number.isFinite(corpus) ? corpus : undefined; await Promise.all( opts.locations.map(async (loc) => { - const there = await stat(loc.root) - .then((st) => st.isDirectory()) - .catch(() => false); - if (!there) { + const st = await stat(loc.root).catch(() => null); + if (!st?.isDirectory()) { out[loc.id] = undefined; return; } + // THE DIRECTORY EXISTING IS NOT THE DRIVE BEING THERE, and this is the + // half the stat above could not catch. An unmounted mountpoint is a real, + // empty directory ON ITS PARENT'S FILESYSTEM — so `getFreeBytes` succeeds + // and reports the parent volume's free space, which on this machine is + // the disk the operator is trying to empty. The location would then read + // "233 GB free" about a platter that is not plugged in. + // + // Comparing `st.dev` with the PARENT's is what a mount is: crossing a + // mount boundary changes the device number. Only asked for a location + // that has learned a `volume.uuid` — that field is the assertion that + // this root is supposed to be its own volume. A location on a plain + // directory (no identity ever probed, a container, a subdirectory of the + // system disk by design) shares its parent's device legitimately, and + // refusing to report its free space would be wrong. + if (loc.volume?.uuid) { + const parent = await stat(path.dirname(loc.root)).catch(() => null); + if (parent && parent.dev === st.dev) { + out[loc.id] = undefined; + return; + } + } const free = await getFreeBytes(loc.root); out[loc.id] = Number.isFinite(free) ? free : undefined; }), diff --git a/common/lib/settings.ts b/common/lib/settings.ts @@ -30,6 +30,7 @@ import { migrateSweepsToLanes, } from "./laneMigration"; import { + INTERNAL_LOCATION_ID, migrateMediaRootToLocations, type StorageLocation, type StorageSettings, @@ -873,6 +874,17 @@ export function defaultStorage(): StorageSettings { const LOCATION_ID_RE = /^[a-z0-9][a-z0-9-]{0,63}$/; +// "internal" IS TAKEN. It is the synthetic /storage row for the corpus volume +// (INTERNAL_LOCATION_ID), and the regex above admits it — so a hand-edited +// settings.json, or an operator typing the obvious word into the New location +// form, could store a real location under the one id the page assembles for +// itself. The row would then be built twice, the rollup would count channels +// into whichever assembled last, and `locationOfDataDir` would start matching +// unrelocated channels against it. +function isReservedLocationId(id: string): boolean { + return id === INTERNAL_LOCATION_ID; +} + function sanitizeVolume(value: unknown): StorageVolume | undefined { if (!value || typeof value !== "object") return undefined; const v = value as Record<string, unknown>; @@ -934,7 +946,9 @@ export function sanitizeStorage(value: unknown): StorageSettings { if (!entry || typeof entry !== "object") continue; const e = entry as Record<string, unknown>; const id = typeof e.id === "string" ? e.id.trim() : ""; - if (!LOCATION_ID_RE.test(id) || seen.has(id)) continue; + if (!LOCATION_ID_RE.test(id) || isReservedLocationId(id) || seen.has(id)) { + continue; + } const rawRoot = typeof e.root === "string" ? e.root.trim() : ""; if (!path.isAbsolute(rawRoot)) continue; // "/mnt/platter/" and "/mnt/platter" are one root; "/" stays "/". diff --git a/common/lib/storageLocations.ts b/common/lib/storageLocations.ts @@ -18,6 +18,19 @@ import path from "node:path"; // copy of the association to keep in step, and `ChannelConfig`'s whitelisted // round-trip (`channelConfig.ts`) needs no new key. +// THE SYNTHETIC ROW'S ID: the corpus volume itself, assembled where it is +// rendered and NEVER stored in `settings.storage.locations`. A stored entry +// under this id would be deletable, and worse, `locationOfDataDir` would then +// match every unrelocated channel — breaking the one rule the whole design +// rests on (a channel is on location L iff its `dataDir` is under `L.root`, +// and an in-place channel has no `dataDir`). +// +// It is HERE rather than beside the row that uses it because `lib/settings.ts` +// has to refuse it as a stored id, and lib may not import controller. +// `controller/storageLocations.ts` re-exports both, where callers name them. +export const INTERNAL_LOCATION_ID = "internal"; +export const INTERNAL_LOCATION_LABEL = "Internal (in place)"; + export type StorageVolume = { // Filesystem UUID, the one stable name a disk has across mountpoints. This is // what makes "the platter came up somewhere else" a recoverable situation. diff --git a/common/views/freeUpSelection.test.ts b/common/views/freeUpSelection.test.ts @@ -77,3 +77,31 @@ test("nothing measurable says so rather than proposing an empty selection", () = assert.deepEqual(r.slugs, []); assert.match(r.note, /Nothing in place has a measured size/); }); + +test("a channel mid-relocation is excluded and counted, not proposed", () => { + // The bulk move refuses a channel whose media is in transition BY NAME, so + // proposing one puts a guaranteed skip in the deck and counts its bytes + // toward a total the Move will never deliver. + const result = selectToFreeBytes( + [ + { slug: "moving", bytes: 500 * GB, inPlace: true, inTransition: true }, + { slug: "still", bytes: 100 * GB, inPlace: true }, + ], + 200 * GB, + ); + assert.deepEqual(result.slugs, ["still"]); + assert.equal(result.moving, 1); + // And it is NOT counted as unmeasured — it has a size; it is unavailable. + assert.equal(result.unmeasured, 0); + assert.match(result.note, /mid-relocation/); + assert.match(result.note, /still 100\.0 GB short/); +}); + +test("an unmeasured channel that is also moving is counted once, as moving", () => { + const result = selectToFreeBytes( + [{ slug: "moving", bytes: null, inPlace: true, inTransition: true }], + 10 * GB, + ); + assert.equal(result.moving, 1); + assert.equal(result.unmeasured, 0); +}); diff --git a/common/views/freeUpSelection.ts b/common/views/freeUpSelection.ts @@ -14,9 +14,13 @@ // // THREE RULES, each of which is a refusal to guess: // -// 1. ONLY CHANNELS THAT ARE IN PLACE. Moving a channel that is already on the -// platter frees nothing on the disk the operator is trying to empty; it -// would be counted as progress and deliver none. +// 1. ONLY CHANNELS THAT ARE IN PLACE AND NOT ALREADY MOVING. A channel on the +// platter frees nothing on the disk being emptied; it would be counted as +// progress and deliver none. A channel MID-RELOCATION is the same problem +// with a worse ending: `.relocating.json` is present, the bulk move refuses +// it by name, and proposing it puts a row in the deck that is guaranteed to +// skip — so the "N GB selected" figure is a promise the Move cannot keep. +// They are excluded and counted, and the note says which. // 2. A CHANNEL WITH NO MEASUREMENT IS NOT A CHANNEL WITH ZERO BYTES. A // snapshot written before `totalMediaBytes` existed carries no figure, and // ranking it as empty would leave the largest channel on the disk at the @@ -34,6 +38,10 @@ export type FreeUpCandidate = { bytes: number | null; // Its media is on the corpus volume — the disk being freed. inPlace: boolean; + // A relocation marker is present: the media is in transition. The bulk move + // refuses such a channel by name, so selecting it would put a guaranteed + // skip in the deck and inflate the total. + inTransition?: boolean; }; export type FreeUpSelection = { @@ -46,6 +54,8 @@ export type FreeUpSelection = { shortfall: number; // Channels skipped for want of a measurement (rule 2). unmeasured: number; + // In-place channels skipped because their media is already moving (rule 1). + moving: number; // A sentence for the deck, built here so the two surfaces that could show it // cannot word it differently. note: string; @@ -55,13 +65,17 @@ export function selectToFreeBytes( candidates: ReadonlyArray<FreeUpCandidate>, targetBytes: number, ): FreeUpSelection { + const moving = candidates.filter((c) => c.inPlace && c.inTransition).length; const unmeasured = candidates.filter( - (c) => c.inPlace && c.bytes === null, + (c) => c.inPlace && !c.inTransition && c.bytes === null, ).length; const eligible = candidates .filter( (c): c is FreeUpCandidate & { bytes: number } => - c.inPlace && typeof c.bytes === "number" && c.bytes > 0, + c.inPlace && + !c.inTransition && + typeof c.bytes === "number" && + c.bytes > 0, ) .sort((a, b) => b.bytes - a.bytes || a.slug.localeCompare(b.slug)); @@ -75,7 +89,14 @@ export function selectToFreeBytes( } } const shortfall = Math.max(0, targetBytes - bytes); - return { slugs, bytes, shortfall, unmeasured, note: noteFor(slugs.length, shortfall, unmeasured) }; + return { + slugs, + bytes, + shortfall, + unmeasured, + moving, + note: noteFor(slugs.length, shortfall, unmeasured, moving), + }; } function gb(n: number): string { @@ -86,11 +107,15 @@ function noteFor( picked: number, shortfall: number, unmeasured: number, + moving: number, ): string { const tail = - unmeasured > 0 + (unmeasured > 0 ? ` ${unmeasured} channel(s) have no size in their report yet and were not considered — refresh them for a better answer.` - : ""; + : "") + + (moving > 0 + ? ` ${moving} channel(s) are mid-relocation and were not considered — the move would refuse them.` + : ""); if (picked === 0 && shortfall > 0) { return `Nothing in place has a measured size to move.${tail}`; } diff --git a/common/views/storage.ts b/common/views/storage.ts @@ -153,10 +153,20 @@ export type StorageRowsPayload = { // carry, lifted out so a caller that wants the totals (the /channels meter // bridge) does not have to re-fold the rows. bytesOnLocation: Record<string, number>; + // HOW MANY CHANNELS ON EACH ROW COULD NOT CONTRIBUTE A FIGURE, paired with + // the map above and for the same reason the ROWS carry `unknownBytes` beside + // `bytes`: a total that silently omits an unmeasured 400 GB channel is worse + // than no total. A caller lifting `bytesOnLocation` out of the rows to draw + // its own meter (the /channels volume bar) was getting the sum without the + // caveat, which is exactly how a bar ends up claiming a full drive is empty. + unknownBytesOnLocation: Record<string, number>; // The corpus volume's share — `bytesOnLocation.internal`, named because it is // the number the whole exercise is about: what is still on the disk that is // 96 % full. bytesInPlace: number; + // `unknownBytesOnLocation.internal` — the corpus volume's share of the + // caveat, named for the same reason `bytesInPlace` is. + unknownBytesInPlace: number; defaultLocationId: string; // Whether `udisksctl` resolved in this process. False in a container, and the // reason the Mount button is withheld there. @@ -325,6 +335,7 @@ export function buildStorageRows(i: StorageRowsInputs): StorageRowsPayload { const busy = runningRepoint(i.registry); const udisksctlAvailable = i.udisksctlAvailable ?? false; const bytesOnLocation: Record<string, number> = {}; + const unknownBytesOnLocation: Record<string, number> = {}; const configured = i.locations.map((loc): StorageRow => { const probe = i.probes[loc.id]; const status: StorageLocationStatus = probe?.status ?? "missing"; @@ -354,6 +365,7 @@ export function buildStorageRows(i: StorageRowsInputs): StorageRowsPayload { const unknownBytes = roll?.unknownBytes ?? 0; const clipsBytes = roll?.clipsBytes ?? 0; bytesOnLocation[loc.id] = bytes; + unknownBytesOnLocation[loc.id] = unknownBytes; return { id: loc.id, label: loc.label || loc.id, @@ -381,7 +393,7 @@ export function buildStorageRows(i: StorageRowsInputs): StorageRowsPayload { }; }); const rows = i.internal - ? [internalRow(i, bytesOnLocation), ...configured] + ? [internalRow(i, bytesOnLocation, unknownBytesOnLocation), ...configured] : configured; return { rows, @@ -389,7 +401,9 @@ export function buildStorageRows(i: StorageRowsInputs): StorageRowsPayload { ? { savedVideos: savedVideosView(i, i.savedVideos, busy) } : {}), bytesOnLocation, + unknownBytesOnLocation, bytesInPlace: bytesOnLocation[INTERNAL_ROW_ID] ?? 0, + unknownBytesInPlace: unknownBytesOnLocation[INTERNAL_ROW_ID] ?? 0, defaultLocationId: i.defaultLocationId, udisksctlAvailable, }; @@ -449,6 +463,7 @@ function savedVideosView( function internalRow( i: StorageRowsInputs, bytesOnLocation: Record<string, number>, + unknownBytesOnLocation: Record<string, number>, ): StorageRow { const internal = i.internal as NonNullable<StorageRowsInputs["internal"]>; const roll = i.rollups[INTERNAL_ROW_ID]; @@ -457,6 +472,7 @@ function internalRow( const unknownBytes = roll?.unknownBytes ?? 0; const clipsBytes = roll?.clipsBytes ?? 0; bytesOnLocation[INTERNAL_ROW_ID] = bytes; + unknownBytesOnLocation[INTERNAL_ROW_ID] = unknownBytes; const withheld = "The corpus volume is where media lives when it has not been moved " + "anywhere. It is not a configured location: there is nothing to re-point, " + diff --git a/editor/app/channels/components/ChannelsTable.tsx b/editor/app/channels/components/ChannelsTable.tsx @@ -357,6 +357,11 @@ export function ChannelsTable({ slug: c.slug, bytes: c.mediaBytes, inPlace: c.volumeId === INTERNAL_ROW_ID, + // A MARKER IS A GUARANTEED SKIP. The bulk move refuses a channel whose + // media is in transition by name, so proposing one would put a row in + // the deck that cannot move and count its bytes toward a total the + // Move will never deliver. + inTransition: c.media?.status === "in-transition", })), targetGB * 1024 ** 3, ); diff --git a/editor/app/storage/actions.ts b/editor/app/storage/actions.ts @@ -15,6 +15,7 @@ import { import type { StreamActionResult } from "yt-dlp-transcript-common/jobs/streamCommand"; import { channelsOnLocation, + INTERNAL_LOCATION_ID, maybeAutoRepoint, probeLocationMemo, recordProbedIdentity, @@ -60,6 +61,17 @@ function validate(draft: LocationDraft): string | null { if (!ID_RE.test(draft.id)) { return `"${draft.id}" is not a valid id: lower-case letters, digits and dashes, starting with a letter or digit, 64 characters at most.`; } + // "internal" IS TAKEN — it is the synthetic row this page assembles for the + // corpus volume, and the regex above happily admits it. Refused HERE as well + // as in the sanitizer because a settings write that silently drops the entry + // reads to the operator as a button that did nothing. + if (draft.id === INTERNAL_LOCATION_ID) { + return ( + `"${INTERNAL_LOCATION_ID}" is reserved: it is the row this page shows ` + + `for the corpus volume, which is not a configurable location. Pick ` + + `another id.` + ); + } const root = normalizeRoot(draft.root); if (!root.startsWith("/")) { return `The root must be an absolute path (got "${draft.root}"). A relative root would name a different directory in every process that read it.`; diff --git a/editor/app/storage/buildStorage.ts b/editor/app/storage/buildStorage.ts @@ -8,7 +8,7 @@ import { channelsOnLocation, probeAllLocations, } from "yt-dlp-transcript-common/controller/storageLocations"; -import { measureTree } from "yt-dlp-transcript-common/controller/relocateDir"; +import { measureTreeCached } from "./lib/measureStore"; import { inspectSavedVideosStore, savedVideosMarkerPath, @@ -74,11 +74,18 @@ export async function buildStorage(): Promise<StorageRowsPayload> { // ever grows to corpus scale, this is the line that has to change — and // `listSavedVideos` (which reads the pointers, each carrying its own `bytes`) // is the cheaper answer waiting. + // + // MEMOIZED FOR 60 SECONDS (see lib/measureStore.ts). This page is + // `force-dynamic` and the global AutoRefresh re-renders it on a timer, so an + // un-cached walk got slower exactly as the store grew — which is the + // situation the operator opened the page to understand. The store's SIZE is + // the only thing cached; its location, status and marker are read fresh every + // render, because those are the safety facts. const store = await inspectSavedVideosStore(paths, settings); const storeMeasured = store.status === "unreachable" || store.status === "in-transition" ? { bytes: 0, files: 0 } - : await measureTree(paths.savedVideosDir); + : await measureTreeCached(paths.savedVideosDir); return buildStorageRows({ locations, savedVideos: { diff --git a/editor/app/storage/lib/measureStore.ts b/editor/app/storage/lib/measureStore.ts @@ -0,0 +1,68 @@ +import { measureTree } from "yt-dlp-transcript-common/controller/relocateDir"; + +// THE SAVED-VIDEO STORE'S SIZE, MEASURED AT MOST ONCE EVERY 60 SECONDS. +// +// `measureTree` is a recursive walk. On /storage it is the one thing on the +// page that touches the disk per render rather than reading a report, and it +// is there because the store holds ONE container per pinned or kept-latest +// video — a few dozen files — not one per video. That is what makes a walk +// affordable at all. +// +// But "affordable" was doing a lot of work for a `force-dynamic` page the +// global AutoRefresh re-renders on a timer, and the store is the one directory +// in the corpus that can grow without anybody deciding it should: every pin, +// every `keepSourceVideo`, every umtool full-source fetch lands a container in +// it. A page that walks a growing directory on every refresh gets slower in +// exactly the situation the operator opened it to understand. +// +// SIXTY SECONDS, keyed by directory. Short enough that a move or a prune shows +// up on the next reload the operator was going to do anyway; long enough that +// an auto-refresh cycle costs one walk rather than one per tick. The store's +// SIZE is not a safety number — the location, the status and the marker are, +// and all three are read fresh every render. +// +// `maxEntries` caps the map because the key is a path and a re-point changes +// it: without a cap a long-lived process that re-pointed repeatedly would keep +// one entry per historical root forever. The oldest insertion goes first (a +// Map iterates in insertion order), which is the right victim — the current +// root is always the most recently written. +// +// `measureTreeCached` is named in `common/controller/noCorpusWalkInRenderPaths.test.ts`'s +// allow map: `measureTree` is banned from render paths, and this module is the +// one place allowed to call it, precisely because of the cache above it. + +export type MeasuredTree = { bytes: number; files: number }; + +const TTL_MS = 60_000; +const MAX_ENTRIES = 32; + +type Entry = { at: number; value: MeasuredTree }; +const cache = new Map<string, Entry>(); + +// Test seam, and the escape hatch for a process that has just moved the store. +export function resetStoreMeasureCache(): void { + cache.clear(); +} + +export async function measureTreeCached( + dir: string, + opts: { now?: number; refresh?: boolean } = {}, +): Promise<MeasuredTree> { + const now = opts.now ?? Date.now(); + if (!opts.refresh) { + const hit = cache.get(dir); + if (hit && now - hit.at < TTL_MS) return hit.value; + } + const value = await measureTree(dir); + // Delete before set so a refreshed key moves to the END of the insertion + // order; otherwise the entry the page uses every minute would be the first + // one evicted. + cache.delete(dir); + cache.set(dir, { at: now, value }); + while (cache.size > MAX_ENTRIES) { + const oldest = cache.keys().next(); + if (oldest.done) break; + cache.delete(oldest.value); + } + return value; +} diff --git a/editor/scripts/measure-nav.mjs b/editor/scripts/measure-nav.mjs @@ -36,7 +36,7 @@ const BASE = argOf("base", "http://localhost:3001").replace(/\/$/, ""); const RUNS = Number(argOf("runs", "3")); // Routes worth timing, with the segment name used to build a state tree. THE -// ELEVEN SINGLE-SEGMENT PAGES IN THE SIDEBAR (editor/app/lib/nav.ts), and +// TWELVE SINGLE-SEGMENT PAGES IN THE SIDEBAR (editor/app/lib/nav.ts), and // only those: stateTree() below encodes ONE segment, so /sites/<id>/charts // and /operations/<id> cannot be listed — each would need the nested // [segment, {children: [param, …]}] form, a different tree per route. A @@ -54,6 +54,7 @@ const ROUTES = [ ["/workers", "workers"], ["/cleanup", "cleanup"], ["/saved-videos", "saved-videos"], + ["/storage", "storage"], ["/settings", "settings"], ["/changelog", "changelog"], ];