library: address artist art review findings

Cap the resolver at four covers, re-rank after deduplication so duplicate covers do not consume the grid budget, carry a thumbnail per fallback step, and advance to the grid when both the full image and its thumbnail fail. Negative-cache failed provider-image fetches per artist URL, clear materialized downloaded covers when all downloads are removed, and drop redundant plumbing.

app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt

@@ -209,8 +209,10 @@ internal const val ARTIST_ART_UPDATE_SQL =
         "FROM credits JOIN albums ON albums.sourceId = :sourceId AND albums.id = credits.albumId " +
         "WHERE albums.coverArt IS NOT NULL), " +
         "unique_ranked AS (SELECT artistId, coverArt, MIN(coverRank) AS coverRank FROM ranked GROUP BY artistId, coverArt), " +
+        "deduped AS (SELECT artistId, coverArt, " +
+        "ROW_NUMBER() OVER (PARTITION BY artistId ORDER BY coverRank) AS coverRank FROM unique_ranked), " +
         "covers AS (SELECT artistId, group_concat(coverArt, char(31)) AS coverArts FROM (" +
-        "SELECT artistId, coverArt, coverRank FROM unique_ranked WHERE coverRank <= 4 ORDER BY artistId, coverRank" +
+        "SELECT artistId, coverArt, coverRank FROM deduped WHERE coverRank <= 4 ORDER BY artistId, coverRank" +
         ") GROUP BY artistId), " +
         "downloaded_albums AS (SELECT DISTINCT artistId, albumId FROM ($DOWNLOADED_COVER_ALBUM_ROWS)), " +
         "downloaded AS (SELECT dl.artistId AS artistId, albums.coverArt AS coverArt, " +
@@ -219,8 +221,10 @@ internal const val ARTIST_ART_UPDATE_SQL =
         "FROM downloaded_albums dl JOIN albums ON albums.sourceId = :sourceId AND albums.id = dl.albumId " +
         "WHERE albums.coverArt IS NOT NULL), " +
         "unique_downloaded AS (SELECT artistId, coverArt, MIN(coverRank) AS coverRank FROM downloaded GROUP BY artistId, coverArt), " +
+        "downloaded_deduped AS (SELECT artistId, coverArt, " +
+        "ROW_NUMBER() OVER (PARTITION BY artistId ORDER BY coverRank) AS coverRank FROM unique_downloaded), " +
         "downloaded_covers AS (SELECT artistId, group_concat(coverArt, char(31)) AS coverArts FROM (" +
-        "SELECT artistId, coverArt, coverRank FROM unique_downloaded WHERE coverRank <= 4 ORDER BY artistId, coverRank" +
+        "SELECT artistId, coverArt, coverRank FROM downloaded_deduped WHERE coverRank <= 4 ORDER BY artistId, coverRank" +
         ") GROUP BY artistId) " +
         "UPDATE artists SET " +
         "coverArts = COALESCE((SELECT coverArts FROM covers WHERE covers.artistId = artists.id), ''), " +
@@ -237,8 +241,10 @@ internal const val DOWNLOADED_ARTIST_ART_UPDATE_SQL =
         "FROM downloaded_albums dl JOIN albums ON albums.sourceId = :sourceId AND albums.id = dl.albumId " +
         "WHERE albums.coverArt IS NOT NULL), " +
         "unique_downloaded AS (SELECT artistId, coverArt, MIN(coverRank) AS coverRank FROM downloaded GROUP BY artistId, coverArt), " +
+        "downloaded_deduped AS (SELECT artistId, coverArt, " +
+        "ROW_NUMBER() OVER (PARTITION BY artistId ORDER BY coverRank) AS coverRank FROM unique_downloaded), " +
         "downloaded_covers AS (SELECT artistId, group_concat(coverArt, char(31)) AS coverArts FROM (" +
-        "SELECT artistId, coverArt, coverRank FROM unique_downloaded WHERE coverRank <= 4 ORDER BY artistId, coverRank" +
+        "SELECT artistId, coverArt, coverRank FROM downloaded_deduped WHERE coverRank <= 4 ORDER BY artistId, coverRank" +
         ") GROUP BY artistId) " +
         "UPDATE artists SET downloadedCoverArts = COALESCE((SELECT coverArts FROM downloaded_covers " +
         "WHERE downloaded_covers.artistId = artists.id), '') " +
@@ -406,6 +412,9 @@ interface LibraryDao {
         artistIds: List<String>,
     )
 
+    @Query("UPDATE artists SET downloadedCoverArts = '' WHERE sourceId = :sourceId AND downloadedCoverArts != ''")
+    suspend fun clearDownloadedArtistArt(sourceId: Long)
+
     @Transaction
     suspend fun recomputeAggregates(sourceId: Long) {
         recomputeAlbumPlayData(sourceId)

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

@@ -68,6 +68,7 @@ class DownloadRepository(
 
     private val statesFlow = MutableStateFlow<Map<String, SongDownload>>(emptyMap())
     private val attemptedBios = ConcurrentHashMap.newKeySet<Pair<Long, String>>()
+    private val failedArtistImages = ConcurrentHashMap.newKeySet<String>()
 
     fun start() {
         if (started) return
@@ -377,6 +378,7 @@ class DownloadRepository(
                     .mapNotNull { it.engineId }
                     .let(engine::cancel)
                 db.downloadDao().deleteSource(sourceId)
+                db.libraryDao().clearDownloadedArtistArt(sourceId)
                 dir(sourceId).deleteRecursively()
             }
         }
@@ -422,13 +424,9 @@ class DownloadRepository(
         val albumArtist = album?.artistId?.let { db.libraryDao().artistOnce(sourceId, it) }
         listOfNotNull(song?.artistId, album?.artistId).forEach(artistIds::add)
         listOfNotNull(songArtist, albumArtist).forEach { storeArtistImage(sourceId, it) }
-        val artistFallbackCovers =
-            listOfNotNull(songArtist, albumArtist)
-                .filter { it.artistImageUrl.isNullOrBlank() }
-                .mapNotNull { it.coverArt }
         val coverArt =
             (
-                listOfNotNull(album?.coverArt) + artistFallbackCovers +
+                listOfNotNull(album?.coverArt) +
                     db.downloadDao().playlistCoversForSong(sourceId, songId)
             ).distinct()
         coverArt.forEach { cover ->
@@ -445,17 +443,21 @@ class DownloadRepository(
         if (!imageUrl.isNullOrBlank()) {
             val cacheKey = artistImageKey(sourceId, artist.id)
             if (artworkStore.file(sourceId, cacheKey).exists()) return
-            val bytes =
+            val attemptKey = "$sourceId:${artist.id}:$imageUrl"
-                try {
+            if (attemptKey !in failedArtistImages) {
-                    artworkFetcher.fetch(imageUrl)
+                val bytes =
-                } catch (e: CancellationException) {
+                    try {
-                    throw e
+                        artworkFetcher.fetch(imageUrl)
-                } catch (_: Exception) {
+                    } catch (e: CancellationException) {
-                    null
+                        throw e
+                    } catch (_: Exception) {
+                        null
+                    }
+                if (bytes != null && bytes.isNotEmpty()) {
+                    artworkStore.write(sourceId, cacheKey, bytes)
+                    return
                 }
-            if (bytes != null && bytes.isNotEmpty()) {
+                failedArtistImages += attemptKey
-                artworkStore.write(sourceId, cacheKey, bytes)
-                return
             }
         }
         artist.coverArt?.let {

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

@@ -29,6 +29,7 @@ import kotlin.math.sin
 
 data class ArtistArt(
     val image: CoverArtRef? = null,
+    val thumbnail: CoverArtRef? = null,
     val covers: List<CoverArtRef> = emptyList(),
     val fallback: ArtistArt? = null,
 ) {
@@ -57,15 +58,22 @@ internal fun resolveArtistArt(
     val coverIds = if (offline) artist.downloadedCoverArts else artist.coverArts
     val grid =
         ArtistArt(
-            covers = coverIds.mapNotNull { coverArt(it, forIcon || coverIds.size > 1) },
+            thumbnail = coverIds.singleOrNull()?.let { coverArt(it, true) },
+            covers = coverIds.take(4).mapNotNull { coverArt(it, forIcon || coverIds.size > 1) },
         )
     val own =
-        artist.coverArt
+        artist.coverArt?.let { coverId ->
-            ?.let { coverArt(it, forIcon) }
+            coverArt(coverId, forIcon)?.let { image ->
-            ?.let { ArtistArt(image = it, fallback = grid) }
+                ArtistArt(
+                    image = image,
+                    thumbnail = if (forIcon) image else coverArt(coverId, true),
+                    fallback = grid,
+                )
+            }
+        }
     return artist.artistImageUrl
         ?.let { artistImage(artist.id, it) }
-        ?.let { ArtistArt(image = it, fallback = own ?: grid) }
+        ?.let { ArtistArt(image = it, thumbnail = it, fallback = own ?: grid) }
         ?: own
         ?: grid
 }
@@ -104,7 +112,6 @@ fun ArtistIcon(
 fun ArtistHeroArt(
     art: ArtistArt,
     name: String,
-    thumbnail: CoverArtRef?,
     modifier: Modifier = Modifier,
 ) {
     var shown by remember(art) { mutableStateOf(art) }
@@ -116,7 +123,7 @@ fun ArtistHeroArt(
             CoverArt(
                 ref = shown.image,
                 name = name,
-                thumbnailRef = thumbnail,
+                thumbnailRef = shown.thumbnail,
                 showPlaceholder = false,
                 modifier = modifier,
                 onError = showFallback,
@@ -127,7 +134,7 @@ fun ArtistHeroArt(
             ArtistArtGrid(
                 covers = shown.covers,
                 name = name,
-                singleThumbnail = thumbnail,
+                singleThumbnail = shown.thumbnail,
                 modifier = modifier,
             )
         }

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

@@ -93,6 +93,7 @@ fun CoverArt(
     val hasThumbnailFallback = thumbnailRef != null && thumbnailRef != ref
     var failed by remember(refKey, thumbnailKey) { mutableStateOf(false) }
     var thumbnailLoaded by remember(refKey, thumbnailKey) { mutableStateOf(false) }
+    var thumbnailFailed by remember(refKey, thumbnailKey) { mutableStateOf(false) }
     var thumbnailRatio by remember(refKey, thumbnailKey) { mutableStateOf(thumbnailKey?.let(ArtworkRatioCache::get)) }
     val letterScheme =
         if (showPlaceholder || (failed && !thumbnailLoaded)) {
@@ -117,6 +118,10 @@ fun CoverArt(
                 thumbnailLoaded = thumbnailLoaded,
                 onThumbnailLoaded = { thumbnailLoaded = true },
                 onThumbnailRatio = { thumbnailRatio = it },
+                onThumbnailError = {
+                    thumbnailFailed = true
+                    if (failed) onError?.invoke()
+                },
                 contentScale = ContentScale.Crop,
                 colorFilter = colorFilter,
                 placeholderContentColor = letterScheme?.onPrimaryContainer,
@@ -135,7 +140,7 @@ fun CoverArt(
                         },
                         onError = {
                             failed = true
-                            if (!hasThumbnailFallback) onError?.invoke()
+                            if (!hasThumbnailFallback || thumbnailFailed) onError?.invoke()
                         },
                         modifier = Modifier.fillMaxSize(),
                     )
@@ -167,7 +172,7 @@ fun CoverArt(
                         },
                     onError = {
                         failed = true
-                        if (!hasThumbnailFallback) onError?.invoke()
+                        if (!hasThumbnailFallback || thumbnailFailed) onError?.invoke()
                     },
                     contentScale = ContentScale.Fit,
                     filterQuality = FilterQuality.High,
@@ -187,6 +192,10 @@ fun CoverArt(
                     thumbnailLoaded = thumbnailLoaded,
                     onThumbnailLoaded = { thumbnailLoaded = true },
                     onThumbnailRatio = { thumbnailRatio = it },
+                    onThumbnailError = {
+                        thumbnailFailed = true
+                        if (failed) onError?.invoke()
+                    },
                     contentScale = ContentScale.Fit,
                     colorFilter = colorFilter,
                     placeholderContentColor = letterScheme?.onPrimaryContainer,
@@ -216,6 +225,7 @@ private fun BoxScope.CoverArtContent(
     thumbnailLoaded: Boolean,
     onThumbnailLoaded: () -> Unit,
     onThumbnailRatio: (Float) -> Unit,
+    onThumbnailError: () -> Unit,
     contentScale: ContentScale,
     colorFilter: ColorFilter?,
     placeholderContentColor: Color?,
@@ -254,6 +264,7 @@ private fun BoxScope.CoverArtContent(
                         ArtworkRatioCache.put(thumbnailRef.cacheKey, it)
                     }
             },
+            onError = { onThumbnailError() },
             modifier = Modifier.fillMaxSize(),
         )
     }

app/src/main/kotlin/com/subtracks/ui/home/HomeScreen.kt

@@ -320,7 +320,7 @@ fun OfflineHomeScreen(
         ) {
             homeTitle()
             downloadedAlbumRow(albums, coverArt, onAlbumClick, onLongClick) { onMore(OfflineListKind.Albums) }
-            downloadedArtistRow(artists, coverArt, artistArt, onArtistClick, onLongClick) { onMore(OfflineListKind.Artists) }
+            downloadedArtistRow(artists, artistArt, onArtistClick, onLongClick) { onMore(OfflineListKind.Artists) }
             downloadedPlaylistRow(playlists, coverArt, onPlaylistClick, onLongClick) { onMore(OfflineListKind.Playlists) }
             songList(
                 key = "downloaded-songs",
@@ -372,7 +372,6 @@ private fun LazyListScope.downloadedAlbumRow(
 
 private fun LazyListScope.downloadedArtistRow(
     artists: LazyPagingItems<Artist>,
-    coverArt: (String?, Boolean) -> CoverArtRef?,
     artistArt: (Artist) -> ArtistArt,
     onArtistClick: (Artist) -> Unit,
     onLongClick: (MenuTarget) -> Unit,

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

@@ -286,7 +286,6 @@ fun ArtistDetailScreen(
     bioExpanded: Boolean = false,
     art: CoverArtRef?,
     artThumbnail: CoverArtRef? = null,
-    artCovers: List<CoverArtRef> = emptyList(),
     heroArt: ArtistArt? = null,
     artwork: ArtworkColors? = null,
     coverArt: (String?, Boolean) -> CoverArtRef?,
@@ -406,13 +405,8 @@ fun ArtistDetailScreen(
                         ) {
                             Box(Modifier.fillMaxWidth()) {
                                 ArtistHeroArt(
-                                    art =
+                                    art = heroArt ?: ArtistArt(image = art, thumbnail = artThumbnail),
-                                        heroArt ?: ArtistArt(
-                                            image = art.takeIf { artCovers.isEmpty() },
-                                            covers = artCovers,
-                                        ),
                                     name = displayName,
-                                    thumbnail = artThumbnail,
                                     modifier = Modifier.fillMaxWidth().height(ART_HEIGHT),
                                 )
                                 if (nameScrimAlpha > 0f) {

app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt

@@ -682,6 +682,27 @@ class LibraryDaoTest {
             assertEquals(listOf("art-2", "art-1"), dao.artistOnce(sourceId, "ar-1")!!.downloadedCoverArts)
         }
 
+    @Test
+    fun artistArtDeduplicatesCoversBeforeApplyingTheFourCoverBudget() =
+        runTest {
+            val sourceId = source()
+            val dao = db.libraryDao()
+            dao.upsertArtists(listOf(artist(sourceId, "ar-1", "Artist", albumCount = 6)))
+            dao.upsertAlbums(
+                listOf("A", "A", "O", "B", "C", "D").mapIndexed { index, cover ->
+                    album(sourceId, "al-$index", "Album $index", year = 2021L - index, starred = null)
+                        .copy(artistId = "ar-1", coverArt = cover)
+                },
+            )
+
+            dao.recomputeArtistArt(sourceId)
+
+            assertEquals(
+                listOf("A", "O", "B", "C"),
+                dao.artistOnce(sourceId, "ar-1")!!.coverArts,
+            )
+        }
+
     @Test
     fun artistArtMaterializationUsesTheCreditIndexes() =
         runTest {

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

@@ -346,7 +346,10 @@ class DownloadRepositoryTest {
         runTest {
             seedLibrary()
             writeFile(1, "s1", "audio")
-            runBlocking { repository.download(1, "s1") }
+            runBlocking {
+                repository.download(1, "s1")
+                db.libraryDao().upsertArtists(listOf(artist(ARTIST_ART).copy(downloadedCoverArts = listOf(ALBUM_ART))))
+            }
             val id = engine.requests.single().first
 
             runBlocking { repository.removeSource(1) }
@@ -354,6 +357,13 @@ class DownloadRepositoryTest {
             assertTrue(engine.cancelled.contains(id))
             assertFalse(file(1, "s1").exists())
             assertTrue(allRows().isEmpty())
+            assertTrue(
+                db
+                    .libraryDao()
+                    .artistOnce(1, "ar1")!!
+                    .downloadedCoverArts
+                    .isEmpty(),
+            )
         }
 
     @Test

app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt

@@ -334,11 +334,14 @@ class AppScreenshotTest {
                         ),
                     art = CoverArtRef("art-al-kid-a", "test:art-al-kid-a"),
                     artThumbnail = CoverArtRef("art-al-kid-a", "test:art-al-kid-a"),
-                    artCovers =
+                    heroArt =
-                        Fixtures.albums
+                        ArtistArt(
-                            .take(3)
+                            covers =
-                            .mapNotNull { it.coverArt }
+                                Fixtures.albums
-                            .map { CoverArtRef(it, "test:$it") },
+                                    .take(3)
+                                    .mapNotNull { it.coverArt }
+                                    .map { CoverArtRef(it, "test:$it") },
+                        ),
                     artwork = artworkColorsFromSeed(Color.rgb(120, 80, 200)),
                     coverArt = { id, _ -> id?.let { CoverArtRef(it, "test:$it") } },
                     onBack = {},

app/src/test/kotlin/com/subtracks/ui/components/ArtistArtTest.kt

@@ -76,8 +76,16 @@ class ArtistArtTest {
         }
         val artistImage: (String, String?) -> CoverArtRef? = { _, _ -> null }
 
-        resolveArtistArt(artist(coverArts = listOf("a")), coverArt, artistImage, offline = false, forIcon = false)
+        val single =
-        assertEquals(listOf(false), thumbnails)
+            resolveArtistArt(
+                artist(coverArts = listOf("a")),
+                coverArt,
+                artistImage,
+                offline = false,
+                forIcon = false,
+            )
+        assertEquals(listOf(true, false), thumbnails)
+        assertEquals("a", single.thumbnail?.url)
 
         thumbnails.clear()
         resolveArtistArt(artist(coverArts = listOf("a", "b")), coverArt, artistImage, offline = false, forIcon = false)