From 1ddde129597f860976ff3a100b6e0d06fe23339d Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 22:18:58 -0400 Subject: [PATCH 1/8] perf: lazy player source build + Cache-Control on byte endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two unrelated wins as a single batch. Flutter — lazy source building in setQueueFromTracks: Today: Future.wait builds all N AudioSource objects (drift queries + LockCaching ctor) before the player can call setAudioSources → play(). Measured at 83ms for a 25-track playlist, on top of the ~285ms initial-source preload. New flow: build only the initial source, hand it to setAudioSources ([initial], initialIndex: 0) so play() can start, then background- fill the rest. Forward direction (skipNext targets) added via addAudioSource. Backward direction (skipPrev) inserted at index 0.. initialIndex-1 with _suppressIndexUpdates true so the unavoidable currentIndex shifts don't push the wrong MediaItem onto the stream. Saves the up-front source-build wait — tap-to-audio for long queues should drop by ~80-100ms even on cache hits. Server — Cache-Control on the three byte-serving endpoints: - /api/albums/{id}/cover: max-age=86400, must-revalidate. Covers change rarely (re-scan, MBID enrichment); a day of cache is safe and skips conditional GETs for the bulk of a session. - /api/playlists/{id}/cover: max-age=300, must-revalidate. Collages recompute when contents change; short enough for edits to feel fresh, long enough to skip repeat fetches during a session. - /api/tracks/{id}/stream: max-age=31536000, immutable. Track bytes are immutable for a given id (scanner re-indexes by file_path; new files get new ids). LockCachingAudioSource on the Flutter side already disk-caches, but proper headers let it skip even the conditional 304 on repeat plays. --- flutter_client/lib/player/audio_handler.dart | 80 ++++++++++++++++---- internal/api/media.go | 12 +++ internal/api/playlists.go | 5 ++ 3 files changed, 84 insertions(+), 13 deletions(-) diff --git a/flutter_client/lib/player/audio_handler.dart b/flutter_client/lib/player/audio_handler.dart index 3bade142..663f9c28 100644 --- a/flutter_client/lib/player/audio_handler.dart +++ b/flutter_client/lib/player/audio_handler.dart @@ -60,6 +60,16 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl /// time the buffered-position stream emits (~200ms cadence). String? _cacheDirPath; + /// True while _fillRemainingSources is doing backward-fill inserts + /// at index 0..initialIndex-1. Each insert shifts the player's + /// currentIndex (it tracks the actively-playing source through + /// list mutations), and the resulting _onCurrentIndexChanged + /// callbacks would push the wrong MediaItem onto the stream + /// (queue.value[shifted_idx] != actively-playing track). When + /// the fill completes, currentIndex == initialIndex, mediaItem + /// is already correct, and we re-enable normal listener behavior. + bool _suppressIndexUpdates = false; + /// Volume stream for UI subscribers. Mirrors the just_audio player's /// volume directly; set via setVolume(double). Stream get volumeStream => _player.volumeStream; @@ -90,31 +100,74 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl } Future setQueueFromTracks(List tracks, {int initialIndex = 0}) async { + if (tracks.isEmpty) return; + final clampedInitial = initialIndex.clamp(0, tracks.length - 1); + + // Populate the visible queue + current mediaItem immediately so + // the player UI reflects the user's tap before any source has + // been built. Source list at the just_audio layer fills in + // asynchronously below. final items = tracks.map(_toMediaItem).toList(); queue.add(items); - if (items.isNotEmpty) { - mediaItem.add(items[initialIndex.clamp(0, items.length - 1)]); - } + mediaItem.add(items[clampedInitial]); debugPrint('audio_handler.setQueueFromTracks: ' '_baseUrl="$_baseUrl" trackCount=${tracks.length}'); final sw = Stopwatch()..start(); - final sources = await Future.wait(tracks.map(_buildAudioSource)); - debugPrint('audio_handler: built ${sources.length} sources in ' + + // Fast path: build only the initial source so the player can + // start. Remaining sources stream in via _fillRemainingSources() + // in the background — addAudioSource for next/auto-advance + // tracks, insertAudioSource for skipPrev tracks. + final initial = await _buildAudioSource(tracks[clampedInitial]); + debugPrint('audio_handler: built initial source in ' '${sw.elapsedMilliseconds}ms'); sw.reset(); sw.start(); - await _player.setAudioSources( - sources, - initialIndex: initialIndex, - ); - debugPrint('audio_handler: setAudioSources ${sw.elapsedMilliseconds}ms'); + await _player.setAudioSources([initial], initialIndex: 0); + debugPrint('audio_handler: setAudioSources(1) ${sw.elapsedMilliseconds}ms'); - // Kick the cover fetch for the initial item — async, doesn't block - // playback. Subsequent track changes are handled by the - // currentIndexStream listener. unawaited(_loadArtForCurrentItem()); + unawaited(_fillRemainingSources(tracks, clampedInitial)); + } + + /// Background fill of the rest of the just_audio source list after + /// the initial source is playing. Forward direction first (most + /// common skipNext target). Backward inserts shift the player's + /// currentIndex; we suppress _onCurrentIndexChanged side effects + /// for those so the mediaItem stream doesn't bounce to the wrong + /// queue entry. + Future _fillRemainingSources( + List tracks, int initialIndex) async { + final sw = Stopwatch()..start(); + for (var i = initialIndex + 1; i < tracks.length; i++) { + try { + final src = await _buildAudioSource(tracks[i]); + await _player.addAudioSource(src); + } catch (e) { + debugPrint('audio_handler: forward fill failed for ${tracks[i].id}: $e'); + } + } + debugPrint('audio_handler: forward fill (${tracks.length - initialIndex - 1}) ' + '${sw.elapsedMilliseconds}ms'); + if (initialIndex > 0) { + sw.reset(); + sw.start(); + _suppressIndexUpdates = true; + try { + for (var i = 0; i < initialIndex; i++) { + final src = await _buildAudioSource(tracks[i]); + await _player.insertAudioSource(i, src); + } + } catch (e) { + debugPrint('audio_handler: backward fill failed: $e'); + } finally { + _suppressIndexUpdates = false; + } + debugPrint('audio_handler: backward fill ($initialIndex) ' + '${sw.elapsedMilliseconds}ms'); + } } String _resolveStreamUrl(TrackRef t) { @@ -259,6 +312,7 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl void _onCurrentIndexChanged(int? idx) { if (idx == null) return; + if (_suppressIndexUpdates) return; // Push the new track's MediaItem onto the mediaItem stream so // the player UI rebuilds with the new title/artist/album/cover. // Without this, the bar and full player stayed pinned to whichever diff --git a/internal/api/media.go b/internal/api/media.go index 1e0e7d8e..0ff6cbbb 100644 --- a/internal/api/media.go +++ b/internal/api/media.go @@ -112,6 +112,12 @@ func (h *handlers) handleGetCover(w http.ResponseWriter, r *http.Request) { return } w.Header().Set("Content-Type", imageContentType(path)) + // Cover bytes change rarely (cover-source enrichment, manual rescan, + // rare MBID-driven re-fetch). One-day max-age + must-revalidate means + // clients skip the conditional GET for the bulk of a session, but + // stale art clears within 24h after a re-scan. ServeContent below + // still emits Last-Modified for the conditional path when needed. + w.Header().Set("Cache-Control", "public, max-age=86400, must-revalidate") http.ServeContent(w, r, filepath.Base(path), info.ModTime(), f) } @@ -140,5 +146,11 @@ func (h *handlers) handleGetStream(w http.ResponseWriter, r *http.Request) { } w.Header().Set("Content-Type", audioContentType(track.FileFormat)) w.Header().Set("Accept-Ranges", "bytes") + // Track bytes are immutable for a given track id (the scanner + // indexes by file_path; re-encoded files take new ids). One-year + // max-age + immutable lets the client cache (LockCachingAudioSource + // on the Flutter side, browser cache on web) skip even the + // conditional GET on repeat plays. + w.Header().Set("Cache-Control", "public, max-age=31536000, immutable") http.ServeContent(w, r, filepath.Base(track.FilePath), info.ModTime(), f) } diff --git a/internal/api/playlists.go b/internal/api/playlists.go index 4c8079bb..4d845cee 100644 --- a/internal/api/playlists.go +++ b/internal/api/playlists.go @@ -386,6 +386,11 @@ func (h *handlers) handleGetPlaylistCover(w http.ResponseWriter, r *http.Request return } full := filepath.Join(h.dataDir, *detail.CoverPath) + // Playlist collages recompute when the playlist's tracks change + // (system playlists re-rendered on rebuild, user playlists when + // modified). 5 minutes is short enough for normal edits to feel + // fresh, long enough to skip repeat fetches during a session. + w.Header().Set("Cache-Control", "public, max-age=300, must-revalidate") http.ServeFile(w, r, full) } From a3c0aed63eb91b81795706ae4f9ae008adf50352 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 22:24:02 -0400 Subject: [PATCH 2/8] =?UTF-8?q?feat(flutter):=20playlist=20detail=20nav=20?= =?UTF-8?q?hydration=20=E2=80=94=20header=20renders=20before=20fetch?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same pattern as album + artist detail. PlaylistDetailScreen accepts optional Playlist seed via go_router extra. While playlistDetailProvider is still resolving, render the header (description if any + track count) plus a "loading tracks…" inline spinner instead of an opaque full-screen CircularProgressIndicator. The body fills in when tracks arrive via drift watch re-emit. Routing reads s.extra as Playlist. PlaylistCard + PlaylistsListScreen pass the playlist via extra. Surfaces with no seed available (deep links etc.) fall back to the original full- screen spinner. Track row rendering already uses ListView.builder so visible row count was never the bottleneck — the wait was for the detail fetch itself. Header-on-tap is the win that makes the screen feel snappy. --- .../lib/playlists/playlist_detail_screen.dart | 74 ++++++++++++++++--- .../lib/playlists/playlists_list_screen.dart | 2 +- .../lib/playlists/widgets/playlist_card.dart | 3 +- flutter_client/lib/shared/routing.dart | 6 +- 4 files changed, 72 insertions(+), 13 deletions(-) diff --git a/flutter_client/lib/playlists/playlist_detail_screen.dart b/flutter_client/lib/playlists/playlist_detail_screen.dart index 468ce5a2..37d7ce9a 100644 --- a/flutter_client/lib/playlists/playlist_detail_screen.dart +++ b/flutter_client/lib/playlists/playlist_detail_screen.dart @@ -12,13 +12,26 @@ import '../theme/theme_extension.dart'; import 'playlists_provider.dart'; class PlaylistDetailScreen extends ConsumerWidget { - const PlaylistDetailScreen({required this.id, super.key}); + const PlaylistDetailScreen({required this.id, this.seed, super.key}); final String id; + /// Optional Playlist passed via go_router extra so the header + /// (title, cover, track count, play/download CTAs) can render + /// before the full detail fetch resolves. Same pattern as album + /// + artist nav hydration. + final Playlist? seed; + @override Widget build(BuildContext context, WidgetRef ref) { final fs = Theme.of(context).extension()!; final detail = ref.watch(playlistDetailProvider(id)); + + // Resolve the best playlist info available for the AppBar title. + final livePlaylist = detail.value?.playlist; + final headerName = (livePlaylist != null && livePlaylist.name.isNotEmpty) + ? livePlaylist.name + : (seed?.name ?? ''); + return Scaffold( backgroundColor: fs.obsidian, appBar: AppBar( @@ -28,17 +41,21 @@ class PlaylistDetailScreen extends ConsumerWidget { icon: Icon(Icons.arrow_back, color: fs.parchment), onPressed: () => context.pop(), ), - title: detail.maybeWhen( - data: (d) => Text( - d.playlist.name, - style: TextStyle(color: fs.parchment), - overflow: TextOverflow.ellipsis, - ), - orElse: () => const SizedBox.shrink(), - ), + title: headerName.isEmpty + ? const SizedBox.shrink() + : Text( + headerName, + style: TextStyle(color: fs.parchment), + overflow: TextOverflow.ellipsis, + ), ), body: detail.when( - loading: () => const Center(child: CircularProgressIndicator()), + // Loading from a seed: render the header immediately + a + // small inline "loading tracks" hint, instead of an opaque + // full-screen spinner. The body fills in when tracks arrive. + loading: () => seed == null + ? const Center(child: CircularProgressIndicator()) + : _SeedBody(playlist: seed!), error: (e, _) => Center(child: Text('$e', style: TextStyle(color: fs.error))), data: (d) => _Body(detail: d), @@ -47,6 +64,43 @@ class PlaylistDetailScreen extends ConsumerWidget { } } +/// Loading-state body: renders just the header from the seed +/// playlist + a "loading tracks…" placeholder. CTA buttons are +/// hidden until the real track list arrives (they need playable +/// refs to do anything useful). +class _SeedBody extends StatelessWidget { + const _SeedBody({required this.playlist}); + final Playlist playlist; + + @override + Widget build(BuildContext context) { + final fs = Theme.of(context).extension()!; + return ListView(children: [ + Padding( + padding: const EdgeInsets.fromLTRB(16, 8, 16, 16), + child: Column(crossAxisAlignment: CrossAxisAlignment.start, children: [ + if (playlist.description.isNotEmpty) + Padding( + padding: const EdgeInsets.only(bottom: 12), + child: Text( + playlist.description, + style: TextStyle(color: fs.ash, fontSize: 13), + ), + ), + Text( + '${playlist.trackCount} ${playlist.trackCount == 1 ? "track" : "tracks"}', + style: TextStyle(color: fs.ash, fontSize: 12), + ), + ]), + ), + const Padding( + padding: EdgeInsets.symmetric(vertical: 24), + child: Center(child: CircularProgressIndicator()), + ), + ]); + } +} + class _Body extends ConsumerWidget { const _Body({required this.detail}); final PlaylistDetail detail; diff --git a/flutter_client/lib/playlists/playlists_list_screen.dart b/flutter_client/lib/playlists/playlists_list_screen.dart index 7cf4172a..bfddbf9a 100644 --- a/flutter_client/lib/playlists/playlists_list_screen.dart +++ b/flutter_client/lib/playlists/playlists_list_screen.dart @@ -51,7 +51,7 @@ class PlaylistsListScreen extends ConsumerWidget { final p = items[i]; return _PlaylistTile( playlist: p, - onTap: () => ctx.push('/playlists/${p.id}'), + onTap: () => ctx.push('/playlists/${p.id}', extra: p), ); }, ), diff --git a/flutter_client/lib/playlists/widgets/playlist_card.dart b/flutter_client/lib/playlists/widgets/playlist_card.dart index 060351a4..515bd55d 100644 --- a/flutter_client/lib/playlists/widgets/playlist_card.dart +++ b/flutter_client/lib/playlists/widgets/playlist_card.dart @@ -21,7 +21,8 @@ class PlaylistCard extends StatelessWidget { child: Material( color: Colors.transparent, child: InkWell( - onTap: () => context.push('/playlists/${playlist.id}'), + onTap: () => + context.push('/playlists/${playlist.id}', extra: playlist), child: Padding( padding: const EdgeInsets.symmetric(horizontal: 8), child: Column(crossAxisAlignment: CrossAxisAlignment.start, children: [ diff --git a/flutter_client/lib/shared/routing.dart b/flutter_client/lib/shared/routing.dart index 73c3eac5..cc29e387 100644 --- a/flutter_client/lib/shared/routing.dart +++ b/flutter_client/lib/shared/routing.dart @@ -9,6 +9,7 @@ import '../library/album_detail_screen.dart'; import '../library/artist_detail_screen.dart'; import '../models/album.dart'; import '../models/artist.dart'; +import '../models/playlist.dart'; import '../discover/discover_screen.dart'; import '../library/home_screen.dart'; import '../library/library_screen.dart'; @@ -120,7 +121,10 @@ GoRouter buildRouter(Ref ref) { GoRoute(path: '/playlists', builder: (_, __) => const PlaylistsListScreen()), GoRoute( path: '/playlists/:id', - builder: (_, s) => PlaylistDetailScreen(id: s.pathParameters['id']!), + builder: (_, s) => PlaylistDetailScreen( + id: s.pathParameters['id']!, + seed: s.extra is Playlist ? s.extra as Playlist : null, + ), ), GoRoute(path: '/requests', builder: (_, __) => const RequestsScreen()), GoRoute(path: '/admin', builder: (_, __) => const AdminLandingScreen()), From ab62a3d118066030fd69d5a7fb68db5b4809bc73 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 22:31:45 -0400 Subject: [PATCH 3/8] fix(flutter): cancel stale background fills when queue changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lazy source-build commit (1ddde12) introduced a race: when the user taps play on a new track while a previous queue's _fillRemainingSources is still working, the stale fill keeps calling _player.addAudioSource() on the new player state — appending old-playlist tracks into the new queue and confusing the player into the "locked to one song" symptom. Fix: queue-generation counter. setQueueFromTracks bumps _queueGeneration first thing; the background fill captures its gen at start and aborts before any further player mutation if a newer queue has taken over. The previous play() never gets to mutate the new player state. Also resets _suppressIndexUpdates at the start of every setQueueFromTracks (defensive — covers the case where a prior backward-fill bailed on its gen check before reaching `finally`) and only releases the flag in finally if we're still the active gen. Symptoms this should resolve: - "locked to one song" after rapid play taps - Late `play() returned 73189ms` lines indicating a previous hung play() call finally resolving and stepping on current state - Player stuck in odd processingState after queue swaps --- flutter_client/lib/player/audio_handler.dart | 46 ++++++++++++++++++-- 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/flutter_client/lib/player/audio_handler.dart b/flutter_client/lib/player/audio_handler.dart index 663f9c28..bec44789 100644 --- a/flutter_client/lib/player/audio_handler.dart +++ b/flutter_client/lib/player/audio_handler.dart @@ -70,6 +70,13 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl /// is already correct, and we re-enable normal listener behavior. bool _suppressIndexUpdates = false; + /// Increments on every setQueueFromTracks call. The background + /// fill task captures the value at start and bails if the live + /// counter has moved on — without this, a stale fill will keep + /// addAudioSource'ing tracks from the previous playlist into the + /// new queue, leaving the player "locked" to a corrupted state. + int _queueGeneration = 0; + /// Volume stream for UI subscribers. Mirrors the just_audio player's /// volume directly; set via setVolume(double). Stream get volumeStream => _player.volumeStream; @@ -103,6 +110,15 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl if (tracks.isEmpty) return; final clampedInitial = initialIndex.clamp(0, tracks.length - 1); + // Bump the generation FIRST. Any in-flight _fillRemainingSources + // from a previous play will see the mismatch on its next gen + // check and stop calling player mutations — important so a stale + // fill doesn't append old-playlist tracks into the new queue. + final myGen = ++_queueGeneration; + // Reset suppress flag in case a prior backward-fill bailed on + // gen check before reaching its `finally`. + _suppressIndexUpdates = false; + // Populate the visible queue + current mediaItem immediately so // the player UI reflects the user's tap before any source has // been built. Source list at the just_audio layer fills in @@ -112,7 +128,7 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl mediaItem.add(items[clampedInitial]); debugPrint('audio_handler.setQueueFromTracks: ' - '_baseUrl="$_baseUrl" trackCount=${tracks.length}'); + '_baseUrl="$_baseUrl" trackCount=${tracks.length} gen=$myGen'); final sw = Stopwatch()..start(); // Fast path: build only the initial source so the player can @@ -120,6 +136,10 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl // in the background — addAudioSource for next/auto-advance // tracks, insertAudioSource for skipPrev tracks. final initial = await _buildAudioSource(tracks[clampedInitial]); + if (myGen != _queueGeneration) { + debugPrint('audio_handler: superseded before setAudioSources (gen=$myGen)'); + return; + } debugPrint('audio_handler: built initial source in ' '${sw.elapsedMilliseconds}ms'); sw.reset(); @@ -129,7 +149,7 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl debugPrint('audio_handler: setAudioSources(1) ${sw.elapsedMilliseconds}ms'); unawaited(_loadArtForCurrentItem()); - unawaited(_fillRemainingSources(tracks, clampedInitial)); + unawaited(_fillRemainingSources(tracks, clampedInitial, myGen)); } /// Background fill of the rest of the just_audio source list after @@ -138,12 +158,23 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl /// currentIndex; we suppress _onCurrentIndexChanged side effects /// for those so the mediaItem stream doesn't bounce to the wrong /// queue entry. + /// + /// `gen` is the queue generation captured when this fill started. + /// Every loop iteration checks against _queueGeneration; if a + /// newer setQueueFromTracks has run (user tapped play on something + /// else), bail immediately so we don't pollute the new queue with + /// addAudioSource calls from this stale fill. Future _fillRemainingSources( - List tracks, int initialIndex) async { + List tracks, int initialIndex, int gen) async { final sw = Stopwatch()..start(); for (var i = initialIndex + 1; i < tracks.length; i++) { + if (gen != _queueGeneration) { + debugPrint('audio_handler: forward fill aborted (gen=$gen, current=$_queueGeneration)'); + return; + } try { final src = await _buildAudioSource(tracks[i]); + if (gen != _queueGeneration) return; await _player.addAudioSource(src); } catch (e) { debugPrint('audio_handler: forward fill failed for ${tracks[i].id}: $e'); @@ -157,13 +188,20 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl _suppressIndexUpdates = true; try { for (var i = 0; i < initialIndex; i++) { + if (gen != _queueGeneration) { + debugPrint('audio_handler: backward fill aborted (gen=$gen, current=$_queueGeneration)'); + return; + } final src = await _buildAudioSource(tracks[i]); + if (gen != _queueGeneration) return; await _player.insertAudioSource(i, src); } } catch (e) { debugPrint('audio_handler: backward fill failed: $e'); } finally { - _suppressIndexUpdates = false; + // Only release the flag if we're still the active gen — a + // newer setQueueFromTracks already reset it for itself. + if (gen == _queueGeneration) _suppressIndexUpdates = false; } debugPrint('audio_handler: backward fill ($initialIndex) ' '${sw.elapsedMilliseconds}ms'); From e856172d608523928d65774836195e9cc7ec6a61 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 23:16:42 -0400 Subject: [PATCH 4/8] chore(flutter): drop success-path diagnostic prints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cleanup pass on the noisy debugPrints we added during the recent debugging sessions. Remaining prints are error-path only: - AlbumCoverCache fetch failed - album/playlist cold-cache fetch failed - audio_handler playbackEventStream error - audio_handler queue supersession (rare race) - audio_handler forward/backward fill failed - artist_detail play failed - cacheFirst fetchAndPopulate failed (only when tag is set) Removed per-event chatter: - audio_handler player-state and processingState subscribers - audio_handler timing measurements (built initial / setAudioSources / forward fill / backward fill) - audio_handler cache hit/miss per-track (fired N times per play) - audio_handler register stream cache (verifiable via Settings) - audio_handler.configure log - albumProvider step-by-step lines (drift miss / online / drift hit / album hit but no tracks) - playlistDetailProvider step-by-step lines (calling get / drift write done / 404 evicted / drift hit / playlist hit but no tracks) - playlistsListProvider wire returned + drift rows lines - playTracks stage timings (serverUrl / token / configure / setQueue / play returned) - metadataPrefetcher warming N artists - artist_detail play tapped — N tracks success log - cacheFirst step-by-step (drift hit / drift miss / online / done) `tag` parameter on cacheFirst preserved for ad-hoc instrumentation. --- flutter_client/lib/cache/cache_first.dart | 16 +++---- .../lib/cache/metadata_prefetcher.dart | 2 - .../lib/library/artist_detail_screen.dart | 2 - .../lib/library/library_providers.dart | 11 +---- flutter_client/lib/player/audio_handler.dart | 45 +------------------ .../lib/player/player_provider.dart | 18 -------- .../lib/playlists/playlists_provider.dart | 25 ----------- 7 files changed, 10 insertions(+), 109 deletions(-) diff --git a/flutter_client/lib/cache/cache_first.dart b/flutter_client/lib/cache/cache_first.dart index ad4e3beb..8d24d11b 100644 --- a/flutter_client/lib/cache/cache_first.dart +++ b/flutter_client/lib/cache/cache_first.dart @@ -19,6 +19,10 @@ import 'dart:async'; import 'package:flutter/foundation.dart' show debugPrint; +// `tag` parameter is preserved for future ad-hoc instrumentation. +// Normal operation only logs failure paths so the per-screen log +// noise stays low. + /// Wraps the watch + cold-cache fallback pattern. Generic over: /// D — the drift row type (or TypedResult for joins) /// T — the result type the caller wants (e.g. `List`) @@ -43,13 +47,9 @@ Stream cacheFirst({ // refresh for this stream subscription, so we don't fire one on every // drift re-emission (otherwise the populate cycles forever). var revalidated = false; - void log(String msg) { - if (tag != null) debugPrint('cacheFirst[$tag]: $msg'); - } await for (final rows in driftStream) { if (rows.isNotEmpty) { - log('drift hit (${rows.length} rows)'); yield toResult(rows); // Stale-while-revalidate: yield cache immediately, then kick off // a REST refresh in the background. Drift watch() picks up the @@ -62,20 +62,18 @@ Stream cacheFirst({ } continue; } - log('drift miss; checking connectivity'); if (await isOnline()) { - log('online; calling fetchAndPopulate'); try { await fetchAndPopulate(); - log('fetchAndPopulate done; awaiting drift re-emit'); // The drift watch() stream re-emits with the populated rows on // the next loop iteration. Don't yield here. } catch (e, st) { - log('fetchAndPopulate failed: $e\n$st'); + if (tag != null) { + debugPrint('cacheFirst[$tag]: fetchAndPopulate failed: $e\n$st'); + } yield toResult(rows); // empty result; caller surfaces error } } else { - log('offline; yielding empty'); yield toResult(rows); // empty result; offline } } diff --git a/flutter_client/lib/cache/metadata_prefetcher.dart b/flutter_client/lib/cache/metadata_prefetcher.dart index 2eef60cb..7acee535 100644 --- a/flutter_client/lib/cache/metadata_prefetcher.dart +++ b/flutter_client/lib/cache/metadata_prefetcher.dart @@ -1,4 +1,3 @@ -import 'package:flutter/foundation.dart' show debugPrint; import 'package:flutter_riverpod/flutter_riverpod.dart'; import '../library/library_providers.dart'; @@ -38,7 +37,6 @@ class MetadataPrefetcher { if (n++ >= _topN) break; _swallow(_ref.read(artistProvider(id).future)); } - if (n > 0) debugPrint('metadataPrefetcher: warming $n artists'); } void _warmHome(HomeData h) { diff --git a/flutter_client/lib/library/artist_detail_screen.dart b/flutter_client/lib/library/artist_detail_screen.dart index 3f624418..9f83ec38 100644 --- a/flutter_client/lib/library/artist_detail_screen.dart +++ b/flutter_client/lib/library/artist_detail_screen.dart @@ -95,8 +95,6 @@ class ArtistDetailScreen extends ConsumerWidget { try { final tracks = await ref.read(artistTracksProvider(id).future); - debugPrint( - 'artist_detail: play tapped — ${tracks.length} tracks for $id'); if (tracks.isEmpty) { if (context.mounted) { ScaffoldMessenger.of(context).showSnackBar( diff --git a/flutter_client/lib/library/library_providers.dart b/flutter_client/lib/library/library_providers.dart index 28e0a296..26ecd296 100644 --- a/flutter_client/lib/library/library_providers.dart +++ b/flutter_client/lib/library/library_providers.dart @@ -187,11 +187,9 @@ final albumProvider = StreamProvider.family< Future fetchAndPopulate() async { try { final api = await ref.read(libraryApiProvider.future); - debugPrint('albumProvider($albumId): calling getAlbum'); final fresh = await api .getAlbum(albumId) .timeout(const Duration(seconds: 10)); - debugPrint('albumProvider($albumId): got ${fresh.tracks.length} tracks, writing to drift'); // Collect every artist mentioned by the album + its tracks so // the JOINs that drive both the album header and the track rows // have something to bind to. Without this, drift returns null @@ -224,7 +222,6 @@ final albumProvider = StreamProvider.family< b.insertAllOnConflictUpdate(db.cachedTracks, fresh.tracks.map((t) => t.toDrift()).toList()); }); - debugPrint('albumProvider($albumId): drift write done; awaiting watch re-emit'); return true; } catch (e, st) { debugPrint('albumProvider($albumId): cold-cache fetch failed: $e\n$st'); @@ -235,7 +232,6 @@ final albumProvider = StreamProvider.family< await for (final albumRows in albumQuery.watch()) { // Case 1: no album row at all → cold-fetch. if (albumRows.isEmpty) { - debugPrint('albumProvider($albumId): drift miss, attempting cold-cache fetch'); if (fetchAttempted) { // Already tried and got nothing back. yield ( @@ -245,10 +241,7 @@ final albumProvider = StreamProvider.family< continue; } fetchAttempted = true; - final online = await isOnline(); - debugPrint('albumProvider($albumId): online=$online'); - if (!online) { - debugPrint('albumProvider($albumId): offline, yielding empty'); + if (!await isOnline()) { yield ( album: const AlbumRef(id: '', title: '', artistId: ''), tracks: const [], @@ -265,7 +258,6 @@ final albumProvider = StreamProvider.family< // On success, drift watch re-emits with rows; loop continues. continue; } - debugPrint('albumProvider($albumId): drift hit (${albumRows.length} rows)'); final albumRow = albumRows.first; final album = albumRow.readTable(db.cachedAlbums).toRef( @@ -280,7 +272,6 @@ final albumProvider = StreamProvider.family< // fetchAndPopulate so the album becomes complete; drift watch // re-emits and we land in the populated branch on the next pass. if (trackRows.isEmpty && !fetchAttempted) { - debugPrint('albumProvider($albumId): album hit but no tracks; fetching'); fetchAttempted = true; if (await isOnline()) { final ok = await fetchAndPopulate(); diff --git a/flutter_client/lib/player/audio_handler.dart b/flutter_client/lib/player/audio_handler.dart index bec44789..db622187 100644 --- a/flutter_client/lib/player/audio_handler.dart +++ b/flutter_client/lib/player/audio_handler.dart @@ -33,15 +33,6 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl // the index and the eviction loop can't reclaim them. _player.bufferedPositionStream .listen((_) => unawaited(_maybeRegisterStreamCache())); - // Diagnostic: log every player-state transition so silent stops - // surface as something we can read in the log. - _player.playerStateStream.listen((s) { - debugPrint( - 'audio_handler: state playing=${s.playing} processing=${s.processingState}'); - }); - _player.processingStateStream.listen((ps) { - debugPrint('audio_handler: processingState=$ps'); - }); } final AudioPlayer _player = AudioPlayer(); @@ -100,10 +91,6 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl _token = token; if (coverCache != null) _coverCache = coverCache; if (audioCacheManager != null) _audioCacheManager = audioCacheManager; - debugPrint('audio_handler.configure: baseUrl="$baseUrl" ' - 'tokenPresent=${token != null && token.isNotEmpty} ' - 'coverCachePresent=${_coverCache != null} ' - 'audioCachePresent=${_audioCacheManager != null}'); } Future setQueueFromTracks(List tracks, {int initialIndex = 0}) async { @@ -127,10 +114,6 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl queue.add(items); mediaItem.add(items[clampedInitial]); - debugPrint('audio_handler.setQueueFromTracks: ' - '_baseUrl="$_baseUrl" trackCount=${tracks.length} gen=$myGen'); - final sw = Stopwatch()..start(); - // Fast path: build only the initial source so the player can // start. Remaining sources stream in via _fillRemainingSources() // in the background — addAudioSource for next/auto-advance @@ -140,13 +123,8 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl debugPrint('audio_handler: superseded before setAudioSources (gen=$myGen)'); return; } - debugPrint('audio_handler: built initial source in ' - '${sw.elapsedMilliseconds}ms'); - sw.reset(); - sw.start(); await _player.setAudioSources([initial], initialIndex: 0); - debugPrint('audio_handler: setAudioSources(1) ${sw.elapsedMilliseconds}ms'); unawaited(_loadArtForCurrentItem()); unawaited(_fillRemainingSources(tracks, clampedInitial, myGen)); @@ -166,12 +144,8 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl /// addAudioSource calls from this stale fill. Future _fillRemainingSources( List tracks, int initialIndex, int gen) async { - final sw = Stopwatch()..start(); for (var i = initialIndex + 1; i < tracks.length; i++) { - if (gen != _queueGeneration) { - debugPrint('audio_handler: forward fill aborted (gen=$gen, current=$_queueGeneration)'); - return; - } + if (gen != _queueGeneration) return; try { final src = await _buildAudioSource(tracks[i]); if (gen != _queueGeneration) return; @@ -180,18 +154,11 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl debugPrint('audio_handler: forward fill failed for ${tracks[i].id}: $e'); } } - debugPrint('audio_handler: forward fill (${tracks.length - initialIndex - 1}) ' - '${sw.elapsedMilliseconds}ms'); if (initialIndex > 0) { - sw.reset(); - sw.start(); _suppressIndexUpdates = true; try { for (var i = 0; i < initialIndex; i++) { - if (gen != _queueGeneration) { - debugPrint('audio_handler: backward fill aborted (gen=$gen, current=$_queueGeneration)'); - return; - } + if (gen != _queueGeneration) return; final src = await _buildAudioSource(tracks[i]); if (gen != _queueGeneration) return; await _player.insertAudioSource(i, src); @@ -203,8 +170,6 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl // newer setQueueFromTracks already reset it for itself. if (gen == _queueGeneration) _suppressIndexUpdates = false; } - debugPrint('audio_handler: backward fill ($initialIndex) ' - '${sw.elapsedMilliseconds}ms'); } } @@ -234,7 +199,6 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl if (mgr != null) { final path = await mgr.pathFor(t.id); if (path != null) { - debugPrint('audio_handler: cache hit for track.id=${t.id} → file://$path'); return AudioSource.uri(Uri.file(path)); } } @@ -260,15 +224,12 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl if (mgr != null) { _cacheDirPath ??= (await getApplicationCacheDirectory()).path; final cacheFile = File('${_cacheDirPath!}/audio_cache/${t.id}.mp3'); - debugPrint('audio_handler: cache miss for track.id=${t.id}, ' - 'using LockCachingAudioSource → ${cacheFile.path}'); // ignore: experimental_member_use return LockCachingAudioSource(parsed, headers: headers, cacheFile: cacheFile); } // 3. No manager configured: plain network source (legacy path). - debugPrint('audio_handler: no cache manager; track.id=${t.id} → "$url"'); return AudioSource.uri(parsed, headers: headers); } @@ -344,8 +305,6 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl _streamCacheRegistered.add(trackId); await mgr.registerStreamCache(trackId, path, size); - debugPrint( - 'audio_handler: registered stream cache for $trackId ($size bytes)'); } void _onCurrentIndexChanged(int? idx) { diff --git a/flutter_client/lib/player/player_provider.dart b/flutter_client/lib/player/player_provider.dart index 47e51879..baca1af3 100644 --- a/flutter_client/lib/player/player_provider.dart +++ b/flutter_client/lib/player/player_provider.dart @@ -1,5 +1,4 @@ import 'package:audio_service/audio_service.dart'; -import 'package:flutter/foundation.dart' show debugPrint; import 'package:flutter_riverpod/flutter_riverpod.dart'; import '../api/endpoints/radio.dart'; @@ -54,22 +53,8 @@ class PlayerActions { final Ref _ref; Future playTracks(List tracks, {int initialIndex = 0}) async { - // Stage-by-stage timing so we can see exactly where the - // "tap → audio" delay lives. Look for "playTracks: ..." lines - // in the next log capture; each stage label is followed by its - // elapsed millis since the previous stage. - final sw = Stopwatch()..start(); - int lap() { - final ms = sw.elapsedMilliseconds; - sw.reset(); - sw.start(); - return ms; - } - final url = await _ref.read(serverUrlProvider.future); - debugPrint('playTracks: serverUrl ${lap()}ms'); final token = await _ref.read(secureStorageProvider).read(key: 'session_token'); - debugPrint('playTracks: token ${lap()}ms'); final cache = _ref.read(albumCoverCacheProvider); final audioCache = _ref.read(audioCacheManagerProvider); final h = _ref.read(audioHandlerProvider) @@ -79,11 +64,8 @@ class PlayerActions { coverCache: cache, audioCacheManager: audioCache, ); - debugPrint('playTracks: configure ${lap()}ms'); await h.setQueueFromTracks(tracks, initialIndex: initialIndex); - debugPrint('playTracks: setQueue (${tracks.length} tracks) ${lap()}ms'); await h.play(); - debugPrint('playTracks: play() returned ${lap()}ms'); } Future playNext(TrackRef track) async { diff --git a/flutter_client/lib/playlists/playlists_provider.dart b/flutter_client/lib/playlists/playlists_provider.dart index dc4105d0..9b4e9c64 100644 --- a/flutter_client/lib/playlists/playlists_provider.dart +++ b/flutter_client/lib/playlists/playlists_provider.dart @@ -40,11 +40,6 @@ final playlistsListProvider = fetchAndPopulate: () async { final api = await ref.read(playlistsApiProvider.future); final fresh = await api.list(kind: kind); - final ownedSysCount = - fresh.owned.where((p) => p.systemVariant != null).length; - debugPrint( - 'playlistsListProvider($kind): wire returned owned=${fresh.owned.length} ' - '(system=$ownedSysCount) public=${fresh.public.length}'); // Reconcile: BuildSystemPlaylists rotates system-playlist UUIDs // every rebuild, so insertOrReplace alone leaves stale rows in @@ -81,13 +76,6 @@ final playlistsListProvider = .where((r) => r.userId != user.id && r.isPublic) .map((r) => r.toRef()) .toList(); - final ownedSys = owned.where((p) => p.isSystem).length; - final driftSys = filtered.where((r) => r.systemVariant != null).length; - debugPrint( - 'playlistsListProvider($kind): drift rows=${rows.length} ' - 'filtered=${filtered.length} (system=$driftSys) ' - 'owned=${owned.length} (system=$ownedSys) pub=${pub.length} ' - 'userId=${user.id}'); return PlaylistsList(owned: owned, public: pub); }, isOnline: () async => (await ref @@ -153,10 +141,7 @@ final playlistDetailProvider = Future fetchAndPopulate() async { try { final api = await ref.read(playlistsApiProvider.future); - debugPrint('playlistDetailProvider($id): calling get'); final fresh = await api.get(id).timeout(const Duration(seconds: 10)); - debugPrint( - 'playlistDetailProvider($id): got ${fresh.tracks.length} tracks, writing'); // Collect the track + artist + album rows referenced by these // playlist entries. Without writing the cachedTracks rows @@ -262,8 +247,6 @@ final playlistDetailProvider = ); } }); - debugPrint( - 'playlistDetailProvider($id): drift write done; awaiting watch re-emit'); return true; } catch (e, st) { // 404 = the playlist row in drift is stale (BuildSystemPlaylists @@ -272,8 +255,6 @@ final playlistDetailProvider = // stale row + its track positions so the home tile disappears // on next render and we don't keep trying. if (e is DioException && e.response?.statusCode == 404) { - debugPrint( - 'playlistDetailProvider($id): server says 404, evicting stale drift rows'); await db.batch((b) { b.deleteWhere( db.cachedPlaylistTracks, (t) => t.playlistId.equals(id)); @@ -292,7 +273,6 @@ final playlistDetailProvider = await for (final playlistRows in playlistQuery.watch()) { if (playlistRows.isEmpty) { - debugPrint('playlistDetailProvider($id): drift miss, cold-fetching'); if (fetchAttempted) { yield emptyDetail(); continue; @@ -307,9 +287,6 @@ final playlistDetailProvider = continue; } - debugPrint( - 'playlistDetailProvider($id): drift hit (${playlistRows.length} rows)'); - final playlist = playlistRows.first.toRef(); final trackRows = await tracksQuery.get(); @@ -318,8 +295,6 @@ final playlistDetailProvider = // tracks for system playlists). Trigger the same fetch, drift // watch re-emits with populated tracks. if (trackRows.isEmpty && !fetchAttempted) { - debugPrint( - 'playlistDetailProvider($id): playlist hit but no tracks; fetching'); fetchAttempted = true; if (await isOnline()) { final ok = await fetchAndPopulate(); From 96aa2407d9aca83898e90724207c9250fbcad8e1 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 23:41:01 -0400 Subject: [PATCH 5/8] feat(home): surface Discover system playlist as a tile (web + flutter) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Server has been generating system_variant='discover' playlists since M6a (internal/playlists/system.go and POST /api/playlists/system/discover/refresh) but neither client surfaced it. The Flutter home filtered system playlists by exact match on 'for_you' and 'songs_like_artist', dropping Discover. The web home did the same. Tiles were generated server-side and silently discarded by both clients. Add a Discover slot to the playlists row in both: - Flutter: 5-slot row now (For You, Discover, 3× Songs-like) with matching placeholder state machine. - Web: same shape, same placeholderVariant() call. When the engine has built it, the real playlist tile renders; otherwise a placeholder with the same building/pending/failed status semantics as the other system tiles. --- flutter_client/lib/library/home_screen.dart | 11 ++++++++++- web/src/routes/+page.svelte | 17 +++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/flutter_client/lib/library/home_screen.dart b/flutter_client/lib/library/home_screen.dart index bf455251..41dc5d04 100644 --- a/flutter_client/lib/library/home_screen.dart +++ b/flutter_client/lib/library/home_screen.dart @@ -135,7 +135,16 @@ List<_PlaylistRowItem> _buildPlaylistsRow( ? _RealPlaylist(forYou) : _PlaceholderPlaylist('For You', _variantFor('for-you', status))); - // Slots 2-4: Songs-like (real first, padded to 3). + // Slot 2: Discover. Server emits this with system_variant='discover' + // when the recommendation engine has eligible tracks; before then, + // show a placeholder in the same "building/pending" state machine + // as For-You. + final discover = findFirst((p) => p.systemVariant == 'discover'); + out.add(discover != null + ? _RealPlaylist(discover) + : _PlaceholderPlaylist('Discover', _variantFor('discover', status))); + + // Slots 3-5: Songs-like (real first, padded to 3). final songsLike = ownedAll .where((p) => p.systemVariant == 'songs_like_artist') .take(3) diff --git a/web/src/routes/+page.svelte b/web/src/routes/+page.svelte index 1ecdfb48..5649a904 100644 --- a/web/src/routes/+page.svelte +++ b/web/src/routes/+page.svelte @@ -39,6 +39,10 @@ (systemPlaylistsQ.data?.owned ?? []).find((p) => p.system_variant === 'for_you') ?? null ); + const discoverPlaylist = $derived( + (systemPlaylistsQ.data?.owned ?? []).find((p) => p.system_variant === 'discover') ?? null + ); + const songsLikePlaylists = $derived( (systemPlaylistsQ.data?.owned ?? []) .filter((p) => p.system_variant === 'songs_like_artist') @@ -48,7 +52,7 @@ const userPlaylists = $derived(userPlaylistsQ.data?.owned ?? []); type PlaceholderVariant = 'building' | 'failed' | 'pending' | 'seed-needed'; - function placeholderVariant(slot: 'for-you' | 'songs-like'): PlaceholderVariant { + function placeholderVariant(slot: 'for-you' | 'discover' | 'songs-like'): PlaceholderVariant { const s = systemStatusQ.data; if (!s) return 'pending'; if (s.in_flight) return 'building'; @@ -71,7 +75,16 @@ out.push({ kind: 'placeholder', label: 'For You', variant: placeholderVariant('for-you') }); } - // Slots 2-4: Songs-like (real first, padded with placeholders). + // Slot 2: Discover (real or placeholder). Server emits this with + // system_variant='discover' once the recommendation engine has + // eligible tracks. + if (discoverPlaylist) { + out.push({ kind: 'real', playlist: discoverPlaylist }); + } else { + out.push({ kind: 'placeholder', label: 'Discover', variant: placeholderVariant('discover') }); + } + + // Slots 3-5: Songs-like (real first, padded with placeholders). for (let i = 0; i < 3; i++) { if (songsLikePlaylists[i]) { out.push({ kind: 'real', playlist: songsLikePlaylists[i] }); From 356f8f5d6cc3f9c4484bb21275e98f0f4ad2d0c7 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 23:46:07 -0400 Subject: [PATCH 6/8] test(web): update home placeholder counts for new Discover slot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 5-slot system row (For-You, Discover, 3× Songs-like) means: - empty state: 5 placeholders (was 4) - For-You only: 4 placeholders (was 3) — Discover + 3 songs-like --- web/src/routes/page.test.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/web/src/routes/page.test.ts b/web/src/routes/page.test.ts index 10a5466e..884dd067 100644 --- a/web/src/routes/page.test.ts +++ b/web/src/routes/page.test.ts @@ -54,10 +54,11 @@ beforeEach(() => { afterEach(() => vi.clearAllMocks()); describe('home Playlists section', () => { - test('renders 4 placeholder cards when no playlists exist', () => { + test('renders 5 placeholder cards when no playlists exist', () => { + // Slot layout: For-You, Discover, 3× Songs-like. render(Page); const placeholders = screen.queryAllByTestId('playlist-placeholder-card'); - expect(placeholders).toHaveLength(4); + expect(placeholders).toHaveLength(5); }); test('building status sets variant=building on placeholders', () => { @@ -103,6 +104,8 @@ describe('home Playlists section', () => { render(Page); expect(screen.getByText('For You')).toBeInTheDocument(); const placeholders = screen.queryAllByTestId('playlist-placeholder-card'); - expect(placeholders).toHaveLength(3); // 3 songs-like slots + // 1 Discover + 3 Songs-like slots = 4 placeholders alongside the + // real For-You tile. + expect(placeholders).toHaveLength(4); }); }); From a09b636e1af42c6ea3d64126ee961772fe8e7da3 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 23:50:42 -0400 Subject: [PATCH 7/8] fix(flutter): full player kebab Go to album/artist navigates cleanly Same root cause as the /queue duplicate-page-key crash: /now-playing is a top-level route (lives outside the ShellRoute), but /artists/:id and /albums/:id are shell-children. Pushing a shell- child from a top-level route makes go_router attempt to mount a second ShellRoute on top of the active one, leaving navigation in a broken state. The mini player works because it's already inside the shell. Add an optional onBeforeNavigate callback to TrackActionsSheet (forwarded through TrackActionsButton). When set, fires after sheet.pop() and before context.push() of the destination route. Wire the full player's TrackActionsButton with onBeforeNavigate: () => Navigator.of(context).maybePop() so /now-playing dismisses itself before the detail route is pushed. Result: clean navigation into the destination, mini player visible underneath as expected. Mini player keeps the default (no callback) since it's already in the shell. --- flutter_client/lib/player/now_playing_screen.dart | 6 ++++++ .../widgets/track_actions/track_actions_button.dart | 7 +++++++ .../widgets/track_actions/track_actions_sheet.dart | 13 +++++++++++++ 3 files changed, 26 insertions(+) diff --git a/flutter_client/lib/player/now_playing_screen.dart b/flutter_client/lib/player/now_playing_screen.dart index 804c4ef3..e7547b7e 100644 --- a/flutter_client/lib/player/now_playing_screen.dart +++ b/flutter_client/lib/player/now_playing_screen.dart @@ -397,6 +397,12 @@ class _SecondaryControls extends StatelessWidget { streamUrl: '', ), hideQueueActions: true, + // /now-playing lives outside the ShellRoute. Pushing + // /artists/:id or /albums/:id (both shell-children) on top + // of it would make go_router try to mount a duplicate + // ShellRoute. Pop ourselves first so the destination route + // mounts cleanly inside the existing shell. + onBeforeNavigate: () => Navigator.of(context).maybePop(), ), ], ); diff --git a/flutter_client/lib/shared/widgets/track_actions/track_actions_button.dart b/flutter_client/lib/shared/widgets/track_actions/track_actions_button.dart index cd293209..61b67575 100644 --- a/flutter_client/lib/shared/widgets/track_actions/track_actions_button.dart +++ b/flutter_client/lib/shared/widgets/track_actions/track_actions_button.dart @@ -11,6 +11,7 @@ class TrackActionsButton extends StatelessWidget { super.key, required this.track, this.hideQueueActions = false, + this.onBeforeNavigate, }); final TrackRef track; @@ -19,6 +20,11 @@ class TrackActionsButton extends StatelessWidget { /// screen where the menu's track IS the currently-playing one. final bool hideQueueActions; + /// Forwarded to TrackActionsSheet.onBeforeNavigate. Use to dismiss + /// the host route before "Go to album" / "Go to artist" pushes the + /// detail screen. + final void Function()? onBeforeNavigate; + @override Widget build(BuildContext context) { final fs = Theme.of(context).extension()!; @@ -29,6 +35,7 @@ class TrackActionsButton extends StatelessWidget { context, track, hideQueueActions: hideQueueActions, + onBeforeNavigate: onBeforeNavigate, ), ); } diff --git a/flutter_client/lib/shared/widgets/track_actions/track_actions_sheet.dart b/flutter_client/lib/shared/widgets/track_actions/track_actions_sheet.dart index c3adaf7e..b689f70d 100644 --- a/flutter_client/lib/shared/widgets/track_actions/track_actions_sheet.dart +++ b/flutter_client/lib/shared/widgets/track_actions/track_actions_sheet.dart @@ -21,15 +21,25 @@ class TrackActionsSheet extends ConsumerWidget { super.key, required this.track, required this.hideQueueActions, + this.onBeforeNavigate, }); final TrackRef track; final bool hideQueueActions; + /// Optional hook invoked just before "Go to album" / "Go to artist" + /// pushes the destination route. Lets callers that live OUTSIDE the + /// ShellRoute (e.g. the full player at /now-playing) pop themselves + /// first, so go_router doesn't try to mount a second ShellRoute on + /// top of the active top-level route — the same duplicate-page-key + /// failure mode we hit when /queue lived inside the shell. + final void Function()? onBeforeNavigate; + static Future show( BuildContext context, TrackRef track, { bool hideQueueActions = false, + void Function()? onBeforeNavigate, }) { return showModalBottomSheet( context: context, @@ -37,6 +47,7 @@ class TrackActionsSheet extends ConsumerWidget { builder: (_) => TrackActionsSheet( track: track, hideQueueActions: hideQueueActions, + onBeforeNavigate: onBeforeNavigate, ), ); } @@ -141,6 +152,7 @@ class TrackActionsSheet extends ConsumerWidget { label: 'Go to album', onTap: () { Navigator.pop(context); + onBeforeNavigate?.call(); context.push('/albums/${track.albumId}'); }, ), @@ -150,6 +162,7 @@ class TrackActionsSheet extends ConsumerWidget { label: 'Go to artist', onTap: () { Navigator.pop(context); + onBeforeNavigate?.call(); context.push('/artists/${track.artistId}'); }, ), From 5fc04f14b79f8609ba5142b1aca9745820dbb05b Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 23:54:11 -0400 Subject: [PATCH 8/8] fix(flutter): full-player kebab nav + restore artistAlbums SWR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two unrelated issues, batched. Full-player kebab → "Go to album/artist" still crashed with _debugCheckDuplicatedPageKeys despite the prior onBeforeNavigate fix. Cause: pop() and push() ran in the same frame, so go_router's page-key reservation table briefly contained both /now-playing AND the destination shell-child, tripping the duplicate-key assert. Refactor: replace onBeforeNavigate with onNavigate(path), where the host receives the path AFTER the sheet pops and owns the navigation entirely. The full player wires: onNavigate: (path) async { await Navigator.of(context).maybePop(); if (context.mounted) GoRouter.of(context).push(path); } Awaiting the pop guarantees /now-playing is fully gone from the navigator's page list before the push starts. Mini player + track rows + everywhere else use the default (no onNavigate) path that does context.push(path) inline — they're already inside the ShellRoute, no race possible. Restored alwaysRefresh: true on artistAlbumsProvider. Dropping it in the cache-loop fix had a side effect: if drift's cachedAlbums held only a subset of an artist's albums (user previously visited just one album by them), the artist detail page rendered that partial list forever — provider only fetched on empty drift, never on partial drift. The metadata prefetcher only mass-warms artistProvider (single row), so re-enabling SWR on artistAlbums won't recreate the storm. --- .../lib/library/library_providers.dart | 8 ++++ .../lib/player/now_playing_screen.dart | 11 ++++-- .../track_actions/track_actions_button.dart | 12 +++--- .../track_actions/track_actions_sheet.dart | 38 ++++++++++++------- 4 files changed, 46 insertions(+), 23 deletions(-) diff --git a/flutter_client/lib/library/library_providers.dart b/flutter_client/lib/library/library_providers.dart index 26ecd296..9482c05c 100644 --- a/flutter_client/lib/library/library_providers.dart +++ b/flutter_client/lib/library/library_providers.dart @@ -101,6 +101,14 @@ final artistAlbumsProvider = isOnline: () async => (await ref .read(connectivityProvider.future) .timeout(const Duration(seconds: 3), onTimeout: () => true)), + // SWR: re-fetch the full album list on every visit. Without this, + // a previously-incomplete drift cache (e.g. user had only opened + // one album by this artist, so cachedAlbums had just that row) + // would render forever as a partial list. The prefetcher only + // warms artistProvider (single row), so this provider isn't + // mass-instantiated and the storm risk that motivated dropping + // alwaysRefresh elsewhere doesn't apply here. + alwaysRefresh: true, tag: 'artistAlbums($artistId)', ); }); diff --git a/flutter_client/lib/player/now_playing_screen.dart b/flutter_client/lib/player/now_playing_screen.dart index e7547b7e..e04a2d64 100644 --- a/flutter_client/lib/player/now_playing_screen.dart +++ b/flutter_client/lib/player/now_playing_screen.dart @@ -400,9 +400,14 @@ class _SecondaryControls extends StatelessWidget { // /now-playing lives outside the ShellRoute. Pushing // /artists/:id or /albums/:id (both shell-children) on top // of it would make go_router try to mount a duplicate - // ShellRoute. Pop ourselves first so the destination route - // mounts cleanly inside the existing shell. - onBeforeNavigate: () => Navigator.of(context).maybePop(), + // ShellRoute (the same _debugCheckDuplicatedPageKeys crash + // we hit before with /queue). Await our own pop fully + // before pushing the destination so go_router never sees + // both pages active in the same frame. + onNavigate: (path) async { + await Navigator.of(context).maybePop(); + if (context.mounted) GoRouter.of(context).push(path); + }, ), ], ); diff --git a/flutter_client/lib/shared/widgets/track_actions/track_actions_button.dart b/flutter_client/lib/shared/widgets/track_actions/track_actions_button.dart index 61b67575..6937e9a9 100644 --- a/flutter_client/lib/shared/widgets/track_actions/track_actions_button.dart +++ b/flutter_client/lib/shared/widgets/track_actions/track_actions_button.dart @@ -11,7 +11,7 @@ class TrackActionsButton extends StatelessWidget { super.key, required this.track, this.hideQueueActions = false, - this.onBeforeNavigate, + this.onNavigate, }); final TrackRef track; @@ -20,10 +20,10 @@ class TrackActionsButton extends StatelessWidget { /// screen where the menu's track IS the currently-playing one. final bool hideQueueActions; - /// Forwarded to TrackActionsSheet.onBeforeNavigate. Use to dismiss - /// the host route before "Go to album" / "Go to artist" pushes the - /// detail screen. - final void Function()? onBeforeNavigate; + /// Forwarded to TrackActionsSheet.onNavigate. The host receives the + /// destination path AFTER the sheet has popped and is responsible + /// for navigating to it (typically: pop self first, then push). + final Future Function(String path)? onNavigate; @override Widget build(BuildContext context) { @@ -35,7 +35,7 @@ class TrackActionsButton extends StatelessWidget { context, track, hideQueueActions: hideQueueActions, - onBeforeNavigate: onBeforeNavigate, + onNavigate: onNavigate, ), ); } diff --git a/flutter_client/lib/shared/widgets/track_actions/track_actions_sheet.dart b/flutter_client/lib/shared/widgets/track_actions/track_actions_sheet.dart index b689f70d..bb5aeef5 100644 --- a/flutter_client/lib/shared/widgets/track_actions/track_actions_sheet.dart +++ b/flutter_client/lib/shared/widgets/track_actions/track_actions_sheet.dart @@ -21,25 +21,27 @@ class TrackActionsSheet extends ConsumerWidget { super.key, required this.track, required this.hideQueueActions, - this.onBeforeNavigate, + this.onNavigate, }); final TrackRef track; final bool hideQueueActions; - /// Optional hook invoked just before "Go to album" / "Go to artist" - /// pushes the destination route. Lets callers that live OUTSIDE the - /// ShellRoute (e.g. the full player at /now-playing) pop themselves - /// first, so go_router doesn't try to mount a second ShellRoute on - /// top of the active top-level route — the same duplicate-page-key - /// failure mode we hit when /queue lived inside the shell. - final void Function()? onBeforeNavigate; + /// Optional hook for "Go to album" / "Go to artist". Called with + /// the destination route path AFTER the sheet has popped. Lets + /// callers that live OUTSIDE the ShellRoute (e.g. the full player + /// at /now-playing) pop themselves first then push, so go_router + /// doesn't try to mount a second ShellRoute on top of the active + /// top-level route. When null, the sheet does its own + /// context.push, which is correct for surfaces already inside the + /// shell (mini player, track rows on detail screens, etc.). + final Future Function(String path)? onNavigate; static Future show( BuildContext context, TrackRef track, { bool hideQueueActions = false, - void Function()? onBeforeNavigate, + Future Function(String path)? onNavigate, }) { return showModalBottomSheet( context: context, @@ -47,7 +49,7 @@ class TrackActionsSheet extends ConsumerWidget { builder: (_) => TrackActionsSheet( track: track, hideQueueActions: hideQueueActions, - onBeforeNavigate: onBeforeNavigate, + onNavigate: onNavigate, ), ); } @@ -152,8 +154,12 @@ class TrackActionsSheet extends ConsumerWidget { label: 'Go to album', onTap: () { Navigator.pop(context); - onBeforeNavigate?.call(); - context.push('/albums/${track.albumId}'); + final path = '/albums/${track.albumId}'; + if (onNavigate != null) { + onNavigate!(path); + } else { + context.push(path); + } }, ), _MenuItem( @@ -162,8 +168,12 @@ class TrackActionsSheet extends ConsumerWidget { label: 'Go to artist', onTap: () { Navigator.pop(context); - onBeforeNavigate?.call(); - context.push('/artists/${track.artistId}'); + final path = '/artists/${track.artistId}'; + if (onNavigate != null) { + onNavigate!(path); + } else { + context.push(path); + } }, ), const _Divider(),