commit dbc7fdf0e18df729041e93f6f89668a2240f4c46
parent 45a37f16b987efe990b0ab460a84ff0ab1039931
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Mon, 21 Sep 2026 01:34:42 -0400
download filter: a rejection takes its own prefetch directory with it
The metadata prefetch writes data/<id>/metadata.info.json BEFORE the filters
get to look at it — that is the point of the split — so by the time the
operator's own "not this one" is known, the directory exists. That directory
is not a neutral leftover: buildIndex admits ANY dir holding a
metadata.info.json to the LMDB index and to the published site, transcript or
not, and deriveChannelSets reads the dir NAME as "ever fetched", which is what
takes a vanished video out of missingNeverFetched. The metadata scan creates
none of them for exactly those two reasons; a rejection that ran through the
downloader now creates none either, so the same channel cannot get two
different answers depending on which path reached the video first.
It only fires for a video the filter needed a LIVE prefetch for — one the scan
has not read yet. A scanned video is settled and yt-dlp is never invoked for
it at all.
The count does not move, and that is checked rather than assumed:
skippedByTitleFilter is derived in the snapshot from the metadata-scan store
against the channel's CURRENT filter (settledIdsFrom), never from a
download-outcome sidecar — the new channelSnapshot test runs a whole channel
with no data/ dirs at all and still gets the bucket. The RETRYABLE
skippedByFilter bucket IS read off download-outcome.json, which is why every
other filter (skip-live) still writes one.
discardPrefetchDir refuses rather than guesses: anything isVideoDownloaded
says is downloaded, a clips/ window dir, or a saved-video pointer, and the
directory stands exactly as it is.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Diffstat:
3 files changed, 251 insertions(+), 8 deletions(-)
diff --git a/common/controller/channelSnapshot.test.ts b/common/controller/channelSnapshot.test.ts
@@ -358,3 +358,87 @@ test("generateChannelSnapshot refuses an unreachable channel rather than writing
await rm(dir, { recursive: true, force: true });
}
});
+
+// ---------------------------------------------------------------------------
+// The filtered channel, end to end through generateChannelSnapshot.
+//
+// It needs nothing but a directory: a config, a playlist and the metadata-scan
+// store. That matters here because the whole point of the two facts below is
+// that they are derived from the STORE and not from anything on a video's
+// disk — a claim only a run with no video dirs at all can actually pin.
+
+type FilterFixture = {
+ filter?: Record<string, unknown>;
+ listed: string[];
+ scanned?: Record<string, { title?: string; liveStatus?: string }>;
+};
+
+async function filteredChannel(
+ fixture: FilterFixture,
+): Promise<{ dir: string; paths: Paths; channelDir: string }> {
+ const dir = await mkdtemp(path.join(tmpdir(), "ttb-snap-filter-"));
+ const paths = {
+ channelsDir: path.join(dir, "channels"),
+ transcriptsDir: dir,
+ savedVideosDir: path.join(dir, "saved-videos"),
+ } as Paths;
+ const channelDir = path.join(paths.channelsDir, "alpha");
+ await mkdir(path.join(channelDir, "data"), { recursive: true });
+ await writeFile(
+ path.join(channelDir, "config.json"),
+ JSON.stringify({
+ handling: "youtube",
+ url: "https://www.youtube.com/@alpha/videos",
+ ...(fixture.filter ? { downloadFilter: fixture.filter } : {}),
+ }),
+ );
+ await writeFile(
+ path.join(channelDir, "playlist"),
+ fixture.listed
+ .map((id) => `https://www.youtube.com/watch?v=${id}`)
+ .join("\n"),
+ );
+ const entries: Record<string, unknown> = {};
+ for (const [id, e] of Object.entries(fixture.scanned ?? {})) {
+ entries[id] = {
+ title: e.title ?? `Synthetic ${id}`,
+ description: "",
+ uploadDate: "20240101",
+ ...(e.liveStatus ? { liveStatus: e.liveStatus } : {}),
+ scannedAt: "2026-01-01T00:00:00.000Z",
+ };
+ }
+ await writeFile(
+ path.join(channelDir, "metadata-scan.json"),
+ JSON.stringify({ version: 1, entries, errors: {}, lastRun: null }),
+ );
+ return { dir, paths, channelDir };
+}
+
+test("a settled video is in skippedByTitleFilter with NO directory of its own", async () => {
+ // THE BUCKET IS SOURCED FROM THE SCAN STORE, not from a download-outcome
+ // sidecar — which is what lets a title-filter rejection delete the prefetch
+ // directory it made (ytdlp/downloadOneManaged.ts, discardPrefetchDir) without
+ // the count moving. This fixture has no data/ dirs at all; if the bucket
+ // depended on one, it would be empty here.
+ const { dir, paths } = await filteredChannel({
+ filter: { include: "keep" },
+ listed: ["aaaa0000001", "bbbb0000002"],
+ scanned: {
+ aaaa0000001: { title: "keep this one" },
+ bbbb0000002: { title: "drop this one" },
+ },
+ });
+ try {
+ const snap = await generateChannelSnapshot(paths, "alpha");
+ assert.deepEqual(snap.buckets.skippedByTitleFilter, ["bbbb0000002"]);
+ // And it is out of the download lane's queue, which is the point of it.
+ assert.deepEqual(snap.undownloadedIds, ["aaaa0000001"]);
+ // A settled id was never a video: it has no directory, so it was never in
+ // totals.videos either.
+ assert.equal(snap.totals.videos, 0);
+ } finally {
+ await rm(dir, { recursive: true, force: true });
+ }
+});
+
diff --git a/common/ytdlp/downloadOneManaged.ts b/common/ytdlp/downloadOneManaged.ts
@@ -28,8 +28,16 @@ import {
} from "../controller/keptVideos";
import { isDoNotClean } from "../lib/doNotClean-server";
import { transcodeAudio } from "../controller/transcode";
-import { findSourceMedia } from "../lib/videoStatus";
-import { savedVideoDir, type SavedVideoOrigin } from "../lib/savedVideo";
+import {
+ findSourceMedia,
+ isVideoDownloaded,
+ readVideoFiles,
+} from "../lib/videoStatus";
+import {
+ SAVED_VIDEO_POINTER_FILENAME,
+ savedVideoDir,
+ type SavedVideoOrigin,
+} from "../lib/savedVideo";
import { persistSourceVideo } from "../lib/savedVideo-server";
import {
type AudioCheckAttemptStats,
@@ -46,6 +54,7 @@ import {
isLivestreamMetadata,
} from "../lib/transcripts-server";
import { evaluateDownloadFilters } from "../lib/downloadFilters";
+import { CLIPS_DIR_NAME } from "../lib/clipWindow";
import {
upsertMetadataScan,
type MetadataScanEntry,
@@ -321,6 +330,65 @@ function transcribeHandlingArgsForAudioCheck(
// the body lives in ytdlp/channelArgs.ts so the clip-window fetch shares it.
const channelConfigArgs = channelExtraArgs;
+// A TITLE-FILTER REJECTION MUST NOT LEAVE A VIDEO DIRECTORY BEHIND.
+//
+// The prefetch writes `data/<id>/metadata.info.json` BEFORE the filters get to
+// look at it — that is the whole point of the split — so by the time the
+// operator's own "not this one" is known, the directory exists. And a directory
+// holding a metadata.info.json is not a neutral leftover: `buildIndex.ts`
+// admits ANY such dir to the LMDB index and to the published site (transcript
+// or not), and `deriveChannelSets` reads the dir NAME as "ever fetched", which
+// is what takes a vanished video out of `missingNeverFetched`. The metadata
+// scan creates none of them for exactly these two reasons
+// (controller/metadataScanStore.ts); a rejection that ran through the
+// downloader must not create them either, or the same channel gets both
+// answers depending on which path reached the video first.
+//
+// THE BUCKET DOES NOT COME FROM THIS DIRECTORY. `skippedByTitleFilter` is
+// derived in the snapshot from the metadata-scan store against the channel's
+// CURRENT filter (channelSnapshot.ts, settledIdsFrom) — the entry recorded a
+// few lines above this call — so removing the dir costs the count nothing. The
+// retryable `skippedByFilter` bucket IS read off `download-outcome.json`, which
+// is why every OTHER filter (skip-live) still writes one: that skip says "we
+// will try again", and a video with no directory and no outcome would silently
+// leave it.
+//
+// REFUSES RATHER THAN GUESSES. The prefetch is not the only thing that can have
+// written into this dir — a video downloaded before the filter was authored is
+// DOWNLOADED, which is a fact and not a preference, and a clip window or a
+// saved-video pointer is somebody else's data. Any of those and the directory
+// stays exactly as it is; the caller's outcome record is unaffected either way.
+async function discardPrefetchDir(
+ videoDir: string,
+ onLog: (line: string) => void,
+): Promise<boolean> {
+ let entries: string[];
+ try {
+ const files = await readVideoFiles(videoDir);
+ if (isVideoDownloaded(files)) return false;
+ entries = files.entries;
+ } catch {
+ // No directory at all (the legacy single-call path never made one), or it
+ // is unreadable. Either way there is nothing of ours to remove.
+ return false;
+ }
+ if (
+ entries.includes(CLIPS_DIR_NAME) ||
+ entries.includes(SAVED_VIDEO_POINTER_FILENAME)
+ ) {
+ return false;
+ }
+ try {
+ await rm(videoDir, { recursive: true, force: true });
+ return true;
+ } catch (err) {
+ onLog(
+ `Could not remove the prefetch directory ${videoDir}: ${(err as Error).message}\n`,
+ );
+ return false;
+ }
+}
+
// The one-invocation runner moved to ytdlp/runOneYtdlp.ts (the clip-window
// fetch needs the same log tee, stderr tail and archive scrape). This wrapper
// keeps the ManagedDownloadOpts-shaped call sites below unchanged.
@@ -643,13 +711,28 @@ async function runManagedDownload(
attempts,
filter: { name: decision.filter, reason: decision.reason },
};
- try {
- await mkdir(videoDir, { recursive: true });
- await writeDownloadOutcome(videoDir, record);
- } catch (err) {
+ // The operator's own rejection takes its prefetch directory with it —
+ // see discardPrefetchDir for why a metadata-only dir is not a neutral
+ // leftover. Every other filter's skip is RETRYABLE and its outcome
+ // sidecar is what `skippedByFilter` is derived from, so only this one
+ // discards.
+ const discarded =
+ decision.filter === "titleFilter" && metadata
+ ? await discardPrefetchDir(videoDir, opts.onLog)
+ : false;
+ if (discarded) {
opts.onLog(
- `Failed to write download-outcome.json: ${(err as Error).message}\n`,
+ `Removed the metadata-only directory for ${canonicalId}: a filtered video is not a member of this corpus.\n`,
);
+ } else {
+ try {
+ await mkdir(videoDir, { recursive: true });
+ await writeDownloadOutcome(videoDir, record);
+ } catch (err) {
+ opts.onLog(
+ `Failed to write download-outcome.json: ${(err as Error).message}\n`,
+ );
+ }
}
return record;
}
diff --git a/editor/e2e/title-filter.spec.ts b/editor/e2e/title-filter.spec.ts
@@ -1,4 +1,4 @@
-import { readFile, writeFile } from "node:fs/promises";
+import { mkdir, readFile, writeFile } from "node:fs/promises";
import { test, expect, type Page } from "@playwright/test";
import {
channelStage,
@@ -201,6 +201,82 @@ test("a settled video is never downloaded", async ({ page }) => {
);
});
+test("a title-filter rejection leaves no video directory behind", async ({
+ page,
+}) => {
+ test.setTimeout(180_000);
+ // NO SCAN FIRST, deliberately: that is the only way a rejection reaches the
+ // downloader at all. Once the scan has read a video the filter settles it and
+ // yt-dlp is never invoked for it (the test above). The leftover this pins
+ // belongs to the other case — a newly listed video the download lane reaches
+ // before any scan does — where the metadata PREFETCH has already written
+ // data/<id>/metadata.info.json by the time the filter gets to say no.
+ //
+ // A directory holding a metadata.info.json is admitted to the LMDB index and
+ // the published site by buildIndex, and its name is what deriveChannelSets
+ // reads as "ever fetched". The scan creates none of them; a rejection must
+ // not either, or the same channel gets two different answers depending on
+ // which path reached the video first.
+ await resetData("title-filter-channel");
+ await generateReport(page, CHANNEL);
+
+ await download(page);
+
+ const invocations = await readInvocations();
+ for (const id of GUEST) {
+ expect(await pathExists(`${ROOT}/data/${id}/transcript.en.vtt`)).toBe(true);
+ }
+ for (const id of SETTLED_BY_GUEST) {
+ // It WAS prefetched — that is the whole difference from the settled case —
+ // and the directory that prefetch made is gone again.
+ expect(invocations).toContain(
+ `prefetch:https://www.youtube.com/watch?v=${id}`,
+ );
+ expect(await pathExists(`${ROOT}/data/${id}`)).toBe(false);
+ expect(await readArchive()).not.toContain(id);
+ }
+
+ // AND THE COUNT DOES NOT MOVE. The bucket is derived from the metadata-scan
+ // store — the rejection recorded its entry there before discarding the
+ // directory — so removing the directory costs it nothing.
+ const before = await readJson<Snapshot>(`${ROOT}/snapshot.json`);
+ const snapshot = await refreshReport(
+ page,
+ before.generatedAt,
+ (s) => (s.buckets.skippedByTitleFilter ?? []).length > 0,
+ );
+ expect(snapshot.buckets.skippedByTitleFilter ?? []).toEqual(SETTLED_BY_GUEST);
+ const scan = await readJson<MetadataScan>(`${ROOT}/metadata-scan.json`);
+ expect(Object.keys(scan.entries).sort()).toEqual(SETTLED_BY_GUEST);
+});
+
+test("a rejection never deletes a directory that already holds artifacts", async ({
+ page,
+}) => {
+ test.setTimeout(180_000);
+ // A video downloaded BEFORE the filter was written is downloaded — a fact,
+ // not a preference. The discard refuses anything but its own prefetch, and
+ // the retryable outcome sidecar is written for it exactly as before.
+ await resetData("title-filter-channel");
+ const kept = PLAIN[0];
+ await mkdir(resolvePath(`${ROOT}/data/${kept}`), { recursive: true });
+ await writeFile(
+ resolvePath(`${ROOT}/data/${kept}/transcript.en.vtt`),
+ "WEBVTT\n\n00:00:00.000 --> 00:00:01.000\nold.\n",
+ );
+ await generateReport(page, CHANNEL);
+
+ await download(page);
+
+ expect(await pathExists(`${ROOT}/data/${kept}/transcript.en.vtt`)).toBe(true);
+ const outcome = await readJson<{ status: string }>(
+ `${ROOT}/data/${kept}/download-outcome.json`,
+ );
+ expect(outcome.status).toBe("skipped-filtered");
+ // The other rejections, which had nothing of their own, are still gone.
+ expect(await pathExists(`${ROOT}/data/${PLAIN[1]}`)).toBe(false);
+});
+
test("changing the filter re-decides the channel with no rescan", async ({
page,
}) => {
