From cf7b489fec2c92d44d01fd4d7f20e5e674e0e621 Mon Sep 17 00:00:00 2001
From: Bryan Van Deusen <bryan.vandeusen@fabledsword.com>
Date: Sat, 1 Aug 2026 12:04:22 -0400
Subject: [PATCH] fix(playlists): make the playlist-track replace atomic
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`refreshDetail` did an un-transacted `deleteByPlaylist` + `upsertAll` — the
same shape as the Home index write that #2327 just fixed. Room's
InvalidationTracker fires after the DELETE, so an observer of
`observeByPlaylist` would see `emptyList()` before the new rows land, which is
exactly what made every Home row visibly collapse to empty and refill.

Nothing consumes `observeByPlaylist` today, so this is not a live defect — it's
a landmine. Making playlist detail cache-first later would have silently
reintroduced the flicker, and the reason would have been three layers away from
the symptom. One `@Transaction` now costs nothing and removes that.

`deleteByPlaylist` is left in place as the building block but is no longer
called from outside the DAO.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../cache/db/dao/CachedPlaylistTrackDao.kt    | 26 ++++++++++++++++++-
 .../playlists/data/PlaylistsRepository.kt     |  6 ++---
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/android/app/src/main/java/com/fabledsword/minstrel/cache/db/dao/CachedPlaylistTrackDao.kt b/android/app/src/main/java/com/fabledsword/minstrel/cache/db/dao/CachedPlaylistTrackDao.kt
index 02ff83b0..072cc11b 100644
--- a/android/app/src/main/java/com/fabledsword/minstrel/cache/db/dao/CachedPlaylistTrackDao.kt
+++ b/android/app/src/main/java/com/fabledsword/minstrel/cache/db/dao/CachedPlaylistTrackDao.kt
@@ -4,6 +4,7 @@ import androidx.room.Dao
 import androidx.room.Insert
 import androidx.room.OnConflictStrategy
 import androidx.room.Query
+import androidx.room.Transaction
 import com.fabledsword.minstrel.cache.db.entities.CachedPlaylistTrackEntity
 import kotlinx.coroutines.flow.Flow
 
@@ -35,10 +36,33 @@ interface CachedPlaylistTrackDao {
     @Query("SELECT MAX(position) FROM cached_playlist_tracks WHERE playlistId = :playlistId")
     suspend fun maxPosition(playlistId: String): Int?
 
-    /** Replace-all pattern for a playlist; called after a sync delta lands. */
     @Query("DELETE FROM cached_playlist_tracks WHERE playlistId = :playlistId")
     suspend fun deleteByPlaylist(playlistId: String)
 
+    /**
+     * Replaces a playlist's whole membership in ONE transaction; called
+     * after a refresh or a sync delta lands.
+     *
+     * Atomic on purpose. Room's InvalidationTracker only notifies observers
+     * after the transaction commits, so [observeByPlaylist] never sees the
+     * empty gap between the delete and the re-insert. Un-transacted, that
+     * gap is a real observed state — it's what made every Home row visibly
+     * collapse to empty and refill before issue #2327 fixed the equivalent
+     * write in `CachedHomeIndexDao`.
+     *
+     * Nothing observes [observeByPlaylist] live today, so this is
+     * pre-emptive: it means making playlist detail cache-first later can't
+     * silently reintroduce that flicker.
+     */
+    @Transaction
+    suspend fun replacePlaylistTracks(
+        playlistId: String,
+        rows: List<CachedPlaylistTrackEntity>,
+    ) {
+        deleteByPlaylist(playlistId)
+        if (rows.isNotEmpty()) upsertAll(rows)
+    }
+
     @Query(
         "DELETE FROM cached_playlist_tracks " +
             "WHERE playlistId = :playlistId AND trackId IN (:trackIds)",
diff --git a/android/app/src/main/java/com/fabledsword/minstrel/playlists/data/PlaylistsRepository.kt b/android/app/src/main/java/com/fabledsword/minstrel/playlists/data/PlaylistsRepository.kt
index b68db292..d7b697ce 100644
--- a/android/app/src/main/java/com/fabledsword/minstrel/playlists/data/PlaylistsRepository.kt
+++ b/android/app/src/main/java/com/fabledsword/minstrel/playlists/data/PlaylistsRepository.kt
@@ -119,9 +119,9 @@ class PlaylistsRepository @Inject constructor(
             throw e
         }
         playlistDao.upsertAll(listOf(wire.toPlaylistEntity()))
-        playlistTrackDao.deleteByPlaylist(id)
-        playlistTrackDao.upsertAll(
-            wire.tracks.mapNotNull { row ->
+        playlistTrackDao.replacePlaylistTracks(
+            playlistId = id,
+            rows = wire.tracks.mapNotNull { row ->
                 row.trackId?.let { trackId ->
                     CachedPlaylistTrackEntity(
                         playlistId = id,