fix(home): hold the updating veil until Home actually settles — #2327
android / Build + lint + test (push) Failing after 3m7s

The "Updating your mixes…" veil wiped on and straight back off before the
update finished, and a number of churn paths never raised it at all.

Three reasons it lowered early. refreshBehindVeil held it for
refresh().join() + a flat 500ms, but finishing the network pull is nowhere
near the end of the visible work: refreshIndex writes only the section id
lists, then each tile hydrates through MetadataProvider (null → skeleton →
album), and only then does the cover art load. Second, updatingInternal was
a plain Boolean cleared in a finally — reconnect and playlist.system_rebuilt
routinely arrive together, so whichever pull finished first wiped the veil
off while the other was still running. Third, refresh() swallowed every
failure in runCatching, so join() returned "fine" after a failed pull: veil
off, content unchanged, no retry.

So the veil's lifetime is now driven by watching the screen instead of by a
guess. UpdateVeilController raises, runs the work (retrying behind the veil),
then holds until the content signature has been unchanged for a quiet window
AND nothing is still loading — floored by a minimum hold so it cannot flash,
capped by a hard ceiling so it cannot strand, and with overlapping triggers
folded into one session rather than racing it. Giving up is silent and sets
no latch: the reconnect-driven recovery and the freshness sweeper keep
retrying afterwards exactly as before.

Cover art was the most visible pop-in and the refresh coroutine cannot see
it, so the composition reports it upward: ServerImage — the single choke
point behind CoverTile for every album/artist/playlist cover — counts its
in-flight loads into an ArtSettleTracker the veil waits on. Art also
crossfades now (set once on the ImageLoader, so it applies app-wide) with
the placeholder fading out over the same window, which softens the pop
everywhere the veil isn't involved.

Underneath all of it, the churn is largely no longer generated. replaceSection
was delete-then-insert per section, un-transacted, so observeBySection emitted
emptyList() — a visible collapse — before refilling, seven times in sequence.
It is now one @Transaction across all sections (Room notifies once, on commit,
so the empty gap is never observed), and the index flow dedups on the id list,
so a section whose contents did not move no longer tears down and rebuilds
every tile's hydration flow. fetchedAt is restamped on every write, which is
why the dedup compares ids rather than rows. Same fix CachedQuarantineDao
already carried for the same reason.

Trigger set widened per the operator's call: the initial load over a warm
cache (a full re-pull that churned every section completely unveiled), manual
pull-to-refresh, scan.run_finished (Home never reacted to it at all), and the
playlist.created/updated/deleted/tracks_changed kinds. The veil waits for
content to be on screen before raising, so a genuinely cold load still gets
its skeleton rather than an opaque panel over nothing.

refreshError is now cleared on success rather than at the start of each
attempt — with retries, clearing it up front made a failing cold start flash
the "Welcome to Minstrel" empty state between attempts.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-07-31 20:31:30 -04:00
co-authored by Claude Opus 5
parent fa0827f668
commit 5044e7a055
8 changed files with 822 additions and 110 deletions
@@ -6,6 +6,7 @@ import androidx.work.Configuration
import coil3.ImageLoader import coil3.ImageLoader
import coil3.SingletonImageLoader import coil3.SingletonImageLoader
import coil3.network.okhttp.OkHttpNetworkFetcherFactory import coil3.network.okhttp.OkHttpNetworkFetcherFactory
import coil3.request.crossfade
import com.fabledsword.minstrel.cache.CacheIndexer import com.fabledsword.minstrel.cache.CacheIndexer
import com.fabledsword.minstrel.cache.mutations.MutationReplayer import com.fabledsword.minstrel.cache.mutations.MutationReplayer
import com.fabledsword.minstrel.cache.sync.SyncController import com.fabledsword.minstrel.cache.sync.SyncController
@@ -29,6 +30,10 @@ import okhttp3.OkHttpClient
import timber.log.Timber import timber.log.Timber
import javax.inject.Inject import javax.inject.Inject
// Cover-art fade-in. Coil skips the transition for memory-cache hits, so
// already-loaded art still appears instantly — only a genuine fetch fades.
private const val ART_CROSSFADE_MS = 220
@HiltAndroidApp @HiltAndroidApp
class MinstrelApplication : class MinstrelApplication :
Application(), Application(),
@@ -213,11 +218,18 @@ class MinstrelApplication :
* OkHttp client as the network fetcher. The `callFactory` lambda * OkHttp client as the network fetcher. The `callFactory` lambda
* is invoked lazily so Hilt has time to inject `okHttpClient` * is invoked lazily so Hilt has time to inject `okHttpClient`
* before Coil makes its first request. * before Coil makes its first request.
*
* Crossfade is set here rather than per-call so every cover surface
* in the app fades its artwork in instead of snapping it. Art
* landing a beat after its tile was the most visible pop-in on Home
* (issue #2327); `ServerImage` fades its placeholder out over the
* same window so the two read as one cross-dissolve.
*/ */
override fun newImageLoader(context: android.content.Context): ImageLoader = override fun newImageLoader(context: android.content.Context): ImageLoader =
ImageLoader.Builder(context) ImageLoader.Builder(context)
.components { .components {
add(OkHttpNetworkFetcherFactory(callFactory = { okHttpClient })) add(OkHttpNetworkFetcherFactory(callFactory = { okHttpClient }))
} }
.crossfade(ART_CROSSFADE_MS)
.build() .build()
} }
@@ -4,6 +4,7 @@ import androidx.room.Dao
import androidx.room.Insert import androidx.room.Insert
import androidx.room.OnConflictStrategy import androidx.room.OnConflictStrategy
import androidx.room.Query import androidx.room.Query
import androidx.room.Transaction
import com.fabledsword.minstrel.cache.db.entities.CachedHomeIndexEntity import com.fabledsword.minstrel.cache.db.entities.CachedHomeIndexEntity
import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.Flow
@@ -21,12 +22,32 @@ interface CachedHomeIndexDao {
) )
suspend fun getBySection(section: String): List<CachedHomeIndexEntity> suspend fun getBySection(section: String): List<CachedHomeIndexEntity>
/** True when Home has any cached section rows to render. */
@Query("SELECT EXISTS(SELECT 1 FROM cached_home_index)")
suspend fun hasAny(): Boolean
@Insert(onConflict = OnConflictStrategy.REPLACE) @Insert(onConflict = OnConflictStrategy.REPLACE)
suspend fun upsertAll(rows: List<CachedHomeIndexEntity>) suspend fun upsertAll(rows: List<CachedHomeIndexEntity>)
/** Replace-all pattern; sync wipes a section then re-inserts. */ @Query("DELETE FROM cached_home_index WHERE section IN (:sections)")
@Query("DELETE FROM cached_home_index WHERE section = :section") suspend fun deleteSections(sections: List<String>)
suspend fun deleteBySection(section: String)
/**
* Swaps every listed section's rows in ONE transaction.
*
* Atomicity is the point, not just tidiness: Room's
* InvalidationTracker only notifies observers after the transaction
* commits, so [observeBySection] never sees the empty gap between the
* delete and the re-insert. Replacing sections one at a time (and
* un-transacted) made each Home row emit `emptyList()` — visibly
* collapsing — before refilling, and made the seven sections do it in
* sequence rather than as a single content swap.
*/
@Transaction
suspend fun replaceSections(sections: List<String>, rows: List<CachedHomeIndexEntity>) {
deleteSections(sections)
if (rows.isNotEmpty()) upsertAll(rows)
}
@Query("DELETE FROM cached_home_index") @Query("DELETE FROM cached_home_index")
suspend fun clear() suspend fun clear()
@@ -14,8 +14,10 @@ import com.fabledsword.minstrel.models.TrackRef
import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flatMapLatest
import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.flow.map
import retrofit2.Retrofit import retrofit2.Retrofit
import retrofit2.create import retrofit2.create
import javax.inject.Inject import javax.inject.Inject
@@ -34,9 +36,10 @@ import javax.inject.Singleton
* reveals when the fetch lands and Room re-emits. Mirrors Flutter's * reveals when the fetch lands and Room re-emits. Mirrors Flutter's
* per-item tile providers. * per-item tile providers.
* *
* `refreshIndex()` pulls `GET /api/home/index`, replaces each section * `refreshIndex()` pulls `GET /api/home/index`, swaps all sections in
* in-place (delete-then-insert, so the section Flows re-fire), and * one transaction (so the rows update together in a single emission
* pre-warms the top artists via [HomeArtistPrewarmer]. * rather than collapsing and refilling), and pre-warms the top artists
* via [HomeArtistPrewarmer].
*/ */
@Singleton @Singleton
// Per-section observe accessors (one per Home row) inflate the function // Per-section observe accessors (one per Home row) inflate the function
@@ -85,72 +88,98 @@ class HomeRepository @Inject constructor(
fun observeYouMightLikeArtists(): Flow<List<HomeTile<ArtistRef>>> = fun observeYouMightLikeArtists(): Flow<List<HomeTile<ArtistRef>>> =
observeArtistSection(SECTION_YOU_MIGHT_LIKE_ARTISTS) observeArtistSection(SECTION_YOU_MIGHT_LIKE_ARTISTS)
/** True when the index cache already has content on screen to protect. */
suspend fun hasCachedIndex(): Boolean = homeIndexDao.hasAny()
/** /**
* Pulls /api/home/index, replaces each cached_home_index section, * Pulls /api/home/index and swaps every cached_home_index section in
* and pre-warms the top artists. The section Flows re-fire on the * a single transaction, then pre-warms the top artists. Missing
* index change; missing entity rows hydrate via the on-miss path. * entity rows hydrate via the on-miss path.
*
* One transaction for all seven sections is deliberate: Room notifies
* observers once, on commit, so Home swaps from the old content to
* the new in a single emission. Per-section, un-transacted writes
* made every row visibly collapse to empty and refill, one after
* another (issue #2327).
*/ */
suspend fun refreshIndex() { suspend fun refreshIndex() {
val wire = api.getHomeIndex() val wire = api.getHomeIndex()
replaceSection(SECTION_RECENTLY_ADDED_ALBUMS, "album", wire.recentlyAddedAlbums) homeIndexDao.replaceSections(
replaceSection(SECTION_REDISCOVER_ALBUMS, "album", wire.rediscoverAlbums) sections = ALL_SECTIONS,
replaceSection(SECTION_REDISCOVER_ARTISTS, "artist", wire.rediscoverArtists) rows = rowsFor(SECTION_RECENTLY_ADDED_ALBUMS, "album", wire.recentlyAddedAlbums) +
replaceSection(SECTION_MOST_PLAYED_TRACKS, "track", wire.mostPlayedTracks) rowsFor(SECTION_REDISCOVER_ALBUMS, "album", wire.rediscoverAlbums) +
replaceSection(SECTION_LAST_PLAYED_ARTISTS, "artist", wire.lastPlayedArtists) rowsFor(SECTION_REDISCOVER_ARTISTS, "artist", wire.rediscoverArtists) +
replaceSection(SECTION_YOU_MIGHT_LIKE_ALBUMS, "album", wire.youMightLikeAlbums) rowsFor(SECTION_MOST_PLAYED_TRACKS, "track", wire.mostPlayedTracks) +
replaceSection(SECTION_YOU_MIGHT_LIKE_ARTISTS, "artist", wire.youMightLikeArtists) rowsFor(SECTION_LAST_PLAYED_ARTISTS, "artist", wire.lastPlayedArtists) +
rowsFor(SECTION_YOU_MIGHT_LIKE_ALBUMS, "album", wire.youMightLikeAlbums) +
rowsFor(SECTION_YOU_MIGHT_LIKE_ARTISTS, "artist", wire.youMightLikeArtists),
)
prewarmer.warm( prewarmer.warm(
wire.rediscoverArtists + wire.lastPlayedArtists + wire.youMightLikeArtists, wire.rediscoverArtists + wire.lastPlayedArtists + wire.youMightLikeArtists,
) )
} }
private suspend fun replaceSection(section: String, entityType: String, ids: List<String>) { private fun rowsFor(
homeIndexDao.deleteBySection(section) section: String,
if (ids.isEmpty()) return entityType: String,
homeIndexDao.upsertAll( ids: List<String>,
ids.mapIndexed { index, id -> ): List<CachedHomeIndexEntity> = ids.mapIndexed { index, id ->
CachedHomeIndexEntity( CachedHomeIndexEntity(
section = section, section = section,
position = index, position = index,
entityType = entityType, entityType = entityType,
entityId = id, entityId = id,
) )
},
)
} }
/**
* The section's ordered entity ids, deduplicated.
*
* Room re-runs the query on every write to `cached_home_index` — and
* `CachedHomeIndexEntity.fetchedAt` is stamped fresh each time — so
* comparing whole rows would call every rewrite a change. Comparing
* the id list instead means a section whose contents didn't actually
* move never restarts the `flatMapLatest` below, which would
* otherwise tear down and rebuild all of its tiles' hydration flows
* and flicker unchanged tiles (issue #2327).
*/
private fun observeSectionIds(section: String): Flow<List<String>> =
homeIndexDao.observeBySection(section)
.map { rows -> rows.map { it.entityId } }
.distinctUntilChanged()
@OptIn(ExperimentalCoroutinesApi::class) @OptIn(ExperimentalCoroutinesApi::class)
private fun observeAlbumSection(section: String): Flow<List<HomeTile<AlbumRef>>> = private fun observeAlbumSection(section: String): Flow<List<HomeTile<AlbumRef>>> =
homeIndexDao.observeBySection(section).flatMapLatest { rows -> observeSectionIds(section).flatMapLatest { ids ->
if (rows.isEmpty()) { if (ids.isEmpty()) {
flowOf(emptyList()) flowOf(emptyList())
} else { } else {
combine(rows.map { metadataProvider.observeAlbum(it.entityId) }) { refs -> combine(ids.map { metadataProvider.observeAlbum(it) }) { refs ->
rows.mapIndexed { i, r -> HomeTile(r.entityId, refs[i]) } ids.mapIndexed { i, id -> HomeTile(id, refs[i]) }
} }
} }
} }
@OptIn(ExperimentalCoroutinesApi::class) @OptIn(ExperimentalCoroutinesApi::class)
private fun observeArtistSection(section: String): Flow<List<HomeTile<ArtistRef>>> = private fun observeArtistSection(section: String): Flow<List<HomeTile<ArtistRef>>> =
homeIndexDao.observeBySection(section).flatMapLatest { rows -> observeSectionIds(section).flatMapLatest { ids ->
if (rows.isEmpty()) { if (ids.isEmpty()) {
flowOf(emptyList()) flowOf(emptyList())
} else { } else {
combine(rows.map { metadataProvider.observeArtist(it.entityId) }) { refs -> combine(ids.map { metadataProvider.observeArtist(it) }) { refs ->
rows.mapIndexed { i, r -> HomeTile(r.entityId, refs[i]) } ids.mapIndexed { i, id -> HomeTile(id, refs[i]) }
} }
} }
} }
@OptIn(ExperimentalCoroutinesApi::class) @OptIn(ExperimentalCoroutinesApi::class)
private fun observeTrackSection(section: String): Flow<List<HomeTile<TrackRef>>> = private fun observeTrackSection(section: String): Flow<List<HomeTile<TrackRef>>> =
homeIndexDao.observeBySection(section).flatMapLatest { rows -> observeSectionIds(section).flatMapLatest { ids ->
if (rows.isEmpty()) { if (ids.isEmpty()) {
flowOf(emptyList()) flowOf(emptyList())
} else { } else {
combine(rows.map { metadataProvider.observeTrack(it.entityId) }) { refs -> combine(ids.map { metadataProvider.observeTrack(it) }) { refs ->
rows.mapIndexed { i, r -> HomeTile(r.entityId, refs[i]) } ids.mapIndexed { i, id -> HomeTile(id, refs[i]) }
} }
} }
} }
@@ -163,5 +192,16 @@ class HomeRepository @Inject constructor(
const val SECTION_LAST_PLAYED_ARTISTS = "last_played_artists" const val SECTION_LAST_PLAYED_ARTISTS = "last_played_artists"
const val SECTION_YOU_MIGHT_LIKE_ALBUMS = "you_might_like_albums" const val SECTION_YOU_MIGHT_LIKE_ALBUMS = "you_might_like_albums"
const val SECTION_YOU_MIGHT_LIKE_ARTISTS = "you_might_like_artists" const val SECTION_YOU_MIGHT_LIKE_ARTISTS = "you_might_like_artists"
/** Every section [refreshIndex] owns — the unit of one atomic swap. */
val ALL_SECTIONS = listOf(
SECTION_RECENTLY_ADDED_ALBUMS,
SECTION_REDISCOVER_ALBUMS,
SECTION_REDISCOVER_ARTISTS,
SECTION_MOST_PLAYED_TRACKS,
SECTION_LAST_PLAYED_ARTISTS,
SECTION_YOU_MIGHT_LIKE_ALBUMS,
SECTION_YOU_MIGHT_LIKE_ARTISTS,
)
} }
} }
@@ -43,6 +43,7 @@ import androidx.compose.material3.SnackbarHost
import androidx.compose.material3.SnackbarHostState import androidx.compose.material3.SnackbarHostState
import androidx.compose.material3.Text import androidx.compose.material3.Text
import androidx.compose.runtime.Composable import androidx.compose.runtime.Composable
import androidx.compose.runtime.CompositionLocalProvider
import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.getValue import androidx.compose.runtime.getValue
import androidx.compose.runtime.remember import androidx.compose.runtime.remember
@@ -85,10 +86,14 @@ import com.fabledsword.minstrel.playlists.widgets.OfflinePoolCard
import com.fabledsword.minstrel.playlists.widgets.PlaylistCard import com.fabledsword.minstrel.playlists.widgets.PlaylistCard
import com.fabledsword.minstrel.playlists.widgets.PlaylistPlaceholderCard import com.fabledsword.minstrel.playlists.widgets.PlaylistPlaceholderCard
import com.fabledsword.minstrel.shared.UiState import com.fabledsword.minstrel.shared.UiState
import com.fabledsword.minstrel.shared.UpdateVeilController
import com.fabledsword.minstrel.shared.VeilSettleState
import com.fabledsword.minstrel.shared.asCacheFirstStateFlow import com.fabledsword.minstrel.shared.asCacheFirstStateFlow
import com.fabledsword.minstrel.shared.widgets.ArtSettleTracker
import com.fabledsword.minstrel.shared.widgets.EmptyState import com.fabledsword.minstrel.shared.widgets.EmptyState
import com.fabledsword.minstrel.shared.widgets.ErrorRetry import com.fabledsword.minstrel.shared.widgets.ErrorRetry
import com.fabledsword.minstrel.shared.widgets.HorizontalScrollRow import com.fabledsword.minstrel.shared.widgets.HorizontalScrollRow
import com.fabledsword.minstrel.shared.widgets.LocalArtSettleTracker
import com.fabledsword.minstrel.shared.widgets.MinstrelTopAppBar import com.fabledsword.minstrel.shared.widgets.MinstrelTopAppBar
import com.fabledsword.minstrel.shared.widgets.PullToRefreshScaffold import com.fabledsword.minstrel.shared.widgets.PullToRefreshScaffold
import com.fabledsword.minstrel.shared.widgets.SkeletonAlbumTile import com.fabledsword.minstrel.shared.widgets.SkeletonAlbumTile
@@ -96,19 +101,22 @@ import com.fabledsword.minstrel.shared.widgets.SkeletonArtistTile
import com.fabledsword.minstrel.shared.widgets.SkeletonSectionHeader import com.fabledsword.minstrel.shared.widgets.SkeletonSectionHeader
import dagger.hilt.android.lifecycle.HiltViewModel import dagger.hilt.android.lifecycle.HiltViewModel
import kotlinx.coroutines.Job import kotlinx.coroutines.Job
import kotlinx.coroutines.async
import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.delay import kotlinx.coroutines.coroutineScope
import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.flow.receiveAsFlow
import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import kotlinx.coroutines.withTimeoutOrNull
import javax.inject.Inject import javax.inject.Inject
private const val SHARE_STOP_TIMEOUT_MS = 5_000L private const val SHARE_STOP_TIMEOUT_MS = 5_000L
@@ -120,16 +128,20 @@ private const val BOTTOM_PADDING_FOR_MINIPLAYER_DP = 140
private const val RECENTLY_ADDED_GRID_ROWS = 2 private const val RECENTLY_ADDED_GRID_ROWS = 2
private const val RECENTLY_ADDED_GRID_HEIGHT_DP = 440 private const val RECENTLY_ADDED_GRID_HEIGHT_DP = 440
// "Updating your mixes…" veil (automatic refresh). Held through the pull // "Updating your mixes…" veil. Its lifetime is decided by
// plus VEIL_SETTLE_MS so per-tile hydration lands behind it before it wipes // UpdateVeilController watching the screen settle — not by a fixed delay,
// off; near-opaque (VEIL_ALPHA) so the section churn never bleeds through. // which lowered it while tiles and artwork were still landing (#2327).
private const val VEIL_SETTLE_MS = 500L // Near-opaque (VEIL_ALPHA) so the section churn never bleeds through.
private const val VEIL_WIPE_MS = 280 private const val VEIL_WIPE_MS = 280
private const val VEIL_ALPHA = 0.96f private const val VEIL_ALPHA = 0.96f
private const val VEIL_SPINNER_DP = 22 private const val VEIL_SPINNER_DP = 22
private const val VEIL_SPINNER_STROKE_DP = 2 private const val VEIL_SPINNER_STROKE_DP = 2
private const val VEIL_LABEL_GAP_DP = 12 private const val VEIL_LABEL_GAP_DP = 12
// How long a manual pull keeps its own indicator while waiting for the
// veil to take over, so the two don't both vanish for a frame mid-handoff.
private const val PULL_HANDOFF_TIMEOUT_MS = 2_000L
// ─── State ─────────────────────────────────────────────────────────── // ─── State ───────────────────────────────────────────────────────────
data class HomeSections( data class HomeSections(
@@ -197,38 +209,13 @@ class HomeViewModel @Inject constructor(
*/ */
private val refreshError = MutableStateFlow<String?>(null) private val refreshError = MutableStateFlow<String?>(null)
private val updatingInternal = MutableStateFlow(false)
/** /**
* True while an automatic background refresh (the 03:00 daily rebuild * Cover-art loads in flight on Home, reported by every [ServerImage]
* or a reconnect re-pull) is repopulating Home. Drives the "Updating * under [LocalArtSettleTracker]. The veil waits on this so artwork
* your mixes…" veil so the section churn — delete-then-insert in * arriving a beat after its tile lands behind the veil rather than
* [HomeRepository.refreshIndex] plus per-tile hydration — happens * popping in on screen.
* hidden behind the veil instead of on screen. Manual pull-to-refresh
* and cold start are NOT veiled (they own the pull spinner / skeleton).
*/ */
val isUpdating: StateFlow<Boolean> = updatingInternal.asStateFlow() val artTracker = ArtSettleTracker()
init {
refresh()
// Screen-level auto-recovery (issue #1245): a Home that failed to
// load while the server was unreachable re-pulls itself the moment
// health returns — same idiom as SyncController, one layer up.
// Veiled: content is already on screen and would otherwise churn.
viewModelScope.launch {
networkStatus.recoveries().collect { refreshBehindVeil() }
}
// #968: the daily 03:00 rebuild (and manual refresh) emit
// playlist.system_rebuilt; re-pull Home so the system-playlist tiles
// and You-might-like rows reflect the new snapshot without a manual
// reload. Mirrors the web SSE consumer. Veiled so the multi-section
// rebuild churn hides behind "Updating your mixes…".
viewModelScope.launch {
eventsStream.events
.filter { it.kind == "playlist.system_rebuilt" }
.collect { refreshBehindVeil() }
}
}
/** /**
* Tap an offline pool: shuffle + play its cached tracks. Empty * Tap an offline pool: shuffle + play its cached tracks. Empty
@@ -334,47 +321,50 @@ class HomeViewModel @Inject constructor(
} }
/** /**
* Pulls both /home/index and the playlists list. Returns the Job * Pulls /home/index, the playlists list and the system-playlist
* for the combined refresh so a pull-to-refresh wrapper can await * status. Returns true when the load-bearing /home/index pull
* actual completion before hiding the indicator. * succeeded — the veil controller retries on false, so this must
* report failure rather than swallow it the way the fire-and-forget
* [refresh] entry point does.
*/ */
fun refresh(): Job = viewModelScope.launch { private suspend fun runRefresh(): Boolean = coroutineScope {
refreshError.value = null
val home = launch {
// /home/index is the load-bearing pull: its failure drives the // /home/index is the load-bearing pull: its failure drives the
// empty-cache Error state. A failure over a populated cache // empty-cache Error state. A failure over a populated cache
// stays silent — cached sections beat a full-screen error. // stays silent — cached sections beat a full-screen error.
//
// Cleared on success, NOT at the start of each attempt: with the
// veil's retries, clearing up front made a failing cold start
// flash the "Welcome to Minstrel" empty state (empty cache + no
// error reads as Empty) between one attempt and the next.
val home = async {
runCatching { homeRepository.refreshIndex() } runCatching { homeRepository.refreshIndex() }
.onSuccess { refreshError.value = null }
.onFailure { refreshError.value = ErrorCopy.fromThrowable(it) } .onFailure { refreshError.value = ErrorCopy.fromThrowable(it) }
.isSuccess
} }
val lists = launch { runCatching { playlistsRepository.refreshList() } } val lists = launch { runCatching { playlistsRepository.refreshList() } }
val status = launch { val status = launch {
runCatching { homeRepository.getSystemPlaylistsStatus() } runCatching { homeRepository.getSystemPlaylistsStatus() }
.onSuccess { systemStatusInternal.value = it } .onSuccess { systemStatusInternal.value = it }
} }
home.join()
lists.join() lists.join()
status.join() status.join()
home.await()
} }
/** Unveiled refresh, for the Error state's explicit Retry button. */
fun refresh(): Job = viewModelScope.launch { runRefresh() }
/** /**
* Automatic background refresh with the "Updating your mixes…" veil * Manual pull-to-refresh. Goes behind the veil like every other
* raised (see [isUpdating]). Used by the daily-rebuild + reconnect * refresh (operator call, 2026-07-31: the churn a pull causes is
* paths where Home is already on screen. Holds the veil through the * identical to the automatic paths, and a small spinner didn't hide
* pull plus a short settle so per-tile hydration lands behind it, then * it). Suspends only long enough to hand the indicator off to the
* lets it wipe off. Overlapping automatic refreshes are rare enough * veil; the refresh itself continues in the controller's session.
* (once-daily rebuild, reconnect) that a plain flag beats a counter.
*/ */
private fun refreshBehindVeil() { suspend fun refreshFromPull() {
viewModelScope.launch { veil.request()
updatingInternal.value = true withTimeoutOrNull(PULL_HANDOFF_TIMEOUT_MS) { veil.visible.first { it } }
try {
refresh().join()
delay(VEIL_SETTLE_MS)
} finally {
updatingInternal.value = false
}
}
} }
val uiState: StateFlow<UiState<HomeSections>> = val uiState: StateFlow<UiState<HomeSections>> =
@@ -425,6 +415,100 @@ class HomeViewModel @Inject constructor(
else -> UiState.Empty else -> UiState.Empty
} }
} }
// ─── Updating veil ───────────────────────────────────────────────
// Declared after uiState: these initialisers read it, and Kotlin runs
// property initialisers and init blocks in declaration order.
/**
* What the veil watches to decide Home has stopped moving: the whole
* rendered state, plus how many covers are still loading.
*
* [UiState.Success] wraps a [HomeSections] data class, so any visible
* change — a section swapping ids, one tile hydrating from skeleton to
* album — changes this value and re-arms the veil's quiet window.
*
* Unhydrated tiles deliberately do NOT gate `quiescent`. A tile whose
* on-miss fetch soft-fails keeps a null value indefinitely
* ([MetadataProvider] swallows those errors), so treating "no
* skeletons left" as the settle condition would pin the veil to its
* hard ceiling on every refresh. They're covered by the content key
* instead: each tile that lands re-arms the window, and once they stop
* landing the screen is genuinely still.
*/
private val settleSignal: Flow<VeilSettleState> =
combine(uiState, artTracker.inFlight) { state, artInFlight ->
VeilSettleState(
contentKey = state,
hasContent = state is UiState.Success,
quiescent = artInFlight == 0,
)
}
private val veil = UpdateVeilController(
scope = viewModelScope,
settleSignal = settleSignal,
shouldVeil = {
// Only worth hiding churn when there's already content to
// hide. A cold load over an empty cache keeps its skeleton —
// veiling that would replace a useful affordance with an
// opaque panel. `hasCachedIndex` is the honest check: uiState
// still reads Loading until the screen subscribes, so on a
// process restore over a warm cache it would say "no content"
// right before the cache emits.
uiState.value is UiState.Success || homeRepository.hasCachedIndex()
},
work = ::runRefresh,
)
/**
* True while the "Updating your mixes…" veil should be raised. The
* controller holds it until Home actually settles — sections swapped,
* tiles hydrated, artwork loaded — instead of for a fixed delay after
* the network pull returns (issue #2327).
*/
val isUpdating: StateFlow<Boolean> = veil.visible
init {
// Every refresh path goes through the controller, which decides
// per session whether to raise the veil. That includes the initial
// load: over a warm cache it's a full re-pull that churns every
// section, and it used to run completely unveiled.
veil.request()
// Screen-level auto-recovery (issue #1245): a Home that failed to
// load while the server was unreachable re-pulls itself the moment
// health returns — same idiom as SyncController, one layer up.
// This is also the recovery that keeps trying after the veil has
// given up and lowered; the controller sets no latch against it.
viewModelScope.launch {
networkStatus.recoveries().collect { veil.request() }
}
// Server-side changes that rewrite what Home renders (#968 and
// the 2026-07-31 widening) re-pull behind the veil. Mirrors the
// web SSE consumer.
viewModelScope.launch {
eventsStream.events
.filter { it.kind in VEILED_EVENT_KINDS }
.collect { veil.request() }
}
}
private companion object {
/**
* Events that change what Home shows. `playlist.system_rebuilt`
* is the 03:00 daily rebuild; the other `playlist.*` kinds move
* the Playlists and Songs-like rows; `scan.run_finished` changes
* Recently added (and Home never reacted to it at all before).
*/
private val VEILED_EVENT_KINDS = setOf(
"playlist.system_rebuilt",
"playlist.created",
"playlist.updated",
"playlist.deleted",
"playlist.tracks_changed",
"scan.run_finished",
)
}
} }
// ─── Screen ────────────────────────────────────────────────────────── // ─── Screen ──────────────────────────────────────────────────────────
@@ -454,14 +538,20 @@ fun HomeScreen(
val offline by viewModel.offline.collectAsStateWithLifecycle() val offline by viewModel.offline.collectAsStateWithLifecycle()
val updating by viewModel.isUpdating.collectAsStateWithLifecycle() val updating by viewModel.isUpdating.collectAsStateWithLifecycle()
PullToRefreshScaffold( PullToRefreshScaffold(
onRefresh = { viewModel.refresh().join() }, onRefresh = { viewModel.refreshFromPull() },
modifier = Modifier.fillMaxSize().padding(inner), modifier = Modifier.fillMaxSize().padding(inner),
) { ) {
Box(Modifier.fillMaxSize()) { Box(Modifier.fillMaxSize()) {
// Every cover below reports its load state to the tracker,
// so the veil can wait for artwork instead of guessing.
CompositionLocalProvider(
LocalArtSettleTracker provides viewModel.artTracker,
) {
HomeStateCrossfade(state, systemStatus, offline, navController, viewModel) HomeStateCrossfade(state, systemStatus, offline, navController, viewModel)
// Automatic-refresh veil: the daily rebuild / reconnect }
// churn hides behind an "Updating your mixes…" wipe. Manual // Refresh veil: rebuild / reconnect / pull / event churn all
// pull owns the PullToRefreshBox spinner instead. // hide behind an "Updating your mixes…" wipe that stays up
// until the screen has actually stopped moving.
UpdatingVeil(visible = updating) UpdatingVeil(visible = updating)
} }
} }
@@ -0,0 +1,215 @@
package com.fabledsword.minstrel.shared
import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.FlowPreview
import kotlinx.coroutines.NonCancellable
import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.debounce
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import kotlinx.coroutines.withTimeoutOrNull
// Once raised, the veil stays up at least this long. Without a floor a
// no-op refresh wipes on and straight back off, which reads as a glitch.
private const val DEFAULT_MIN_HOLD_MS = 900L
// The screen must stop changing for this long before the veil lowers.
// Every content change re-arms it, so a refresh that lands in stages
// (index → tile hydration → artwork) holds the veil across all of them.
private const val DEFAULT_QUIET_MS = 700L
// Hard ceiling on visibility. A refresh that never settles must not
// strand the user behind an opaque veil; the work itself is NOT capped.
private const val DEFAULT_MAX_HOLD_MS = 12_000L
// Attempts per session. Retrying behind the veil is the point: a pull
// that fails on the first try gets another go before the user sees
// anything, instead of the veil wiping off over unchanged content.
private const val DEFAULT_ATTEMPTS = 3
private const val DEFAULT_RETRY_BACKOFF_MS = 600L
/** Tunables for [UpdateVeilController]; defaults are the Home values. */
data class VeilTimings(
val minHoldMs: Long = DEFAULT_MIN_HOLD_MS,
val quietMs: Long = DEFAULT_QUIET_MS,
val maxHoldMs: Long = DEFAULT_MAX_HOLD_MS,
val attempts: Int = DEFAULT_ATTEMPTS,
val retryBackoffMs: Long = DEFAULT_RETRY_BACKOFF_MS,
)
/**
* A snapshot of everything that visibly moves on the veiled screen.
*
* @param contentKey any value whose equality tracks what's rendered — a
* change means the screen moved, and re-arms the quiet timer.
* @param hasContent true when real content (not a skeleton or an empty
* state) is on screen. The veil waits for this before raising: there's
* nothing to hide until there's something to hide.
* @param quiescent false while something is still landing (artwork
* loading, tiles hydrating). The veil will not lower until this is
* true, up to [VeilTimings.maxHoldMs].
*/
data class VeilSettleState(
val contentKey: Any?,
val hasContent: Boolean,
val quiescent: Boolean,
)
/**
* Drives an "updating" overlay from *observed content settling* rather
* than from a fixed delay.
*
* The problem this replaces: a veil held for `refresh().join() + 500ms`
* lowers while the screen is still moving, because finishing the network
* pull is nowhere near the end of the visible work — the pull writes id
* lists, then tiles hydrate one by one, then artwork loads. And a plain
* `isUpdating` Boolean set in a `finally` gets cleared by whichever of
* two overlapping refreshes finishes first, wiping the veil off mid-update
* (issue #2327).
*
* So instead: raise, run [work], then hold until [settleSignal] reports
* the screen has stopped changing for [VeilTimings.quietMs] AND is
* quiescent — bounded below by [VeilTimings.minHoldMs] so it can never
* flash, and above by [VeilTimings.maxHoldMs] so it can never strand.
*
* Overlapping triggers extend the running session instead of racing it,
* so the veil stays up continuously rather than lowering and re-raising.
*
* Failure is quiet by design: [work] gets [VeilTimings.attempts] tries
* behind the veil, and if they all fail the veil simply wipes off over
* the cached content with no error surfaced. Giving up here ends only
* *this* session — it sets no latch and blocks nothing, so the caller's
* own recovery paths (reconnect re-pull, freshness sweeps, the next
* event, a manual pull) keep retrying afterwards exactly as before.
*
* @param work one refresh attempt; returns true when it succeeded.
* @param shouldVeil sampled at session start — "is there cached content
* this refresh is about to overwrite?". False means a cold load, where
* a skeleton is the right affordance, and the work runs unveiled.
*/
class UpdateVeilController(
private val scope: CoroutineScope,
private val settleSignal: Flow<VeilSettleState>,
private val shouldVeil: suspend () -> Boolean,
private val timings: VeilTimings = VeilTimings(),
private val work: suspend () -> Boolean,
) {
private val visibleInternal = MutableStateFlow(false)
/** True while the veil should be drawn over the screen. */
val visible: StateFlow<Boolean> = visibleInternal.asStateFlow()
// Conflated: a burst of triggers (reconnect + rebuild event arriving
// together) collapses into one follow-up pass, not a queue of them.
private val requests = Channel<Unit>(Channel.CONFLATED)
init {
// One consumer, so sessions are serialised by construction: two
// triggers can never each own a piece of the veil's state.
scope.launch {
while (true) {
requests.receive()
runSession()
}
}
}
/**
* Ask for a refresh. Safe to call from any trigger at any rate —
* calls arriving during a session extend it rather than starting a
* competing one.
*/
fun request() {
requests.trySend(Unit)
}
private suspend fun runSession() {
if (!shouldVeil()) {
drainWork()
return
}
// Raise only once there's content on screen to hide. Over a warm
// cache that's within a frame or two of here — long before the
// network pull lands — so the churn still gets covered. But on a
// genuinely cold load, content appears only *because* this work
// produced it, and veiling that would delay first paint to hide
// nothing.
val raised = CompletableDeferred<Unit>()
val raiser = scope.launch {
settleSignal.first { it.hasContent }
visibleInternal.value = true
raised.complete(Unit)
}
// Floor and ceiling are measured from the raise, not the request,
// so a late raise still gets its full no-flash minimum.
val floor = scope.launch {
raised.await()
delay(timings.minHoldMs)
}
val ceiling = scope.launch {
raised.await()
delay(timings.maxHoldMs)
visibleInternal.value = false
}
try {
drainWork()
// Always wait for the settle, never conditionally on `visible`:
// work that finishes without suspending would otherwise reach
// here before the raiser has been dispatched, tear the session
// down, and leave the churn uncovered.
withTimeoutOrNull(timings.maxHoldMs) { awaitSettled() }
} finally {
raiser.cancel()
ceiling.cancel()
if (raised.isCompleted) {
// NonCancellable so the floor is honoured (and the veil
// always cleared) even while the scope is torn down; the
// floor job dies with the scope, so this cannot hang.
withContext(NonCancellable) { floor.join() }
}
floor.cancel()
visibleInternal.value = false
}
}
private suspend fun drainWork() {
do {
runWorkWithRetries()
// A trigger that arrived mid-session gets folded into this one.
} while (requests.tryReceive().isSuccess)
}
private suspend fun runWorkWithRetries() {
repeat(timings.attempts) { attempt ->
if (work()) return
if (attempt < timings.attempts - 1) {
delay(timings.retryBackoffMs * (attempt + 1))
}
}
}
/**
* Suspends until the screen has been unchanged for
* [VeilTimings.quietMs] and reports itself quiescent.
*
* `debounce` is what makes this hold across a staged update: every
* change restarts the window, so the veil lowers only once emissions
* actually stop. `first { quiescent }` then rejects a quiet-but-
* still-loading moment and waits for the next lull.
*/
@OptIn(FlowPreview::class)
private suspend fun awaitSettled() {
settleSignal
.distinctUntilChanged()
.debounce(timings.quietMs)
.first { it.quiescent }
}
}
@@ -0,0 +1,60 @@
package com.fabledsword.minstrel.shared.widgets
import androidx.compose.runtime.Stable
import androidx.compose.runtime.staticCompositionLocalOf
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.update
/**
* Counts the cover-art loads that are currently in flight, so a
* screen-level overlay can wait for the artwork to actually land instead
* of guessing with a fixed delay.
*
* Artwork is the most visible pop-in on Home: a tile can be fully
* hydrated (title, artist, counts all present) and still snap its cover
* in a second later, which is exactly the churn the "Updating your
* mixes…" veil exists to hide. The refresh coroutine can't see that —
* it finishes long before Coil does — so the composition reports it
* upward here instead.
*
* [ServerImage] reports into whatever tracker it finds in
* [LocalArtSettleTracker], which means every art surface in the app
* participates for free. Only *composed* images are counted, so a
* LazyRow's off-screen tiles are correctly ignored — the count tracks
* the pop-in a user can actually see.
*
* Provide one per screen that needs it (typically owned by the
* screen's ViewModel so its refresh logic can read [inFlight]):
*
* CompositionLocalProvider(LocalArtSettleTracker provides vm.artTracker) { ... }
*/
@Stable
class ArtSettleTracker {
private val inFlightInternal = MutableStateFlow(0)
/**
* How many on-screen images are still loading. Zero means the
* artwork has settled — every composed cover has either drawn or
* failed to a fallback.
*/
val inFlight: StateFlow<Int> = inFlightInternal.asStateFlow()
fun begin() {
inFlightInternal.update { it + 1 }
}
fun end() {
// Floor at zero: a decrement can outlive its increment when a
// tile is disposed mid-load and the count must not go negative
// and wedge "settled" off forever.
inFlightInternal.update { (it - 1).coerceAtLeast(0) }
}
}
/**
* The tracker [ServerImage] reports load state to, or null on screens
* that don't care (the default) — reporting is then a no-op.
*/
val LocalArtSettleTracker = staticCompositionLocalOf<ArtSettleTracker?> { null }
@@ -1,25 +1,40 @@
package com.fabledsword.minstrel.shared.widgets package com.fabledsword.minstrel.shared.widgets
import androidx.compose.animation.core.animateFloatAsState
import androidx.compose.animation.core.tween
import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.runtime.Composable import androidx.compose.runtime.Composable
import androidx.compose.runtime.DisposableEffect
import androidx.compose.runtime.getValue import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue import androidx.compose.runtime.setValue
import androidx.compose.ui.Alignment import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.alpha
import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.layout.ContentScale
import coil3.compose.AsyncImage import coil3.compose.AsyncImage
import coil3.compose.AsyncImagePainter import coil3.compose.AsyncImagePainter
import com.fabledsword.minstrel.shared.resolveServerUrl import com.fabledsword.minstrel.shared.resolveServerUrl
// The fallback fades out as the artwork crossfades in (Coil's crossfade is
// configured globally on the ImageLoader in MinstrelApplication). Matching
// durations makes the swap read as one cross-dissolve; without the fade the
// placeholder icon vanished a frame before the cover appeared, which is the
// "art popping in" the Home veil exists to hide (issue #2327).
private const val FALLBACK_FADE_MS = 220
/** /**
* Renders a server-hosted image, resolving relative URLs centrally so * Renders a server-hosted image, resolving relative URLs centrally so
* every cover surface loads consistently. Shows [fallback] when the URL * every cover surface loads consistently. Shows [fallback] when the URL
* is blank/unresolvable, while the image is still loading, and when the * is blank/unresolvable, while the image is still loading, and when the
* load fails — so a tile is never left blank (e.g. art not yet backfilled, * load fails — so a tile is never left blank (e.g. art not yet backfilled,
* which the "You might like" row hits often). * which the "You might like" row hits often).
*
* In-flight loads are reported to [LocalArtSettleTracker] when a screen
* provides one, so a screen-level overlay can wait for artwork to land
* instead of guessing with a fixed delay.
*/ */
@Composable @Composable
fun ServerImage( fun ServerImage(
@@ -40,6 +55,23 @@ fun ServerImage(
var state by remember(resolved) { var state by remember(resolved) {
mutableStateOf<AsyncImagePainter.State>(AsyncImagePainter.State.Empty) mutableStateOf<AsyncImagePainter.State>(AsyncImagePainter.State.Empty)
} }
// Empty counts as loading: it's the pre-request state, so treating it
// as settled would let a screen overlay lower before Coil even starts.
val loading = state is AsyncImagePainter.State.Empty ||
state is AsyncImagePainter.State.Loading
val tracker = LocalArtSettleTracker.current
DisposableEffect(tracker, loading) {
if (loading) tracker?.begin()
// Balanced by construction: the effect re-runs when `loading` flips
// (decrement, then no re-increment) and disposes when a tile leaves
// the composition mid-load (scrolled away).
onDispose { if (loading) tracker?.end() }
}
val fallbackAlpha by animateFloatAsState(
targetValue = if (loading || state is AsyncImagePainter.State.Error) 1f else 0f,
animationSpec = tween(FALLBACK_FADE_MS),
label = "art-fallback",
)
Box(modifier = modifier, contentAlignment = Alignment.Center) { Box(modifier = modifier, contentAlignment = Alignment.Center) {
AsyncImage( AsyncImage(
model = resolved, model = resolved,
@@ -48,10 +80,10 @@ fun ServerImage(
contentScale = contentScale, contentScale = contentScale,
onState = { state = it }, onState = { state = it },
) )
if (state is AsyncImagePainter.State.Loading || if (fallbackAlpha > 0f) {
state is AsyncImagePainter.State.Error Box(Modifier.alpha(fallbackAlpha), contentAlignment = Alignment.Center) {
) {
fallback() fallback()
} }
} }
} }
}
@@ -0,0 +1,242 @@
package com.fabledsword.minstrel.shared
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.launch
import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.advanceTimeBy
import kotlinx.coroutines.test.advanceUntilIdle
import kotlinx.coroutines.test.runCurrent
import kotlinx.coroutines.test.runTest
import org.junit.jupiter.api.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertTrue
private const val WORK_MS = 1_000L
private const val SLOW_WORK_MS = 5_000L
private const val CHURN_ROUNDS = 5
private const val ART_IN_FLIGHT = 3
private const val SUCCEED_ON_ATTEMPT = 3
private const val QUIET_WINDOWS_TO_OUTLAST = 3
/**
* The veil's job is to stay up until the screen has stopped moving. Each
* test pins one of the ways the previous fixed-delay implementation
* lowered it too early (issue #2327).
*
* The controller is built on `backgroundScope` throughout: its consumer
* loop runs forever, so hanging it off the test's own scope would stop
* `runTest` from ever completing.
*/
@OptIn(ExperimentalCoroutinesApi::class)
class UpdateVeilControllerTest {
private val timings = VeilTimings(
minHoldMs = 900,
quietMs = 700,
maxHoldMs = 12_000,
attempts = 3,
retryBackoffMs = 600,
)
/** Drives the settle signal by hand: content key, presence, art count. */
private class FakeScreen(hasContent: Boolean = true) {
val state = MutableStateFlow(Triple(0, hasContent, 0))
val signal = state.map { (key, hasContent, art) ->
VeilSettleState(contentKey = key, hasContent = hasContent, quiescent = art == 0)
}
/** Something visibly changed — re-arms the quiet window. */
fun churn() {
state.value = state.value.copy(first = state.value.first + 1)
}
fun artLoading(count: Int) {
state.value = state.value.copy(third = count)
}
fun contentAppears() {
state.value = state.value.copy(second = true)
}
}
private fun TestScope.controllerOn(
screen: FakeScreen,
shouldVeil: suspend () -> Boolean = { true },
work: suspend () -> Boolean,
) = UpdateVeilController(
scope = backgroundScope,
settleSignal = screen.signal,
shouldVeil = shouldVeil,
timings = timings,
work = work,
)
/** Records every visibility transition, so a blink can't hide. */
private fun TestScope.recordVisibility(controller: UpdateVeilController): List<Boolean> {
val seen = mutableListOf<Boolean>()
backgroundScope.launch { controller.visible.collect { seen.add(it) } }
return seen
}
@Test
fun `veil outlasts content that keeps churning after the pull returns`() = runTest {
val screen = FakeScreen()
val controller = controllerOn(screen) { true }
controller.request()
runCurrent()
assertTrue(controller.visible.value, "veil is up while the screen is still moving")
// Tiles hydrating one after another, each inside the quiet window.
// The old implementation had already wiped off after a flat 500ms.
repeat(CHURN_ROUNDS) {
advanceTimeBy(timings.quietMs / 2)
screen.churn()
runCurrent()
assertTrue(controller.visible.value, "veil must hold across staged churn")
}
advanceUntilIdle()
assertFalse(controller.visible.value, "veil lowers once the screen goes quiet")
}
@Test
fun `veil waits for artwork to finish loading`() = runTest {
val screen = FakeScreen()
val controller = controllerOn(screen) { true }
screen.artLoading(ART_IN_FLIGHT)
controller.request()
// Well past the quiet window and the floor — but art is still in
// flight, so lowering now would show the covers popping in.
advanceTimeBy(timings.minHoldMs + timings.quietMs * QUIET_WINDOWS_TO_OUTLAST)
assertTrue(controller.visible.value, "veil must wait on in-flight art")
screen.artLoading(0)
advanceUntilIdle()
assertFalse(controller.visible.value, "veil lowers once art has landed")
}
@Test
fun `veil holds through failed attempts and their retries`() = runTest {
val screen = FakeScreen()
var attempts = 0
val controller = controllerOn(screen) {
attempts++
attempts >= SUCCEED_ON_ATTEMPT // fail twice, succeed on the third
}
val seen = recordVisibility(controller)
controller.request()
runCurrent()
assertTrue(controller.visible.value)
advanceTimeBy(timings.retryBackoffMs + 1)
assertTrue(controller.visible.value, "veil stays up across the backoff")
advanceUntilIdle()
assertEquals(SUCCEED_ON_ATTEMPT, attempts, "retries until the pull succeeds")
assertEquals(listOf(false, true, false), seen, "raised once, lowered once")
}
@Test
fun `giving up lowers the veil quietly and does not block later requests`() = runTest {
val screen = FakeScreen()
var attempts = 0
var succeed = false
val controller = controllerOn(screen) {
attempts++
succeed
}
controller.request()
advanceUntilIdle()
assertEquals(timings.attempts, attempts, "exhausts its attempts")
assertFalse(controller.visible.value, "gives up silently")
// The operator's requirement: giving up must not latch anything off
// — the reconnect-driven recovery still gets to try again later.
succeed = true
controller.request()
advanceUntilIdle()
assertEquals(timings.attempts + 1, attempts, "a later request still runs")
}
@Test
fun `overlapping requests extend one veil instead of racing it`() = runTest {
val screen = FakeScreen()
var started = 0
val controller = controllerOn(screen) {
started++
delay(WORK_MS)
true
}
val seen = recordVisibility(controller)
// Reconnect and the rebuild event arriving together is what made the
// old Boolean flag clear mid-update: whichever pull finished first
// wiped the veil off while the other was still running.
controller.request()
runCurrent()
controller.request()
advanceTimeBy(WORK_MS + 1)
assertTrue(controller.visible.value, "second trigger extends the same veil")
advanceUntilIdle()
assertEquals(2, started, "the mid-session trigger still did its pull")
assertEquals(listOf(false, true, false), seen, "one veil session, not two")
}
@Test
fun `a never-settling screen still releases the veil at the ceiling`() = runTest {
val screen = FakeScreen()
val controller = controllerOn(screen) { true }
screen.artLoading(1) // an image that never completes
controller.request()
advanceTimeBy(timings.maxHoldMs + 1)
assertFalse(controller.visible.value, "the hard ceiling must never strand the user")
}
@Test
fun `a cold load runs unveiled`() = runTest {
val screen = FakeScreen()
var ran = false
val controller = controllerOn(
screen,
shouldVeil = { false }, // empty cache: the skeleton owns this
) {
ran = true
true
}
controller.request()
advanceUntilIdle()
assertTrue(ran, "the refresh still happens")
assertFalse(controller.visible.value, "but no veil over a skeleton")
}
@Test
fun `veil waits for content to paint before raising`() = runTest {
val screen = FakeScreen(hasContent = false)
val controller = controllerOn(screen) {
delay(SLOW_WORK_MS)
true
}
controller.request()
advanceTimeBy(WORK_MS)
assertFalse(controller.visible.value, "nothing to hide until content is up")
screen.contentAppears()
runCurrent()
assertTrue(controller.visible.value, "raises the moment cached content paints")
advanceUntilIdle()
assertFalse(controller.visible.value)
}
}