timothy/ersatztv
1
0
Fork 0
Code Issues 54 Pull Requests 6 Actions Packages Projects Releases 9 Wiki Activity

Media-server scanner: ratio-threshold + projection-failure detection for the empty-fetch guard #484

New Issue
Closed
opened 2026-07-19 23:31:23 +02:00 by timothy · 3 comments
timothy commented 2026-07-19 23:31:23 +02:00
Owner
Copy Link
Copy Source

Follow-up to #477 (PR #483), which added a deterministic zero-count anti-nuke guard to the three library-level media-server sweeps (MediaServerReconciliationGuard). Two ideas from #477 were deliberately deferred there and recorded in docs/decisions.md:

  1. Ratio / missing-fraction threshold. #477's guard only fires when the incoming set is exactly empty. A fetch that returns a small non-zero subset of a large library (partial pagination failure, partial upstream corruption) would still flag the missing majority FileNotFound. A "skip if missing fraction > X%" guard would catch that — but it risks suppressing a legitimate bulk deletion, so it needs a tunable, telemetry-backed policy (and a decision on the default threshold / whether it's configurable).

  2. Projection-failure detection. JellyfinApiClient.GetPagedLibraryItems silently drops items whose ProjectToShow/ProjectToEpisode returns None (via .Somes()). Such an item the server did return is then indistinguishable from a deletion at the reconciliation step, so a projection regression could mass-flag healthy items. Fixing this needs a dropped-count threaded out of the API-client layer through to the scanner and used to suppress/limit the sweep.

Both are larger, cross-layer changes than the zero-count guard. Not urgent (the zero-count guard covers the reported catastrophic whole-library case with no false positives).

Done-when

  • Decided whether a ratio threshold is worth the false-suppression risk; if yes, implemented + configurable + tested — REJECTED, rationale recorded
  • Projection-failure count threaded from the API client and used to bound/skip the sweep, or explicitly rejected with rationale — implemented, across all six sweeps
  • docs/decisions.md updated
  • Adversarial review passed
Follow-up to #477 (PR #483), which added a deterministic zero-count anti-nuke guard to the three library-level media-server sweeps (`MediaServerReconciliationGuard`). Two ideas from #477 were deliberately deferred there and recorded in `docs/decisions.md`: 1. **Ratio / missing-fraction threshold.** #477's guard only fires when the incoming set is *exactly* empty. A fetch that returns a small non-zero subset of a large library (partial pagination failure, partial upstream corruption) would still flag the missing majority `FileNotFound`. A "skip if missing fraction > X%" guard would catch that — but it risks suppressing a *legitimate* bulk deletion, so it needs a tunable, telemetry-backed policy (and a decision on the default threshold / whether it's configurable). 2. **Projection-failure detection.** `JellyfinApiClient.GetPagedLibraryItems` silently drops items whose `ProjectToShow`/`ProjectToEpisode` returns None (via `.Somes()`). Such an item the server *did* return is then indistinguishable from a deletion at the reconciliation step, so a projection regression could mass-flag healthy items. Fixing this needs a dropped-count threaded out of the API-client layer through to the scanner and used to suppress/limit the sweep. Both are larger, cross-layer changes than the zero-count guard. Not urgent (the zero-count guard covers the reported catastrophic whole-library case with no false positives). ## Done-when - [x] Decided whether a ratio threshold is worth the false-suppression risk; if yes, implemented + configurable + tested — **REJECTED**, rationale recorded - [x] Projection-failure count threaded from the API client and used to bound/skip the sweep, or explicitly rejected with rationale — **implemented**, across all six sweeps - [x] docs/decisions.md updated - [x] Adversarial review passed
timothy added the enhancementpriority: low labels 2026-07-19 23:31:23 +02:00
timothy commented 2026-07-22 19:29:23 +02:00
Author
Owner
Copy Link
Copy Source

🔗 Session bundle — media-server scanner hardening: #460, #484, #491, #496, #500

Scan/reconcile data-integrity & anti-nuke robustness in the media-server sweeps, all carved from the #477/#488/#494/#497/#409 review chain. #491/#496/#500 share the check-then-insert / remove-stale+add-new reconcile idiom (duplicate rows on races or duplicate names / cross-library identity); #460 normalizes the MinValue LastScan write; #484 extends the empty-fetch guard to a ratio/projection-failure threshold. Same subsystem (Infrastructure/Scanner repositories + MediaServerReconciliationGuard) — sweepable in one session.

When you claim any member, check this cluster for co-workable siblings and sweep the disjoint set in one session (per docs/handoffs/chicorytv-issue-queue.md → bundles).

🔗 **Session bundle — media-server scanner hardening:** #460, #484, #491, #496, #500 Scan/reconcile data-integrity & anti-nuke robustness in the media-server sweeps, all carved from the #477/#488/#494/#497/#409 review chain. #491/#496/#500 share the check-then-insert / remove-stale+add-new reconcile idiom (duplicate rows on races or duplicate names / cross-library identity); #460 normalizes the MinValue LastScan write; #484 extends the empty-fetch guard to a ratio/projection-failure threshold. Same subsystem (Infrastructure/Scanner repositories + MediaServerReconciliationGuard) — sweepable in one session. When you claim any member, check this cluster for co-workable siblings and sweep the disjoint set in one session (per docs/handoffs/chicorytv-issue-queue.md → bundles).
timothy added the in-progress label 2026-07-25 14:04:26 +02:00
timothy commented 2026-07-25 14:04:28 +02:00
Author
Owner
Copy Link
Copy Source

Claiming — top-ranked eligible backlog pickup from scripts/select-queue.sh (prio-low tier, no active arc, deps clear, no prior claim).

Scanning the media-server scanner-hardening bundle (#460/#484/#491/#496/#500) for co-workable siblings before starting; #460 and #500 are closed, so the live disjoint set is #484 + #491 (+ #496, which looks too large to sweep alongside).

Claiming — top-ranked eligible backlog pickup from `scripts/select-queue.sh` (prio-low tier, no active arc, deps clear, no prior claim). Scanning the media-server scanner-hardening bundle (#460/#484/#491/#496/#500) for co-workable siblings before starting; #460 and #500 are closed, so the live disjoint set is #484 + #491 (+ #496, which looks too large to sweep alongside).
timothy commented 2026-07-25 17:09:56 +02:00
Author
Owner
Copy Link
Copy Source

Closing record

Outcome: Shipped in PR #612 (merged). Both deferred ideas resolved: the ratio/missing-fraction threshold is rejected with rationale, and projection-failure detection is implemented across all six media-server sweeps (movies, other videos, music videos, shows, and the nested per-show seasons and per-season episodes) for Jellyfin and Emby.

Root cause: n/a — this was deferred design work from #477, not a defect. But the sweep's underlying hazard is worth restating: existing.Except(incoming) treats any absence as a deletion, and EmptyTrashHandler deletes FileNotFound rows permanently, so anything that silently shrinks incoming is a data-loss vector.

Decisions/conventions changed: added scan.projection-failure-sweep-guard (sibling to, not superseding, scan.zero-item-fetch-guard — #477's rule is unchanged and still in force).

Reusable knowledge: four things.

  1. .Somes() was hiding two different failure classes. A guard-clause None (STRM file, LocationType non-FileSystem/Virtual, unknown item type) is a deliberate, permanent skip; a None from a catch is a failure and is what's indistinguishable from a deletion. Only the second may suppress a sweep. Conflating them is the seductive wrong answer — it permanently disables reconciliation for any library holding a single STRM file, so stale rows accumulate forever.
  2. A deferral rationale does not transfer to a new failure mode. #477 left the nested TV sweeps unguarded because "blast radius is one show's seasons / one season's episodes." True for a per-parent empty fetch; false for a projection failure, which is systematic — one bad code path fires on every parent, so every season enumerates zero episodes and the whole episode library is swept in one scan. This was missed by the implementer and by me, and caught only by cold review. When extending a guard to a new mode, re-derive every inherited "deliberately not guarded" against the new mode.
  3. Emby's skips are not Jellyfin's. Emby keys on MediaSources empty and LocationType == "Virtual", not STRM. And Emby's MediaSources-empty check is response-shape-dependent, unlike Jellyfin's genuinely permanent guards: a Refit/DTO drift presents as a mass Skip, which by construction leaves the sweep enabled — a total regression is caught by #477's zero branch, a partial one is not. Documented as a residual rather than re-classified, since re-classifying reintroduces STRM-style permanent suppression.
  4. Plex needs no counting at all — its projections return bare entities with no Option and no catch, so a bad item throws and unwinds the scan rather than dropping silently.

Verification: Scanner.Tests 1504, Infrastructure.Tests 114, ErsatzTV.Tests 1873, Core.Tests 652 (+1 skipped), Architecture.Tests 5 — all 0 failed. Negative controls run in both polarities (neutralizing the guard to => true fails exactly the refusal tests; to => false fails the positive controls). Join tests pin the same-counter-instance wiring at five places, so a refactor handing the client a fresh counter goes red rather than silently killing the protection. Full CI green. No live-E2E: scanner reconciliation logic, not an API write path or UI, and the failure mode requires a projection regression that can't be induced against a real server without shipping broken code.

Deferred: #604 — JellyfinCollectionScanner's remove-all-then-re-add over GetCollectionItems has the identical seam and is still uncounted; a swallowed drop silently empties a collection. Not permanent media deletion, so genuinely lower priority, but the classification work is already done.

Docs updated: docs/decisions.md (+ regenerated docs/decisions/README.md catalog). No route, endpoint, or SPA screen changed, so blazor-route-parity.md / api-conventions.md / spa-conventions.md needed no update.

## Closing record **Outcome:** Shipped in PR #612 (merged). Both deferred ideas resolved: the ratio/missing-fraction threshold is **rejected** with rationale, and projection-failure detection is **implemented** across all six media-server sweeps (movies, other videos, music videos, shows, and the nested per-show seasons and per-season episodes) for Jellyfin and Emby. **Root cause:** n/a — this was deferred design work from #477, not a defect. But the sweep's underlying hazard is worth restating: `existing.Except(incoming)` treats *any* absence as a deletion, and `EmptyTrashHandler` deletes `FileNotFound` rows permanently, so anything that silently shrinks `incoming` is a data-loss vector. **Decisions/conventions changed:** added `scan.projection-failure-sweep-guard` (sibling to, not superseding, `scan.zero-item-fetch-guard` — #477's rule is unchanged and still in force). **Reusable knowledge:** four things. 1. **`.Somes()` was hiding two different failure classes.** A guard-clause `None` (STRM file, `LocationType` non-FileSystem/Virtual, unknown item type) is a *deliberate, permanent* skip; a `None` from a `catch` is a *failure* and is what's indistinguishable from a deletion. Only the second may suppress a sweep. Conflating them is the seductive wrong answer — it permanently disables reconciliation for any library holding a single STRM file, so stale rows accumulate forever. 2. **A deferral rationale does not transfer to a new failure mode.** #477 left the nested TV sweeps unguarded because "blast radius is one show's seasons / one season's episodes." True for a *per-parent empty fetch*; false for a projection failure, which is *systematic* — one bad code path fires on every parent, so every season enumerates zero episodes and the whole episode library is swept in one scan. This was missed by the implementer and by me, and caught only by cold review. When extending a guard to a new mode, re-derive every inherited "deliberately not guarded" against the new mode. 3. **Emby's skips are not Jellyfin's.** Emby keys on `MediaSources` empty and `LocationType == "Virtual"`, not STRM. And Emby's `MediaSources`-empty check is **response-shape-dependent**, unlike Jellyfin's genuinely permanent guards: a Refit/DTO drift presents as a mass *Skip*, which by construction leaves the sweep enabled — a total regression is caught by #477's zero branch, a **partial** one is not. Documented as a residual rather than re-classified, since re-classifying reintroduces STRM-style permanent suppression. 4. **Plex needs no counting at all** — its projections return bare entities with no `Option` and no `catch`, so a bad item throws and unwinds the scan rather than dropping silently. **Verification:** Scanner.Tests 1504, Infrastructure.Tests 114, ErsatzTV.Tests 1873, Core.Tests 652 (+1 skipped), Architecture.Tests 5 — all 0 failed. Negative controls run in **both** polarities (neutralizing the guard to `=> true` fails exactly the refusal tests; to `=> false` fails the positive controls). Join tests pin the same-counter-instance wiring at five places, so a refactor handing the client a fresh counter goes red rather than silently killing the protection. Full CI green. No live-E2E: scanner reconciliation logic, not an API write path or UI, and the failure mode requires a projection regression that can't be induced against a real server without shipping broken code. **Deferred:** #604 — `JellyfinCollectionScanner`'s remove-all-then-re-add over `GetCollectionItems` has the identical seam and is still uncounted; a swallowed drop silently empties a collection. Not permanent media deletion, so genuinely lower priority, but the classification work is already done. **Docs updated:** `docs/decisions.md` (+ regenerated `docs/decisions/README.md` catalog). No route, endpoint, or SPA screen changed, so `blazor-route-parity.md` / `api-conventions.md` / `spa-conventions.md` needed no update.
timothy removed the in-progress label 2026-07-25 17:09:59 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
ad-hoc
One-off / ad-hoc work not tracked by a dedicated issue
 api
REST API / HTTP endpoints
 bug
Something isn't working
 ci-cd
Build, test, deploy pipeline
 content
Channel content / schedules / playlists
 dependencies
Dependency updates (Renovate)
 enhancement
New feature or improvement
 frontend
ChicoryTV React SPA frontend
 in-progress
Claimed by an active session — do not pick up
 jellyfin
Jellyfin tuner / IPTV integration
 parked
Excluded from automatic queue pickup; work only when explicitly selected
 priority: high
priority: low
priority: medium
review
Adversarial review finding
 security
Security / vulnerability fix
No labels enhancement priority: low
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
renovate timothy (Timothy)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: timothy/ersatztv#484