Fix a queue bug that deleted episodes, and delete dead DAO methods

Review found a data-losing bug. advance() drops the head of the queue, but
play() never put the played episode there. Play something from a show list,
let it finish, and advance dropped whatever was actually at the head -- a
different episode, removed from the queue having never been played.

The fix is an invariant rather than a special case: anything played becomes
the head, so advance always drops the episode that just finished. playNow()
does that, play() calls it, and the tests name the scenario.

Two DAO methods had no caller -- getQueue, superseded by observeQueue, and
observeUnplayed, written for a screen that was never built. Deleted. Room
generates an implementation for each, so they were not free.

The reachability guard now covers DAO methods too, which is the shape that
bit FeeDreader: three superseded methods sat undetected until a manual audit.
Interfaces need looser matching than objects, since a call's receiver varies
by site and a @Transaction default method calls its siblings unqualified, so
it matches both forms and errs toward "called" -- a false alarm costs more
attention than a missed one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B

10 files changed, 118 insertions, 16 deletions

diff --git a/.coverage-baseline b/.coverage-baseline
index 09c9fdf..e9816e1 100644
--- a/.coverage-baseline
+++ b/.coverage-baseline
@@ -1 +1 @@
-71.31
+71.37
diff --git a/CLAUDE.md b/CLAUDE.md
index e78257a..67de4bd 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -54,11 +54,23 @@ with `adb shell monkey`. Use the shared wrappers, which report from exit codes,
 and a logcat capture started before launch:
 
 ```
-/workspace/devtools/bin/gradle-test /workspace/listen -- --rerun-tasks
+/workspace/devtools/bin/gradle-test   /workspace/listen -- --rerun-tasks
+/workspace/devtools/bin/gradle-report /workspace/listen detekt
 /workspace/devtools/bin/android-launch-check org.terst.listen \
     --apk app/build/outputs/apk/debug/app-debug.apk --settle 15
+/workspace/devtools/bin/android-ui org.terst.listen dump
+/workspace/devtools/bin/publish-apk \
+    app/build/outputs/apk/debug/app-debug.apk listen-debug.apk
 ```
 
+**Never read `app/build/reports/detekt/detekt.xml` directly.** An invalid config aborts detekt
+before it analyses anything and leaves the previous run's report in place; it was read as
+current findings three times. `gradle-report` refuses a report older than the run.
+
+**Never `adb shell input tap X Y`.** A blind tap landed in FeeDreader's settings and the
+screenshot after it was reported as Listen. `android-ui` checks the foreground package first
+and taps by label.
+
 See `/workspace/devtools/README.md`. These exist because grepped console text has produced wrong
 claims three separate times across these repos.
 
@@ -130,3 +142,13 @@ intro/outro skipping. Schema fields for manual skip offsets exist but nothing re
 
 - **Feeds are big.** A real subscription in testing produced 2753 episodes in one refresh. Any
   per-episode work — a query in a loop, an O(n²) match — needs to be considered at that scale.
+
+- **The playing episode is always the head of the queue.** `advance()` drops the head, so if
+  something played from a show list isn't there, finishing it deletes a queued episode unplayed.
+  `play()` calls `playNow()` first to hold that invariant. Don't add a playback entry point that
+  skips it.
+
+- **Two reachability tests guard dead code**, one for helper objects and one for DAO methods.
+  Both exist because code was written, tested, documented and never called: `findFeedLink` (the
+  share-sheet bug) and `QueueRepository.advance` (the queue silently stopped). Adding to their
+  allowlists needs a reason, not just a name.
diff --git a/app/src/main/java/org/terst/listen/data/Daos.kt b/app/src/main/java/org/terst/listen/data/Daos.kt
index 2376644..deb350a 100644
--- a/app/src/main/java/org/terst/listen/data/Daos.kt
+++ b/app/src/main/java/org/terst/listen/data/Daos.kt
@@ -84,9 +84,6 @@ interface EpisodeDao {
         before: Long,
         downloaded: DownloadState = DownloadState.DOWNLOADED,
     ): List<EpisodeEntity>
-
-    @Query("SELECT * FROM episodes WHERE playedAtMs IS NULL ORDER BY publishedAtMs DESC LIMIT :limit")
-    fun observeUnplayed(limit: Int): Flow<List<EpisodeEntity>>
 }
 
 @Dao
@@ -98,13 +95,6 @@ interface QueueDao {
     )
     fun observeQueue(): Flow<List<EpisodeEntity>>
 
-    @Query(
-        """SELECT e.* FROM episodes e
-           INNER JOIN queue q ON q.episodeId = e.id
-           ORDER BY q.position"""
-    )
-    suspend fun getQueue(): List<EpisodeEntity>
-
     @Query("SELECT * FROM queue ORDER BY position")
     suspend fun getEntries(): List<QueueEntity>
 
diff --git a/app/src/main/java/org/terst/listen/data/Repositories.kt b/app/src/main/java/org/terst/listen/data/Repositories.kt
index fb869d8..60d1590 100644
--- a/app/src/main/java/org/terst/listen/data/Repositories.kt
+++ b/app/src/main/java/org/terst/listen/data/Repositories.kt
@@ -76,6 +76,9 @@ class QueueRepository(private val dao: QueueDao) {
 
     suspend fun playNext(episodeId: String) = rewrite { QueueOrdering.playNext(it, episodeId) }
 
+    /** Promotes an episode to the head, where "what is playing" lives. */
+    suspend fun playNow(episodeId: String) = rewrite { QueueOrdering.playNow(it, episodeId) }
+
     suspend fun remove(episodeId: String) = rewrite { QueueOrdering.remove(it, episodeId) }
 
     suspend fun move(from: Int, to: Int) = rewrite { QueueOrdering.move(it, from, to) }
diff --git a/app/src/main/java/org/terst/listen/queue/QueueOrdering.kt b/app/src/main/java/org/terst/listen/queue/QueueOrdering.kt
index 4cc6543..d657477 100644
--- a/app/src/main/java/org/terst/listen/queue/QueueOrdering.kt
+++ b/app/src/main/java/org/terst/listen/queue/QueueOrdering.kt
@@ -26,6 +26,15 @@ object QueueOrdering {
         return listOf(rest.first(), episodeId) + rest.drop(1)
     }
 
+    /**
+     * Makes an episode the head, which is what is playing.
+     *
+     * The invariant this exists to hold: [advance] drops the head, so if the playing episode
+     * isn't there, finishing it deletes a different episode from the queue unplayed.
+     */
+    fun playNow(queue: List<String>, episodeId: String): List<String> =
+        listOf(episodeId) + queue.filterNot { it == episodeId }
+
     fun remove(queue: List<String>, episodeId: String): List<String> =
         queue.filterNot { it == episodeId }
 
diff --git a/app/src/main/java/org/terst/listen/ui/PlayerViewModel.kt b/app/src/main/java/org/terst/listen/ui/PlayerViewModel.kt
index fa501a4..aef40ac 100644
--- a/app/src/main/java/org/terst/listen/ui/PlayerViewModel.kt
+++ b/app/src/main/java/org/terst/listen/ui/PlayerViewModel.kt
@@ -58,6 +58,11 @@ class PlayerViewModel(app: Application) : AndroidViewModel(app) {
     /** Starts an episode, resuming where the listener left off and at the show's speed. */
     fun play(episode: EpisodeEntity) {
         viewModelScope.launch {
+            // Anything played becomes the head of the queue. Without that, finishing an
+            // episode started from a show list would advance past -- and silently discard --
+            // whatever was actually queued.
+            graph.queue.playNow(episode.id)
+
             val show = graph.subscriptions.getById(episode.subscriptionId)
             val speed = show?.playbackSpeed ?: DEFAULT_SPEED
             connection.play(
diff --git a/app/src/test/java/org/terst/listen/ReachabilityTest.kt b/app/src/test/java/org/terst/listen/ReachabilityTest.kt
index 4eaed9e..d129756 100644
--- a/app/src/test/java/org/terst/listen/ReachabilityTest.kt
+++ b/app/src/test/java/org/terst/listen/ReachabilityTest.kt
@@ -70,6 +70,35 @@ class ReachabilityTest {
             .toList()
     }
 
+    /**
+     * Room DAOs are the other place dead code hides -- FeeDreader accumulated three superseded
+     * methods that an audit, not a test, eventually found. Matching is on `.name(` across all
+     * of main with declarations stripped, because a DAO call's receiver varies by site and a
+     * @Transaction default method legitimately calls its siblings in the same file.
+     */
+    @Test
+    fun `every dao method has a caller`() {
+        val sources = mainDir.walkTopDown().filter { it.extension == "kt" }.toList()
+        val declaration = Regex("""(?:suspend )?fun (\w+)\(""")
+        val corpus = sources.joinToString("\n") { declaration.replace(it.readText(), "DECL(") }
+
+        val daoFile = File(mainDir, "data/Daos.kt")
+        assertTrue("expected ${daoFile.path}", daoFile.isFile)
+
+        val dead = Regex("""^    (?:suspend )?fun (\w+)""", RegexOption.MULTILINE)
+            .findAll(daoFile.readText())
+            .map { it.groupValues[1] }
+            .distinct()
+            // Both a qualified call and a bare one: a @Transaction default method calls its
+            // siblings unqualified. Heuristic on purpose -- it errs toward saying "called",
+            // since a false alarm here costs more attention than a missed one.
+            .filterNot { corpus.contains(".$it(") || Regex("""\b$it\(""").containsMatchIn(corpus) }
+            .toList()
+
+        assertTrue("No caller for DAO method(s): ${dead.sorted()}. Wire them up or delete them.",
+            dead.isEmpty())
+    }
+
     @Test
     fun `every helper object function has a caller`() {
         val sources = mainDir.walkTopDown().filter { it.extension == "kt" }.toList()
diff --git a/app/src/test/java/org/terst/listen/data/FakeDaos.kt b/app/src/test/java/org/terst/listen/data/FakeDaos.kt
index 821e1b2..f313687 100644
--- a/app/src/test/java/org/terst/listen/data/FakeDaos.kt
+++ b/app/src/test/java/org/terst/listen/data/FakeDaos.kt
@@ -103,9 +103,6 @@ class FakeEpisodeDao : EpisodeDao {
             it.downloadState == downloaded && it.playedAtMs != null && it.playedAtMs!! < before
         }
 
-    override fun observeUnplayed(limit: Int): Flow<List<EpisodeEntity>> =
-        flowOf(rows.filter { it.playedAtMs == null }.take(limit))
-
     private fun replace(id: String, transform: (EpisodeEntity) -> EpisodeEntity) {
         val index = rows.indexOfFirst { it.id == id }
         if (index >= 0) {
@@ -125,7 +122,6 @@ class FakeQueueDao : QueueDao {
     }
 
     override fun observeQueue(): Flow<List<EpisodeEntity>> = flowOf(ordered())
-    override suspend fun getQueue() = ordered()
     override suspend fun getEntries() = entries.sortedBy { it.position }
 
     override suspend fun upsertAll(entries: List<QueueEntity>) {
diff --git a/app/src/test/java/org/terst/listen/data/QueueRepositoryTest.kt b/app/src/test/java/org/terst/listen/data/QueueRepositoryTest.kt
index a3ed294..dde5bdb 100644
--- a/app/src/test/java/org/terst/listen/data/QueueRepositoryTest.kt
+++ b/app/src/test/java/org/terst/listen/data/QueueRepositoryTest.kt
@@ -81,6 +81,29 @@ class QueueRepositoryTest {
         assertContiguous()
     }
 
+    /**
+     * The bug this prevents: play an episode that isn't in the queue, let it finish, and the
+     * old advance dropped whatever was at the head -- a different episode, removed unplayed.
+     */
+    @Test
+    fun `playing an unqueued episode does not consume a queued one`() = runTest {
+        listOf("a", "b").forEach { repo.append(it) }
+        repo.playNow("d")
+        assertEquals(listOf("d", "a", "b"), ids())
+
+        assertEquals("a", repo.advance())
+        assertEquals("nothing may be dropped except the episode that finished", listOf("a", "b"), ids())
+        assertContiguous()
+    }
+
+    @Test
+    fun `play now promotes to the head`() = runTest {
+        listOf("a", "b", "c").forEach { repo.append(it) }
+        repo.playNow("c")
+        assertEquals(listOf("c", "a", "b"), ids())
+        assertContiguous()
+    }
+
     @Test
     fun `advance drops the finished episode and reports the next`() = runTest {
         listOf("a", "b", "c").forEach { repo.append(it) }
diff --git a/app/src/test/java/org/terst/listen/queue/QueueOrderingTest.kt b/app/src/test/java/org/terst/listen/queue/QueueOrderingTest.kt
index 556351f..93ee0a0 100644
--- a/app/src/test/java/org/terst/listen/queue/QueueOrderingTest.kt
+++ b/app/src/test/java/org/terst/listen/queue/QueueOrderingTest.kt
@@ -69,6 +69,31 @@ class QueueOrderingTest {
         assertEquals(queue, QueueOrdering.move(queue, from = 99, to = 0))
     }
 
+    /**
+     * Playing an episode makes it the head, which is what keeps advance honest: advance drops
+     * the head, so unless the playing episode is there, finishing it silently deletes somebody
+     * else's place in the queue.
+     */
+    @Test
+    fun `play now puts the episode at the head`() {
+        assertEquals(listOf("d", "a", "b", "c"), QueueOrdering.playNow(queue, "d"))
+    }
+
+    @Test
+    fun `play now on a queued episode moves it to the head without duplicating`() {
+        assertEquals(listOf("c", "a", "b"), QueueOrdering.playNow(queue, "c"))
+    }
+
+    @Test
+    fun `play now on the current head changes nothing`() {
+        assertEquals(listOf("a", "b", "c"), QueueOrdering.playNow(queue, "a"))
+    }
+
+    @Test
+    fun `play now on an empty queue starts it`() {
+        assertEquals(listOf("d"), QueueOrdering.playNow(emptyList(), "d"))
+    }
+
     @Test
     fun `advancing drops the finished episode and returns the next`() {
         val (next, remaining) = QueueOrdering.advance(queue)