fix(497): reconcile music-video metadata collections on Jellyfin rescan #499

Merged
timothy merged 1 commits from fix/497-musicvideo-metadata-update into main 2026-07-20 18:53:42 +02:00
Owner

Fixes #497.

Problem

JellyfinMusicVideoLibraryScanner.UpdateMetadata copied only scalar fields for an existing music video, so genres/tags/studios/artists edited in Jellyfin never reached ErsatzTV — the update path silently dropped every child collection (only the Add path persisted them).

Root cause (inherited from upstream)

Unlike the movie/episode Jellyfin path — which reconciles child collections inside the tracked repository GetOrAdd — MusicVideoRepository.GetOrAdd is AsNoTracking, and the scanner never reconciled the collections itself. This is upstream ErsatzTV code (present verbatim at our fork import boundary), not something introduced in the fork. Movie/Episode paths do not have this gap, so no separate fix is needed there (Done-when #4).

Fix

Mirrors PlexMovieLibraryScanner.UpdateMetadata's remove-stale + add-new idiom, reconciling exactly the collections that BOTH the Add path persists AND GetOrAdd eager-loads: Genres, Tags, Studios, Artists.

  • Guids: add-persisted but not eager-loaded → reconciling would see an empty existing and duplicate-insert every scan → out of scope.
  • Directors: eager-loaded but not add-persisted for music videos → out of scope (keeps update consistent with add).

Test

Interaction test (substituted repos, canned existing item) asserting the exact reconcile calls; proven non-vacuous (reverting the Reconcile* calls drops every Received() and the test fails).

The real-DB double-scan approach can't drive this here: the in-memory test harness shares one SQLite connection across contexts, and mid-scan GetOrAdd's MediaVersions.First().MediaFiles.First().Path predicate mis-resolves once the existing item carries metadata children — a harness-only quirk (prod uses per-context pooled connections; music videos use that predicate only because they carry no server ItemId).

Done-when

  • A tag/genre added in Jellyfin to an existing music video appears after a normal scan
  • A tag/genre removed in Jellyfin is removed (no orphan accumulation)
  • Test covers the update path, proven non-vacuous
  • Confirmed Movie/Episode update paths do NOT have the same gap (reconcile in tracked repo GetOrAdd)
  • Adversarial review passed

Docs

No API/SPA/route/migration change → no convention-doc update required.

🤖 Generated with Claude Code

Fixes #497. ## Problem `JellyfinMusicVideoLibraryScanner.UpdateMetadata` copied only scalar fields for an **existing** music video, so genres/tags/studios/artists edited in Jellyfin never reached ErsatzTV — the update path silently dropped every child collection (only the *Add* path persisted them). ## Root cause (inherited from upstream) Unlike the movie/episode Jellyfin path — which reconciles child collections inside the **tracked** repository `GetOrAdd` — `MusicVideoRepository.GetOrAdd` is `AsNoTracking`, and the scanner never reconciled the collections itself. This is upstream ErsatzTV code (present verbatim at our fork import boundary), not something introduced in the fork. Movie/Episode paths do **not** have this gap, so no separate fix is needed there (Done-when #4). ## Fix Mirrors `PlexMovieLibraryScanner.UpdateMetadata`'s remove-stale + add-new idiom, reconciling exactly the collections that BOTH the Add path persists AND `GetOrAdd` eager-loads: **Genres, Tags, Studios, Artists**. - Guids: add-persisted but *not* eager-loaded → reconciling would see an empty `existing` and duplicate-insert every scan → out of scope. - Directors: eager-loaded but *not* add-persisted for music videos → out of scope (keeps update consistent with add). ## Test Interaction test (substituted repos, canned existing item) asserting the exact reconcile calls; **proven non-vacuous** (reverting the `Reconcile*` calls drops every `Received()` and the test fails). The real-DB double-scan approach can't drive this here: the in-memory test harness shares one SQLite connection across contexts, and mid-scan `GetOrAdd`'s `MediaVersions.First().MediaFiles.First().Path` predicate mis-resolves once the existing item carries metadata children — a harness-only quirk (prod uses per-context pooled connections; music videos use that predicate only because they carry no server ItemId). ## Done-when - [x] A tag/genre added in Jellyfin to an existing music video appears after a normal scan - [x] A tag/genre removed in Jellyfin is removed (no orphan accumulation) - [x] Test covers the update path, proven non-vacuous - [x] Confirmed Movie/Episode update paths do NOT have the same gap (reconcile in tracked repo GetOrAdd) - [ ] Adversarial review passed ## Docs No API/SPA/route/migration change → no convention-doc update required. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
timothy added 1 commit 2026-07-20 18:32:59 +02:00
fix(497): reconcile music-video metadata collections on Jellyfin rescan
Build ErsatzTV Image / CI image pin matches docker/ci (pull_request) Successful in 9s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 1m12s
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 51s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 1m25s
Build ErsatzTV Image / decisions.md append-only (pull_request) Successful in 7s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 5m56s
Build ErsatzTV Image / Functional E2E (curl contracts) (pull_request) Successful in 15m37s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 19m3s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
c633b89ad9
JellyfinMusicVideoLibraryScanner.UpdateMetadata copied only scalar fields
for an EXISTING music video, so genres/tags/studios/artists edited in
Jellyfin never reached ErsatzTV — the update path silently dropped every
child collection (only the Add path ever persisted them). Root cause is
inherited from upstream: unlike the movie/episode Jellyfin path, which
reconciles collections inside the tracked repository GetOrAdd,
MusicVideoRepository.GetOrAdd is AsNoTracking and the scanner never
reconciled the collections itself.

Fix mirrors PlexMovieLibraryScanner.UpdateMetadata's remove-stale +
add-new idiom, reconciling exactly the collections that BOTH the Add path
persists AND GetOrAdd eager-loads: Genres, Tags, Studios, Artists. Guids
(add-persisted but not eager-loaded — would duplicate) and Directors
(eager-loaded but not add-persisted for music videos) are deliberately
out of scope.

Movie/Episode paths do NOT have this gap (they reconcile in the tracked
repo GetOrAdd), so no separate fix is needed there.

Test is an interaction test (substituted repos, canned existing item)
verifying the exact reconcile calls; proven non-vacuous. The real-DB
double-scan approach can't drive this: the in-memory harness shares one
SQLite connection across contexts and mid-scan GetOrAdd's First()-nav
path predicate mis-resolves once the existing item carries metadata
children — a harness-only quirk (prod uses per-context pooled
connections).

fixes #497

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author
Owner

Cold adversarial review (review-only, cross-context)

No blockers. Independently verified: scope exclusion of Guids (not eager-loaded → would duplicate) and Directors (not add-persisted for music videos) is correct against MetadataRepository.Add + MusicVideoRepository.GetOrAdd; remove seams delete by direct FK Id (no shared-row cross-delete); updated = await Reconcile() || updated always awaits (only the right || operand short-circuits); all four methods materialize with .ToList() before mutating (no iterator invalidation); the interaction test is non-vacuous.

Low (not blocking, not introduced here): duplicate-name double-insert — if Jellyfin ever emits two identical genre/tag names for one item, both pass the add filter and both INSERT. This is inherited verbatim from the mirrored PlexMovieLibraryScanner.UpdateMetadata, so keeping 1:1 parity here and filing a follow-up to fix BOTH scanners uniformly (rather than diverging in one place). Follow-up: filed as a new issue.

Review-verdict: MERGEABLE @ c633b89a

## Cold adversarial review (review-only, cross-context) **No blockers.** Independently verified: scope exclusion of Guids (not eager-loaded → would duplicate) and Directors (not add-persisted for music videos) is correct against `MetadataRepository.Add` + `MusicVideoRepository.GetOrAdd`; remove seams delete by direct FK Id (no shared-row cross-delete); `updated = await Reconcile() || updated` always awaits (only the right `||` operand short-circuits); all four methods materialize with `.ToList()` before mutating (no iterator invalidation); the interaction test is non-vacuous. **Low (not blocking, not introduced here):** duplicate-name double-insert — if Jellyfin ever emits two identical genre/tag names for one item, both pass the add filter and both INSERT. This is inherited verbatim from the mirrored `PlexMovieLibraryScanner.UpdateMetadata`, so keeping 1:1 parity here and filing a follow-up to fix BOTH scanners uniformly (rather than diverging in one place). Follow-up: filed as a new issue. Review-verdict: MERGEABLE @ c633b89a
timothy merged commit c4ad4a73ba into main 2026-07-20 18:53:42 +02:00
timothy deleted branch fix/497-musicvideo-metadata-update 2026-07-20 18:53:43 +02:00
Sign in to join this conversation.