cleanup

ErsatzTV.Core.Tests/Scheduling/ClassicScheduling/AlternateScheduleAnchorTests.cs

@@ -11,24 +11,16 @@ using Testably.Abstractions.Testing;
 
 namespace ErsatzTV.Core.Tests.Scheduling.ClassicScheduling;
 
-/// <summary>
+// a collection used only by an alternate schedule still gets an enumerator on days that alternate
-///     A collection used only by an alternate schedule still gets an enumerator on days when that
+// isn't active, because media items are gathered once for the primary schedule and every alternate
-///     alternate isn't active, because the collection media items are gathered once for the primary
-///     schedule and every alternate. On those days it falls into the filler branch and gets a
-///     shuffled enumerator, whose index domain is the grouped item count rather than the media item
-///     count.
-/// </summary>
 [TestFixture]
 public class AlternateScheduleAnchorTests : PlayoutBuilderTestBase
 {
     private const int PrimaryCollectionId = 1;
     private const int AlternateCollectionId = 2;
 
-    /// <summary>
+    // season/episode order over 10 episodes indexes 0-9; with a multi-part pair, the shuffled
-    ///     The alternate's collection has 10 episodes and its schedule orders them by season/episode,
+    // enumerator used on inactive days sees 9 groups, so index 9 is outside its domain
-    ///     so its own enumerator's index domain is 0-9. With a multi-part pair the shuffled enumerator
-    ///     used on inactive days groups those 10 items into 9, so index 9 is out of its domain.
-    /// </summary>
     [TestCase(9, true, ExpectedResult = 9, TestName = "Index at the grouped count")]
     [TestCase(8, true, ExpectedResult = 8, TestName = "Index below the grouped count")]
     [TestCase(9, false, ExpectedResult = 9, TestName = "Index at the grouped count, nothing grouped")]
@@ -47,8 +39,7 @@ public class AlternateScheduleAnchorTests : PlayoutBuilderTestBase
                 EnumeratorState = new CollectionEnumeratorState { Seed = 1234, Index = index }
             });
 
-        // a Wednesday, so the alternate (Sundays only) is not active and the alternate's
+        // a Wednesday, so the alternate (Sundays only) is not active
-        // collection is not scheduled at all
         DateTimeOffset now = LocalTime(new DateTime(2025, 6, 11), 20);
         now.DayOfWeek.ShouldBe(DayOfWeek.Wednesday);
 

ErsatzTV.Core.Tests/Scheduling/ClassicScheduling/ContinuePlayoutTests.cs

@@ -468,8 +468,7 @@ public class ContinuePlayoutTests : PlayoutBuilderTestBase
             result.AddedItems.Count.ShouldBe(53);
         }
 
-        // two collections, each with a checkpoint for the two midnights the build crossed
+        // two collections, each with a continue anchor and a checkpoint per midnight crossed
-        // plus a continue anchor
         playout.ProgramScheduleAnchors.Count.ShouldBe(6);
         playout.ProgramScheduleAnchors.Count(x => x.AnchorDate is not null).ShouldBe(4);
         PlayoutProgramScheduleAnchor lastCheckpoint = playout.ProgramScheduleAnchors

ErsatzTV.Core.Tests/Scheduling/ClassicScheduling/MultiDayBuildTests.cs

@@ -11,11 +11,8 @@ using Testably.Abstractions.Testing;
 
 namespace ErsatzTV.Core.Tests.Scheduling.ClassicScheduling;
 
-/// <summary>
+// unit-test version of the "fast forward a playout" debugging endpoint that used to live in
-///     Unit-test version of the "fast forward a playout" debugging endpoint that used to live in
+// ChannelController
-///     ChannelController: run hourly continue builds across several days and assert invariants at
-///     every step.
-/// </summary>
 [TestFixture]
 public class MultiDayBuildTests : PlayoutBuilderTestBase
 {
@@ -25,13 +22,11 @@ public class MultiDayBuildTests : PlayoutBuilderTestBase
     [Test]
     public async Task Continue_Should_Keep_A_Checkpoint_For_The_Current_Day()
     {
-        // 6 hour movies scheduled 3 at a time, so a block spans 18 hours and regularly
+        // 6 hour movies 3 at a time, so an 18 hour block regularly crosses midnight
-        // crosses midnight - which is expected and intended
         (PlayoutBuilder builder, Playout playout, PlayoutReferenceData referenceData) = MultipleTestData(
             TimeSpan.FromHours(6),
             multipleCount: "3");
 
-        // a playout created at 20:00, then built every hour for a week
         DateTimeOffset start = LocalTime(20);
         var missing = new List<string>();
 
@@ -48,15 +43,13 @@ public class MultiDayBuildTests : PlayoutBuilderTestBase
 
             result.IsRight.ShouldBeTrue($"build failed at {now:yyyy-MM-dd HH:mm}");
 
-            // a build never writes a checkpoint for its own first day, and doesn't need to:
+            // no checkpoint is written for a build's own first day, and none is needed: there is no
-            // there is no earlier state to restore, so refreshing starts from index 0 and
+            // earlier state, so refreshing from index 0 reproduces the same schedule
-            // reproduces the same schedule
             if (now.Date == start.Date)
             {
                 continue;
             }
 
-            // from here on, a refresh has to have somewhere to rewind to
             bool hasCheckpointForToday = playout.ProgramScheduleAnchors.Any(a =>
                 a.AnchorDateOffset.HasValue && a.AnchorDateOffset.Value.Date == now.Date);
 
@@ -74,8 +67,7 @@ public class MultiDayBuildTests : PlayoutBuilderTestBase
     [Test]
     public async Task Refresh_Should_Not_Reset_Collection_Progress()
     {
-        // a collection large enough that the index can't wrap over the whole run, so the
+        // large enough that the index can't wrap, so it measures progress directly
-        // index is a straightforward measure of how much progress has been made
         (PlayoutBuilder builder, Playout playout, PlayoutReferenceData referenceData) = MultipleTestData(
             TimeSpan.FromHours(6),
             multipleCount: "3",
@@ -83,8 +75,7 @@ public class MultiDayBuildTests : PlayoutBuilderTestBase
 
         DateTimeOffset start = LocalTime(20);
 
-        // three weeks of hourly builds, so that progress is far enough along to be clearly
+        // long enough that progress is clearly distinguishable from a playout that restarted
-        // distinguishable from a playout that restarted at the beginning
         for (var hour = 0; hour < 24 * 21; hour++)
         {
             await builder.Build(
@@ -107,9 +98,8 @@ public class MultiDayBuildTests : PlayoutBuilderTestBase
 
         result.IsRight.ShouldBeTrue();
 
-        // a refresh rewinds to the start of today, so the index legitimately moves back by
+        // rewinding to the start of today legitimately moves the index back by up to a day; a refresh
-        // up to a day. a refresh that lost its checkpoints restarts at 0 and only advances
+        // that lost its checkpoints lands an order of magnitude lower
-        // through the build window, landing an order of magnitude lower
         int after = HeadIndex(playout);
         after.ShouldBeGreaterThan(
             before / 2,
@@ -119,8 +109,7 @@ public class MultiDayBuildTests : PlayoutBuilderTestBase
     [Test]
     public async Task Continue_Should_Keep_A_Checkpoint_For_Every_Day_In_The_Build_Window()
     {
-        // shaped like the playouts that were missing checkpoints in a real user database: a week
+        // shaped like the playouts that were missing checkpoints in a real user database
-        // long build window, shuffled schedule items, and blocks of five ~100 minute items
         (PlayoutBuilder builder, Playout playout, PlayoutReferenceData referenceData) = MultipleTestData(
             TimeSpan.FromMinutes(100),
             multipleCount: "5",
@@ -145,9 +134,7 @@ public class MultiDayBuildTests : PlayoutBuilderTestBase
 
             result.IsRight.ShouldBeTrue($"build failed at {now:yyyy-MM-dd HH:mm}");
 
-            // every day the playout covers needs a checkpoint, so that a refresh on any of them has
+            // every day covered needs a checkpoint, except the build's own first day; past days are pruned
-            // somewhere to rewind to. the build's own first day is the one exception, and days
-            // already in the past are pruned
             DateTime firstExpected = now.Date > start.Date ? now.Date : start.Date.AddDays(1);
             DateTime lastExpected = now.AddDays(WeekOfDaysToBuild).Date;
 
@@ -263,14 +250,9 @@ public class MultiDayBuildTests : PlayoutBuilderTestBase
         return (builder, playout, referenceData);
     }
 
-    /// <summary>
+    // anchor dates are compared in machine-local time, so the test clock has to be local too or day
-    ///     Anchor dates are compared in machine-local time
+    // boundaries won't line up. that means running in whatever zone the machine is in, hence mid-June,
-    ///     (<see cref="PlayoutProgramScheduleAnchor.AnchorDateOffset" />), so the test clock has to be local
+    // where no zone has a DST transition
-    ///     too, or day boundaries won't line up. That means these tests run in whatever zone the machine is
-    ///     in - UTC on CI, something else locally - so the window below is deliberately placed in mid-June,
-    ///     where no zone has a DST transition. Day-boundary behaviour across a DST change is a separate
-    ///     concern and wants its own test.
-    /// </summary>
     private static DateTimeOffset LocalTime(int hour)
     {
         var date = new DateTime(2025, 6, 10, 0, 0, 0, DateTimeKind.Unspecified);

ErsatzTV.Core.Tests/Scheduling/MarathonEnumeratorTests.cs

@@ -8,11 +8,8 @@ using Shouldly;
 
 namespace ErsatzTV.Core.Tests.Scheduling;
 
-/// <summary>
+// marathon content builds a PlaylistEnumerator over one playlist item per group, so it inherits that
-///     Marathon content builds a <see cref="PlaylistEnumerator" /> over one playlist item per group,
+// enumerator's save/restore behaviour; these cover the state handling, not the grouping
-///     so it inherits that enumerator's save/restore behaviour. These cover the state handling, not
-///     the grouping.
-/// </summary>
 [TestFixture]
 public class MarathonEnumeratorTests
 {
@@ -23,11 +20,7 @@ public class MarathonEnumeratorTests
 
     private CancellationToken _cancellationToken;
 
-    /// <summary>
+    // a cycle completes when every media item has been played, which resets the index to 0
-    ///     A cycle completes when every media item has been played, which resets the index to 0 and
-    ///     reshuffles the groups. Two shows of three episodes each is six items, so the index has to
-    ///     come back to 0 within six moves.
-    /// </summary>
     [Test]
     public async Task Marathon_Should_Complete_A_Cycle()
     {
@@ -44,10 +37,8 @@ public class MarathonEnumeratorTests
         indexes.ShouldContain(0, "the index never returned to 0, so the cycle never completed");
     }
 
-    /// <summary>
+    // season 0 is filtered out by the season/episode enumerator but still counted as an item the cycle
-    ///     Season 0 episodes are filtered out by the season/episode enumerator but still counted as
+    // is waiting on, so the cycle can never complete
-    ///     items the cycle is waiting on, so the cycle can never complete.
-    /// </summary>
     [Test]
     public async Task Marathon_Should_Complete_A_Cycle_With_Specials()
     {
@@ -65,10 +56,8 @@ public class MarathonEnumeratorTests
         indexes.ShouldContain(0, "the index never returned to 0, so the cycle never completed");
     }
 
-    /// <summary>
+    // the flood and duration schedulers hand back a snapshot when an item doesn't fit, so ResetState
-    ///     The flood and duration schedulers snapshot every enumerator's state before trying an item
+    // has to actually rewind
-    ///     and hand it back when the item doesn't fit, so ResetState has to actually rewind.
-    /// </summary>
     [Test]
     public async Task Marathon_Should_Rewind_On_ResetState()
     {
@@ -94,10 +83,6 @@ public class MarathonEnumeratorTests
         enumerator.Current.Map(mi => mi.Id).IfNone(-1).ShouldBe(expected);
     }
 
-    /// <summary>
-    ///     Rebuilding a playout restores the enumerator from the persisted state, which has to land on
-    ///     the same item the previous build stopped at.
-    /// </summary>
     [Test]
     public async Task Marathon_Should_Resume_Where_It_Left_Off()
     {

ErsatzTV.Core.Tests/Scheduling/PlaylistEnumeratorTests.cs

@@ -457,18 +457,14 @@ public class PlaylistEnumeratorTests
         continued.ShouldBe(reference.Skip(6).Take(6));
     }
 
-    /// <summary>
+    // a rebuild only has the persisted seed and index, so a shuffle that depends on anything else
-    ///     Completing a cycle reshuffles the playlist items, and the state persisted at that point is only a
+    // diverges from the playout the previous build was part way through
-    ///     seed and an index. Rebuilding starts from the playlist order and applies the seed, so the shuffle
-    ///     has to be a function of those two things - otherwise the rebuilt playout diverges from the one
-    ///     the previous build was part way through.
-    /// </summary>
     [Test]
     public async Task Shuffled_Playlist_Should_Resume_The_Same_Way_After_A_Cycle()
     {
         IMediaCollectionRepository repo = Substitute.For<IMediaCollectionRepository>();
 
-        // four single-item entries, so a cycle is four moves and each move names its own playlist item
+        // single-item entries, so each move names its own playlist item
         Dictionary<PlaylistItem, List<MediaItem>> BuildMap() =>
             Range(1, 4).ToDictionary(
                 i => new PlaylistItem
@@ -493,7 +489,7 @@ public class PlaylistEnumeratorTests
             randomStartPoint: false,
             CancellationToken.None);
 
-        // play past the end of the first cycle, which reseeds and reshuffles
+        // past the end of the first cycle, which reseeds and reshuffles
         for (var i = 0; i < 6; i++)
         {
             live.MoveNext(Option<DateTimeOffset>.None);

ErsatzTV.Core.Tests/Scheduling/ShuffledContentTests.cs

@@ -134,11 +134,8 @@ public class ShuffledContentTests
         shuffledContent.State.Seed.ShouldNotBe(MagicSeed);
     }
 
-    /// <summary>
+    // MoveNext wraps at the flattened item count, so any index below that is valid state the
-    ///     MoveNext walks (and wraps at) the flattened item count, so an index up to one less than
+    // enumerator produced itself
-    ///     that is valid state. The constructor validates against the group count instead, so any
-    ///     index at or above it is thrown away even though the enumerator itself produced it.
-    /// </summary>
     [Test]
     public void State_Should_Not_Reset_When_Valid_For_Grouped_Items()
     {

ErsatzTV.Core/Scheduling/PlaylistEnumerator.cs

@@ -15,8 +15,7 @@ public class PlaylistEnumerator : IMediaCollectionEnumerator
     private bool _shufflePlaylistItems;
     private List<EnumeratorPlayAllCount> _sortedEnumerators;
 
-    // a cycle start is the only position this enumerator can be put back to directly; every other
+    // a cycle start is the only position that can be restored directly; see RewindToCycleStart
-    // position is reached by replaying from one. see RewindToCycleStart
     private List<EnumeratorPlayAllCount> _enumeratorsInPlaylistOrder;
     private Dictionary<IMediaCollectionEnumerator, CollectionEnumeratorState> _childStatesAtCycleStart;
 
@@ -37,11 +36,7 @@ public class PlaylistEnumerator : IMediaCollectionEnumerator
 
     public int EnumeratorIndex { get; private set; }
 
-    /// <summary>
+    // index and seed don't describe a playlist position, so the only way back to one is to replay to it
-    ///     A playlist position isn't described by index and seed alone - it also lives in
-    ///     <see cref="EnumeratorIndex" />, the batch progress, the ids the cycle still owes and the state of
-    ///     every child enumerator - so the only way back to a position is to replay to it.
-    /// </summary>
     public void ResetState(CollectionEnumeratorState state)
     {
         // nothing has moved since this state was handed out, so there is nothing to rewind
@@ -164,8 +159,7 @@ public class PlaylistEnumerator : IMediaCollectionEnumerator
         {
             List<MediaItem> items = playlistItemMap[playlistItem];
 
-            // a cycle ends when every item has been played, so an item that can never be played would
+            // an item that can never play would leave the cycle permanently incomplete
-            // leave the cycle permanently incomplete; season, episode order won't play season 0
             if (playlistItem.PlaybackOrder is PlaybackOrder.SeasonEpisode)
             {
                 items = SeasonEpisodeMediaCollectionEnumerator.Playable(items);
@@ -226,8 +220,7 @@ public class PlaylistEnumerator : IMediaCollectionEnumerator
                         }
 
                         enumerator = new SeasonEpisodeMediaCollectionEnumerator(items, initState);
-                        // every season 0 episode was already removed above, so this is a playlist item
+                        // season 0 is already gone, so this item had nothing else in it
-                        // that had nothing else in it
                         if (enumerator.Count == 0)
                         {
                             enumerator = null;
@@ -309,10 +302,7 @@ public class PlaylistEnumerator : IMediaCollectionEnumerator
         }
     }
 
-    /// <summary>
+    // must leave exactly what Create leaves for this seed, or replaying from here won't match a rebuild
-    ///     Puts everything back the way <see cref="Create" /> leaves it for the given seed: children at their
-    ///     starting positions, the playlist items back in seed order, and the cycle owing every item again.
-    /// </summary>
     private void RewindToCycleStart(int seed)
     {
         foreach ((IMediaCollectionEnumerator enumerator, CollectionEnumeratorState childState) in
@@ -358,11 +348,8 @@ public class PlaylistEnumerator : IMediaCollectionEnumerator
         }
     }
 
-    /// <summary>
+    // shuffles playlist order rather than current order, so the result depends only on that order and the
-    ///     Always shuffles the playlist order rather than the current order, so that the result is a function
+    // seed - all a rebuild has to work from
-    ///     of that order and the seed - which is all the persisted state gives a rebuild to work from. Cycles
-    ///     reshuffle from the same starting point rather than from wherever the last shuffle landed.
-    /// </summary>
     private List<EnumeratorPlayAllCount> ShufflePlaylistItems()
     {
         if (_enumeratorsInPlaylistOrder.Count < 3)

ErsatzTV.Core/Scheduling/PlayoutBuilder.cs

@@ -169,9 +169,8 @@ public class PlayoutBuilder : IPlayoutBuilder
 
         // _logger.LogDebug("All anchors: {@Anchors}", playout.ProgramScheduleAnchors);
 
-        // remove the "continue" anchors (null anchor date), which hold the state at the head of
+        // these have to go rather than just be ignored; GetMediaCollectionEnumerator ranks a null
-        // the playout; they have to go rather than just be ignored, because GetMediaCollectionEnumerator
+        // anchor date above every checkpoint, and we want to rewind to today's checkpoint
-        // ranks a null anchor date above every checkpoint and we want to rewind to today's checkpoint
         playout.ProgramScheduleAnchors.RemoveAll(a => a.AnchorDate is null);
 
         // _logger.LogDebug("Checkpoint anchors: {@Anchors}", playout.ProgramScheduleAnchors);
@@ -457,8 +456,8 @@ public class PlayoutBuilder : IPlayoutBuilder
             return result;
         }
 
-        // build one whole day at a time; this is also how alternate schedules switch per day,
+        // one whole day at a time; this is also how alternate schedules switch, since the active
-        // since the active schedule is selected once per pass
+        // schedule is selected once per pass
         while (finish < playoutFinish)
         {
             if (cancellationToken.IsCancellationRequested)
@@ -497,7 +496,6 @@ public class PlayoutBuilder : IPlayoutBuilder
 
         if (start < playoutFinish)
         {
-            // build the remaining partial day
             _logger.LogDebug("Building final playout from {Start} to {Finish}", start, playoutFinish);
             Either<BaseError, PlayoutBuildResult> buildResult = await BuildPlayoutItemsForPass(
                 playout,
@@ -901,10 +899,8 @@ public class PlayoutBuilder : IPlayoutBuilder
 
             result.AddedItems.AddRange(playoutItems);
 
-            // save a checkpoint anchor whenever scheduling crosses a local midnight; this has to
+            // has to happen here rather than at the end of a pass; pass boundaries only aspirationally
-            // happen here rather than at the end of a pass, because pass boundaries only
+            // line up with midnights, and the final partial pass never ends on one
-            // aspirationally line up with midnights - the final partial pass never ends on one, and
-            // a long item can carry a whole-day pass well past its own boundary
             if (nextState.CurrentTime.ToLocalTime().Date > playoutBuilderState.CurrentTime.ToLocalTime().Date)
             {
                 SaveCheckpointAnchors(playout, collectionEnumerators, nextState.CurrentTime);
@@ -1212,11 +1208,7 @@ public class PlayoutBuilder : IPlayoutBuilder
         && anchor.PlaylistId == collectionKey.PlaylistId
         && anchor.SearchQuery == collectionKey.SearchQuery;
 
-    /// <summary>
+    // refresh rewinds to the checkpoint dated today, so every local day of the playout needs one
-    ///     Records the collection progress as of <paramref name="checkpointTime" />, which is the first
-    ///     resumable point at or after a local midnight. Refresh rewinds to the checkpoint dated today,
-    ///     so every local day of the playout needs one.
-    /// </summary>
     private static void SaveCheckpointAnchors(
         Playout playout,
         Dictionary<CollectionKey, IMediaCollectionEnumerator> collectionEnumerators,
@@ -1227,15 +1219,13 @@ public class PlayoutBuilder : IPlayoutBuilder
 
         foreach ((CollectionKey collectionKey, IMediaCollectionEnumerator enumerator) in collectionEnumerators)
         {
-            // one checkpoint per collection key per local date; rebuilding a day that already has a
+            // rebuilding a day replaces its checkpoint; the new state is the one matching the new items
-            // checkpoint replaces it, since the new state is the one that matches the new items
             Option<PlayoutProgramScheduleAnchor> maybeExisting = playout.ProgramScheduleAnchors.FirstOrDefault(a =>
                 AnchorMatchesCollectionKey(a, collectionKey)
                 && a.AnchorDateOffset.HasValue
                 && a.AnchorDateOffset.Value.Date == checkpointDate);
 
-            // the enumerator keeps mutating its own state as the build continues, so the checkpoint
+            // the enumerator keeps mutating its own state as the build continues
-            // needs a copy of it, frozen at this instant
             CollectionEnumeratorState state = enumerator.State.Clone();
 
             foreach (PlayoutProgramScheduleAnchor existing in maybeExisting)

ErsatzTV.Core/Scheduling/SeasonEpisodeMediaCollectionEnumerator.cs

@@ -34,10 +34,7 @@ public sealed class SeasonEpisodeMediaCollectionEnumerator : IMediaCollectionEnu
         }
     }
 
-    /// <summary>
+    // shared with callers that need to know what this will play, not what they handed it
-    ///     Season 0 is never played in season, episode order. Callers that need to know which items this
-    ///     enumerator will actually play - rather than which items they handed it - share the rule here.
-    /// </summary>
     public static List<MediaItem> Playable(IEnumerable<MediaItem> mediaItems) =>
         mediaItems.Filter(mi => (mi is not Episode episode) || (episode.Season?.SeasonNumber ?? 0) > 0).ToList();