fix(494): reconcile removed music videos in Jellyfin scanner #495

Merged

timothy merged 2 commits from feat/494-musicvideo-reconcile into main 2026-07-20 17:22:15 +02:00

Conversation 1 Commits 2 Files Changed 3

+354 -1

timothy commented 2026-07-20 16:56:28 +02:00

Owner

Copy Link

Copy Source

Fixes #494.

JellyfinMusicVideoLibraryScanner did add/update only — a music video (or artist) deleted on the Jellyfin side lingered in ErsatzTV forever and could still be scheduled. Adds a trash sweep at the end of ScanLibrary (TrashMissingMusicVideos).

Approach (option 1 from the issue)

• Identity = (LibraryPathId, path), not server itemId. Music videos carry no JellyfinMusicVideo entity / ItemId/Etag, and the scanner injects the local IMusicVideoRepository (no itemId-keyed existing-set or flag seam). So the sweep diffs incoming local paths against FindMusicVideoPaths(libraryPath), hard-deletes the remainder via DeleteByPath + IScannerProxy.RemoveMediaItems, and cleans now-empty artists via IArtistRepository.DeleteEmptyArtists. Hard delete (not soft FileNotFound trash) because there is no per-item FileNotFound seam on this path and Done-when is "removed".
• Cross-kind safety is a query property, not a media-kind property. MediaItem is TPT with LibraryPathId on the base, so a Movie/Show/MusicVideo can share one LibraryPath (mixed library). FindMusicVideoPaths and DeleteByPath both filter LibraryPathId and join the concrete MusicVideo table, so the sweep can only see/delete music videos. Pinned by a cross-delete test.
• Reuses the #477 empty-fetch guard (MediaServerReconciliationGuard.ShouldFlagMissing): a successful-but-empty fetch does not wipe the library. Pinned as a negative control.

Known limitation (deferred)

MusicVideoRepository.GetOrAdd dedups a path globally (no LibraryPathId predicate), so a file served by two libraries with overlapping local paths is a single row owned by whichever library scanned it first; if that owner later stops reporting it while another library still serves it, the sweep removes the shared row. Proper fix = per-library music-video identity (a JellyfinMusicVideo etag entity + migration) — the issue's "option 2". Follow-up filed.

Coordination

File-disjoint from the in-flight #489/PR #493 (mixed libraries) — that PR touches the dispatch/wiring, not JellyfinMusicVideoLibraryScanner.cs. The #493 session's four warnings (esp. the global-GetOrAdd identity trap) are addressed above.

Tests

Integration tests (real ArtistRepository/MusicVideoRepository/LibraryRepository over in-memory SQLite, extending the #488 harness): removal, empty-artist cleanup, cross-kind safety, empty-fetch guard. Proven non-vacuous (all fail pre-fix except the guard control). Full local pass: Scanner.Tests 1481, ErsatzTV.Tests 1748, 0 failures.

Docs

docs/decisions.md records the identity choice + deferred follow-up.

🤖 Generated with Claude Code

Fixes #494. `JellyfinMusicVideoLibraryScanner` did add/update only — a music video (or artist) deleted on the Jellyfin side lingered in ErsatzTV forever and could still be scheduled. Adds a trash sweep at the end of `ScanLibrary` (`TrashMissingMusicVideos`). ## Approach (option 1 from the issue) - **Identity = (LibraryPathId, path), not server itemId.** Music videos carry no `JellyfinMusicVideo` entity / `ItemId`/`Etag`, and the scanner injects the *local* `IMusicVideoRepository` (no itemId-keyed existing-set or flag seam). So the sweep diffs incoming local paths against `FindMusicVideoPaths(libraryPath)`, hard-deletes the remainder via `DeleteByPath` + `IScannerProxy.RemoveMediaItems`, and cleans now-empty artists via `IArtistRepository.DeleteEmptyArtists`. Hard delete (not soft `FileNotFound` trash) because there is no per-item FileNotFound seam on this path and Done-when is "removed". - **Cross-kind safety is a query property, not a media-kind property.** `MediaItem` is TPT with `LibraryPathId` on the base, so a Movie/Show/MusicVideo can share one `LibraryPath` (mixed library). `FindMusicVideoPaths` and `DeleteByPath` both filter `LibraryPathId` **and** join the concrete `MusicVideo` table, so the sweep can only see/delete music videos. Pinned by a cross-delete test. - **Reuses the #477 empty-fetch guard** (`MediaServerReconciliationGuard.ShouldFlagMissing`): a successful-but-empty fetch does not wipe the library. Pinned as a negative control. ## Known limitation (deferred) `MusicVideoRepository.GetOrAdd` dedups a path **globally** (no `LibraryPathId` predicate), so a file served by two libraries with overlapping local paths is a single row owned by whichever library scanned it first; if that owner later stops reporting it while another library still serves it, the sweep removes the shared row. Proper fix = per-library music-video identity (a `JellyfinMusicVideo` etag entity + migration) — the issue's "option 2". Follow-up filed. ## Coordination File-disjoint from the in-flight #489/PR #493 (mixed libraries) — that PR touches the dispatch/wiring, not `JellyfinMusicVideoLibraryScanner.cs`. The #493 session's four warnings (esp. the global-`GetOrAdd` identity trap) are addressed above. ## Tests Integration tests (real `ArtistRepository`/`MusicVideoRepository`/`LibraryRepository` over in-memory SQLite, extending the #488 harness): removal, empty-artist cleanup, cross-kind safety, empty-fetch guard. Proven non-vacuous (all fail pre-fix except the guard control). Full local pass: Scanner.Tests 1481, ErsatzTV.Tests 1748, 0 failures. ## Docs `docs/decisions.md` records the identity choice + deferred follow-up. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

timothy added 1 commit 2026-07-20 16:56:29 +02:00

fix(494): reconcile removed music videos in Jellyfin scanner ... 2351841ff4

JellyfinMusicVideoLibraryScanner did add/update only, so a music video (or
artist) deleted on the Jellyfin side lingered in ErsatzTV forever and could
still be scheduled. Add a trash sweep at the end of ScanLibrary
(TrashMissingMusicVideos) mirroring the MediaServer* base scanners' "gone
upstream => remove" pattern.

Identity is library-scoped by (LibraryPathId, path) rather than server itemId:
music videos carry no JellyfinMusicVideo entity / ItemId, so the sweep diffs the
incoming local paths against FindMusicVideoPaths(libraryPath) and hard-deletes
the remainder via DeleteByPath + RemoveMediaItems, then cleans now-empty artists
via DeleteEmptyArtists. Both repo queries filter LibraryPathId AND join the
concrete MusicVideo table, so a Movie/Show sharing the same LibraryPath (a mixed
library) can never be cross-deleted. Gated by the #477 MediaServerReconciliationGuard
so a successful-but-empty fetch does not wipe the library.

Known limitation: MusicVideoRepository.GetOrAdd dedups paths globally, so a file
served by two libraries with overlapping local paths is a single row owned by the
first library; a proper fix needs per-library music-video identity (deferred).

Integration tests (real repos over in-memory SQLite) pin removal, empty-artist
cleanup, cross-kind safety, and the empty-fetch guard. decisions.md records the
identity choice and the deferred follow-up.

fixes #494

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

timothy referenced this pull request 2026-07-20 16:57:44 +02:00

Music videos have no per-library identity — global GetOrAdd path dedup can cross-library false-trash #496

timothy added 1 commit 2026-07-20 17:03:00 +02:00

test(494): mixed-library zero-incoming regression + tighten removal test ... fdef908929

Address the #493 session's field finding: a mixed Jellyfin library runs the
music-video arm with a legitimately empty incoming set on every scan while
Movies/Shows exist under the same LibraryPath (the real "Standup" case). Add
ScanLibrary_Should_Not_Touch_Movies_Or_Shows_When_No_MusicVideos_Present — seeds
a Movie (with a real MediaFile path) + Show, scans with zero incoming, asserts
both survive and RemoveMediaItems is never called. This pins that the
MusicVideo-joined FindMusicVideoPaths (not just the empty-fetch guard) is what
keeps existing.Except([]) from wiping the movies/episodes.

Also tighten the removal test to pin the trashed music-video id in the
RemoveMediaItems assertion (cold-review nit), and name the deferred per-library
identity follow-up (#496) in decisions.md.

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

timothy commented 2026-07-20 17:05:08 +02:00

Author

Owner

Copy Link

Copy Source

Cold-context adversarial review (independent, review-only agent — mandatory here: write-path + hard-delete semantics) completed on 2351841f: no Blocker/High, verdict MERGEABLE. It independently confirmed the path-diff symmetry (GetLocalPath ↔ stored MF.Path), partial-set protection (incoming paths added before ProcessMusicVideo so an errored-but-present item is never trashed), the #477 empty-fetch guard, cross-library row exclusion, and DeleteEmptyArtists scoping.

Addressed the two nits in fdef9089:

• Tightened the removal test to pin the trashed music-video id in the RemoveMediaItems assertion.
• Added ScanLibrary_Should_Not_Touch_Movies_Or_Shows_When_No_MusicVideos_Present — the #493 session's real "Standup" case (mixed library, zero incoming music videos, Movies+Show present), proving the MusicVideo-joined FindMusicVideoPaths (not just the guard) prevents the wipe.

Deferred (low, consistent with every existing scanner): RemoveMediaItems/DeleteEmptyArtists failures warn rather than abort after the DB row is deleted — a pre-existing repo-wide pattern, out of #494 scope.

The delta 2351841f..fdef9089 is test-only + one docs line; the production scanner file is byte-identical, so the verdict carries.

Review-verdict: MERGEABLE @ fdef908929

Cold-context adversarial review (independent, review-only agent — mandatory here: write-path + hard-delete semantics) completed on `2351841f`: **no Blocker/High**, verdict MERGEABLE. It independently confirmed the path-diff symmetry (GetLocalPath ↔ stored MF.Path), partial-set protection (incoming paths added *before* ProcessMusicVideo so an errored-but-present item is never trashed), the #477 empty-fetch guard, cross-library row exclusion, and `DeleteEmptyArtists` scoping. Addressed the two nits in `fdef9089`: - **Tightened** the removal test to pin the trashed music-video id in the `RemoveMediaItems` assertion. - **Added** `ScanLibrary_Should_Not_Touch_Movies_Or_Shows_When_No_MusicVideos_Present` — the #493 session's real "Standup" case (mixed library, zero incoming music videos, Movies+Show present), proving the `MusicVideo`-joined `FindMusicVideoPaths` (not just the guard) prevents the wipe. Deferred (low, consistent with every existing scanner): `RemoveMediaItems`/`DeleteEmptyArtists` failures warn rather than abort after the DB row is deleted — a pre-existing repo-wide pattern, out of #494 scope. The delta `2351841f..fdef9089` is **test-only + one docs line**; the production scanner file is byte-identical, so the verdict carries. Review-verdict: MERGEABLE @ fdef90892979e2d4054195c520c6bece34f68057

timothy referenced this pull request 2026-07-20 17:05:48 +02:00

JellyfinMusicVideoLibraryScanner does no reconciliation — music videos removed from Jellyfin are never removed from ErsatzTV #494

timothy merged commit 5b0d8b5d09 into main 2026-07-20 17:22:15 +02:00

timothy deleted branch feat/494-musicvideo-reconcile 2026-07-20 17:22:16 +02:00

timothy referenced this issue from a commit 2026-07-20 17:22:17 +02:00

fix(494): reconcile removed music videos in Jellyfin scanner (#495)

timothy referenced this pull request 2026-07-20 17:23:03 +02:00

JellyfinMusicVideoLibraryScanner does no reconciliation — music videos removed from Jellyfin are never removed from ErsatzTV #494

timothy referenced this pull request 2026-07-20 17:23:47 +02:00

ChicoryTV rewrite — queue tracker (goal, arc, session protocol) #237

timothy referenced this pull request 2026-07-20 17:52:59 +02:00

JellyfinMusicVideoLibraryScanner.UpdateMetadata drops Tags/Genres/Studios on existing items — Jellyfin metadata edits never sync #497

Sign in to join this conversation.

Reviewers

No Reviewers

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

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

renovate timothy (Timothy)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: timothy/ersatztv#495