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.
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)
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>
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.
## 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 main2026-07-20 18:53:42 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes #497.
Problem
JellyfinMusicVideoLibraryScanner.UpdateMetadatacopied 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.GetOrAddisAsNoTracking, 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 ANDGetOrAddeager-loads: Genres, Tags, Studios, Artists.existingand duplicate-insert every scan → out of scope.Test
Interaction test (substituted repos, canned existing item) asserting the exact reconcile calls; proven non-vacuous (reverting the
Reconcile*calls drops everyReceived()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'sMediaVersions.First().MediaFiles.First().Pathpredicate 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
Docs
No API/SPA/route/migration change → no convention-doc update required.
🤖 Generated with Claude Code
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() || updatedalways 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