address the second offline review: cancel guard, bounded reconcile

app/src/main/kotlin/com/subtracks/data/download/SystemDownloadEngine.kt

@@ -43,6 +43,9 @@ class SystemDownloadEngine(
     }
 
     override fun cancel(ids: List<Long>) {
+        // DownloadManager.remove throws on an empty id list, and queued rows waiting for a slot
+        // have no engine id yet, so an offline cancel can legitimately select none.
+        if (ids.isEmpty()) return
         manager().remove(*ids.toLongArray())
     }
 

app/src/main/kotlin/com/subtracks/data/repo/DownloadRepository.kt

@@ -327,7 +327,8 @@ class DownloadRepository(
 
     private suspend fun removeRows(rows: List<SongDownload>) {
         if (rows.isEmpty()) return
-        engine.cancel(rows.mapNotNull { it.engineId })
+        val engineIds = rows.mapNotNull { it.engineId }
+        if (engineIds.isNotEmpty()) engine.cancel(engineIds)
         rows.groupBy { it.sourceId }.forEach { (sourceId, group) ->
             group.forEach { file(sourceId, it.songId).delete() }
             group.map { it.songId }.chunked(DELETE_CHUNK).forEach { db.downloadDao().deleteSongs(sourceId, it) }

app/src/main/kotlin/com/subtracks/data/repo/SourceRepository.kt

@@ -75,6 +75,9 @@ class SourceRepository(
             prefs.offlineMode().collect { _offline.value = it }
         }
         scope.launch {
+            // Read the persisted flag before the source config can build a source or probe it, so a
+            // cold start with offline on never touches the network.
+            _offline.value = prefs.offlineMode().first()
             combine(
                 db.sourcesDao().activeSubsonicConfig(),
                 networkMode,
@@ -88,7 +91,7 @@ class SourceRepository(
                 _quality.value = quality
                 val sourceChanged = config?.id != activeSourceId
                 activeSourceId = config?.id
-                if (sourceChanged && config != null && config.useTokenAuth && !prefs.offlineMode().first()) {
+                if (sourceChanged && config != null && config.useTokenAuth && !_offline.value) {
                     scope.launch { runCatching { config.toClient().check("ping") } }
                 }
             }

app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt

@@ -24,6 +24,7 @@ import kotlinx.coroutines.flow.drop
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
+import kotlinx.coroutines.withContext
 import kotlin.random.Random
 
 data class QueueItem(
@@ -1266,17 +1267,18 @@ class PlaybackController(
         snapshot: QueueSnapshot,
         from: Long,
         step: Long,
-    ): Long? {
+    ): Long? =
-        var position = from
+        withContext(Dispatchers.IO) {
-        var scanned = 0L
+            var position = from
-        while (position in 0 until snapshot.size && scanned < OFFLINE_SCAN_LIMIT) {
+            var scanned = 0L
-            val songId = queueRepository.itemAt(snapshot, position)?.song?.id
+            while (position in 0 until snapshot.size && scanned < OFFLINE_SCAN_LIMIT) {
-            if (songId != null && downloads.localUri(songId) != null) return position
+                val songId = queueRepository.itemAt(snapshot, position)?.song?.id
-            position += step
+                if (songId != null && downloads.localUri(songId) != null) return@withContext position
-            scanned++
+                position += step
+                scanned++
+            }
+            null
         }
-        return null
-    }
 
     private suspend fun reloadForOffline() =
         startLock.withLock {

app/src/main/kotlin/com/subtracks/ui/components/Components.kt

@@ -22,6 +22,7 @@ import androidx.compose.material3.CircularProgressIndicator
 import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.Text
 import androidx.compose.runtime.Composable
+import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
@@ -309,6 +310,22 @@ fun FilteredEmptyState(
     }
 }
 
+@Composable
+fun CloseWhenDownloadsGone(
+    downloaded: Long,
+    offline: Boolean,
+    onClose: () -> Unit,
+) {
+    var hadDownloads by remember { mutableStateOf(false) }
+    LaunchedEffect(downloaded, offline) {
+        if (downloaded > 0) {
+            hadDownloads = true
+        } else if (hadDownloads && offline) {
+            onClose()
+        }
+    }
+}
+
 @Composable
 fun Modifier.statusBarScrim(): Modifier {
     val top = with(LocalDensity.current) { WindowInsets.statusBars.getTop(this).toDp() }

app/src/main/kotlin/com/subtracks/ui/library/AlbumDetailScreen.kt

@@ -34,6 +34,7 @@ import com.subtracks.data.model.Song
 import com.subtracks.data.model.SongDownload
 import com.subtracks.data.source.StarType
 import com.subtracks.playback.PlaybackController
+import com.subtracks.ui.components.CloseWhenDownloadsGone
 import com.subtracks.ui.components.ContextMenuHost
 import com.subtracks.ui.components.DeleteDownloadsDialog
 import com.subtracks.ui.components.HeroDetailScaffold
@@ -67,14 +68,7 @@ fun AlbumDetailRoute(
     val downloads by viewModel.downloads.collectAsStateWithLifecycle()
     val downloadStatus by viewModel.downloadStatus.collectAsStateWithLifecycle()
     val offline by viewModel.offline.collectAsStateWithLifecycle()
-    var hadDownloads by remember { mutableStateOf(false) }
+    CloseWhenDownloadsGone(downloadStatus.downloaded, offline, onBack)
-    LaunchedEffect(downloadStatus.downloaded, offline) {
-        if (downloadStatus.downloaded > 0) {
-            hadDownloads = true
-        } else if (hadDownloads && offline) {
-            onBack()
-        }
-    }
     val scope = rememberCoroutineScope()
     var pendingDelete by remember { mutableStateOf<Long?>(null) }
     val playback by playbackController.state.collectAsStateWithLifecycle()

app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt

@@ -82,6 +82,7 @@ import com.subtracks.data.model.CoverArtRef
 import com.subtracks.data.model.ListDownloadStatus
 import com.subtracks.data.source.StarType
 import com.subtracks.playback.PlaybackController
+import com.subtracks.ui.components.CloseWhenDownloadsGone
 import com.subtracks.ui.components.ContextMenuHost
 import com.subtracks.ui.components.CoverArt
 import com.subtracks.ui.components.DeleteDownloadsDialog
@@ -150,14 +151,7 @@ fun ArtistDetailRoute(
     val artThumbnail by viewModel.artThumbnail.collectAsStateWithLifecycle()
     val downloadStatus by viewModel.downloadStatus.collectAsStateWithLifecycle()
     val offline by viewModel.offline.collectAsStateWithLifecycle()
-    var hadDownloads by remember { mutableStateOf(false) }
+    CloseWhenDownloadsGone(downloadStatus.downloaded, offline, onBack)
-    LaunchedEffect(downloadStatus.downloaded, offline) {
-        if (downloadStatus.downloaded > 0) {
-            hadDownloads = true
-        } else if (hadDownloads && offline) {
-            onBack()
-        }
-    }
     val albumDownloads by viewModel.albumDownloads.collectAsStateWithLifecycle()
     val scope = rememberCoroutineScope()
     var pendingDelete by remember { mutableStateOf<PendingDownloadDelete?>(null) }

app/src/main/kotlin/com/subtracks/ui/library/PlaylistDetailScreen.kt

@@ -22,6 +22,7 @@ import com.subtracks.data.model.QueueKind
 import com.subtracks.data.model.SongDownload
 import com.subtracks.data.source.StarType
 import com.subtracks.playback.PlaybackController
+import com.subtracks.ui.components.CloseWhenDownloadsGone
 import com.subtracks.ui.components.ContextMenuHost
 import com.subtracks.ui.components.DeleteDownloadsDialog
 import com.subtracks.ui.components.HeroDetailScaffold
@@ -53,14 +54,7 @@ fun PlaylistDetailRoute(
     val downloads by viewModel.downloads.collectAsStateWithLifecycle()
     val downloadStatus by viewModel.downloadStatus.collectAsStateWithLifecycle()
     val offline by viewModel.offline.collectAsStateWithLifecycle()
-    var hadDownloads by remember { mutableStateOf(false) }
+    CloseWhenDownloadsGone(downloadStatus.downloaded, offline, onBack)
-    LaunchedEffect(downloadStatus.downloaded, offline) {
-        if (downloadStatus.downloaded > 0) {
-            hadDownloads = true
-        } else if (hadDownloads && offline) {
-            onBack()
-        }
-    }
     val scope = rememberCoroutineScope()
     var pendingDelete by remember { mutableStateOf<Long?>(null) }
     val playback by playbackController.state.collectAsStateWithLifecycle()

app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt

@@ -74,10 +74,9 @@ private const val POSITION_TIMEOUT_MS = 1_000L
 
 internal const val QUEUE_WINDOW_ROWS = QUEUE_CHUNK * 3
 
-// Offline the view hides non-downloaded rows, so resolving a contiguous position range can span far
+// Offline the view hides non-downloaded rows, so a displayed window can span far more positions than
-// more than it shows. Cap the positions resolved per load (and per reconcile) so a sparse large
+// it shows. Reconcile resolves only this many positions from the head and keeps the rest on screen,
-// playlist cannot make the view resolve the whole queue; online every position is shown, so the cap
+// so a sparse large playlist cannot make a layout change resolve the whole queue.
-// is never reached.
 private const val MAX_SCAN_POSITIONS = QUEUE_WINDOW_ROWS * 4
 
 data class QueueRow(
@@ -169,11 +168,21 @@ class QueueViewModel(
         val loaded = queueRepository.range(snapshot, start, end).filter(::visible)
         val reused = reusedIds()
         val updated = loaded.map { rowFor(it, reused) }
-        if (updated.size == rows.size) {
+        if (end >= rawEnd) {
-            updated.forEachIndexed { index, row -> if (rows[index] != row) rows[index] = row }
+            if (updated.size == rows.size) {
+                updated.forEachIndexed { index, row -> if (rows[index] != row) rows[index] = row }
+            } else {
+                rows.clear()
+                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 }
             rows.clear()
             rows.addAll(updated)
+            rows.addAll(tail)
         }
     }
 
@@ -187,11 +196,9 @@ class QueueViewModel(
                 val snapshot = queueRepository.snapshot()
                 val collected = mutableListOf<QueueWindowItem>()
                 var end = first - 1
-                var scanned = 0L
+                while (end >= 0L && collected.size < QUEUE_CHUNK) {
-                while (end >= 0L && collected.size < QUEUE_CHUNK && scanned < MAX_SCAN_POSITIONS) {
                     val start = (end - QUEUE_CHUNK + 1).coerceAtLeast(0)
                     collected.addAll(0, queueRepository.range(snapshot, start, end).filter(::visible))
-                    scanned += end - start + 1
                     end = start - 1
                 }
                 if (collected.isEmpty()) return@withLock
@@ -210,11 +217,9 @@ class QueueViewModel(
                 if (last >= snapshot.size - 1) return@withLock
                 val collected = mutableListOf<QueueWindowItem>()
                 var start = last + 1
-                var scanned = 0L
+                while (start < snapshot.size && collected.size < QUEUE_CHUNK) {
-                while (start < snapshot.size && collected.size < QUEUE_CHUNK && scanned < MAX_SCAN_POSITIONS) {
                     val to = (start + QUEUE_CHUNK - 1).coerceAtMost(snapshot.size - 1)
                     collected.addAll(queueRepository.range(snapshot, start, to).filter(::visible))
-                    scanned += to - start + 1
                     start = to + 1
                 }
                 if (collected.isEmpty()) return@withLock

app/src/test/kotlin/com/subtracks/data/download/FakeDownloadEngine.kt

@@ -9,6 +9,7 @@ class FakeDownloadEngine : DownloadEngine {
     var cancelCalls = 0
     var enqueueDelayMs = 0L
     var failEnqueueFor: String? = null
+    var failCancelOnEmpty = false
 
     override fun enqueue(request: EngineRequest): Long {
         if (enqueueDelayMs > 0) Thread.sleep(enqueueDelayMs)
@@ -23,6 +24,7 @@ class FakeDownloadEngine : DownloadEngine {
     override fun download(id: Long): EngineDownload? = downloads[id]
 
     override fun cancel(ids: List<Long>) {
+        if (ids.isEmpty() && failCancelOnEmpty) throw IllegalArgumentException("no ids")
         cancelCalls++
         cancelled += ids
     }

app/src/test/kotlin/com/subtracks/data/repo/DownloadRepositoryTest.kt

@@ -362,6 +362,23 @@ class DownloadRepositoryTest {
             assertTrue(engine.cancelled.isNotEmpty())
         }
 
+    @Test
+    fun cancellingQueuedRowsWithoutEngineIdsLeavesTheEngineAlone() =
+        runTest {
+            seedLibrary()
+            runBlocking {
+                db.downloadDao().upsert(SongDownload(1, "s1", DownloadStatus.Queued))
+                db.downloadDao().upsert(SongDownload(1, "s2", DownloadStatus.Queued))
+            }
+            // DownloadManager.remove throws on an empty id list; a queue of only waiting rows has
+            // no engine ids, so cancelling must not call the engine at all.
+            engine.failCancelOnEmpty = true
+
+            runBlocking { repository.cancelActive() }
+
+            assertTrue(allRows().isEmpty())
+        }
+
     @Test
     fun theDownloadStartsBeforeItsArtworkIsFetched() =
         runTest {

app/src/test/kotlin/com/subtracks/data/sync/SyncManagerTest.kt

@@ -224,6 +224,7 @@ class SyncManagerTest {
                 "<song id=\"s1\" title=\"Song One\" albumId=\"al1\" artistId=\"ar1\" track=\"1\"/>" +
                 "</searchResult3></subsonic-response>"
     }
+
     @Test
     fun aSyncRequestWhileOfflineIsRefusedWithoutTouchingTheNetwork() =
         runBlocking {