From 313745a65301d40367be3f471eb6847f469b75d3 Mon Sep 17 00:00:00 2001
From: Simon Cropp <simon.cropp@gmail.com>
Date: Sat, 22 Aug 2026 23:31:32 +1000
Subject: [PATCH] Do not step over the change block just above the viewport

PreviousChange started one row above the viewport and then stepped off any block it
found there. That row is not in the viewport, so a block ending on it is one the
reader has not been taken to - and it was skipped for the block before it. From row
17 with a change at 16, previous went to 2. From row 3 with a change at 2 it went
nowhere at all, there being nothing before it to fall through to, so the first
block in a file could not be reached from the row below it.

It starts at the top row of the viewport now, the row NextChange starts at, and
steps off a block only when that row is itself a change - which is the case
stepping off was for.
---
 .../PreviousChangeTests.cs                    | 64 +++++++++++++++++++
 src/DiffEngineViewer/ViewerSession.cs         |  8 ++-
 2 files changed, 71 insertions(+), 1 deletion(-)
 create mode 100644 src/DiffEngineViewer.Tests/PreviousChangeTests.cs

diff --git a/src/DiffEngineViewer.Tests/PreviousChangeTests.cs b/src/DiffEngineViewer.Tests/PreviousChangeTests.cs
new file mode 100644
index 00000000..f8c5ef43
--- /dev/null
+++ b/src/DiffEngineViewer.Tests/PreviousChangeTests.cs
@@ -0,0 +1,64 @@
+/// <summary>
+/// Stepping back through the changed blocks of the entry on screen. Forty rows with changes at 3,
+/// 17 and 33 - so rows 2, 16 and 32 - and sixteen body rows to show them in.
+/// </summary>
+public class PreviousChangeTests
+{
+    /// <summary>
+    /// A block ending on the row immediately above the viewport is one the reader has not been
+    /// taken to, and it used to be stepped over as though it were the block they were already in.
+    /// </summary>
+    [Test]
+    public async Task Lands_on_a_block_ending_just_above_the_viewport()
+    {
+        var state = At(17);
+
+        var moved = ViewerSession.Apply(state, CommandKind.PreviousChange);
+
+        await Assert.That(moved.ScrollTop).IsEqualTo(16);
+    }
+
+    /// <summary>
+    /// The same thing with nothing above it to fall through to, where the result was no movement
+    /// at all rather than the wrong movement.
+    /// </summary>
+    [Test]
+    public async Task Reaches_the_first_block_from_the_row_below_it()
+    {
+        var state = At(3);
+
+        var moved = ViewerSession.Apply(state, CommandKind.PreviousChange);
+
+        await Assert.That(moved.ScrollTop).IsEqualTo(2);
+    }
+
+    /// <summary>
+    /// A viewport that really is inside a block still steps off it, or previous would never leave
+    /// the block it is in.
+    /// </summary>
+    [Test]
+    public async Task Steps_off_the_block_the_viewport_is_in()
+    {
+        var state = At(16);
+
+        var moved = ViewerSession.Apply(state, CommandKind.PreviousChange);
+
+        await Assert.That(moved.ScrollTop).IsEqualTo(2);
+    }
+
+    [Test]
+    public async Task Stays_at_the_first_block()
+    {
+        var state = At(2);
+
+        var moved = ViewerSession.Apply(state, CommandKind.PreviousChange);
+
+        await Assert.That(moved.ScrollTop).IsEqualTo(2);
+    }
+
+    static SessionState At(int scrollTop) =>
+        Fixtures.File(Fixtures.Long(true), Fixtures.Long(false)) with
+        {
+            ScrollTop = scrollTop
+        };
+}
diff --git a/src/DiffEngineViewer/ViewerSession.cs b/src/DiffEngineViewer/ViewerSession.cs
index d59969e7..daf70d93 100644
--- a/src/DiffEngineViewer/ViewerSession.cs
+++ b/src/DiffEngineViewer/ViewerSession.cs
@@ -1039,7 +1039,13 @@ static int NextChange(IReadOnlyList<Row> rows, int from)
 
     static int PreviousChange(IReadOnlyList<Row> rows, int from)
     {
-        var index = Math.Min(from, rows.Count) - 1;
+        // The top row of the viewport, not the one above it. Stepping off from there took the
+        // block ending immediately above the viewport for the block the viewport was in, and
+        // skipped past it to the one before - or, with nothing before it, refused to move at all.
+        var index = Math.Min(from, rows.Count - 1);
+
+        // So step off only when the viewport really is sitting in a block, which is when its top
+        // row is itself a change.
         while (index >= 0 &&
                IsChange(rows[index]))
         {