From 096a3c0b15e35a208f11ed02133e11eb3e742b8f Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 20 Jun 2026 12:29:05 -0400 Subject: [PATCH 01/11] =?UTF-8?q?fix(clients):=20never=20leave=20cover=20t?= =?UTF-8?q?iles=20blank=20=E2=80=94=20shared=20web=20=20+=20Android?= =?UTF-8?q?=20Coil=20placeholder/error?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cover tiles (worst in the "You might like" home row, which surfaces unplayed items whose art is often not yet backfilled) sat empty while loading and stayed blank on a 404. The server returns a fast 404; the gap was missing client-side loading/fallback states. Web: new shared Cover.svelte owns the loading placeholder + onerror fallback (static cover, or Disc3 for artists). AlbumCard, ArtistCard and CompactTrackCard now reuse it instead of three hand-rolled tags that disagreed on fallback handling — notably ArtistCard had no onerror. Android: ServerImage tracks Coil's load state so the per-caller fallback doubles as a placeholder (loading) and an error state (404 / unreachable), instead of only guarding the null-URL case. All five call sites pass an explicit size modifier, so the new Box wrapper is layout-safe. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../minstrel/shared/widgets/ServerImage.kt | 30 ++++++++- web/src/lib/components/AlbumCard.svelte | 13 +--- web/src/lib/components/ArtistCard.svelte | 21 +++--- .../lib/components/CompactTrackCard.svelte | 15 +---- web/src/lib/components/Cover.svelte | 66 +++++++++++++++++++ 5 files changed, 107 insertions(+), 38 deletions(-) create mode 100644 web/src/lib/components/Cover.svelte diff --git a/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/ServerImage.kt b/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/ServerImage.kt index f30b9732..0560b76e 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/ServerImage.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/ServerImage.kt @@ -1,15 +1,25 @@ package com.fabledsword.minstrel.shared.widgets +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue +import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.layout.ContentScale import coil3.compose.AsyncImage +import coil3.compose.AsyncImagePainter import com.fabledsword.minstrel.shared.resolveServerUrl /** * Renders a server-hosted image, resolving relative URLs centrally so * every cover surface loads consistently. Shows [fallback] when the URL - * is blank or unresolvable. + * 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, + * which the "You might like" row hits often). */ @Composable fun ServerImage( @@ -22,12 +32,26 @@ fun ServerImage( val resolved = resolveServerUrl(url) if (resolved == null) { fallback() - } else { + return + } + // Track Coil's load state so the fallback doubles as a placeholder + // (loading) and an error state (404 / unreachable) — not just a + // null-URL guard, which left present-but-failing URLs blank. + var state by remember(resolved) { + mutableStateOf(AsyncImagePainter.State.Empty) + } + Box(modifier = modifier, contentAlignment = Alignment.Center) { AsyncImage( model = resolved, contentDescription = contentDescription, - modifier = modifier, + modifier = Modifier.fillMaxSize(), contentScale = contentScale, + onState = { state = it }, ) + if (state is AsyncImagePainter.State.Loading || + state is AsyncImagePainter.State.Error + ) { + fallback() + } } } diff --git a/web/src/lib/components/AlbumCard.svelte b/web/src/lib/components/AlbumCard.svelte index 69712910..20b6426f 100644 --- a/web/src/lib/components/AlbumCard.svelte +++ b/web/src/lib/components/AlbumCard.svelte @@ -1,18 +1,14 @@ + +
+ {#if useIcon} +
+ +
+ {:else if imgSrc} + (loaded = true)} + onerror={() => (failed = true)} + /> + {/if} +
From 2a8de82a172ff789c6f331f2407f7a7aa3ca4cea Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 20 Jun 2026 12:40:08 -0400 Subject: [PATCH 02/11] fix(web/player): auto-skip a failed track instead of dead-ending on "Try again" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A track that fails to load (e.g. a stale system-playlist snapshot pointing at a rebuilt/removed file) hard-set the player to the 'error' state and stranded the user on a "Try again" button that just re-queued the same failing track. Now a load error advances to the next track; the error state only surfaces once the whole queue has proven unplayable — every track failed, or we reached the end. A failure streak capped at queue length stops a fully-broken queue from cycling, and resets on the next successful play. Next (Track B cont.): self-heal a stale system-playlist / radio queue by re-pulling the fresh snapshot on total failure, plus the Android equivalent. Issue #968. Co-Authored-By: Claude Opus 4.8 (1M context) --- web/src/lib/player/store.svelte.ts | 30 ++++++++++++++++++++++++++++-- web/src/lib/player/store.test.ts | 18 ++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/web/src/lib/player/store.svelte.ts b/web/src/lib/player/store.svelte.ts index 42650296..ef8f9c1d 100644 --- a/web/src/lib/player/store.svelte.ts +++ b/web/src/lib/player/store.svelte.ts @@ -83,6 +83,12 @@ let _radioRefreshInFlight = false; // queue clears it). let _queueSource = $state(null); +// Track B (#968): consecutive load failures since the last successful +// 'playing'. A bad track auto-advances instead of dead-ending; the streak +// (capped against queue length) stops a fully-unplayable queue from +// looping forever. Reset on a successful play and on a fresh playQueue. +let _failureStreak = 0; + let _audioEl: HTMLAudioElement | null = null; export const player = { @@ -142,6 +148,7 @@ export function playQueue( _position = 0; _duration = 0; _error = null; + _failureStreak = 0; } export function togglePlay(): void { @@ -263,6 +270,7 @@ export function reportStateFromAudio( case 'playing': _state = 'playing'; _error = null; + _failureStreak = 0; return; case 'paused': _state = 'paused'; @@ -271,8 +279,7 @@ export function reportStateFromAudio( _state = 'loading'; return; case 'error': - _state = 'error'; - _error = detail ?? 'Playback failed.'; + handleLoadFailure(detail); return; case 'ended': if (_repeat === 'one') { @@ -301,6 +308,25 @@ export function reportStateFromAudio( } } +// Track B (#968): a single failed track must not strand the player on the +// "Try again" dead-end. Advance past it; only surface the error once the +// whole queue has proven unplayable — every track failed, or we reached the +// end with nothing playable. The streak cap (vs queue length) stops a +// fully-broken queue from cycling forever instead of settling. +function handleLoadFailure(detail?: string): void { + _failureStreak++; + if (_failureStreak < _queue.length && _index + 1 < _queue.length) { + _index++; + _position = 0; + _duration = 0; + _state = 'loading'; + _error = null; + return; + } + _state = 'error'; + _error = detail ?? 'Playback failed.'; +} + export function enqueueTrack(t: TrackRef): void { _radioSeedId = null; // M4c _queue = [..._queue, t]; diff --git a/web/src/lib/player/store.test.ts b/web/src/lib/player/store.test.ts index f4c144fe..2b5228f1 100644 --- a/web/src/lib/player/store.test.ts +++ b/web/src/lib/player/store.test.ts @@ -288,6 +288,24 @@ describe('player store — shuffle + repeat + audio reports', () => { expect(player.state).toBe('error'); expect(player.error).toBe('network lost'); }); + + test('reportStateFromAudio("error") auto-advances past a bad track', () => { + playQueue([track('1'), track('2'), track('3')]); + reportStateFromAudio('error', 'boom'); + expect(player.index).toBe(1); + expect(player.state).toBe('loading'); + expect(player.error).toBeNull(); + }); + + test('reportStateFromAudio("error") dead-ends once the whole queue is unplayable', () => { + playQueue([track('1'), track('2')]); + reportStateFromAudio('error', 'boom'); // track 1 fails → skip to track 2 + expect(player.index).toBe(1); + expect(player.state).toBe('loading'); + reportStateFromAudio('error', 'boom'); // track 2 also fails → exhausted + expect(player.state).toBe('error'); + expect(player.error).toBe('boom'); + }); }); import { From 335d7822158a4f29bba62350c0647ae2225951c2 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 20 Jun 2026 12:41:30 -0400 Subject: [PATCH 03/11] fix(android/player): auto-skip a failed track on load error, not just zero-duration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit onPlayerError fired a `load_failed` event and the snackbar reporter coalesced it into "Skipped N unplayable tracks" — but nothing actually skipped, so a bad/stale track stranded playback while the toast claimed otherwise. Mirror the zero_duration path: advance to the next item and re-prepare (a load error leaves the player IDLE), or stop at the end. Forward-only bounds a fully- unplayable queue. Web parity with 2a8de82a. Issue #968. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../fabledsword/minstrel/player/PlayerController.kt | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt index f571f6a9..b84b55b0 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt @@ -417,6 +417,16 @@ class PlayerController @Inject constructor( detail = error.message, ), ) + // A failed load leaves the player IDLE; advance past the bad + // track and re-prepare so one unplayable item doesn't strand + // playback (mirrors the zero_duration skip). Forward-only — + // stop at the end — bounds a fully-unplayable queue. + if (controller.hasNextMediaItem()) { + controller.seekToNextMediaItem() + controller.prepare() + } else { + controller.stop() + } } override fun onMediaItemTransition( From 27766ae0631118ab5b43b118ebd7e91166af634e Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 20 Jun 2026 12:51:47 -0400 Subject: [PATCH 04/11] feat(web/player): self-heal a stale system-playlist / radio queue on total failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the whole queue proves unplayable (e.g. a tab left open across the daily system-playlist rebuild — the exact stale-snapshot case), the player now re-pulls the fresh snapshot and resumes instead of dead-ending on "Try again". The seeder hands the store an opaque refetch closure so the store stays decoupled from the playlist API and the per-artist (songs_like_artist) identity problem: single-instance variants re-pull via systemShuffle, per-artist mixes via getPlaylist(id), radio re-seeds from its track. Bounded to one self-heal per exhaustion (reset on the next successful play) so a still-broken refresh can't loop; "Try again" stays the genuine last resort. Wired from PlaylistCard, the playlist detail page, and playRadio. Issue #968. Co-Authored-By: Claude Opus 4.8 (1M context) --- web/src/lib/components/PlaylistCard.svelte | 19 +++++- web/src/lib/player/store.svelte.ts | 77 ++++++++++++++++++++-- web/src/lib/player/store.test.ts | 26 ++++++++ web/src/lib/playlists/systemRefetch.ts | 33 ++++++++++ web/src/routes/playlists/[id]/+page.svelte | 25 +++++-- 5 files changed, 169 insertions(+), 11 deletions(-) create mode 100644 web/src/lib/playlists/systemRefetch.ts diff --git a/web/src/lib/components/PlaylistCard.svelte b/web/src/lib/components/PlaylistCard.svelte index 84e358c0..95642a6b 100644 --- a/web/src/lib/components/PlaylistCard.svelte +++ b/web/src/lib/components/PlaylistCard.svelte @@ -5,6 +5,7 @@ import { user } from '$lib/auth/store.svelte'; import { getPlaylist, systemShuffle, refreshSystem } from '$lib/api/playlists'; import { playlistTrackToRef } from '$lib/playlists/playlistTrackToRef'; + import { systemPlaylistRefetch } from '$lib/playlists/systemRefetch'; import { errCode } from '$lib/api/errors'; import { qk } from '$lib/api/queries'; import { playQueue } from '$lib/player/store.svelte'; @@ -80,7 +81,14 @@ const detail = await systemShuffle(variant); const refs = toTrackRefs(detail.tracks); if (refs.length > 0) { - playQueue(refs, 0, { source: variant }); + playQueue(refs, 0, { + source: variant, + refetch: systemPlaylistRefetch({ + variant, + playlistId: playlist.id, + perArtist: false, + }), + }); } } else { const detail = await getPlaylist(playlist.id); @@ -93,7 +101,14 @@ // so source attribution stays absent — the PlaylistCard // test pins this contract. if (variant != null) { - playQueue(refs, 0, { source: variant }); + playQueue(refs, 0, { + source: variant, + refetch: systemPlaylistRefetch({ + variant, + playlistId: playlist.id, + perArtist: true, + }), + }); } else { playQueue(refs, 0); } diff --git a/web/src/lib/player/store.svelte.ts b/web/src/lib/player/store.svelte.ts index ef8f9c1d..1e41432b 100644 --- a/web/src/lib/player/store.svelte.ts +++ b/web/src/lib/player/store.svelte.ts @@ -89,6 +89,17 @@ let _queueSource = $state(null); // looping forever. Reset on a successful play and on a fresh playQueue. let _failureStreak = 0; +// Track B (#968): when a queue is seeded from a refreshable source (a system +// playlist or radio), the seeder hands us a closure that re-pulls the fresh +// snapshot. On total failure (whole queue unplayable — likely a stale tab +// left open across the daily rebuild) we call it once to self-heal before +// surfacing the dead-end. Kept opaque so the store stays decoupled from the +// playlist API and the per-artist (songs_like_artist) identity problem. +let _queueRefetch: (() => Promise) | null = null; +let _selfHealInFlight = false; +let _selfHealAttempts = 0; +const MAX_SELF_HEAL_ATTEMPTS = 1; + let _audioEl: HTMLAudioElement | null = null; export const player = { @@ -115,13 +126,20 @@ export function registerAudioEl(el: HTMLAudioElement | null): void { export function playQueue( tracks: TrackRef[], startIndex = 0, - opts: { shuffle?: boolean; source?: string | null } = {}, + opts: { + shuffle?: boolean; + source?: string | null; + refetch?: () => Promise; + } = {}, ): void { _radioSeedId = null; // M4c: non-radio enqueue clears the radio refresh state // #415: a fresh queue resets the system-playlist source. Set only // when seeded from a system playlist (server already returned the // rotation-aware order, so no client shuffle in that path). _queueSource = opts.source ?? null; + // #968: a fresh play replaces the self-heal closure + resets its budget. + _queueRefetch = opts.refetch ?? null; + _selfHealAttempts = 0; if (opts.shuffle && tracks.length > 1) { // Fisher-Yates over the whole list. startIndex is ignored — the // caller is asking for "random play from this pool," so the first @@ -271,6 +289,7 @@ export function reportStateFromAudio( _state = 'playing'; _error = null; _failureStreak = 0; + _selfHealAttempts = 0; return; case 'paused': _state = 'paused'; @@ -323,10 +342,53 @@ function handleLoadFailure(detail?: string): void { _error = null; return; } + // Whole queue is unplayable. If it came from a refreshable source, the + // snapshot is probably stale — re-pull it and resume before giving up. + if (trySelfHeal()) return; _state = 'error'; _error = detail ?? 'Playback failed.'; } +// #968: re-pull a stale refreshable queue. Returns true if a self-heal was +// started (caller must not dead-end). Bounded to one attempt per exhaustion +// (reset on the next successful play) so a still-broken refresh can't loop. +function trySelfHeal(): boolean { + if (_selfHealInFlight) return true; + if (_queueRefetch === null) return false; + if (_selfHealAttempts >= MAX_SELF_HEAL_ATTEMPTS) return false; + _selfHealAttempts++; + _selfHealInFlight = true; + _state = 'loading'; + _error = null; + const refetch = _queueRefetch; + void refetch() + .then((refs) => { + if (refs.length > 0) { + // Re-seed in place — keep _queueSource / _queueRefetch so the new + // snapshot can itself self-heal, and don't reset _selfHealAttempts + // (only a successful 'playing' clears the budget). + _queue = refs; + _index = 0; + _position = 0; + _duration = 0; + _failureStreak = 0; + _state = 'loading'; + _error = null; + } else { + _state = 'error'; + _error = 'Nothing playable in this mix right now.'; + } + }) + .catch(() => { + _state = 'error'; + _error = 'Couldn’t refresh this mix. Try again.'; + }) + .finally(() => { + _selfHealInFlight = false; + }); + return true; +} + export function enqueueTrack(t: TrackRef): void { _radioSeedId = null; // M4c _queue = [..._queue, t]; @@ -371,12 +433,19 @@ export function playNextMany(ts: TrackRef[]): void { _queue = [..._queue.slice(0, next), ...ts, ..._queue.slice(next)]; } -export async function playRadio(seedTrackId: string): Promise { +async function fetchRadioTracks(seedTrackId: string): Promise { const resp = await api.get( `/api/radio?seed_track=${encodeURIComponent(seedTrackId)}` ); - if (resp.tracks.length === 0) return; - playQueue(resp.tracks, 0); + return resp.tracks; +} + +export async function playRadio(seedTrackId: string): Promise { + const tracks = await fetchRadioTracks(seedTrackId); + if (tracks.length === 0) return; + // #968: hand the player a self-heal closure so a fully-stale radio queue + // re-seeds from the same track instead of dead-ending. + playQueue(tracks, 0, { refetch: () => fetchRadioTracks(seedTrackId) }); _radioSeedId = seedTrackId; // M4c: set AFTER playQueue (which clears it) } diff --git a/web/src/lib/player/store.test.ts b/web/src/lib/player/store.test.ts index 2b5228f1..3d2044d2 100644 --- a/web/src/lib/player/store.test.ts +++ b/web/src/lib/player/store.test.ts @@ -306,6 +306,32 @@ describe('player store — shuffle + repeat + audio reports', () => { expect(player.state).toBe('error'); expect(player.error).toBe('boom'); }); + + test('reportStateFromAudio("error") self-heals a refreshable queue on total failure', async () => { + const fresh = [track('a'), track('b')]; + const refetch = vi.fn().mockResolvedValue(fresh); + playQueue([track('1')], 0, { source: 'for_you', refetch }); + reportStateFromAudio('error', 'boom'); // sole track fails → exhausted → self-heal + expect(refetch).toHaveBeenCalledOnce(); + expect(player.state).toBe('loading'); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + expect(player.queue.map((t) => t.id)).toEqual(['a', 'b']); + expect(player.index).toBe(0); + }); + + test('self-heal fires at most once per exhaustion, then dead-ends', async () => { + const refetch = vi.fn().mockResolvedValue([track('x')]); + playQueue([track('1')], 0, { source: 'for_you', refetch }); + reportStateFromAudio('error', 'boom'); // exhausted → self-heal #1 + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + reportStateFromAudio('error', 'boom'); // re-pulled track also fails; budget spent → error + expect(player.state).toBe('error'); + expect(refetch).toHaveBeenCalledOnce(); + }); }); import { diff --git a/web/src/lib/playlists/systemRefetch.ts b/web/src/lib/playlists/systemRefetch.ts new file mode 100644 index 00000000..ef5a19ba --- /dev/null +++ b/web/src/lib/playlists/systemRefetch.ts @@ -0,0 +1,33 @@ +import type { TrackRef, PlaylistDetail } from '$lib/api/types'; +import { systemShuffle, getPlaylist } from '$lib/api/playlists'; +import { playlistTrackToRef } from './playlistTrackToRef'; + +// #968: builds the self-heal closure for a system-playlist queue. On total +// playback failure (every queued track unplayable — typically a stale tab +// left open across the daily rebuild) the player calls this to re-pull the +// current snapshot and resume. +// +// Single-instance variants (for_you, discover, deep_cuts, …) re-pull by +// variant via the rotation-aware shuffle endpoint. Per-artist mixes +// (songs_like_artist — one playlist per seed artist) can't be addressed by +// variant alone, so they re-pull by playlist id. Mirrors the play routing +// in PlaylistCard. + +function toRefs(detail: PlaylistDetail): TrackRef[] { + return detail.tracks + .map((r) => playlistTrackToRef(r)) + .filter((t): t is TrackRef => t !== null); +} + +export function systemPlaylistRefetch(opts: { + variant: string; + playlistId: string; + perArtist: boolean; +}): () => Promise { + return async () => { + const detail = opts.perArtist + ? await getPlaylist(opts.playlistId) + : await systemShuffle(opts.variant); + return toRefs(detail); + }; +} diff --git a/web/src/routes/playlists/[id]/+page.svelte b/web/src/routes/playlists/[id]/+page.svelte index ee23199d..fd80bcf9 100644 --- a/web/src/routes/playlists/[id]/+page.svelte +++ b/web/src/routes/playlists/[id]/+page.svelte @@ -18,6 +18,7 @@ import { user } from '$lib/auth/store.svelte'; import { errCode, errMessage } from '$lib/api/errors'; import { playQueue } from '$lib/player/store.svelte'; + import { systemPlaylistRefetch } from '$lib/playlists/systemRefetch'; import { pushToast } from '$lib/stores/toast.svelte'; import type { TrackRef } from '$lib/api/types'; @@ -61,16 +62,30 @@ function onPlay(position: number) { if (!playlistQuery?.data) return; + const data = playlistQuery.data; // Skip rows where track_id is null (track was removed from library). - const live = playlistQuery.data.tracks - .filter((t) => t.track_id !== null) - .map(toTrackRef); + const live = data.tracks.filter((t) => t.track_id !== null).map(toTrackRef); // Find which `live` index corresponds to the clicked position. Some // positions in the original list may be unavailable (filtered out), // so the index in the playable list != the playlist position. - const clickedTrackID = playlistQuery.data.tracks.find((t) => t.position === position)?.track_id; + const clickedTrackID = data.tracks.find((t) => t.position === position)?.track_id; const startIdx = live.findIndex((t) => t.id === clickedTrackID); - playQueue(live, Math.max(0, startIdx)); + // #968: a system playlist played from its detail page gets the same + // self-heal closure as the home tile, so a stale snapshot re-pulls on + // total failure. (Source attribution is intentionally left to the home + // tile / shuffle path — this only adds the recovery closure.) + const variant = data.system_variant; + if (variant != null) { + playQueue(live, Math.max(0, startIdx), { + refetch: systemPlaylistRefetch({ + variant, + playlistId: id, + perArtist: data.seed_artist_id != null + }) + }); + } else { + playQueue(live, Math.max(0, startIdx)); + } } function toTrackRef(t: { From d4cc177db4f9ac071f39d0c1a2734001c31d1fef Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 20 Jun 2026 12:56:01 -0400 Subject: [PATCH 05/11] feat(android/player): self-heal a stale system-playlist / radio queue on total failure Web parity with 27766ae0. When a load error exhausts a fully-unplayable queue, re-pull the source instead of stopping: a bare-variant source is a refreshable system playlist (re-pull via PlaylistsRepository.systemShuffle), "radio:" re-seeds via RadioController. Reads the source from the current MediaItem extra; bounded to one re-pull per exhaustion (reset when a track next loads with real audio) so a still-stale refresh can't loop. Album / artist / user-playlist / offline sources have nothing to refresh and still stop. Issue #968. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../minstrel/player/PlayerController.kt | 80 ++++++++++++++++++- 1 file changed, 76 insertions(+), 4 deletions(-) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt index b84b55b0..5e200171 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt @@ -14,6 +14,8 @@ import androidx.media3.session.MediaController import androidx.media3.session.SessionToken import com.fabledsword.minstrel.di.ApplicationScope import com.fabledsword.minstrel.models.TrackRef +import com.fabledsword.minstrel.playlists.data.PlaylistsRepository +import com.fabledsword.minstrel.playlists.data.toPlayableTrackRefs import com.fabledsword.minstrel.shared.resolveServerUrl import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.CoroutineScope @@ -63,6 +65,7 @@ class PlayerController @Inject constructor( @ApplicationContext private val context: Context, @ApplicationScope private val scope: CoroutineScope, private val radio: RadioController, + private val playlists: PlaylistsRepository, private val playerFactory: PlayerFactory, private val activeUpnpHolder: com.fabledsword.minstrel.player.output.ActiveUpnpHolder, private val remoteState: RemotePlayerState, @@ -111,6 +114,13 @@ class PlayerController @Inject constructor( */ private var lastEvaluatedItemIndex: Int = -1 + /** + * #968: self-heal budget — re-pulls of a stale refreshable queue since + * the last successful play. Capped so a still-broken refresh can't loop; + * reset when any track loads with real audio. + */ + private var selfHealAttempts = 0 + /** * Stable queue snapshot kept in sync with the player's MediaItems — * the player's own getMediaItem(index) returns Media3 types; we keep @@ -351,7 +361,12 @@ class PlayerController @Inject constructor( if (current == null || upnpEngaged) return val duration = controller.duration val isZeroDuration = duration <= 0L || duration == androidx.media3.common.C.TIME_UNSET - if (!isZeroDuration) return + if (!isZeroDuration) { + // A track loaded with real audio — clear the self-heal budget so + // a later stale queue can recover again. + selfHealAttempts = 0 + return + } playbackErrorEventsChannel.trySend( PlaybackErrorEvent( trackId = current.id, @@ -367,6 +382,58 @@ class PlayerController @Inject constructor( } } + /** + * #968: the queue is fully unplayable (every track failed to load). If it + * came from a refreshable source — a system playlist (bare-variant source) + * or radio ("radio:") — the snapshot is probably stale (app/tab left + * open across the daily rebuild); re-pull it and resume instead of stopping. + * Bounded by [selfHealAttempts]. Album / artist / user-playlist / offline + * sources have nothing to refresh — stop. + */ + private fun selfHealOrStop(controller: MediaController) { + val source = controller.currentMediaItem + ?.mediaMetadata?.extras?.getString(MINSTREL_SOURCE_KEY) + if (source == null || selfHealAttempts >= MAX_SELF_HEAL_ATTEMPTS) { + controller.stop() + return + } + when { + source.startsWith("radio:") -> { + selfHealAttempts++ + scope.launch { selfHealRadio(source.removePrefix("radio:")) } + } + !source.contains(':') -> { // bare variant = refreshable system playlist + selfHealAttempts++ + scope.launch { selfHealSystem(source) } + } + else -> controller.stop() + } + } + + private suspend fun selfHealSystem(variant: String) { + val refs = runCatching { playlists.systemShuffle(variant).tracks.toPlayableTrackRefs() } + .getOrDefault(emptyList()) + if (refs.isEmpty()) { + stopOnControllerThread() + return + } + setQueue(refs, initialIndex = 0, source = variant) + } + + private suspend fun selfHealRadio(seedTrackId: String) { + val refs = runCatching { radio.seed(seedTrackId) }.getOrDefault(emptyList()) + if (refs.isEmpty()) { + stopOnControllerThread() + return + } + setQueue(refs, initialIndex = 0, source = "radio:$seedTrackId") + } + + private fun stopOnControllerThread() { + val controller = mediaController ?: return + runOnControllerThread(controller) { controller.stop() } + } + // ── Internal: async connect + Listener-driven UI state sync ────────── private suspend fun connectAndObserve() { @@ -419,13 +486,14 @@ class PlayerController @Inject constructor( ) // A failed load leaves the player IDLE; advance past the bad // track and re-prepare so one unplayable item doesn't strand - // playback (mirrors the zero_duration skip). Forward-only — - // stop at the end — bounds a fully-unplayable queue. + // playback (mirrors the zero_duration skip). At the end of a + // fully-unplayable queue, try to self-heal a stale refreshable + // source before stopping. if (controller.hasNextMediaItem()) { controller.seekToNextMediaItem() controller.prepare() } else { - controller.stop() + selfHealOrStop(controller) } } @@ -773,6 +841,10 @@ class PlayerController @Inject constructor( // they don't need this poll. private const val POSITION_POLL_INTERVAL_MS = 500L +// #968: at most one stale-queue self-heal re-pull per exhaustion (reset on +// the next successful play) so a still-broken refresh can't loop. +private const val MAX_SELF_HEAL_ATTEMPTS = 1 + /** * Structured playback failure event for [PlaybackErrorReporter]. Drives * both the user-facing snackbar ("Couldn't play X — skipping") and the From 16f76ea707f94a83a79b8f68e81e26bae082e0f5 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 20 Jun 2026 12:59:27 -0400 Subject: [PATCH 06/11] feat(server): emit playlist.system_rebuilt on daily + manual system-playlist rebuild MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The daily 03:00 scheduler rebuild (and the manual refresh endpoint) replace a user's system playlists + You-might-like rows but published no event, so a client left open across the rebuild served yesterday's snapshot until a manual reload — the stale-tab case behind #968. Add a user-scoped playlist.system_rebuilt event (envelope {kind,user_id,data:{}}) from both the scheduler (bus threaded into NewScheduler) and handleSystemPlaylistRefresh. Clients consume it to invalidate home / system-playlist views and proactively re-pull a stale active queue. Issue #968. Co-Authored-By: Claude Opus 4.8 (1M context) --- cmd/minstrel/main.go | 7 +++++- internal/api/events_publish.go | 15 +++++++++++ internal/api/playlists_system_shuffle.go | 2 ++ internal/playlists/scheduler.go | 32 ++++++++++++++++++++++-- 4 files changed, 53 insertions(+), 3 deletions(-) diff --git a/cmd/minstrel/main.go b/cmd/minstrel/main.go index 4657f232..4e887257 100644 --- a/cmd/minstrel/main.go +++ b/cmd/minstrel/main.go @@ -242,7 +242,12 @@ func run() error { // active user's daily build at 03:00 in their stored timezone. // Replaces the 24h-anchored cron loop (removed in the next commit // of this arc). - playlistScheduler, err := playlists.NewScheduler(pool, logger.With("component", "playlist_scheduler"), cfg.Storage.DataDir) + playlistScheduler, err := playlists.NewScheduler( + pool, + logger.With("component", "playlist_scheduler"), + cfg.Storage.DataDir, + bus, + ) if err != nil { return fmt.Errorf("init playlist scheduler: %w", err) } diff --git a/internal/api/events_publish.go b/internal/api/events_publish.go index cdf3289a..7b77edb4 100644 --- a/internal/api/events_publish.go +++ b/internal/api/events_publish.go @@ -88,6 +88,21 @@ func (h *handlers) publishPlaylistEvent(kind string, ownerID, playlistID pgtype. }) } +// publishSystemRebuilt notifies the owner's clients that their system +// playlists (and You-might-like rows) were regenerated, so they invalidate +// the home / system-playlist providers and a stale active queue can re-pull. +// Mirrors the daily scheduler's event; fired here from the manual refresh. +func (h *handlers) publishSystemRebuilt(userID pgtype.UUID) { + if h.eventbus == nil { + return + } + h.eventbus.Publish(eventbus.Event{ + Kind: "playlist.system_rebuilt", + UserID: uuidToString(userID), + Data: map[string]any{}, + }) +} + // publishRequestStatusChanged broadcasts a Lidarr request status flip to // the request's original requester so their /requests page reflects the // new state without manual refresh. Admin actors (approve / reject) still diff --git a/internal/api/playlists_system_shuffle.go b/internal/api/playlists_system_shuffle.go index bca19717..84be5c1a 100644 --- a/internal/api/playlists_system_shuffle.go +++ b/internal/api/playlists_system_shuffle.go @@ -73,6 +73,8 @@ func (h *handlers) handleSystemPlaylistRefresh(w http.ResponseWriter, r *http.Re writeErr(w, apierror.InternalMsg("build failed", err)) return } + // #968: announce the rebuild so the user's other clients refresh. + h.publishSystemRebuilt(user.ID) q := dbq.New(h.pool) v := kind pl, err := q.GetSystemPlaylistByVariantForUser(r.Context(), diff --git a/internal/playlists/scheduler.go b/internal/playlists/scheduler.go index 6d325378..97ab098f 100644 --- a/internal/playlists/scheduler.go +++ b/internal/playlists/scheduler.go @@ -28,6 +28,7 @@ import ( "github.com/jackc/pgx/v5/pgxpool" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/eventbus" "git.fabledsword.com/bvandeusen/minstrel/internal/taste" ) @@ -47,14 +48,21 @@ type Scheduler struct { pool *pgxpool.Pool logger *slog.Logger dataDir string + bus *eventbus.Bus // #968: announce rebuilds so clients self-refresh mu sync.Mutex jobs map[pgtype.UUID]uuid.UUID // user_id → gocron job id } // NewScheduler builds an idle scheduler. Caller must invoke Start -// before any builds fire. -func NewScheduler(pool *pgxpool.Pool, logger *slog.Logger, dataDir string) (*Scheduler, error) { +// before any builds fire. bus may be nil (tests); rebuild events are +// then skipped. +func NewScheduler( + pool *pgxpool.Pool, + logger *slog.Logger, + dataDir string, + bus *eventbus.Bus, +) (*Scheduler, error) { g, err := gocron.NewScheduler() if err != nil { return nil, fmt.Errorf("init gocron: %w", err) @@ -64,6 +72,7 @@ func NewScheduler(pool *pgxpool.Pool, logger *slog.Logger, dataDir string) (*Sch pool: pool, logger: logger, dataDir: dataDir, + bus: bus, jobs: map[pgtype.UUID]uuid.UUID{}, }, nil } @@ -234,7 +243,26 @@ func (s *Scheduler) rebuildUserDaily(ctx context.Context, userID pgtype.UUID, no if err := BuildSystemPlaylists(ctx, s.pool, s.logger, userID, now, s.dataDir); err != nil { s.logger.Warn("scheduler: build failed", "user_id", uuidStringPL(userID), "err", err) + return } + // #968: tell the user's connected clients their home content was + // regenerated, so a tab/app left open across the rebuild refreshes its + // system-playlist + You-might-like views (and a stale active queue can + // re-pull) instead of serving yesterday's snapshot until a manual reload. + s.publishRebuilt(userID) +} + +// publishRebuilt broadcasts a user-scoped "system playlists rebuilt" event. +// No-op when the bus is nil (test construction). +func (s *Scheduler) publishRebuilt(userID pgtype.UUID) { + if s.bus == nil { + return + } + s.bus.Publish(eventbus.Event{ + Kind: "playlist.system_rebuilt", + UserID: uuidStringPL(userID), + Data: map[string]any{}, + }) } // Stop drains gocron and stops the scheduler loop. From 5a80a1e46032890e680c05b0aa5eb68ddafa7170 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 20 Jun 2026 15:44:05 -0400 Subject: [PATCH 07/11] feat(web): subscribe to SSE and refresh home/playlists on playlist.system_rebuilt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The web client only ever SENT events; it had no inbound SSE listener, so a tab left open across the daily system-playlist rebuild kept showing yesterday's home + playlist snapshots until a manual reload (the stale- browse-view bug behind #968). Add useServerEvents(): opens /api/events/stream while authenticated and, on playlist.system_rebuilt, invalidates the home, playlists, and system-playlist-status query caches. Deliberately does not disturb the active playback queue — that self-heals on the failure path. Issue #968. Co-Authored-By: Claude Opus 4.8 (1M context) --- web/src/lib/serverEvents.svelte.ts | 37 ++++++++++++++++++++++++++++++ web/src/routes/+layout.svelte | 2 ++ 2 files changed, 39 insertions(+) create mode 100644 web/src/lib/serverEvents.svelte.ts diff --git a/web/src/lib/serverEvents.svelte.ts b/web/src/lib/serverEvents.svelte.ts new file mode 100644 index 00000000..d3ab5bcc --- /dev/null +++ b/web/src/lib/serverEvents.svelte.ts @@ -0,0 +1,37 @@ +import { queryClient } from '$lib/query/client'; +import { qk } from '$lib/api/queries'; +import { user } from '$lib/auth/store.svelte'; + +// Inbound server events over SSE (#968). The web client's OUTBOUND half +// (play events) lives in player/events.svelte.ts; this is the inbound +// listener it never had. Today it reacts to `playlist.system_rebuilt` — the +// daily 03:00 rebuild or a manual refresh — by invalidating the home + +// system-playlist query caches so a tab left open across the rebuild stops +// serving yesterday's snapshot (the stale-browse-view bug). Active playback +// self-heals separately on the failure path (store.handleLoadFailure); we +// deliberately do NOT yank a playing queue here — that would interrupt a +// mid-song listen to restart the new mix at track 0. + +function onSystemRebuilt(): void { + queryClient.invalidateQueries({ queryKey: qk.home() }); + // Prefix match invalidates every kind ('user' | 'system' | 'all'). + queryClient.invalidateQueries({ queryKey: ['playlists'] }); + queryClient.invalidateQueries({ queryKey: qk.systemPlaylistsStatus() }); +} + +// Wire once from +layout.svelte's mount. Opens the stream only while +// authenticated and closes it on logout; EventSource auto-reconnects on +// transient network drops. +export function useServerEvents(): void { + $effect(() => { + if (!user.value) return; + // Absent under SSR / jsdom — the listener simply no-ops there. + if (typeof EventSource === 'undefined') return; + const es = new EventSource('/api/events/stream'); + es.addEventListener('playlist.system_rebuilt', onSystemRebuilt); + return () => { + es.removeEventListener('playlist.system_rebuilt', onSystemRebuilt); + es.close(); + }; + }); +} diff --git a/web/src/routes/+layout.svelte b/web/src/routes/+layout.svelte index e6eb4821..4b71d934 100644 --- a/web/src/routes/+layout.svelte +++ b/web/src/routes/+layout.svelte @@ -23,6 +23,7 @@ } from '$lib/player/store.svelte'; import { useMediaSession } from '$lib/player/mediaSession.svelte'; import { useEventsDispatcher } from '$lib/player/events.svelte'; + import { useServerEvents } from '$lib/serverEvents.svelte'; import { useGlobalShortcuts } from '$lib/player/shortcuts.svelte'; import { applyMetaThemeColor } from '$lib/theme/applyMetaThemeColor.svelte'; import { audioLoader } from '$lib/player/audioLoader'; @@ -132,6 +133,7 @@ useMediaSession(); useEventsDispatcher(); + useServerEvents(); useGlobalShortcuts(); applyMetaThemeColor(); From a23e2e36caad16ac09e2c8a99d2bbec7f3ffc53f Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 20 Jun 2026 15:46:35 -0400 Subject: [PATCH 08/11] feat(android/home): refresh Home on playlist.system_rebuilt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Parity with the web SSE consumer (5a80a1e4). HomeViewModel now subscribes to EventsStream and re-pulls Home (refreshIndex + system-playlist status) when the server emits playlist.system_rebuilt — the daily 03:00 rebuild or a manual refresh — so the system-playlist tiles and You-might-like rows reflect the new snapshot without a manual reload. Browse-only: the active playback queue is left to self-heal on the failure path. Issue #968. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../com/fabledsword/minstrel/home/ui/HomeScreen.kt | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/home/ui/HomeScreen.kt b/android/app/src/main/java/com/fabledsword/minstrel/home/ui/HomeScreen.kt index e3d047fb..598b88c1 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/home/ui/HomeScreen.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/home/ui/HomeScreen.kt @@ -95,6 +95,7 @@ import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.flow.stateIn +import kotlinx.coroutines.flow.filter import kotlinx.coroutines.launch import javax.inject.Inject @@ -139,6 +140,7 @@ class HomeViewModel @Inject constructor( private val libraryRepository: LibraryRepository, private val player: com.fabledsword.minstrel.player.PlayerController, private val shuffleSource: com.fabledsword.minstrel.cache.ShuffleSource, + private val eventsStream: com.fabledsword.minstrel.events.EventsStream, networkStatus: com.fabledsword.minstrel.connectivity.NetworkStatusController, ) : ViewModel() { @@ -167,6 +169,15 @@ class HomeViewModel @Inject constructor( init { refresh() + // #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. + viewModelScope.launch { + eventsStream.events + .filter { it.kind == "playlist.system_rebuilt" } + .collect { refresh() } + } } /** From d05264ff8029de034fdd67223e9a7354839ee76b Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 20 Jun 2026 17:35:45 -0400 Subject: [PATCH 09/11] fix(web/playlists): attribute source when playing a system playlist from its detail page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Playing a system playlist from /playlists/ previously sent no source, so it never advanced that playlist's rotation — inconsistent with the home tile (and the Android detail screen, which already tags the variant). Pass source: variant alongside the existing self-heal closure so a play is attributed regardless of the surface it started from. Issue #968. Co-Authored-By: Claude Opus 4.8 (1M context) --- web/src/routes/playlists/[id]/+page.svelte | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/web/src/routes/playlists/[id]/+page.svelte b/web/src/routes/playlists/[id]/+page.svelte index fd80bcf9..f35fcd06 100644 --- a/web/src/routes/playlists/[id]/+page.svelte +++ b/web/src/routes/playlists/[id]/+page.svelte @@ -70,13 +70,14 @@ // so the index in the playable list != the playlist position. const clickedTrackID = data.tracks.find((t) => t.position === position)?.track_id; const startIdx = live.findIndex((t) => t.id === clickedTrackID); - // #968: a system playlist played from its detail page gets the same - // self-heal closure as the home tile, so a stale snapshot re-pulls on - // total failure. (Source attribution is intentionally left to the home - // tile / shuffle path — this only adds the recovery closure.) + // #968: a system playlist played from its detail page is tagged with its + // source (so play_started advances the rotation, matching the home tile and + // the Android detail screen) and gets the self-heal closure so a stale + // snapshot re-pulls on total failure. const variant = data.system_variant; if (variant != null) { playQueue(live, Math.max(0, startIdx), { + source: variant, refetch: systemPlaylistRefetch({ variant, playlistId: id, From e932ab438ca2b473e0c2c1000826d09b0ebee2e6 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 20 Jun 2026 19:48:52 -0400 Subject: [PATCH 10/11] feat(web/playlists): stale-view banner + Refresh on an open system-playlist after rebuild MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #980. When the daily rebuild fires while a system-playlist detail page is open, its cached data goes stale and can't be refetched in place — the playlist id rotated, so the old id 404s. serverEvents now exposes a monotonic rebuild counter; the detail page shows a "this mix was refreshed" banner with a Refresh that re-resolves the variant (systemShuffle) to the new playlist id and navigates there. No forced redirect, no auto-reload — the user refreshes on their terms. Functional behaviors were already correct (tapping a song plays it; tiles load the current mix); this closes the cosmetic list-staleness. Co-Authored-By: Claude Opus 4.8 (1M context) --- web/src/lib/serverEvents.svelte.ts | 13 +++++ web/src/routes/playlists/[id]/+page.svelte | 57 +++++++++++++++++++++- 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/web/src/lib/serverEvents.svelte.ts b/web/src/lib/serverEvents.svelte.ts index d3ab5bcc..98699045 100644 --- a/web/src/lib/serverEvents.svelte.ts +++ b/web/src/lib/serverEvents.svelte.ts @@ -12,7 +12,20 @@ import { user } from '$lib/auth/store.svelte'; // deliberately do NOT yank a playing queue here — that would interrupt a // mid-song listen to restart the new mix at track 0. +// Monotonic rebuild counter (#980). Bumped on every playlist.system_rebuilt. +// An open system-playlist DETAIL page can't be invalidated in place (its id +// rotates on rebuild — a refetch would 404), so instead it watches this +// counter and offers a "this mix was refreshed → Refresh" affordance that +// re-resolves the variant to the new playlist. +let _systemRebuildCount = $state(0); +export const systemRebuilt = { + get count(): number { + return _systemRebuildCount; + } +}; + function onSystemRebuilt(): void { + _systemRebuildCount++; queryClient.invalidateQueries({ queryKey: qk.home() }); // Prefix match invalidates every kind ('user' | 'system' | 'all'). queryClient.invalidateQueries({ queryKey: ['playlists'] }); diff --git a/web/src/routes/playlists/[id]/+page.svelte b/web/src/routes/playlists/[id]/+page.svelte index f35fcd06..5581aecb 100644 --- a/web/src/routes/playlists/[id]/+page.svelte +++ b/web/src/routes/playlists/[id]/+page.svelte @@ -1,6 +1,7 @@ @@ -187,6 +224,24 @@ playlistQuery.refetch()} /> {:else if playlistQuery?.data} {@const pl = playlistQuery.data} + {#if staleSystemView} +
+ This mix was refreshed since you opened it. + +
+ {/if} {#if !editing}
From f4f4df77089a5df2ac93a6150e23dc5740f0f85d Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 20 Jun 2026 19:52:13 -0400 Subject: [PATCH 11/11] feat(android/playlists): stale-view snackbar + Refresh on open system playlist after rebuild MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #980, parity with web e932ab43. When playlist.system_rebuilt arrives (SSE) while a system-playlist detail screen is open, the ViewModel marks it stale and the screen shows an indefinite "This mix was refreshed · Refresh" snackbar. Refresh re-resolves the rotated variant via PlaylistsRepository.systemShuffle and reuses the existing regenerated navigate-replace flow to land on the fresh playlist id — without triggering another server rebuild (unlike the manual regenerate button). Dismiss clears the flag. Functional behaviors were already correct; this closes the cosmetic stale-list gap. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../playlists/ui/PlaylistDetailScreen.kt | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/playlists/ui/PlaylistDetailScreen.kt b/android/app/src/main/java/com/fabledsword/minstrel/playlists/ui/PlaylistDetailScreen.kt index 1df819ef..1aa2ddbe 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/playlists/ui/PlaylistDetailScreen.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/playlists/ui/PlaylistDetailScreen.kt @@ -28,6 +28,10 @@ import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme import androidx.compose.material3.OutlinedButton import androidx.compose.material3.Scaffold +import androidx.compose.material3.SnackbarDuration +import androidx.compose.material3.SnackbarHost +import androidx.compose.material3.SnackbarHostState +import androidx.compose.material3.SnackbarResult import androidx.compose.material3.Text import androidx.compose.material3.TopAppBar import androidx.compose.runtime.Composable @@ -139,6 +143,15 @@ class PlaylistDetailViewModel @Inject constructor( private val regeneratedChannel = Channel(Channel.BUFFERED) val regenerated: Flow = regeneratedChannel.receiveAsFlow() + /** + * #980: a system-playlist rebuild landed (SSE) while this screen is open — + * its list is now stale and the uuid has rotated. Drives a "this mix was + * refreshed → Refresh" snackbar rather than yanking the user; Refresh + * re-resolves the variant via [reloadRebuilt]. + */ + private val staleInternal = MutableStateFlow(false) + val stale: StateFlow = staleInternal.asStateFlow() + val likedTrackIds: StateFlow> = likes.observeLikedTrackIds() .stateIn( @@ -163,6 +176,12 @@ class PlaylistDetailViewModel @Inject constructor( * ignored — the playlists-list screen handles those. */ private fun handlePlaylistEvent(event: LiveEvent) { + // #980: a system rebuild carries no playlist_id (it rebuilds all of the + // user's system mixes at once) — handle it before the per-id filter. + if (event.kind == "playlist.system_rebuilt") { + markStaleIfSystem() + return + } val eventPlaylistId = event.data["playlist_id"]?.jsonPrimitive?.contentOrNull if (eventPlaylistId != playlistId) return when (event.kind) { @@ -171,6 +190,14 @@ class PlaylistDetailViewModel @Inject constructor( } } + private fun markStaleIfSystem() { + val playlist = (internal.value as? PlaylistDetailUiState.Success) + ?.detail?.playlist ?: return + if (playlist.systemVariant?.takeIf { playlist.refreshable } != null) { + staleInternal.value = true + } + } + fun toggleLikeTrack(trackId: String) { val desired = trackId !in likedTrackIds.value viewModelScope.launch { @@ -220,6 +247,30 @@ class PlaylistDetailViewModel @Inject constructor( } } + /** + * #980: user tapped Refresh on the stale-view snackbar. The rebuild + * already ran server-side (uuid rotated) — re-resolve the variant to the + * fresh playlist id and hand it to [regenerated] for navigate-replace. + * Unlike [regenerate] this does NOT trigger another rebuild. + */ + fun reloadRebuilt() { + val playlist = (internal.value as? PlaylistDetailUiState.Success) + ?.detail?.playlist ?: return + val variant = playlist.systemVariant?.takeIf { playlist.refreshable } ?: return + staleInternal.value = false + viewModelScope.launch { + runCatching { repository.systemShuffle(variant).playlist.id } + .onSuccess { newId -> + if (newId.isNotEmpty()) regeneratedChannel.trySend(newId) + } + } + } + + /** #980: user dismissed the stale-view snackbar without refreshing. */ + fun dismissStale() { + staleInternal.value = false + } + /** Play the available tracks starting at [startTrackId] (or first available). */ fun play(tracks: List, startTrackId: String?) { val refs = tracks.toPlayableTrackRefs() @@ -261,8 +312,25 @@ fun PlaylistDetailScreen( } } } + // #980: a rebuild landed while this system-playlist screen is open. Offer a + // non-intrusive "refreshed → Refresh" snackbar; Refresh re-resolves the + // rotated uuid (via the regenerated navigate-replace flow). + val snackbarHostState = remember { SnackbarHostState() } + val stale by viewModel.stale.collectAsState() + LaunchedEffect(stale) { + if (stale) { + val result = snackbarHostState.showSnackbar( + message = "This mix was refreshed", + actionLabel = "Refresh", + duration = SnackbarDuration.Indefinite, + ) + if (result == SnackbarResult.ActionPerformed) viewModel.reloadRebuilt() + else viewModel.dismissStale() + } + } Scaffold( modifier = Modifier.fillMaxSize(), + snackbarHost = { SnackbarHost(snackbarHostState) }, topBar = { TopAppBar( title = { Text(currentTitle(state)) },