Merge pull request 'fix(500): dedup incoming metadata collections so a duplicate name inserts once' (#602) from fix/500-distinct-metadata into main

ErsatzTV.Scanner.Tests/Core/Plex/PlexMovieLibraryScannerTests.cs

@@ -0,0 +1,299 @@
+using System.IO.Abstractions;
+using ErsatzTV.Core;
+using ErsatzTV.Core.Domain;
+using ErsatzTV.Core.Interfaces.Plex;
+using ErsatzTV.Core.Interfaces.Repositories;
+using ErsatzTV.Core.Metadata;
+using ErsatzTV.Scanner.Core.Interfaces;
+using ErsatzTV.Scanner.Core.Interfaces.Metadata;
+using ErsatzTV.Scanner.Core.Plex;
+using Microsoft.Extensions.Logging.Abstractions;
+using NSubstitute;
+using NUnit.Framework;
+using Shouldly;
+
+namespace ErsatzTV.Scanner.Tests.Core.Plex;
+
+// ersatztv#500: the remove-stale + add-new idiom materializes the add set (.ToList()) BEFORE the loop
+// mutates existingMetadata, so two identically-named incoming entries both passed the add filter and both
+// got inserted — a duplicate row. This is the ORIGINAL of the pattern (#497 mirrored it into the Jellyfin
+// music-video scanner, covered in ErsatzTV.Tests/Integration/JellyfinMusicVideoLibraryScannerTests).
+// UpdateMetadata is protected, so a minimal test-only subclass exposes it; that is the whole surface under
+// test here, which keeps this off the full ScanLibrary path.
+public class PlexMovieLibraryScannerTests
+{
+    [TestFixture]
+    public class UpdateMetadata
+    {
+        // Non-vacuous: dropping the DistinctBy calls in PlexMovieLibraryScanner.UpdateMetadata turns every
+        // Received(1) below into 2 calls, and each Count.ShouldBe(1) into 2.
+        [Test]
+        public async Task Should_Not_Double_Insert_Duplicate_Named_Incoming_Entries()
+        {
+            var movieRepository = Substitute.For<IMovieRepository>();
+            var metadataRepository = Substitute.For<IMetadataRepository>();
+
+            movieRepository.AddGenre(Arg.Any<MovieMetadata>(), Arg.Any<Genre>()).Returns(true);
+            movieRepository.AddTag(Arg.Any<MovieMetadata>(), Arg.Any<Tag>()).Returns(true);
+            movieRepository.AddStudio(Arg.Any<MovieMetadata>(), Arg.Any<Studio>()).Returns(true);
+            movieRepository.AddActor(Arg.Any<MovieMetadata>(), Arg.Any<Actor>()).Returns(true);
+            movieRepository.AddDirector(Arg.Any<MovieMetadata>(), Arg.Any<Director>()).Returns(true);
+            movieRepository.AddWriter(Arg.Any<MovieMetadata>(), Arg.Any<Writer>()).Returns(true);
+            metadataRepository.AddGuid(Arg.Any<ErsatzTV.Core.Domain.Metadata>(), Arg.Any<MetadataGuid>())
+                .Returns(true);
+            metadataRepository
+                .UpdateSubtitles(
+                    Arg.Any<ErsatzTV.Core.Domain.Metadata>(),
+                    Arg.Any<List<Subtitle>>(),
+                    Arg.Any<CancellationToken>())
+                .Returns(false);
+
+            // the existing item carries none of the incoming names, so every incoming entry is an "add"
+            var existingMetadata = new MovieMetadata
+            {
+                MetadataKind = MetadataKind.External,
+                Genres = [],
+                Tags = [],
+                Studios = [],
+                Actors = [],
+                Directors = [],
+                Writers = [],
+                Guids = [],
+                Subtitles = [],
+                Artwork = []
+            };
+
+            var existing = new PlexMovie { Id = 1, MovieMetadata = [existingMetadata] };
+
+            // Plex reports each name TWICE for the same movie (and the same guid twice)
+            var incoming = new MovieMetadata
+            {
+                MetadataKind = MetadataKind.External,
+                ContentRating = existingMetadata.ContentRating,
+                Plot = existingMetadata.Plot,
+                SortTitle = existingMetadata.SortTitle,
+                Genres = [new Genre { Name = "Comedy" }, new Genre { Name = "Comedy" }],
+                Tags = [new Tag { Name = "DupTag" }, new Tag { Name = "DupTag" }],
+                Studios = [new Studio { Name = "DupStudio" }, new Studio { Name = "DupStudio" }],
+                Actors = [new Actor { Name = "DupActor" }, new Actor { Name = "DupActor" }],
+                Directors = [new Director { Name = "DupDirector" }, new Director { Name = "DupDirector" }],
+                Writers = [new Writer { Name = "DupWriter" }, new Writer { Name = "DupWriter" }],
+                Guids = [new MetadataGuid { Guid = "imdb://tt1" }, new MetadataGuid { Guid = "imdb://tt1" }],
+                Subtitles = [],
+                Artwork = []
+            };
+
+            var scanner = new TestPlexMovieLibraryScanner(movieRepository, metadataRepository);
+
+            Either<BaseError, MediaItemScanResult<PlexMovie>> result = await scanner.Update(
+                new MediaItemScanResult<PlexMovie>(existing),
+                incoming);
+
+            result.IsRight.ShouldBeTrue();
+
+            await movieRepository.Received(1).AddGenre(Arg.Any<MovieMetadata>(), Arg.Is<Genre>(g => g.Name == "Comedy"));
+            await movieRepository.Received(1).AddTag(Arg.Any<MovieMetadata>(), Arg.Is<Tag>(t => t.Name == "DupTag"));
+            await movieRepository.Received(1)
+                .AddStudio(Arg.Any<MovieMetadata>(), Arg.Is<Studio>(s => s.Name == "DupStudio"));
+            await movieRepository.Received(1)
+                .AddActor(Arg.Any<MovieMetadata>(), Arg.Is<Actor>(a => a.Name == "DupActor"));
+            await movieRepository.Received(1)
+                .AddDirector(Arg.Any<MovieMetadata>(), Arg.Is<Director>(d => d.Name == "DupDirector"));
+            await movieRepository.Received(1)
+                .AddWriter(Arg.Any<MovieMetadata>(), Arg.Is<Writer>(w => w.Name == "DupWriter"));
+            await metadataRepository.Received(1).AddGuid(
+                Arg.Any<ErsatzTV.Core.Domain.Metadata>(),
+                Arg.Is<MetadataGuid>(g => g.Guid == "imdb://tt1"));
+
+            // and the in-memory collections mirror the DB: one entry each, not two
+            existingMetadata.Genres.Count.ShouldBe(1);
+            existingMetadata.Tags.Count.ShouldBe(1);
+            existingMetadata.Studios.Count.ShouldBe(1);
+            existingMetadata.Actors.Count.ShouldBe(1);
+            existingMetadata.Directors.Count.ShouldBe(1);
+            existingMetadata.Writers.Count.ShouldBe(1);
+            existingMetadata.Guids.Count.ShouldBe(1);
+        }
+
+        // POSITIVE CONTROL for the dedup above: distinct incoming entries must still ALL be added, and a
+        // stale existing entry must still be removed. Without this, a mis-keyed dedup (or a stray .Take(1))
+        // that collapsed genuinely different entries would pass every assertion in the duplicate test.
+        [Test]
+        public async Task Should_Still_Add_All_Distinct_Entries_And_Remove_Stale_Ones()
+        {
+            var movieRepository = Substitute.For<IMovieRepository>();
+            var metadataRepository = Substitute.For<IMetadataRepository>();
+
+            movieRepository.AddGenre(Arg.Any<MovieMetadata>(), Arg.Any<Genre>()).Returns(true);
+            metadataRepository.RemoveGenre(Arg.Any<Genre>()).Returns(true);
+            metadataRepository
+                .UpdateSubtitles(
+                    Arg.Any<ErsatzTV.Core.Domain.Metadata>(),
+                    Arg.Any<List<Subtitle>>(),
+                    Arg.Any<CancellationToken>())
+                .Returns(false);
+
+            var existingMetadata = NewMetadata();
+            existingMetadata.Genres = [new Genre { Name = "Keep" }, new Genre { Name = "Stale" }];
+
+            var existing = new PlexMovie { Id = 1, MovieMetadata = [existingMetadata] };
+
+            MovieMetadata incoming = NewMetadata();
+            incoming.Genres = [new Genre { Name = "Keep" }, new Genre { Name = "New1" }, new Genre { Name = "New2" }];
+
+            var scanner = new TestPlexMovieLibraryScanner(movieRepository, metadataRepository);
+
+            Either<BaseError, MediaItemScanResult<PlexMovie>> result = await scanner.Update(
+                new MediaItemScanResult<PlexMovie>(existing),
+                incoming);
+
+            result.IsRight.ShouldBeTrue();
+
+            // both distinct new genres added; the kept one is not re-added; the stale one is removed
+            await movieRepository.Received(1).AddGenre(Arg.Any<MovieMetadata>(), Arg.Is<Genre>(g => g.Name == "New1"));
+            await movieRepository.Received(1).AddGenre(Arg.Any<MovieMetadata>(), Arg.Is<Genre>(g => g.Name == "New2"));
+            await movieRepository.DidNotReceive().AddGenre(Arg.Any<MovieMetadata>(), Arg.Is<Genre>(g => g.Name == "Keep"));
+            await metadataRepository.Received(1).RemoveGenre(Arg.Is<Genre>(g => g.Name == "Stale"));
+            await metadataRepository.DidNotReceive().RemoveGenre(Arg.Is<Genre>(g => g.Name == "Keep"));
+
+            existingMetadata.Genres.Select(g => g.Name).OrderBy(n => n)
+                .ShouldBe(new[] { "Keep", "New1", "New2" });
+        }
+
+        // ersatztv#500 review finding: Actors are dedup'd specially. Their REMOVE filter is keyed on
+        // (Name, artwork-presence) — that is the mechanism by which an artwork-less actor is dropped and
+        // re-added WITH artwork. A bare DistinctBy(a => a.Name) keeps the FIRST duplicate, so if Plex lists
+        // the artwork-less copy first the artwork is silently discarded; and because the remove filter would
+        // still see the artwork-less duplicate, its upgrade clause stays false, so the stale row is never
+        // removed and the artwork never arrives on ANY later scan either. Hence the artwork-preferring dedup
+        // shared by both filters.
+        // Non-vacuous: reverting to `.DistinctBy(a => a.Name)` in the add chain fails both assertions.
+        [Test]
+        public async Task Should_Prefer_The_Artwork_Bearing_Duplicate_Actor()
+        {
+            var movieRepository = Substitute.For<IMovieRepository>();
+            var metadataRepository = Substitute.For<IMetadataRepository>();
+
+            movieRepository.AddActor(Arg.Any<MovieMetadata>(), Arg.Any<Actor>()).Returns(true);
+            metadataRepository.RemoveActor(Arg.Any<Actor>()).Returns(true);
+            metadataRepository
+                .UpdateSubtitles(
+                    Arg.Any<ErsatzTV.Core.Domain.Metadata>(),
+                    Arg.Any<List<Subtitle>>(),
+                    Arg.Any<CancellationToken>())
+                .Returns(false);
+
+            var existingMetadata = NewMetadata();
+            var existing = new PlexMovie { Id = 1, MovieMetadata = [existingMetadata] };
+
+            // Plex lists the same person twice, artwork-less copy FIRST
+            MovieMetadata incoming = NewMetadata();
+            incoming.Actors =
+            [
+                new Actor { Name = "Dup", Role = "no-artwork", Artwork = null },
+                new Actor
+                {
+                    Name = "Dup",
+                    Role = "with-artwork",
+                    Artwork = new Artwork { ArtworkKind = ArtworkKind.Thumbnail, Path = "/art/dup.jpg" }
+                }
+            ];
+
+            var scanner = new TestPlexMovieLibraryScanner(movieRepository, metadataRepository);
+
+            (await scanner.Update(new MediaItemScanResult<PlexMovie>(existing), incoming)).IsRight.ShouldBeTrue();
+
+            // exactly one actor, and it is the one that carries the artwork (and its Role came along)
+            await movieRepository.Received(1).AddActor(Arg.Any<MovieMetadata>(), Arg.Any<Actor>());
+            existingMetadata.Actors.Count.ShouldBe(1);
+            existingMetadata.Actors[0].Artwork.ShouldNotBeNull();
+            existingMetadata.Actors[0].Role.ShouldBe("with-artwork");
+        }
+
+        // The follow-on half of the finding: an artwork-less actor ALREADY stored must still be upgraded when
+        // the incoming set contains the same name twice. This is the case that used to be permanently stuck.
+        [Test]
+        public async Task Should_Upgrade_An_Existing_Artworkless_Actor_Despite_Duplicate_Incoming_Names()
+        {
+            var movieRepository = Substitute.For<IMovieRepository>();
+            var metadataRepository = Substitute.For<IMetadataRepository>();
+
+            movieRepository.AddActor(Arg.Any<MovieMetadata>(), Arg.Any<Actor>()).Returns(true);
+            metadataRepository.RemoveActor(Arg.Any<Actor>()).Returns(true);
+            metadataRepository
+                .UpdateSubtitles(
+                    Arg.Any<ErsatzTV.Core.Domain.Metadata>(),
+                    Arg.Any<List<Subtitle>>(),
+                    Arg.Any<CancellationToken>())
+                .Returns(false);
+
+            var existingMetadata = NewMetadata();
+            existingMetadata.Actors = [new Actor { Name = "Dup", Role = "stale", Artwork = null }];
+            var existing = new PlexMovie { Id = 1, MovieMetadata = [existingMetadata] };
+
+            MovieMetadata incoming = NewMetadata();
+            incoming.Actors =
+            [
+                new Actor { Name = "Dup", Role = "no-artwork", Artwork = null },
+                new Actor
+                {
+                    Name = "Dup",
+                    Role = "with-artwork",
+                    Artwork = new Artwork { ArtworkKind = ArtworkKind.Thumbnail, Path = "/art/dup.jpg" }
+                }
+            ];
+
+            var scanner = new TestPlexMovieLibraryScanner(movieRepository, metadataRepository);
+
+            (await scanner.Update(new MediaItemScanResult<PlexMovie>(existing), incoming)).IsRight.ShouldBeTrue();
+
+            // the stale artwork-less row is removed and replaced by the artwork-bearing one
+            await metadataRepository.Received(1).RemoveActor(Arg.Is<Actor>(a => a.Role == "stale"));
+            existingMetadata.Actors.Count.ShouldBe(1);
+            existingMetadata.Actors[0].Artwork.ShouldNotBeNull();
+        }
+
+        private static MovieMetadata NewMetadata() =>
+            new()
+            {
+                MetadataKind = MetadataKind.External,
+                Genres = [],
+                Tags = [],
+                Studios = [],
+                Actors = [],
+                Directors = [],
+                Writers = [],
+                Guids = [],
+                Subtitles = [],
+                Artwork = []
+            };
+    }
+
+    // Exposes the protected UpdateMetadata. Every dependency the metadata path does not touch is a
+    // substitute; nothing here drives ScanLibrary.
+    private sealed class TestPlexMovieLibraryScanner : PlexMovieLibraryScanner
+    {
+        public TestPlexMovieLibraryScanner(
+            IMovieRepository movieRepository,
+            IMetadataRepository metadataRepository)
+            : base(
+                Substitute.For<IScannerProxy>(),
+                Substitute.For<IPlexServerApiClient>(),
+                movieRepository,
+                metadataRepository,
+                Substitute.For<IMediaSourceRepository>(),
+                Substitute.For<IPlexMovieRepository>(),
+                Substitute.For<IPlexMetadataRepository>(),
+                Substitute.For<IPlexPathReplacementService>(),
+                Substitute.For<IFileSystem>(),
+                Substitute.For<ILocalChaptersProvider>(),
+                NullLogger<PlexMovieLibraryScanner>.Instance)
+        {
+        }
+
+        public Task<Either<BaseError, MediaItemScanResult<PlexMovie>>> Update(
+            MediaItemScanResult<PlexMovie> result,
+            MovieMetadata fullMetadata) =>
+            UpdateMetadata(result, fullMetadata, CancellationToken.None);
+    }
+}

ErsatzTV.Scanner/Core/Jellyfin/JellyfinMusicVideoLibraryScanner.cs

@@ -296,7 +296,10 @@ public class JellyfinMusicVideoLibraryScanner : IJellyfinMusicVideoLibraryScanne
     private async Task<bool> ReconcileGenres(MusicVideoMetadata existing, MusicVideoMetadata incoming)
     {
         existing.Genres ??= [];
-        List<Genre> incomingGenres = incoming.Genres ?? [];
+        // ersatztv#500: dedup on Name — the add filter below is materialized (.ToList()) BEFORE the loop
+        // mutates existing.Genres, so two identically-named incoming entries would both pass and both insert.
+        // Deduping here (not just in the add loop) is safe: the remove filter only asks "is this name present".
+        List<Genre> incomingGenres = (incoming.Genres ?? []).DistinctBy(g => g.Name).ToList();
         var updated = false;
 
         foreach (Genre genre in existing.Genres.Filter(g => incomingGenres.All(g2 => g2.Name != g.Name)).ToList())
@@ -317,7 +320,7 @@ public class JellyfinMusicVideoLibraryScanner : IJellyfinMusicVideoLibraryScanne
     private async Task<bool> ReconcileTags(MusicVideoMetadata existing, MusicVideoMetadata incoming)
     {
         existing.Tags ??= [];
-        List<Tag> incomingTags = incoming.Tags ?? [];
+        List<Tag> incomingTags = (incoming.Tags ?? []).DistinctBy(t => t.Name).ToList();
         var updated = false;
 
         foreach (Tag tag in existing.Tags.Filter(t => incomingTags.All(t2 => t2.Name != t.Name)).ToList())
@@ -338,7 +341,7 @@ public class JellyfinMusicVideoLibraryScanner : IJellyfinMusicVideoLibraryScanne
     private async Task<bool> ReconcileStudios(MusicVideoMetadata existing, MusicVideoMetadata incoming)
     {
         existing.Studios ??= [];
-        List<Studio> incomingStudios = incoming.Studios ?? [];
+        List<Studio> incomingStudios = (incoming.Studios ?? []).DistinctBy(s => s.Name).ToList();
         var updated = false;
 
         foreach (Studio studio in existing.Studios.Filter(s => incomingStudios.All(s2 => s2.Name != s.Name)).ToList())
@@ -359,7 +362,7 @@ public class JellyfinMusicVideoLibraryScanner : IJellyfinMusicVideoLibraryScanne
     private async Task<bool> ReconcileArtists(MusicVideoMetadata existing, MusicVideoMetadata incoming)
     {
         existing.Artists ??= [];
-        List<MusicVideoArtist> incomingArtists = incoming.Artists ?? [];
+        List<MusicVideoArtist> incomingArtists = (incoming.Artists ?? []).DistinctBy(a => a.Name).ToList();
         var updated = false;
 
         foreach (MusicVideoArtist artist in existing.Artists

ErsatzTV.Scanner/Core/Plex/PlexMovieLibraryScanner.cs

@@ -1,4 +1,4 @@
-using System.IO.Abstractions;
+using System.IO.Abstractions;
 using ErsatzTV.Core;
 using ErsatzTV.Core.Domain;
 using ErsatzTV.Core.Extensions;
@@ -200,7 +200,11 @@ public class PlexMovieLibraryScanner :
             }
         }
 
+        // ersatztv#500: each add filter below is materialized (.ToList()) BEFORE the loop mutates
+        // existingMetadata, so two identically-named incoming entries would both pass it and both insert.
+        // Deduplicate the incoming set on the same key the filter compares (Name; Guid for Guids).
         foreach (Genre genre in fullMetadata.Genres
+                     .DistinctBy(g => g.Name)
                      .Filter(g => existingMetadata.Genres.All(g2 => g2.Name != g.Name))
                      .ToList())
         {
@@ -223,6 +227,7 @@ public class PlexMovieLibraryScanner :
         }
 
         foreach (Studio studio in fullMetadata.Studios
+                     .DistinctBy(s => s.Name)
                      .Filter(s => existingMetadata.Studios.All(s2 => s2.Name != s.Name))
                      .ToList())
         {
@@ -233,9 +238,23 @@ public class PlexMovieLibraryScanner :
             }
         }
 
+        // ersatztv#500: Actors are the one collection whose dedup cannot be a bare DistinctBy in the add
+        // chain. Unlike the others, the REMOVE filter below is keyed on (Name, artwork-presence) — it is what
+        // drops an artwork-less actor so the add loop can re-add it WITH artwork. So the deduped set has to be
+        // hoisted and used by BOTH filters, and it has to prefer the artwork-bearing duplicate:
+        //  - dedup only the add chain and the remove filter still sees the artwork-less duplicate, which makes
+        //    its artwork-upgrade clause false -> the stale row is never removed and never re-added, so the
+        //    artwork is lost on EVERY subsequent scan, not just this one;
+        //  - keep the FIRST duplicate blindly and we may keep the one without artwork.
+        // Actor also carries Role/Order, so first-wins would silently drop those too.
+        List<Actor> incomingActors = fullMetadata.Actors
+            .GroupBy(a => a.Name)
+            .Map(g => g.FirstOrDefault(a => a.Artwork != null) ?? g.First())
+            .ToList();
+
         foreach (Actor actor in existingMetadata.Actors
                      .Filter(a =>
-                         fullMetadata.Actors.All(a2 => a2.Name != a.Name || a.Artwork == null && a2.Artwork != null))
+                         incomingActors.All(a2 => a2.Name != a.Name || a.Artwork == null && a2.Artwork != null))
                      .ToList())
         {
             existingMetadata.Actors.Remove(actor);
@@ -245,7 +264,7 @@ public class PlexMovieLibraryScanner :
             }
         }
 
-        foreach (Actor actor in fullMetadata.Actors
+        foreach (Actor actor in incomingActors
                      .Filter(a => existingMetadata.Actors.All(a2 => a2.Name != a.Name))
                      .ToList())
         {
@@ -268,6 +287,7 @@ public class PlexMovieLibraryScanner :
         }
 
         foreach (Director director in fullMetadata.Directors
+                     .DistinctBy(g => g.Name)
                      .Filter(g => existingMetadata.Directors.All(g2 => g2.Name != g.Name))
                      .ToList())
         {
@@ -290,6 +310,7 @@ public class PlexMovieLibraryScanner :
         }
 
         foreach (Writer writer in fullMetadata.Writers
+                     .DistinctBy(g => g.Name)
                      .Filter(g => existingMetadata.Writers.All(g2 => g2.Name != g.Name))
                      .ToList())
         {
@@ -312,6 +333,7 @@ public class PlexMovieLibraryScanner :
         }
 
         foreach (MetadataGuid guid in fullMetadata.Guids
+                     .DistinctBy(g => g.Guid)
                      .Filter(g => existingMetadata.Guids.All(g2 => g2.Guid != g.Guid))
                      .ToList())
         {
@@ -335,6 +357,7 @@ public class PlexMovieLibraryScanner :
         }
 
         foreach (Tag tag in fullMetadata.Tags
+                     .DistinctBy(g => g.Name)
                      .Filter(g => existingMetadata.Tags.All(g2 => g2.Name != g.Name))
                      .ToList())
         {

ErsatzTV.Tests/Integration/JellyfinMusicVideoLibraryScannerTests.cs

@@ -468,6 +468,105 @@ public class JellyfinMusicVideoLibraryScannerTests
         existingMetadata.Track.ShouldBe(4);
     }
 
+    // ersatztv#500: if Jellyfin reports two identically-named genres/tags/studios/artists for one item and
+    // that name is not yet on the existing item, BOTH used to pass the add filter (which is materialized with
+    // .ToList() before the loop mutates `existing`) and both got inserted — a duplicate row. Dedup the
+    // incoming collection on the same key the filter compares.
+    // Non-vacuous: dropping the DistinctBy calls in Reconcile* makes every Received(1) below see 2 calls.
+    [Test]
+    public async Task ScanLibrary_Should_Not_Double_Insert_Duplicate_Named_Incoming_Collection_Entries()
+    {
+        JellyfinLibrary library = BuildLibrary(1, "/data/music");
+        var connectionParameters = new JellyfinConnectionParameters("http://jellyfin", "api-key", 1);
+        const string VideoPath = "/data/music/artist1/song1.mkv";
+
+        // the existing item carries NONE of the incoming names, so every incoming entry is an "add"
+        var existing = new MusicVideo
+        {
+            Id = 7,
+            ArtistId = 3,
+            MediaVersions =
+            [
+                new MediaVersion
+                {
+                    MediaFiles = [new MediaFile { Path = VideoPath }],
+                    Streams = []
+                }
+            ],
+            MusicVideoMetadata =
+            [
+                new MusicVideoMetadata
+                {
+                    Id = 11,
+                    Genres = [],
+                    Tags = [],
+                    Studios = [],
+                    Artists = []
+                }
+            ]
+        };
+
+        var artistRepository = Substitute.For<IArtistRepository>();
+        artistRepository.GetArtistByMetadata(Arg.Any<int>(), Arg.Any<ArtistMetadata>())
+            .Returns(Some(new Artist { Id = 3, ArtistMetadata = new List<ArtistMetadata>() }));
+        artistRepository.DeleteEmptyArtists(Arg.Any<LibraryPath>()).Returns(new List<int>());
+
+        var musicVideoRepository = Substitute.For<IMusicVideoRepository>();
+        musicVideoRepository
+            .GetOrAdd(Arg.Any<Artist>(), Arg.Any<LibraryPath>(), Arg.Any<LibraryFolder>(), Arg.Any<string>())
+            .Returns(Right<BaseError, MediaItemScanResult<MusicVideo>>(
+                new MediaItemScanResult<MusicVideo>(existing) { IsAdded = false }));
+        musicVideoRepository.FindMusicVideoPaths(Arg.Any<LibraryPath>())
+            .Returns(new List<string> { VideoPath }.AsEnumerable());
+        musicVideoRepository.AddGenre(Arg.Any<MusicVideoMetadata>(), Arg.Any<Genre>()).Returns(true);
+        musicVideoRepository.AddTag(Arg.Any<MusicVideoMetadata>(), Arg.Any<Tag>()).Returns(true);
+        musicVideoRepository.AddStudio(Arg.Any<MusicVideoMetadata>(), Arg.Any<Studio>()).Returns(true);
+        musicVideoRepository.AddArtist(Arg.Any<MusicVideoMetadata>(), Arg.Any<MusicVideoArtist>()).Returns(true);
+
+        var metadataRepository = Substitute.For<IMetadataRepository>();
+        metadataRepository.Update(Arg.Any<ErsatzTV.Core.Domain.Metadata>()).Returns(true);
+        metadataRepository.UpdateStatistics(Arg.Any<MediaItem>(), Arg.Any<MediaVersion>(), Arg.Any<bool>())
+            .Returns(false);
+
+        var libraryRepository = Substitute.For<ILibraryRepository>();
+        libraryRepository.GetParentFolderId(Arg.Any<LibraryPath>(), Arg.Any<string>(), Arg.Any<CancellationToken>())
+            .Returns(Option<int>.None);
+        libraryRepository.GetOrAddFolder(Arg.Any<LibraryPath>(), Arg.Any<Option<int>>(), Arg.Any<string>())
+            .Returns(new LibraryFolder { Id = 5, Path = "/data/music/artist1" });
+
+        // Jellyfin reports each name TWICE for the same item
+        JellyfinMusicVideoLibraryScanner scanner = BuildScannerWith(
+            FakeApi(() => BuildIncoming(
+                VideoPath, "Artist 1", "Song 1",
+                genres: ["Synthwave", "Synthwave"],
+                tags: ["DupTag", "DupTag"],
+                studios: ["DupStudio", "DupStudio"],
+                artists: ["Artist 1", "Artist 1"])),
+            artistRepository,
+            musicVideoRepository,
+            libraryRepository,
+            metadataRepository);
+
+        (await scanner.ScanLibrary(connectionParameters, library, deepScan: false, CancellationToken.None))
+            .IsRight.ShouldBeTrue();
+
+        await musicVideoRepository.Received(1)
+            .AddGenre(Arg.Any<MusicVideoMetadata>(), Arg.Is<Genre>(g => g.Name == "Synthwave"));
+        await musicVideoRepository.Received(1)
+            .AddTag(Arg.Any<MusicVideoMetadata>(), Arg.Is<Tag>(t => t.Name == "DupTag"));
+        await musicVideoRepository.Received(1)
+            .AddStudio(Arg.Any<MusicVideoMetadata>(), Arg.Is<Studio>(s => s.Name == "DupStudio"));
+        await musicVideoRepository.Received(1)
+            .AddArtist(Arg.Any<MusicVideoMetadata>(), Arg.Is<MusicVideoArtist>(a => a.Name == "Artist 1"));
+
+        // and the in-memory collections mirror the DB: one entry each, not two
+        MusicVideoMetadata metadata = existing.MusicVideoMetadata.Head();
+        metadata.Genres.Count.ShouldBe(1);
+        metadata.Tags.Count.ShouldBe(1);
+        metadata.Studios.Count.ShouldBe(1);
+        metadata.Artists.Count.ShouldBe(1);
+    }
+
     private JellyfinMusicVideoLibraryScanner BuildScannerWith(
         IJellyfinApiClient apiClient,
         IArtistRepository artistRepository,

docs/decisions.md

@@ -2479,8 +2479,8 @@ remote scanner tripped it, and the feature had never run in prod, CI, or locally
 ## 2026-07-20 — `JellyfinMusicVideoLibraryScanner` reconciles by library-scoped path diff + hard delete, not server itemId soft-trash (#494)
 `key: scan.musicvideo-reconciliation` · `status: active` · `since: 2026-07-20` · `supersedes: none` · `superseded-by: none`
 **Rule:** `JellyfinMusicVideoLibraryScanner` reconciles removed music videos by a library-scoped local-path diff plus hard delete (`TrashMissingMusicVideos`), not the server-itemId soft-trash pattern the other media-server scanners use, because music videos carry no server identity.
-**Signals:** music-video trash sweep, path-based identity, cross-kind safety, path-keyed identity, empty-fetch guard reuse · paths: `JellyfinMusicVideoLibraryScanner.TrashMissingMusicVideos`, `FindMusicVideoPaths`/`DeleteByPath`, `IMusicVideoRepository`, `MediaServerReconciliationGuard` · issues: #494, #477, #488, #496
-**Mechanics:** `ScanLibrary_Should_Not_CrossDelete_Movie_Or_Show_Sharing_The_LibraryPath`; `ScanLibrary_Should_Not_Sweep_When_Jellyfin_Returns_Zero_Items`; integration tests extending the #488 harness
+**Signals:** music-video trash sweep, path-based identity, cross-kind safety, path-keyed identity, empty-fetch guard reuse, remove-stale+add-new dedup · paths: `JellyfinMusicVideoLibraryScanner.TrashMissingMusicVideos`, `FindMusicVideoPaths`/`DeleteByPath`, `IMusicVideoRepository`, `MediaServerReconciliationGuard` · issues: #494, #477, #488, #496, #500
+**Mechanics:** `ScanLibrary_Should_Not_CrossDelete_Movie_Or_Show_Sharing_The_LibraryPath`; `ScanLibrary_Should_Not_Sweep_When_Jellyfin_Returns_Zero_Items`; integration tests extending the #488 harness. #500 — when mirroring the remove-stale + add-new idiom, dedup the incoming set on **the same key its add filter compares** (the filter is materialized before the loop mutates `existing`, so duplicates both pass): `Name`, `Guid` for guids, and for Plex `Actors` an artwork-preferring dedup shared with the remove filter (whose key is `(Name, artwork-presence)`). Remaining un-deduped copies of the idiom: #600.
 
 The Jellyfin music-video scanner did add/update only — a music video removed on the Jellyfin side lingered in
 ErsatzTV forever and could still be scheduled. It now runs a trash sweep at the end of `ScanLibrary`

docs/testing.md

@@ -9,7 +9,7 @@ before adding tests, not just `docs/contributing.md` §8 (which now just points
 |---|---|---|
 | `ErsatzTV.Tests` | API controllers + MediatR handlers | In-memory SQLite fixture: a shared `SqliteConnection("Data Source=:memory:;Foreign Keys=False")` kept open + `EnsureCreatedAsync()` (**not** full migration replay) + `PRAGMA foreign_keys=OFF`, then seed; a tiny `IDbContextFactory` wraps `new TvContext(...)`. 828 tests currently. |
 | `ErsatzTV.Core.Tests` | Domain logic, scheduling, IPTV/XMLTV generation | References `ErsatzTV.Application` directly — there is no separate `Application.Tests` project. 543 tests + 1 skipped under `TZ=UTC` (the Block playout golden additionally skips under a non-UTC `TZ`; see Golden-file nets). |
-| `ErsatzTV.Scanner.Tests` | Library scanning: scan handlers, folder scanners, NFO readers | Handler tests substitute the folder scanners + `ILibraryRepository` and assert the resulting repository writes (e.g. `ScanLocalLibraryHandlerTests` pins which `LastScan` levels a scan records — ersatztv#264). Fakes/`Testably` back the file-system-facing scanners. 1471 tests. Additionally contains `Core/FFmpeg/TranscodingTests` — `[Explicit]` + `[Combinatorial]`, so it never runs in CI or a plain `dotnet test` (it needs real ffmpeg/hardware) and contributes 0 to that count; run it by name when touching the transcoding pipeline. |
+| `ErsatzTV.Scanner.Tests` | Library scanning: scan handlers, folder scanners, NFO readers | Handler tests substitute the folder scanners + `ILibraryRepository` and assert the resulting repository writes (e.g. `ScanLocalLibraryHandlerTests` pins which `LastScan` levels a scan records — ersatztv#264). Fakes/`Testably` back the file-system-facing scanners. ~1,485 tests (approximate on purpose — an exact count goes stale on every PR that adds one). Additionally contains `Core/FFmpeg/TranscodingTests` — `[Explicit]` + `[Combinatorial]`, so it never runs in CI or a plain `dotnet test` (it needs real ffmpeg/hardware) and contributes 0 to that count; run it by name when touching the transcoding pipeline. |
 | `ErsatzTV.Architecture.Tests` | Layering rules via NetArchTest.eNhancedEdition | Core↛Infra/App/EF; FFmpeg↛all; App↛concrete providers. 5 tests. See `docs/contributing.md` §1. |
 | `ErsatzTV.FFmpeg.Tests` | FFmpeg command construction | Build a pipeline, assert the exact rendered arg string (`PipelineBuilderBaseTests.cs`). |
 | `web/` (vitest) | React SPA unit tests | 330 tests; run alongside typecheck + build (see below). |