Merge pull request #17756 from Shadowghost/fix-people-creation

Fix people and artist validator creation and deletion handling

Emby.Server.Implementations/Library/LibraryManager.cs

@@ -15,7 +15,6 @@ using Emby.Naming.Common;
 using Emby.Naming.TV;
 using Emby.Naming.Video;
 using Emby.Server.Implementations.Library.Resolvers;
-using Emby.Server.Implementations.Library.Validators;
 using Emby.Server.Implementations.Playlists;
 using Emby.Server.Implementations.ScheduledTasks.Tasks;
 using Emby.Server.Implementations.Sorting;
@@ -35,7 +34,6 @@ using MediaBrowser.Controller.Entities.Movies;
 using MediaBrowser.Controller.IO;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Controller.LiveTv;
-using MediaBrowser.Controller.MediaEncoding;
 using MediaBrowser.Controller.Persistence;
 using MediaBrowser.Controller.Playlists;
 using MediaBrowser.Controller.Providers;
@@ -75,7 +73,6 @@ namespace Emby.Server.Implementations.Library
         private readonly Lazy<IProviderManager> _providerManagerFactory;
         private readonly Lazy<IUserViewManager> _userViewManagerFactory;
         private readonly IServerApplicationHost _appHost;
-        private readonly IMediaEncoder _mediaEncoder;
         private readonly IFileSystem _fileSystem;
         private readonly IItemRepository _itemRepository;
         private readonly IItemPersistenceService _persistenceService;
@@ -122,7 +119,6 @@ namespace Emby.Server.Implementations.Library
         /// <param name="fileSystem">The file system.</param>
         /// <param name="providerManagerFactory">The provider manager.</param>
         /// <param name="userViewManagerFactory">The user view manager.</param>
-        /// <param name="mediaEncoder">The media encoder.</param>
         /// <param name="itemRepository">The item repository.</param>
         /// <param name="persistenceService">The item persistence service.</param>
         /// <param name="nextUpService">The next up service.</param>
@@ -148,7 +144,6 @@ namespace Emby.Server.Implementations.Library
             IFileSystem fileSystem,
             Lazy<IProviderManager> providerManagerFactory,
             Lazy<IUserViewManager> userViewManagerFactory,
-            IMediaEncoder mediaEncoder,
             IItemRepository itemRepository,
             IItemPersistenceService persistenceService,
             INextUpService nextUpService,
@@ -174,7 +169,6 @@ namespace Emby.Server.Implementations.Library
             _fileSystem = fileSystem;
             _providerManagerFactory = providerManagerFactory;
             _userViewManagerFactory = userViewManagerFactory;
-            _mediaEncoder = mediaEncoder;
             _itemRepository = itemRepository;
             _persistenceService = persistenceService;
             _nextUpService = nextUpService;
@@ -1209,6 +1203,12 @@ namespace Emby.Server.Implementations.Library
                 .FirstOrDefault();
         }
 
+        /// <inheritdoc />
+        public Guid GetPersonId(string name)
+        {
+            return GetItemByNameId<Person>(Person.GetPath(name));
+        }
+
         /// <inheritdoc />
         public Person? GetPerson(string name)
         {
@@ -1222,6 +1222,33 @@ namespace Emby.Server.Implementations.Library
             return null;
         }
 
+        /// <inheritdoc />
+        public Person GetOrCreatePerson(string name)
+        {
+            var existing = GetPerson(name);
+            if (existing is not null)
+            {
+                return existing;
+            }
+
+            var path = Person.GetPath(name);
+            var info = Directory.CreateDirectory(path);
+            var item = new Person
+            {
+                Name = name,
+                Id = GetItemByNameId<Person>(path),
+                DateCreated = info.CreationTimeUtc,
+                DateModified = info.LastWriteTimeUtc,
+                Path = path
+            };
+
+            item.PresentationUniqueKey = item.CreatePresentationUniqueKey();
+
+            CreateItem(item, null);
+
+            return item;
+        }
+
         /// <summary>
         /// Gets the studio.
         /// </summary>
@@ -1354,15 +1381,6 @@ namespace Emby.Server.Implementations.Library
             return GetNewItemIdInternal(path, typeof(T), forceCaseInsensitiveId);
         }
 
-        /// <inheritdoc />
-        public Task ValidatePeopleAsync(IProgress<double> progress, CancellationToken cancellationToken)
-        {
-            // Ensure the location is available.
-            Directory.CreateDirectory(_configurationManager.ApplicationPaths.PeoplePath);
-
-            return new PeopleValidator(this, _logger, _fileSystem).ValidatePeople(cancellationToken, progress);
-        }
-
         /// <summary>
         /// Reloads the root media folder.
         /// </summary>
@@ -3746,27 +3764,14 @@ namespace Emby.Server.Implementations.Library
 
                 var itemUpdateType = ItemUpdateType.MetadataDownload;
                 var saveEntity = false;
-                var createEntity = false;
                 var personEntity = GetPerson(person.Name);
 
                 if (personEntity is null)
                 {
                     try
                     {
-                        var path = Person.GetPath(person.Name);
-                        var info = Directory.CreateDirectory(path);
-                        personEntity = new Person()
-                        {
-                            Name = person.Name,
-                            Id = GetItemByNameId<Person>(path),
-                            DateCreated = info.CreationTimeUtc,
-                            DateModified = info.LastWriteTimeUtc,
-                            Path = path
-                        };
-
-                        personEntity.PresentationUniqueKey = personEntity.CreatePresentationUniqueKey();
+                        personEntity = GetOrCreatePerson(person.Name);
                         saveEntity = true;
-                        createEntity = true;
                     }
                     catch (Exception ex)
                     {
@@ -3800,11 +3805,6 @@ namespace Emby.Server.Implementations.Library
 
                 if (saveEntity)
                 {
-                    if (createEntity)
-                    {
-                        CreateItems([personEntity], null, CancellationToken.None);
-                    }
-
                     await RunMetadataSavers(personEntity, itemUpdateType).ConfigureAwait(false);
                     personEntity.DateLastSaved = DateTime.UtcNow;
 

Emby.Server.Implementations/Library/Validators/ArtistsValidator.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.Globalization;
 using System.Linq;
 using System.Threading;
@@ -61,6 +62,9 @@ public class ArtistsValidator
         var count = names.Count;
         var refreshed = 0;
 
+        var liveIds = new HashSet<Guid>();
+        var unresolved = 0;
+
         foreach (var name in names)
         {
             try
@@ -73,13 +77,20 @@ public class ArtistsValidator
 
                 // Fall back to GetArtist if not found (creates new item if needed)
                 item ??= _libraryManager.GetArtist(name);
-                var isNew = !existingArtistIds.Contains(item.Id);
-                var neverRefreshed = item.DateLastRefreshed == default;
 
-                if (isNew || neverRefreshed)
+                // A name with no item is nothing to refresh, and nothing to keep alive either.
+                if (item is not null)
                 {
-                    await item.RefreshMetadata(cancellationToken).ConfigureAwait(false);
-                    refreshed++;
+                    liveIds.Add(item.Id);
+
+                    var isNew = !existingArtistIds.Contains(item.Id);
+                    var neverRefreshed = item.DateLastRefreshed == default;
+
+                    if (isNew || neverRefreshed)
+                    {
+                        await item.RefreshMetadata(cancellationToken).ConfigureAwait(false);
+                        refreshed++;
+                    }
                 }
             }
             catch (OperationCanceledException)
@@ -88,6 +99,7 @@ public class ArtistsValidator
             }
             catch (Exception ex)
             {
+                unresolved++;
                 _logger.LogError(ex, "Error refreshing {ArtistName}", name);
             }
 
@@ -101,13 +113,26 @@ public class ArtistsValidator
 
         _logger.LogInformation("Refreshed metadata for {RefreshedCount} new artists out of {TotalCount} total", refreshed, count);
 
+        // Every name that threw is a name whose artist is missing from the live set, and deleting against
+        // a live set with holes in it deletes artists the library still refers to. Leave the sweep to a
+        // run that got a clean read of them.
+        if (unresolved > 0)
+        {
+            _logger.LogWarning(
+                "Not removing dead artists: {Count} of {TotalCount} names could not be resolved this run",
+                unresolved,
+                count);
+
+            progress.Report(100);
+            return;
+        }
+
         var deadEntities = _libraryManager.GetItemList(new InternalItemsQuery
         {
             IncludeItemTypes = [BaseItemKind.MusicArtist],
-            IsDeadArtist = true,
             IsLocked = false
-        }).Cast<MusicArtist>()
-        .Where(item => item.IsAccessedByName)
+        }).OfType<MusicArtist>()
+        .Where(item => item.IsAccessedByName && !liveIds.Contains(item.Id))
         .ToList();
 
         foreach (var item in deadEntities)

Emby.Server.Implementations/Library/Validators/PeopleValidator.cs

@@ -1,12 +1,12 @@
 using System;
+using System.Collections.Generic;
+using System.Globalization;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Jellyfin.Data.Enums;
 using MediaBrowser.Controller.Entities;
 using MediaBrowser.Controller.Library;
-using MediaBrowser.Controller.Providers;
-using MediaBrowser.Model.IO;
 using Microsoft.Extensions.Logging;
 
 namespace Emby.Server.Implementations.Library.Validators;
@@ -17,112 +17,143 @@ namespace Emby.Server.Implementations.Library.Validators;
 public class PeopleValidator
 {
     /// <summary>
-    /// The _library manager.
+    /// The library manager.
     /// </summary>
     private readonly ILibraryManager _libraryManager;
 
     /// <summary>
-    /// The _logger.
+    /// The logger.
     /// </summary>
-    private readonly ILogger _logger;
-
-    private readonly IFileSystem _fileSystem;
+    private readonly ILogger<PeopleValidator> _logger;
 
     /// <summary>
     /// Initializes a new instance of the <see cref="PeopleValidator" /> class.
     /// </summary>
     /// <param name="libraryManager">The library manager.</param>
     /// <param name="logger">The logger.</param>
-    /// <param name="fileSystem">The file system.</param>
-    public PeopleValidator(ILibraryManager libraryManager, ILogger logger, IFileSystem fileSystem)
+    public PeopleValidator(ILibraryManager libraryManager, ILogger<PeopleValidator> logger)
     {
         _libraryManager = libraryManager;
         _logger = logger;
-        _fileSystem = fileSystem;
     }
 
     /// <summary>
     /// Validates the people.
     /// </summary>
-    /// <param name="cancellationToken">The cancellation token.</param>
     /// <param name="progress">The progress.</param>
+    /// <param name="cancellationToken">The cancellation token.</param>
     /// <returns>Task.</returns>
-    public async Task ValidatePeople(CancellationToken cancellationToken, IProgress<double> progress)
+    public async Task Run(IProgress<double> progress, CancellationToken cancellationToken)
     {
         // Before the refresh below walks them: a credit no item maps to any more stands for nothing,
         // and while it is there the person it names cannot reach the dead-person sweep either.
         var numOrphaned = _libraryManager.DeleteOrphanedCredits();
         if (numOrphaned > 0)
         {
-            _logger.LogDebug("Deleted {Amount} credits no item maps to", numOrphaned);
+            _logger.LogInformation("Deleted {Amount} credits no item maps to", numOrphaned);
         }
 
-        var people = _libraryManager.GetPeopleNames(new InternalPeopleQuery());
+        var names = _libraryManager.GetPeopleNames(new InternalPeopleQuery());
+        var existingPersonIds = _libraryManager.GetItemIds(new InternalItemsQuery
+        {
+            IncludeItemTypes = [BaseItemKind.Person]
+        }).ToHashSet();
+
+        var (newNames, deadIds) = PartitionCreditsByPersonId(names, _libraryManager.GetPersonId, existingPersonIds);
 
         var numComplete = 0;
+        var count = names.Count;
+        var refreshed = 0;
 
-        var numPeople = people.Count;
-
-        IProgress<double> subProgress = new Progress<double>((val) => progress.Report(val / 2));
-
-        _logger.LogDebug("Will refresh {Amount} people", numPeople);
-
-        foreach (var person in people)
+        foreach (var name in names)
         {
             cancellationToken.ThrowIfCancellationRequested();
 
             try
             {
-                var item = _libraryManager.GetPerson(person);
-                if (item is null)
+                var item = _libraryManager.GetOrCreatePerson(name);
+                var isNew = !existingPersonIds.Contains(item.Id);
+                var neverRefreshed = item.DateLastRefreshed == default;
+
+                if (isNew || neverRefreshed)
                 {
-                    _logger.LogWarning("Failed to get person: {Name}", person);
-                    continue;
+                    await item.RefreshMetadata(cancellationToken).ConfigureAwait(false);
+                    refreshed++;
                 }
-
-                var options = new MetadataRefreshOptions(new DirectoryService(_fileSystem))
-                {
-                    ImageRefreshMode = MetadataRefreshMode.ValidationOnly,
-                    MetadataRefreshMode = MetadataRefreshMode.ValidationOnly
-                };
-
-                await item.RefreshMetadata(options, cancellationToken).ConfigureAwait(false);
             }
             catch (OperationCanceledException)
             {
+                // Don't clutter the log
                 throw;
             }
             catch (Exception ex)
             {
-                _logger.LogError(ex, "Error validating IBN entry {Person}", person);
+                _logger.LogError(ex, "Error refreshing {PersonName}", name);
             }
 
-            // Update progress
             numComplete++;
             double percent = numComplete;
-            percent /= numPeople;
+            percent /= count;
+            percent *= 100;
 
-            subProgress.Report(100 * percent);
+            progress.Report(percent);
         }
 
-        var deadEntities = _libraryManager.GetItemList(new InternalItemsQuery
-        {
-            IncludeItemTypes = [BaseItemKind.Person],
-            IsDeadPerson = true,
-            IsLocked = false
-        });
+        _logger.LogInformation(
+            "Refreshed metadata for {RefreshedCount} people out of {TotalCount} total, {NewCount} of which had no item yet",
+            refreshed,
+            count,
+            newNames.Count);
 
-        subProgress = new Progress<double>((val) => progress.Report((val / 2) + 50));
+        // A person somebody locked is theirs, not ours, however little the library still credits them.
+        var deadEntities = deadIds
+            .Select(_libraryManager.GetItemById)
+            .OfType<Person>()
+            .Where(item => !item.IsLocked)
+            .ToList();
 
-        var i = 0;
-        foreach (var item in deadEntities.Chunk(500))
+        foreach (var item in deadEntities)
         {
-            _libraryManager.DeleteItemsUnsafeFast(item, true);
-            subProgress.Report(100f / deadEntities.Count * (i++ * 100));
+            _logger.LogInformation("Deleting dead {ItemType} {ItemId} {ItemName}", item.GetType().Name, item.Id.ToString("N", CultureInfo.InvariantCulture), item.Name);
         }
 
+        _libraryManager.DeleteItemsUnsafeFast(deadEntities, deleteSourceFiles: true);
+
         progress.Report(100);
+    }
 
-        _logger.LogInformation("People validation complete, deleted {Orphaned} orphaned credits", numOrphaned);
+    /// <summary>
+    /// Splits the person items into the ones a credit still calls for and the ones nothing does.
+    /// </summary>
+    /// <param name="creditNames">Every name credited on an item, from the people table.</param>
+    /// <param name="getPersonId">Maps a credit name to the id its person item has.</param>
+    /// <param name="existingPersonIds">The ids of the person items that exist.</param>
+    /// <returns>The credits needing an item, and the ids of the items nothing credits.</returns>
+    internal static (List<string> NewNames, List<Guid> DeadIds) PartitionCreditsByPersonId(
+        IReadOnlyList<string> creditNames,
+        Func<string, Guid> getPersonId,
+        IReadOnlySet<Guid> existingPersonIds)
+    {
+        ArgumentNullException.ThrowIfNull(creditNames);
+        ArgumentNullException.ThrowIfNull(getPersonId);
+        ArgumentNullException.ThrowIfNull(existingPersonIds);
+
+        var newNames = new List<string>();
+        var liveIds = new HashSet<Guid>();
+
+        foreach (var name in creditNames)
+        {
+            var personId = getPersonId(name);
+
+            // Distinct credit names can normalize onto one id; only the first of them needs an item.
+            if (liveIds.Add(personId) && !existingPersonIds.Contains(personId))
+            {
+                newNames.Add(name);
+            }
+        }
+
+        var deadIds = existingPersonIds.Where(id => !liveIds.Contains(id)).ToList();
+
+        return (newNames, deadIds);
     }
 }

Emby.Server.Implementations/ScheduledTasks/Tasks/PeopleValidationTask.cs

@@ -4,6 +4,7 @@ using System.Collections.Generic;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
+using Emby.Server.Implementations.Library.Validators;
 using Jellyfin.Data.Enums;
 using Jellyfin.Database.Implementations;
 using Jellyfin.Database.Implementations.Entities;
@@ -29,6 +30,7 @@ public class PeopleValidationTask : IScheduledTask, IConfigurableScheduledTask
     private readonly IDbContextFactory<JellyfinDbContext> _dbContextFactory;
     private readonly IFileSystem _fileSystem;
     private readonly ILogger<PeopleValidationTask> _logger;
+    private readonly ILogger<PeopleValidator> _validatorLogger;
     private readonly IItemTypeLookup _itemTypeLookup;
 
     /// <summary>
@@ -39,6 +41,7 @@ public class PeopleValidationTask : IScheduledTask, IConfigurableScheduledTask
     /// <param name="dbContextFactory">Instance of the <see cref="IDbContextFactory{TContext}"/> interface.</param>
     /// <param name="fileSystem">Instance of the <see cref="IFileSystem"/> interface.</param>
     /// <param name="logger">Instance of the <see cref="ILogger{PeopleValidationTask}"/> interface.</param>
+    /// <param name="validatorLogger">Instance of the <see cref="ILogger{PeopleValidator}"/> interface.</param>
     /// <param name="itemTypeLookup">Instance of the <see cref="IItemTypeLookup"/> interface.</param>
     public PeopleValidationTask(
         ILibraryManager libraryManager,
@@ -46,6 +49,7 @@ public class PeopleValidationTask : IScheduledTask, IConfigurableScheduledTask
         IDbContextFactory<JellyfinDbContext> dbContextFactory,
         IFileSystem fileSystem,
         ILogger<PeopleValidationTask> logger,
+        ILogger<PeopleValidator> validatorLogger,
         IItemTypeLookup itemTypeLookup)
     {
         _libraryManager = libraryManager;
@@ -53,6 +57,7 @@ public class PeopleValidationTask : IScheduledTask, IConfigurableScheduledTask
         _dbContextFactory = dbContextFactory;
         _fileSystem = fileSystem;
         _logger = logger;
+        _validatorLogger = validatorLogger;
         _itemTypeLookup = itemTypeLookup;
     }
 
@@ -165,7 +170,9 @@ public class PeopleValidationTask : IScheduledTask, IConfigurableScheduledTask
         // Phase 2: Validate people (33-66%). Runs after orphaned PeopleBaseItemMap entries are
         // cleaned up above, so dead people are removed in a single pass instead of requiring a second run.
         IProgress<double> validateProgress = new Progress<double>((val) => progress.Report((val / 3) + 33));
-        await _libraryManager.ValidatePeopleAsync(validateProgress, cancellationToken).ConfigureAwait(false);
+        await new PeopleValidator(_libraryManager, _validatorLogger)
+            .Run(validateProgress, cancellationToken)
+            .ConfigureAwait(false);
 
         // Phase 3: Refresh images for people missing them (66-100%)
         IProgress<double> refreshProgress = new Progress<double>((val) => progress.Report((val / 3) + 66));

MediaBrowser.Controller/Entities/InternalItemsQuery.cs

@@ -432,12 +432,18 @@ namespace MediaBrowser.Controller.Entities
 
         public string? HasNoSubtitleTrackWithLanguage { get; set; }
 
+        /// <summary>
+        /// Gets or sets a value indicating whether to return only items nothing names any more.
+        /// </summary>
         public bool? IsDeadArtist { get; set; }
 
         public bool? IsDeadStudio { get; set; }
 
         public bool? IsDeadGenre { get; set; }
 
+        /// <summary>
+        /// Gets or sets a value indicating whether to return only items nothing names any more.
+        /// </summary>
         public bool? IsDeadPerson { get; set; }
 
         /// <summary>

MediaBrowser.Controller/Library/ILibraryManager.cs

@@ -106,6 +106,13 @@ namespace MediaBrowser.Controller.Library
         /// <returns>Task{Person}.</returns>
         Person? GetPerson(string name);
 
+        /// <summary>
+        /// Gets a Person, creating and persisting it if no item exists for the name yet.
+        /// </summary>
+        /// <param name="name">The name of the person.</param>
+        /// <returns>The person.</returns>
+        Person GetOrCreatePerson(string name);
+
         /// <summary>
         /// Finds the by path.
         /// </summary>
@@ -152,15 +159,6 @@ namespace MediaBrowser.Controller.Library
         /// <exception cref="ArgumentOutOfRangeException">Throws if year is invalid.</exception>
         Year GetYear(int value);
 
-        /// <summary>
-        /// Validate and refresh the People sub-set of the IBN.
-        /// The items are stored in the db but not loaded into memory until actually requested by an operation.
-        /// </summary>
-        /// <param name="progress">The progress.</param>
-        /// <param name="cancellationToken">The cancellation token.</param>
-        /// <returns>Task.</returns>
-        Task ValidatePeopleAsync(IProgress<double> progress, CancellationToken cancellationToken);
-
         /// <summary>
         /// Reloads the root media folder.
         /// </summary>
@@ -709,6 +707,14 @@ namespace MediaBrowser.Controller.Library
         /// <returns><c>true</c> if ignored, <c>false</c> otherwise.</returns>
         bool IgnoreFile(FileSystemMetadata file, BaseItem parent);
 
+        /// <summary>
+        /// Gets the id a <see cref="Person"/> item for the name would have, without looking it up
+        /// or creating it.
+        /// </summary>
+        /// <param name="name">The name of the person.</param>
+        /// <returns>The item id for the name.</returns>
+        Guid GetPersonId(string name);
+
         Guid GetStudioId(string name);
 
         Guid GetGenreId(string name);

tests/Jellyfin.Server.Implementations.Tests/Library/PeopleValidatorPartitionTests.cs

@@ -0,0 +1,117 @@
+using System;
+using System.Collections.Generic;
+using Emby.Server.Implementations.Library.Validators;
+using Xunit;
+
+namespace Jellyfin.Server.Implementations.Tests.Library;
+
+/// <summary>
+/// Tests for how the people validator decides which credits need a person item and which person items
+/// nothing credits any more. Keying either half on the item's name rather than its id put the two halves
+/// in a loop that created, refreshed and deleted the same people on every run, so these pin the id.
+/// </summary>
+public class PeopleValidatorPartitionTests
+{
+    // Stands in for the real item-by-name id: derived from the credit name, case-insensitively, and
+    // from nothing else. The property that matters is that it does not depend on the item's own name.
+    private static Guid PersonId(string creditName)
+    {
+#pragma warning disable CA5351 // Do Not Use Broken Cryptographic Algorithms
+        var hash = System.Security.Cryptography.MD5.HashData(
+            System.Text.Encoding.Unicode.GetBytes(creditName.ToLowerInvariant()));
+#pragma warning restore CA5351 // Do Not Use Broken Cryptographic Algorithms
+        return new Guid(hash);
+    }
+
+    [Fact]
+    public void PartitionCreditsByPersonId_ProviderRenamedThePerson_KeepsThemAndCreatesNothing()
+    {
+        // The credit still says "AURORA"; the item it made has been renamed to "Aurora" by the provider
+        // that refreshed it. Nothing about the library changed, so nothing should be created or deleted.
+        var credits = new[] { "AURORA" };
+        var existing = new HashSet<Guid> { PersonId("AURORA") };
+
+        var (newNames, deadIds) = PeopleValidator.PartitionCreditsByPersonId(credits, PersonId, existing);
+
+        Assert.Empty(newNames);
+        Assert.Empty(deadIds);
+    }
+
+    [Theory]
+    // Every shape of rename seen in the wild on a real library.
+    [InlineData("AURORA")]
+    [InlineData("Amir AboulEla")]
+    [InlineData("Miguel Ángel Fuentes")]
+    [InlineData("a‐ha")]
+    [InlineData("윤현민")]
+    public void PartitionCreditsByPersonId_CreditWithAnItem_IsNeverBothCreatedAndDeleted(string creditName)
+    {
+        var existing = new HashSet<Guid> { PersonId(creditName) };
+
+        var (newNames, deadIds) = PeopleValidator.PartitionCreditsByPersonId([creditName], PersonId, existing);
+
+        Assert.Empty(newNames);
+        Assert.Empty(deadIds);
+    }
+
+    [Fact]
+    public void PartitionCreditsByPersonId_CreditWithNoItem_IsCreated()
+    {
+        var (newNames, deadIds) = PeopleValidator.PartitionCreditsByPersonId(
+            ["Wanted Person"],
+            PersonId,
+            new HashSet<Guid>());
+
+        Assert.Equal(["Wanted Person"], newNames);
+        Assert.Empty(deadIds);
+    }
+
+    [Fact]
+    public void PartitionCreditsByPersonId_ItemNoCreditNames_IsDead()
+    {
+        var orphan = PersonId("Nobody Credits Me");
+        var existing = new HashSet<Guid> { PersonId("Credited"), orphan };
+
+        var (newNames, deadIds) = PeopleValidator.PartitionCreditsByPersonId(["Credited"], PersonId, existing);
+
+        Assert.Empty(newNames);
+        Assert.Equal([orphan], deadIds);
+    }
+
+    [Fact]
+    public void PartitionCreditsByPersonId_CreditsNormalizingOntoOneId_CreateOneItem()
+    {
+        // "AURORA" and "Aurora" are one person as far as the item-by-name id is concerned, so exactly
+        // one of them should create the item and neither should end up dead.
+        var (newNames, deadIds) = PeopleValidator.PartitionCreditsByPersonId(
+            ["AURORA", "Aurora", "aurora"],
+            PersonId,
+            new HashSet<Guid>());
+
+        Assert.Single(newNames);
+        Assert.Empty(deadIds);
+    }
+
+    [Fact]
+    public void PartitionCreditsByPersonId_SecondRunAfterCreating_AsksForNothingFurther()
+    {
+        // The churn showed up as a run that never settled, so drive two rounds: whatever round one
+        // created must leave round two with nothing to do.
+        string[] credits = ["AURORA", "Amir AboulEla", "Miguel Ángel Fuentes"];
+        var existing = new HashSet<Guid>();
+
+        var (firstNames, firstDead) = PeopleValidator.PartitionCreditsByPersonId(credits, PersonId, existing);
+        Assert.Equal(3, firstNames.Count);
+        Assert.Empty(firstDead);
+
+        foreach (var created in firstNames)
+        {
+            existing.Add(PersonId(created));
+        }
+
+        var (secondNames, secondDead) = PeopleValidator.PartitionCreditsByPersonId(credits, PersonId, existing);
+
+        Assert.Empty(secondNames);
+        Assert.Empty(secondDead);
+    }
+}