From 729737895fa27a2f0678dbac592474d73394ecec Mon Sep 17 00:00:00 2001
From: austinried <4966622+austinried@users.noreply.github.com>
Date: Sun, 27 Sep 2026 10:04:56 +0000
Subject: [PATCH] address review: offset fallback for far backward album seeks;
 pin the playlist keyset

- The NULL-aware backward album predicate is not sargable, so a backward seek
  from a far cursor scans the whole album partition. Fall back to the offset
  query when the anchor is more than twice the offset away; near-cursor
  backward reads still use keyset.
- Cover the playlist keyset path: a DAO test for `playlistSongsBefore` and a
  repository test scanning a playlist forward and backward. Both fail if the
  playlist order key stops driving the seek.
---
 .../subtracks/data/repo/QueueRepository.kt    |  5 +++
 .../subtracks/data/repo/QueueKeysetTest.kt    | 42 ++++++++++++++++++-
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt
index bd3b8a12..eca9a3e1 100644
--- a/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt
+++ b/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt
@@ -764,6 +764,11 @@ class QueueRepository(
     ): List<SongItem>? {
         if (!cursor.key.seekable(entry.kind)) return null
         val from = cursor.ordinal
+        // A backward seek from a far cursor is not cheaper than the offset query: the album
+        // predicate is not sargable (it has to include NULL disc/track), so it scans the partition
+        // back to `offset` instead of using a range. Fall back when the anchor is more than twice
+        // the offset away, matching the documented far-jump behaviour.
+        if (from > offset && from - offset >= offset) return null
         return if (offset >= from) {
             rowsFrom(entry, cursor.key, offset - from, limit)
         } else {
diff --git a/app/src/test/kotlin/com/subtracks/data/repo/QueueKeysetTest.kt b/app/src/test/kotlin/com/subtracks/data/repo/QueueKeysetTest.kt
index 17665b25..b1ba328e 100644
--- a/app/src/test/kotlin/com/subtracks/data/repo/QueueKeysetTest.kt
+++ b/app/src/test/kotlin/com/subtracks/data/repo/QueueKeysetTest.kt
@@ -120,6 +120,44 @@ class QueueKeysetTest {
             assertEquals(listOf("n2", "s1"), actual)
         }
 
+    @Test
+    fun playlistKeysetBeforeMatchesTheOffsetWindow() =
+        runTest {
+            seedLibrary()
+            val all = allPlaylistRows()
+            val anchor = all[10]
+
+            val expected = all.drop(5).take(5).map { it.song.id }
+            val actual =
+                db
+                    .queueDao()
+                    .playlistSongsBefore(1, "pl1", anchor.position, skip = 0, limit = 5)
+                    .asReversed()
+                    .map { it.song.id }
+
+            assertEquals(expected, actual)
+        }
+
+    @Test
+    fun scanningAPlaylistThroughTheRepositoryUsesKeysetSeeks() =
+        runTest {
+            seedLibrary()
+            repository.replace(listOf(repository.playlistEntry(1, "pl1")))
+            val snapshot = repository.snapshot()
+            val expected = allPlaylistRows().map { it.song.id }
+            assertEquals(expected.size, snapshot.size.toInt())
+
+            val forward = (0 until snapshot.size).map { repository.itemAt(snapshot, it)?.song?.id }
+            assertEquals(expected, forward)
+            assertTrue("forward keyset seeks: ${repository.keysetSeeks}", repository.keysetSeeks >= snapshot.size - 1)
+            assertEquals(1L, repository.offsetSeeks)
+
+            val beforeBackward = repository.keysetSeeks
+            val backward = (snapshot.size - 1 downTo 0).map { repository.itemAt(snapshot, it)?.song?.id }.asReversed()
+            assertEquals(expected, backward)
+            assertTrue("backward keyset seeks", repository.keysetSeeks > beforeBackward)
+        }
+
     @Test
     fun scanningAnAlbumWithNullDiscAndTrackBackwardsResolvesEveryRow() =
         runTest {
@@ -176,7 +214,9 @@ class QueueKeysetTest {
             val resolved =
                 (snapshot.size - 1 downTo 0).map { repository.itemAt(snapshot, it)?.song?.id }.asReversed()
             assertEquals(expected, resolved)
-            assertTrue(repository.keysetSeeks >= snapshot.size - 1)
+            // Most steps are keyset; the first few near the start fall back to OFFSET, which is
+            // cheaper there.
+            assertTrue("backward keyset seeks: ${repository.keysetSeeks}", repository.keysetSeeks >= snapshot.size * 9 / 10)
         }
 
     @Test