From e2c3f31c26643f59a28aa9fbbeecc8d8cb81cec0 Mon Sep 17 00:00:00 2001
From: austinried <4966622+austinried@users.noreply.github.com>
Date: Sat, 14 Nov 2026 01:47:31 +0000
Subject: [PATCH] ui: serialize queue moves with row updates

---
 .../com/subtracks/ui/playback/QueueScreen.kt  | 18 ++---
 .../ui/playback/QueueViewModelTest.kt         | 71 ++++++++++++++++++-
 2 files changed, 79 insertions(+), 10 deletions(-)

diff --git a/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt b/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt
index d8c0e5de..4a35ee32 100644
--- a/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt
+++ b/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt
@@ -194,13 +194,11 @@ class QueueViewModel(
                 rows.addAll(updated)
             }
         } else {
-            // Only the head of a sparse offline window was resolved; keep the tail on screen,
-            // dropping any tail row whose id was reused for a head row.
             val usedIds = updated.mapTo(HashSet()) { it.id }
-            val tail = rows.filter { it.position > end && it.id !in usedIds }
+            val unresolvedTail = rows.filter { it.id !in usedIds }
             rows.clear()
             rows.addAll(updated)
-            rows.addAll(tail)
+            rows.addAll(unresolvedTail)
         }
     }
 
@@ -307,11 +305,13 @@ class QueueViewModel(
         reorderPending = true
         viewModelScope.launch {
             try {
-                val applied = playbackController.move(from, to, resetVersion)
-                mutex.withLock {
-                    if (!applied) rollbackReorder(edit)
-                    reconcileLocked()
-                }
+                val applied =
+                    mutex.withLock {
+                        val applied = playbackController.move(from, to, resetVersion)
+                        if (!applied) rollbackReorder(edit)
+                        reconcileLocked()
+                        applied
+                    }
                 onComplete(applied)
             } finally {
                 reorderPending = false
diff --git a/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt b/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt
index df4f7ca0..0d7184d0 100644
--- a/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt
+++ b/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt
@@ -509,6 +509,34 @@ class QueueViewModelTest {
         assertEquals(before.map { it.position }, rowsSnapshot().map { it.position })
     }
 
+    @Test
+    fun windowLoadingWaitsForRejectedMoveRollback() {
+        openReconciledSparseOfflineQueue()
+        markDownloaded(1, "s0")
+        val canonical = resolvedQueueIds()
+        val before = rowsSnapshot()
+        val dragged = before[1]
+        val paused = pauseQueueMutation()
+        val completion = CompletableDeferred<Boolean>()
+        try {
+            runOnMainSuspending {
+                viewModel.reorder(1, 2)
+                val move = viewModel.rows.dropMoveFor(dragged.id)!!
+                assertTrue(viewModel.move(move.from, move.to, move.resetVersion, completion::complete))
+                yield()
+                viewModel.loadOlder()
+                yield()
+            }
+        } finally {
+            resumeQueueMutation(paused)
+        }
+
+        assertFalse(awaitCompletion(completion))
+        assertEquals(canonical, resolvedQueueIds())
+        assertEquals(listOf("s0", "s1", "s801", "s901"), rowsSnapshot().map { it.song.song.id })
+        assertEquals(listOf(0L, 1L, 800L, 900L), rowsSnapshot().map { it.position })
+    }
+
     @Test
     fun rejectedSparseTailRemovalRestoresTheVisibleOrder() {
         openReconciledSparseOfflineQueue()
@@ -667,13 +695,13 @@ class QueueViewModelTest {
                 val secondMove = viewModel.rows.dropMoveFor(before[2].id)!!
                 assertFalse(viewModel.move(secondMove.from, secondMove.to, secondMove.resetVersion, secondCompletion::complete))
             }
-            await { rowsSnapshot().map { it.song.song.id } == expectedFirst }
         } finally {
             resumeQueueMutation(paused)
         }
 
         assertTrue(awaitCompletion(firstCompletion))
         assertFalse(awaitCompletion(secondCompletion))
+        assertEquals(expectedFirst, rowsSnapshot().map { it.song.song.id })
         assertEquals(expectedFirst, resolvedQueueIds().filter { it in visibleIds })
     }
 
@@ -699,6 +727,47 @@ class QueueViewModelTest {
         assertEquals(listOf("s1", "s801"), resolvedQueueIds().filter { it in setOf("s1", "s801", "s901") })
     }
 
+    @Test
+    fun removalQueuedBehindMoveDoesNotUseItsOldPosition() {
+        runBlocking { seedSongs(3) }
+        controller.playAlbum(1, "al1", 0)
+        await { controller.state.value.position == 0L }
+        viewModel.open()
+        await { viewModel.ready }
+        val before = rowsSnapshot()
+        val paused = pauseQueueMutation()
+        val moveCompletion = CompletableDeferred<Boolean>()
+        val removalCompletion = CompletableDeferred<Boolean>()
+        try {
+            runOnMainSuspending {
+                assertTrue(
+                    viewModel.move(
+                        before[0].position,
+                        before[2].position,
+                        before[0].resetVersion,
+                        moveCompletion::complete,
+                    ),
+                )
+                yield()
+                assertEquals(
+                    "s2",
+                    viewModel.rows[1]
+                        .song.song.id,
+                )
+                viewModel.remove(before[1].position, removalCompletion::complete)
+                yield()
+            }
+        } finally {
+            resumeQueueMutation(paused)
+        }
+
+        assertTrue(awaitCompletion(moveCompletion))
+        assertFalse(awaitCompletion(removalCompletion))
+        assertEquals(listOf("s2", "s3", "s1"), resolvedQueueIds())
+        assertEquals(listOf("s2", "s3", "s1"), rowsSnapshot().map { it.song.song.id })
+        assertEquals(listOf(0L, 1L, 2L), rowsSnapshot().map { it.position })
+    }
+
     @Test
     fun moveRejectsRowsFromDifferentResetGenerations() {
         runOnMain {