Show how far accept all has got

claude.md

@@ -152,6 +152,18 @@ apart.
   Whether an entry is hidden is always read back out of `VisibleEntries`, never recomputed — the
   rules about when a header exists at all live in one place and must stay there. A fold is a view:
   `AcceptAll` still sweeps what it hides, which `CollapseTests` pins.
+- Accept-all goes an entry at a time, because it takes as long as the queue is long.
+  `ViewerSession.BeginAcceptAll` records an `AcceptBatch`, and `AcceptAllRunner` claims an entry
+  under `SessionHost`'s lock (`ClaimNext`), applies it outside (`ApplyClaimed`), and records it
+  under the lock again - snapshots before files, since whether a delete is held turns on how the
+  snapshots went. The render loop takes that lock every frame, so one transition over the queue
+  froze the window for the whole batch. A window's batch runs on a worker, a wire `AcceptAll` on
+  its listener thread, and `ViewerSession.Apply(AcceptAll)` is the same steps back to back, which
+  is what the tests drive. Owners put `AcceptProgress` on their listings - the tray completes each
+  snapshot with `InlineQueue.AcceptInBatch` rather than all at the end - and `OwnerLink.Run` lists
+  beside an in-flight send rather than after it, so an attached window follows the owner's batch.
+  While `SessionState.Progress` is set the status line shows it and the window refuses anything
+  `ChangesQueue` names.
 - Images (`Images/`, extensions in `DiffEngine/Viewer/ImageExtensions.cs`, linked into the viewer so
   the tool registration and the renderer cannot disagree) are a side, not a mode. `FileSide.Read`
   decides text or picture **by extension**, because the expected side of a new snapshot has no bytes

docs/mdsource/tray.source.md

@@ -59,6 +59,8 @@ Exiting the tray writes any still-pending inline snapshots back to disk, under t
 
 "Accept all" will accept all pending moves, deletes and inline snapshots. Snapshots whose target frameworks disagree about the content are skipped rather than picked between; resolve those in the viewer.
 
+A long queue takes a while to accept. An open [DiffEngineViewer](/docs/viewer.md) window shows how far it has got, with each snapshot leaving the list as it lands.
+
 
 ### Locked files
 

docs/mdsource/viewer.source.md

@@ -94,6 +94,8 @@ What lands on the clipboard is what is on screen: tabs already expanded to the f
 
 A test run that fails several inline snapshots produces one window, not several. Whichever process binds the loopback port holds the queue; everything else hands its patch to that one. The window lists everything pending and offers **Accept all**.
 
+**Accept all** takes as long as the queue is long, so it goes one entry at a time. Each entry leaves the list as it lands, the status line says how far it has got (`Accepting 12 of 40`), and the window keeps responding throughout. **Accept**, **Discard** and **Accept all** are disabled until it finishes; scrolling, selecting and copying are not. It is the same when [DiffEngineTray](/docs/tray.md) holds the queue, and when the accept-all was started from the tray's menu: the window follows the tray's progress.
+
 Failing file comparisons join the same queue, so a run that fails ten snapshots opens one window whether they are inline or on disk. Every other diff tool gets a process per pair, and DiffEngine closes each one as its test starts passing; the viewer is told to drop that row instead.
 
 Rows that came from files follow those files. A re-run that rewrites a received file shows the rewrite, a verified file that appears fills in the other pane, and a row whose received file goes away leaves with it — so nothing is offered for a file that is no longer there, however it went. The window closes once the last row does.

docs/tray.md

@@ -66,6 +66,8 @@ Exiting the tray writes any still-pending inline snapshots back to disk, under t
 
 "Accept all" will accept all pending moves, deletes and inline snapshots. Snapshots whose target frameworks disagree about the content are skipped rather than picked between; resolve those in the viewer.
 
+A long queue takes a while to accept. An open [DiffEngineViewer](/docs/viewer.md) window shows how far it has got, with each snapshot leaving the list as it lands.
+
 
 ### Locked files
 

docs/viewer.md

@@ -101,6 +101,8 @@ What lands on the clipboard is what is on screen: tabs already expanded to the f
 
 A test run that fails several inline snapshots produces one window, not several. Whichever process binds the loopback port holds the queue; everything else hands its patch to that one. The window lists everything pending and offers **Accept all**.
 
+**Accept all** takes as long as the queue is long, so it goes one entry at a time. Each entry leaves the list as it lands, the status line says how far it has got (`Accepting 12 of 40`), and the window keeps responding throughout. **Accept**, **Discard** and **Accept all** are disabled until it finishes; scrolling, selecting and copying are not. It is the same when [DiffEngineTray](/docs/tray.md) holds the queue, and when the accept-all was started from the tray's menu: the window follows the tray's progress.
+
 Failing file comparisons join the same queue, so a run that fails ten snapshots opens one window whether they are inline or on disk. Every other diff tool gets a process per pair, and DiffEngine closes each one as its test starts passing; the viewer is told to drop that row instead.
 
 Rows that came from files follow those files. A re-run that rewrites a received file shows the rewrite, a verified file that appears fills in the other pane, and a row whose received file goes away leaves with it — so nothing is offered for a file that is no longer there, however it went. The window closes once the last row does.

src/DiffEngine.Tests/InlineQueueTests.cs

@@ -399,6 +399,52 @@ public async Task ABatchCompletionSkipsANewcomer()
         await Assert.That(message).IsEqualTo("Accepted 2");
     }
 
+    /// <summary>
+    /// A batch completed an entry at a time, the way an owner applying a long queue completes it
+    /// so the queue can be watched shrinking. Each step leaves the queue a listing would show at
+    /// that point, and the steps together say what the whole batch would have.
+    /// </summary>
+    [Test]
+    public async Task ABatchCompletedAnEntryAtATimeShrinksAsItGoes()
+    {
+        var queue = InlineQueue.Empty
+            .Enqueue(Patch("A.cs", 1))
+            .Enqueue(Patch("B.cs", 2))
+            .Enqueue(Patch("C.cs", 3));
+        var pending = queue.Items;
+        var tally = new AcceptAllTally();
+
+        queue = queue.AcceptInBatch(pending[0], InlineApplyResult.Applied, ref tally);
+        await Assert.That(queue.Items.Select(_ => _.Name)).IsEquivalentTo(["B.cs:2", "C.cs:3"]);
+
+        queue = queue.AcceptInBatch(pending[1], InlineApplyResult.NotFound("no Verify or Throws call"), ref tally);
+        queue = queue.AcceptInBatch(pending[2], InlineApplyResult.Applied, ref tally);
+
+        // The stale one stays, as it does out of a whole batch
+        await Assert.That(queue.Items.Single().Status).IsEqualTo("B.cs:2 not written. no Verify or Throws call");
+        await Assert.That(tally.Refused).IsTrue();
+        await Assert.That(tally.Message(queue.Conflicts))
+            .IsEqualTo("Accepted 2, 1 not written. B.cs:2 not written. no Verify or Throws call");
+    }
+
+    /// <summary>
+    /// A re-run that replaced the entry while its patch applied keeps its new content, and the
+    /// batch does not count an outcome that describes content no longer pending.
+    /// </summary>
+    [Test]
+    public async Task ABatchStepSkipsAnEntryReplacedWhileItApplied()
+    {
+        var queue = InlineQueue.Empty.Enqueue(Patch(content: "first"));
+        var entry = queue.Items.Single();
+        queue = queue.Enqueue(Patch(content: "second"));
+        var tally = new AcceptAllTally();
+
+        var after = queue.AcceptInBatch(entry, InlineApplyResult.Applied, ref tally);
+
+        await Assert.That(after).IsSameReferenceAs(queue);
+        await Assert.That(tally).IsEqualTo(new AcceptAllTally());
+    }
+
     [Test]
     public async Task DiscardRemovesWithoutApplying()
     {

src/DiffEngine.Tests/ViewerProtocolTests.cs

@@ -608,6 +608,56 @@ public async Task AListingWithNoWindowCommandSaysNothing()
         await Assert.That(parsed!.Window).IsNull();
     }
 
+    /// <summary>
+    /// How a viewer displaying someone else's queue learns how far the owner's accept-all has got:
+    /// on a listing taken while it runs, since the accept itself is one exchange that answers only
+    /// once the batch is done.
+    /// </summary>
+    [Test]
+    public async Task AcceptProgressRidesOnAListing()
+    {
+        var text = ViewerResponse.Listing([], progress: new(3, 40)).Build();
+
+        await Assert.That(text).Contains("progress: 3|40\n");
+        await Assert.That(ViewerResponse.TryParse(text, out var parsed)).IsTrue();
+        await Assert.That(parsed!.Progress).IsEqualTo(new AcceptProgress(3, 40));
+    }
+
+    [Test]
+    public async Task AListingWithNoAcceptRunningSaysNothingOfProgress()
+    {
+        var text = ViewerResponse.Listing([]).Build();
+
+        await Assert.That(text).DoesNotContain("progress:");
+        await Assert.That(ViewerResponse.TryParse(text, out var parsed)).IsTrue();
+        await Assert.That(parsed!.Progress).IsNull();
+    }
+
+    /// <summary>
+    /// A progress line is only a count, so one that does not parse is a response that does not,
+    /// the way a malformed move line is.
+    /// </summary>
+    [Test]
+    public async Task AMalformedProgressLineRejectsTheResponse()
+    {
+        var text = ViewerResponse.Listing([], progress: new(3, 40)).Build()
+            .Replace("progress: 3|40\n", "progress: 3\n");
+
+        await Assert.That(ViewerResponse.TryParse(text, out _)).IsFalse();
+    }
+
+    /// <summary>
+    /// The entry being worked on rather than the count finished: the first is "1 of 40" while it
+    /// is applying, and the last is never "41 of 40".
+    /// </summary>
+    [Test]
+    public async Task ProgressNamesTheEntryInHand()
+    {
+        await Assert.That(new AcceptProgress(0, 40).Describe()).IsEqualTo("Accepting 1 of 40");
+        await Assert.That(new AcceptProgress(39, 40).Describe()).IsEqualTo("Accepting 40 of 40");
+        await Assert.That(new AcceptProgress(40, 40).Describe()).IsEqualTo("Accepting 40 of 40");
+    }
+
     /// <summary>
     /// The client's three second default is what a real caller uses to decide the owner has died.
     /// The tests below are about what the owner answers rather than how fast, and CI starts six

src/DiffEngine/Inline/AcceptAllTally.cs

@@ -0,0 +1,27 @@
+namespace DiffEngine;
+
+/// <summary>
+/// How a bulk accept has gone so far, counted an entry at a time.
+/// <para>
+/// Carried between the entries of a batch that completes each one as it lands, rather than all of
+/// them at the end, so what the batch reports once it is done is the sentence
+/// <see cref="InlineQueue.AcceptAllMessage"/> has always built for a bulk accept. A value rather
+/// than something the batch updates in place, because the viewer keeps it in a session state that
+/// is immutable.
+/// </para>
+/// </summary>
+readonly record struct AcceptAllTally(int Accepted, int NotWritten, int Failed, string? Failure)
+{
+    /// <summary>
+    /// A patch in this batch that was not written. What holds a sweep's pending deletes, since one
+    /// of them may be the only copy left of a snapshot that never made it into the source.
+    /// </summary>
+    public bool Refused => NotWritten + Failed > 0;
+
+    /// <summary>
+    /// Conflicted entries are counted by the caller at the end rather than here, because a batch
+    /// never applies one: they are whatever the queue still holds with more than one variant.
+    /// </summary>
+    public string Message(int conflicted) =>
+        InlineQueue.AcceptAllMessage(Accepted, NotWritten, Failed, conflicted, Failure);
+}

src/DiffEngine/Inline/InlineQueue.cs

@@ -464,55 +464,82 @@ public InlineQueue AcceptAll(
         IReadOnlyList<(PendingInline Entry, InlineApplyResult Result)> outcomes,
         out string message)
     {
-        var remaining = new List<PendingInline>();
-        var accepted = 0;
-        var notWritten = 0;
-        var failed = 0;
-        var conflicted = 0;
-        string? failure = null;
-        foreach (var entry in Items)
+        var queue = this;
+        var tally = new AcceptAllTally();
+        foreach (var (entry, result) in outcomes)
         {
-            var outcome = outcomes.FirstOrDefault(_ => ReferenceEquals(_.Entry.Variants, entry.Variants));
-            if (outcome.Entry is null)
-            {
-                if (entry.Conflicted)
-                {
-                    conflicted++;
-                }
+            queue = queue.AcceptInBatch(entry, result, ref tally);
+        }
 
-                remaining.Add(entry);
-                continue;
-            }
+        message = tally.Message(queue.Conflicts);
+        return queue;
+    }
 
-            var (removed, stale, text) = Outcome(entry, outcome.Result);
-            // Dropped on its own, an entry the reader was watching and got an answer about. Dropped
-            // out of a batch of thirty, an entry nobody saw go: no literal written, nothing left in
-            // the queue to say so, and a count of accepts that included it. So it stays, carrying
-            // what the applier said, the way every other unwritten snapshot in the batch does. A
-            // re-run brings the patch back and the arrival clears the status.
-            if (stale)
-            {
-                notWritten++;
-                failure = text;
-                remaining.Add(entry with { Status = text });
-                continue;
-            }
+    /// <summary>
+    /// One entry of a bulk accept, completed on its own rather than with the rest of the batch.
+    /// <para>
+    /// A host applying a long queue commits each outcome as it arrives, so whoever is watching the
+    /// queue sees it shrink as the batch goes, and a listing taken partway through says how far it
+    /// has got. Completing everything at the end left a window showing an untouched queue for as
+    /// long as the batch took, and then emptying all at once.
+    /// </para>
+    /// <para>
+    /// The rules are the batch's rather than a single accept's, because the batch completion above
+    /// is this, once per outcome. An entry that changed while its patch was applying is left alone
+    /// and not counted, found by its variants the way the two phase accept finds it.
+    /// </para>
+    /// </summary>
+    internal InlineQueue AcceptInBatch(PendingInline entry, InlineApplyResult result, ref AcceptAllTally tally)
+    {
+        var items = Items.ToList();
+        var index = items.FindIndex(_ => ReferenceEquals(_.Variants, entry.Variants));
+        if (index < 0)
+        {
+            return this;
+        }
 
-            if (removed)
+        var current = items[index];
+        var (removed, stale, text) = Outcome(current, result);
+        // Dropped on its own, an entry the reader was watching and got an answer about. Dropped
+        // out of a batch of thirty, an entry nobody saw go: no literal written, nothing left in
+        // the queue to say so, and a count of accepts that included it. So it stays, carrying
+        // what the applier said, the way every other unwritten snapshot in the batch does. A
+        // re-run brings the patch back and the arrival clears the status.
+        if (stale)
+        {
+            tally = tally with
             {
-                accepted++;
-                continue;
-            }
-
-            failed++;
-            failure = text;
-            remaining.Add(entry with { Status = text });
+                NotWritten = tally.NotWritten + 1,
+                Failure = text
+            };
+            items[index] = current with { Status = text };
+        }
+        else if (removed)
+        {
+            tally = tally with { Accepted = tally.Accepted + 1 };
+            items.RemoveAt(index);
+        }
+        else
+        {
+            tally = tally with
+            {
+                Failed = tally.Failed + 1,
+                Failure = text
+            };
+            items[index] = current with { Status = text };
         }
 
-        message = AcceptAllMessage(accepted, notWritten, failed, conflicted, failure);
-        return new(remaining);
+        return new(items);
     }
 
+    /// <summary>
+    /// What a bulk accept that has finished counts as still needing review: it never applies an
+    /// entry with more than one variant, so whatever the queue holds of those is left for a
+    /// reviewer to pick from.
+    /// </summary>
+    internal int Conflicts =>
+        Items.Count(_ => _.Conflicted);
+
     public PendingInline? Find(string key) =>
         Items.FirstOrDefault(_ => _.Key == key);
 

src/DiffEngine/Protocol/AcceptProgress.cs

@@ -0,0 +1,51 @@
+namespace DiffEngine;
+
+/// <summary>
+/// How far an accept-all has got: <paramref name="Done"/> of the <paramref name="Total"/> entries
+/// it set out to deal with, whichever way each of them went.
+/// <para>
+/// Accepting a long queue takes as long as the queue is long - every snapshot is a read, a parse
+/// and a write under a cross process mutex, and every move can be retried for seconds while a diff
+/// tool lets go of it - and nothing said so. A window that owned the queue froze for the whole
+/// batch, and one displaying someone else's said "Waiting for the queue owner." over a list that
+/// did not move until everything went at once.
+/// </para>
+/// <para>
+/// On the wire because the batch runs in whichever process owns the queue and the window may
+/// belong to another one. The owner answers a listing taken partway through with this, and the
+/// displaying viewer says it in the same words the owning one would.
+/// </para>
+/// </summary>
+record AcceptProgress(int Done, int Total)
+{
+    /// <summary>
+    /// The entry being worked on rather than the count finished, which is how a progress line
+    /// reads: the first entry is "1 of 40" while it is being applied, not "0 of 40".
+    /// </summary>
+    public string Describe() =>
+        $"Accepting {Math.Min(Done + 1, Total)} of {Total}";
+
+    /// <summary>
+    /// One more entry dealt with, however it went.
+    /// </summary>
+    public AcceptProgress Advance() =>
+        this with { Done = Done + 1 };
+
+    public string Build() =>
+        $"{Done}|{Total}";
+
+    public static bool TryParse(string value, [NotNullWhen(true)] out AcceptProgress? progress)
+    {
+        progress = null;
+        var parts = value.Split('|');
+        if (parts.Length != 2 ||
+            !int.TryParse(parts[0], out var done) ||
+            !int.TryParse(parts[1], out var total))
+        {
+            return false;
+        }
+
+        progress = new(done, total);
+        return true;
+    }
+}