feat(484): suppress the media-server sweep on projection failures; reject the ratio threshold
Extends MediaServerReconciliationGuard (#477) with a second deterministic refusal: when the enumeration that produced the incoming set silently dropped items whose projection THREW, the file-not-found sweep is refused. A dropped item the server did return is indistinguishable from a deletion at the reconcile step, so a projection regression could otherwise mass-flag a healthy library FileNotFound (which EmptyTrash then deletes permanently). Deliberate guard-clause skips (STRM files, virtual items, unsupported types) are explicitly NOT failures and never suppress a sweep — counting them would permanently disable reconciliation for any library holding a single STRM file. The ratio / missing-fraction threshold is REJECTED, not deferred: it is a two-sided heuristic with no tunable default and no telemetry, and the failure it approximates is exactly observable via the projection-failure count (a genuine bulk deletion produces zero failures). Seam is deliberately narrow — the private ProjectTo* contract inside each api client changed from Option<T> to MediaServerProjectionResult<T> (projected/skipped/failed), the paged helper counts IsFailure in one place, and the scanner reads it through an optional trailing MediaServerProjectionFailureCounter on only the five library-level methods that feed a sweep. The counter is per-enumeration state created by the scanner, never a field on an api client. fixes #484
This commit is contained in:
@@ -3777,3 +3777,75 @@ section renders the date itself, never a stale/fresh verdict.
|
||||
much of this corpus is agent-authored, distinguishing agent-inferred from human-confirmed-after-
|
||||
measuring is real signal; it just needs its own actor convention and wasn't worth coupling to this
|
||||
change.
|
||||
## 2026-07-25 — A media-server sweep also refuses when the api client silently dropped items whose projection threw; the ratio threshold is rejected (#484)
|
||||
`key: scan.projection-failure-sweep-guard` · `status: active` · `since: 2026-07-25` · `supersedes: none` · `superseded-by: none`
|
||||
**Rule:** `MediaServerReconciliationGuard.ShouldFlagMissing` takes a per-enumeration projection-failure count and refuses the file-not-found sweep (logged loudly) whenever it is non-zero against a non-empty existing set; deliberate guard-clause skips (STRM, virtual, unsupported type) are explicitly **not** failures and never suppress a sweep. The missing-fraction / ratio threshold floated by #477 is **rejected**, not deferred.
|
||||
**Signals:** projection failure, silently dropped item, deferred ratio threshold rejected, STRM skip vs failure, library sweep anti-nuke follow-up · paths: `MediaServerProjectionResult`, `MediaServerProjectionFailureCounter`, `MediaServerReconciliationGuard`, `JellyfinApiClient.GetPagedLibraryItems`, `EmbyApiClient.GetPagedLibraryContents` · issues: #484, #477, #476
|
||||
**Mechanics:** `MediaServerReconciliationGuardTests` policy table · `JellyfinApiClientTests.ProjectionFailureCounter` · `MediaServerTelevisionLibraryScannerTests.Projection_Failure_Suppresses_The_Partial_Deletion_Sweep`
|
||||
|
||||
This resolves both items the `scan.zero-item-fetch-guard` record (#477) left deferred. That record's rule
|
||||
is unchanged and still in force: a zero-item fetch against a non-empty library still refuses the sweep.
|
||||
This is a **second, independent** refusal on the same guard, plus a decision not to build the first.
|
||||
|
||||
- **The two ways a projection produces nothing are NOT the same thing, and conflating them is the main
|
||||
way to get this wrong.** Each media-server api client maps every item the server returned through a
|
||||
private `ProjectTo*` and drops the ones that yield nothing. A *deliberate skip* is a guard clause at
|
||||
the top of the projection — Jellyfin `ProjectToMovie`/`ProjectToMusicVideo` (`LocationType !=
|
||||
"FileSystem"`, `.strm`), `ProjectToEpisode` (same two), `ProjectToCollectionMediaItem` (both plus an
|
||||
unmatched `item.Type`); Emby `ProjectToMovie` (no `MediaSources`), `ProjectToEpisode` (`LocationType
|
||||
== "Virtual"`), `ProjectToCollectionMediaItem` (unmatched `item.Type`). These are permanent and
|
||||
expected: a library holding one STRM file emits one on **every** scan, forever. A *failure drop* is
|
||||
the `catch (Exception ex) { LogWarning(ex, "Error projecting …"); }` that every `ProjectTo*` in all
|
||||
three clients ends with — the server DID return that item, we just could not build it, and at the
|
||||
reconcile step that is indistinguishable from a deletion. Only failure drops suppress the sweep.
|
||||
Counting deliberate skips would permanently disable reconciliation for any library containing a single
|
||||
STRM file, so stale rows would accumulate forever — a regression, not a safe default.
|
||||
`Jellyfin`/`Emby` `ProjectToShow`, `ProjectToSeason` and `ProjectToCollection` have **no** guard
|
||||
clause at all, so for them every drop is a failure.
|
||||
- **Rejected — the missing-fraction / ratio threshold.** It is a two-sided heuristic. Set low it
|
||||
silently suppresses legitimate bulk deletions (stale rows persist invisibly, and the user's only
|
||||
signal is a warning nobody reads); set high it misses the partial-fetch case it exists for. Choosing
|
||||
the number needs per-install telemetry we do not collect, and no default is defensible for both a
|
||||
20-item library and a 20,000-item one. Decisively: the failure it approximates is **exactly
|
||||
observable** by the mechanism above, so accepting an unbounded false-suppression risk to approximate
|
||||
it is a bad trade. A genuine bulk deletion produces **zero** projection failures (and a
|
||||
correspondingly smaller server-reported total), so the deterministic signal has no false positives on
|
||||
the very case the ratio threshold would have broken.
|
||||
- **A three-state projection result, not a wider tuple.** The seam is deliberately narrow.
|
||||
`IAsyncEnumerable<Tuple<TItem, int>>` appears in ~90 signatures across `ErsatzTV.Core/Interfaces/
|
||||
{Jellyfin,Emby,Plex}`, the three api clients and ~15 scanners; widening it for this would be a
|
||||
disproportionate, risky refactor. Instead the **private** mapper contract inside each client changed
|
||||
from `Option<TItem>` to `MediaServerProjectionResult<TItem>` (projected / skipped / failed) — private,
|
||||
so zero public churn — and the paged helper counts `IsFailure` in exactly one place per client. The
|
||||
scanner reads the count through an **optional trailing parameter** on only the library-level methods
|
||||
that actually feed a sweep (`IJellyfinApiClient.GetMovieLibraryItems` /
|
||||
`GetMusicVideoLibraryItems` / `GetShowLibraryItemsWithoutPeople`, `IEmbyApiClient.GetMovieLibraryItems`
|
||||
/ `GetShowLibraryItems`), so every other call site is untouched. A library that cannot be resolved
|
||||
(`Option<TLibrary>.None` inside a mapper) is treated as a **failure**, not a skip — fail closed.
|
||||
- **The counter is per-enumeration state, never ambient.** `MediaServerProjectionFailureCounter` is
|
||||
created by the scanner that owns the sweep, handed to the single enumeration whose result it will
|
||||
diff, and read only after that enumeration completes. It is never a field on an api client (those are
|
||||
long-lived and shared) and never static, so concurrent scans of different libraries cannot leak
|
||||
failures into each other's sweep decision; increments are interlocked so a paginator that ever fans
|
||||
out stays correct.
|
||||
- **Wired into the four real sweep call sites**, matching #477's scope exactly: the three library-level
|
||||
base scanners (`MediaServerMovieLibraryScanner`, `MediaServerTelevisionLibraryScanner`,
|
||||
`MediaServerOtherVideoLibraryScanner`) and `JellyfinMusicVideoLibraryScanner`. The nested TV
|
||||
season/episode sweeps stay unguarded for the reasons in `scan.zero-item-fetch-guard`, and
|
||||
`ScanLibraryWithoutCleanup` (single-show rescan) passes no counter because it runs no sweep.
|
||||
- **Plex reports zero failures because it has none.** `PlexServerApiClient`'s movie/show/other-video
|
||||
projections return a bare entity with no `Option` and no `catch`, so a bad item throws and unwinds the
|
||||
whole scan — the pre-existing "protection by accident of control flow" that #477 named. The Plex
|
||||
scanner overrides therefore accept the counter and ignore it; that is honest, not a gap. If those
|
||||
projections ever grow a swallowing `catch`, they must report into the counter at the same time.
|
||||
- **Tests.** `MediaServerReconciliationGuardTests` pins the extended policy table: a failure count skips
|
||||
and warns; a short incoming set with **zero** failures (the deliberate-skip case) still sweeps; a
|
||||
1-of-500 bulk deletion with zero failures still sweeps; failures against an empty local set stay a
|
||||
silent no-op; the failure branch is reported when both refusals apply. `JellyfinApiClientTests
|
||||
.ProjectionFailureCounter` drives the real client over real JSON and proves the split at the layer
|
||||
that makes it — a STRM plus a virtual item leave the counter at 0 while the healthy item still flows,
|
||||
a projection made to throw yields no items and a count of 1, and two enumerations on one client
|
||||
instance keep separate counts. `MediaServerTelevisionLibraryScannerTests
|
||||
.Projection_Failure_Suppresses_The_Partial_Deletion_Sweep` is the integration pair to #476's cascade
|
||||
test: identical arrangement, only the counter differs, so the cascade test is the positive control
|
||||
that keeps the new test from passing vacuously.
|
||||
|
||||
Reference in New Issue
Block a user