commit 195ac4c3e440459c37feb8ff677474c1dd2d5191
parent 1ab9eaf9a780135271a6d2ebfee73b0cb2607ddb
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Sun, 20 Sep 2026 03:10:36 -0400
download filter: no metadata is not a verdict — fail open so the failure is classified
The title filter used to fail CLOSED on a null prefetch: no metadata, no match,
so skip. But a prefetch returns nothing for batch-level reasons far more often
than per-video ones — a 429, a bot check, a dropped connection — and calling
those "skipped-filtered" was actively harmful. The video never reached the real
attempt, so `classifyDownloadFailure` never ran, `onPlatformBackoff` never
fired, and abort-on-error never tripped (runYtdlp.ts ~937 vs ~944). A
rate-limited channel would walk its whole playlist "filtering" every video
while the source refused every request, and record a cooldown for none of it.
Failing open costs nothing the filter cares about: the real attempt runs and
fails for the reason it actually failed, and the filter still decides on the
next pass, when there is something to decide on.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Diffstat:
2 files changed, 40 insertions(+), 21 deletions(-)
diff --git a/common/lib/downloadFilters.test.ts b/common/lib/downloadFilters.test.ts
@@ -172,14 +172,31 @@ test("registry: invalid regex -> inert filter, exactly one log line", () => {
assert.match(lines[0], /INERT/);
});
-test("registry: null metadata with a filter configured -> retryable skip", () => {
- const d = evaluateDownloadFilters(
- ctx({ channelConfig: withFilter("guest"), metadata: null }),
+test("registry: null metadata is NOT a filter decision (fail open)", () => {
+ // A prefetch returns nothing for batch-level reasons — 429, bot check,
+ // network — far more often than per-video ones. Calling that a filter skip
+ // stopped the video from ever reaching the real attempt, so its failure was
+ // never classified and no per-platform backoff was ever recorded.
+ assert.equal(
+ evaluateDownloadFilters(
+ ctx({ channelConfig: withFilter("guest"), metadata: null }),
+ ),
+ null,
+ );
+ // Including when the filter would have rejected it on its title: we do not
+ // have the title, so there is nothing to reject it on.
+ assert.equal(
+ evaluateDownloadFilters(
+ ctx({
+ channelConfig: {
+ ...BASE_CONFIG,
+ downloadFilter: { include: "guest", includeLivestreams: true },
+ },
+ metadata: null,
+ }),
+ ),
+ null,
);
- assert.ok(d);
- assert.equal(d.skip, true);
- assert.equal(d.filter, "titleFilter");
- assert.match(d.reason, /failing closed/);
});
test("registry: null metadata with NO filter -> no skip (fail-open)", () => {
diff --git a/common/lib/downloadFilters.ts b/common/lib/downloadFilters.ts
@@ -203,12 +203,21 @@ export function titleFilterReason(
// a video the operator doesn't want is declined on its own terms rather than
// being classified as a live-stream retry.
//
-// FAILS CLOSED on missing metadata: a prefetch that produced nothing cannot be
-// matched, and downloading it anyway would defeat the filter on exactly the
-// videos a flaky extractor hides. The skip is an ORDINARY retryable one (it
-// lands in the existing skippedByFilter bucket and the next run tries again) —
-// nothing here is terminal, because nothing here decides what is settled. The
-// settled set is derived from the metadata-scan store.
+// FAILS OPEN on missing metadata, like skipLive, and that is a correction.
+//
+// It used to fail closed — no metadata, no match, so skip. But a prefetch
+// returns nothing for BATCH-LEVEL reasons far more often than for per-video
+// ones: a 429, a bot check, a network drop. Turning those into
+// `skipped-filtered` was actively harmful. The video never reached the real
+// attempt, so its failure was never classified (runYtdlp.ts ~937 vs ~944):
+// no `classifyDownloadFailure`, no `onPlatformBackoff`, no abort-on-error. A
+// rate-limited channel would walk its entire playlist "filtering" every video
+// while the source refused every request, and record a cooldown for none of it.
+//
+// Returning null costs nothing: the real download attempt runs, fails for the
+// reason it actually failed, and the existing machinery does its job. The
+// filter is not bypassed either — the download writes metadata, and the next
+// pass (or the metadata scan) decides with something to decide on.
const titleFilter: DownloadFilter = {
name: "titleFilter",
evaluate(ctx) {
@@ -229,14 +238,7 @@ const titleFilter: DownloadFilter = {
return null;
}
const m = ctx.metadata;
- if (!m) {
- return {
- skip: true,
- filter: "titleFilter",
- reason:
- "no metadata to match the download filter against (failing closed)",
- };
- }
+ if (!m) return null; // fail open — see above
if (!titleFilterRejects(compiled, m)) return null;
return {
skip: true,
