From a7f35a5d6d3fb582de48291bbb42f4d0864f05f2 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 17:25:27 -0400 Subject: [PATCH 01/16] =?UTF-8?q?feat(flutter):=20full=20player=20?= =?UTF-8?q?=E2=80=94=20combined=20action=20row=20above=20seek?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move shuffle / repeat / queue out from below the play controls and combine with the like + kebab into a single row sitting just above the seek bar. Title row drops back to title-only (truly centered now, no Stack needed since nothing competes for the row's right edge). Layout order is now: art → title → artist → album → [shuffle, repeat, queue, like, kebab] → seek → prev/play/next. The "queue" icon in the top-right of the AppBar stays for now — redundant with the new row but matches what users have already muscle-memoried for opening the queue from any player state. --- .../lib/player/now_playing_screen.dart | 87 ++++++++----------- 1 file changed, 37 insertions(+), 50 deletions(-) diff --git a/flutter_client/lib/player/now_playing_screen.dart b/flutter_client/lib/player/now_playing_screen.dart index ff518406..804c4ef3 100644 --- a/flutter_client/lib/player/now_playing_screen.dart +++ b/flutter_client/lib/player/now_playing_screen.dart @@ -113,20 +113,21 @@ class _NowPlayingScreenState extends ConsumerState { overflow: TextOverflow.ellipsis, ), ], - const SizedBox(height: 32), - _SeekRow(position: pos, duration: dur, fs: fs, ref: ref), - const SizedBox(height: 24), - _PrimaryControls( - fs: fs, - ref: ref, - isPlaying: isPlaying, - ), const SizedBox(height: 24), _SecondaryControls( fs: fs, actions: actions, shuffleOn: shuffleOn, repeatMode: repeatMode, + media: media, + ), + const SizedBox(height: 8), + _SeekRow(position: pos, duration: dur, fs: fs, ref: ref), + const SizedBox(height: 24), + _PrimaryControls( + fs: fs, + ref: ref, + isPlaying: isPlaying, ), const SizedBox(height: 24), ], @@ -219,48 +220,14 @@ class _TitleRow extends StatelessWidget { @override Widget build(BuildContext context) { - // Stack: title centered absolutely in the row; like + kebab pinned - // to the right edge. Padding on the title equals the actions' width - // so it stays optically centered without colliding with them. - return SizedBox( - height: 32, - child: Stack( - alignment: Alignment.center, - children: [ - Padding( - padding: const EdgeInsets.symmetric(horizontal: 64), - child: Text( - media.title, - style: TextStyle(color: fs.parchment, fontSize: 22), - maxLines: 1, - overflow: TextOverflow.ellipsis, - textAlign: TextAlign.center, - ), - ), - Positioned( - right: 0, - child: Row( - mainAxisSize: MainAxisSize.min, - children: [ - LikeButton(kind: LikeKind.track, id: media.id, size: 22), - TrackActionsButton( - track: TrackRef( - id: media.id, - title: media.title, - albumId: (media.extras?['album_id'] as String?) ?? '', - albumTitle: media.album ?? '', - artistId: (media.extras?['artist_id'] as String?) ?? '', - artistName: media.artist ?? '', - durationSec: media.duration?.inSeconds ?? 0, - streamUrl: '', - ), - hideQueueActions: true, - ), - ], - ), - ), - ], - ), + // Title-only — like + kebab moved into _SecondaryControls above + // the seek bar so they share the same row as shuffle/repeat/queue. + return Text( + media.title, + style: TextStyle(color: fs.parchment, fontSize: 22), + maxLines: 1, + overflow: TextOverflow.ellipsis, + textAlign: TextAlign.center, ); } } @@ -365,17 +332,23 @@ class _PrimaryControls extends StatelessWidget { } } +/// Action row sitting just above the seek bar. Holds shuffle / repeat +/// / queue plus the like + kebab that used to live in the title row, +/// so the title can sit truly centered above and this row carries +/// every per-track action in one place. class _SecondaryControls extends StatelessWidget { const _SecondaryControls({ required this.fs, required this.actions, required this.shuffleOn, required this.repeatMode, + required this.media, }); final FabledSwordTheme fs; final PlayerActions actions; final bool shuffleOn; final AudioServiceRepeatMode repeatMode; + final MediaItem media; @override Widget build(BuildContext context) { @@ -411,6 +384,20 @@ class _SecondaryControls extends StatelessWidget { icon: Icon(Icons.queue_music, color: fs.ash), onPressed: () => GoRouter.of(context).push('/queue'), ), + LikeButton(kind: LikeKind.track, id: media.id, size: 22), + TrackActionsButton( + track: TrackRef( + id: media.id, + title: media.title, + albumId: (media.extras?['album_id'] as String?) ?? '', + albumTitle: media.album ?? '', + artistId: (media.extras?['artist_id'] as String?) ?? '', + artistName: media.artist ?? '', + durationSec: media.duration?.inSeconds ?? 0, + streamUrl: '', + ), + hideQueueActions: true, + ), ], ); } From c1df2af9920df5e1c6e48746a916fd0eb53c3a5c Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 17:32:57 -0400 Subject: [PATCH 02/16] =?UTF-8?q?fix(flutter):=20/queue=20at=20top=20level?= =?UTF-8?q?=20=E2=80=94=20fixes=20duplicate-key=20crash=20from=20player?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tapping the queue button from /now-playing crashed with NavigatorState._debugCheckDuplicatedPageKeys. Root cause: when I moved /now-playing out of the ShellRoute earlier, /queue was left inside the shell. Pushing /queue while /now-playing was on top made go_router try to mount a second ShellRoute instance under the existing one — both shells got the same page key. Move /queue out of the ShellRoute too, sibling to /now-playing. Shell stays mounted (with whatever child it had) underneath both top-level routes; pop from /queue returns to /now-playing or the shell's previous child as appropriate. --- flutter_client/lib/shared/routing.dart | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/flutter_client/lib/shared/routing.dart b/flutter_client/lib/shared/routing.dart index 532cf1e3..73c3eac5 100644 --- a/flutter_client/lib/shared/routing.dart +++ b/flutter_client/lib/shared/routing.dart @@ -85,6 +85,14 @@ GoRouter buildRouter(Ref ref) { }, ), ), + // /queue lives outside the ShellRoute too. Why: pushing /queue + // from /now-playing (which is also outside the shell) used to + // cause go_router to mount a second ShellRoute instance under + // the existing one, producing a duplicate page-key assertion + // (NavigatorState._debugCheckDuplicatedPageKeys). Top-level + // routes can stack on each other freely; shell-children can't + // when something on top of the shell is already routing. + GoRoute(path: '/queue', builder: (_, __) => const QueueScreen()), ShellRoute( builder: (ctx, state, child) => VersionGate(child: _ShellWithPlayerBar(child: child)), routes: [ @@ -105,7 +113,6 @@ GoRouter buildRouter(Ref ref) { seed: s.extra is AlbumRef ? s.extra as AlbumRef : null, ), ), - GoRoute(path: '/queue', builder: (_, __) => const QueueScreen()), GoRoute(path: '/search', builder: (_, __) => const SearchScreen()), GoRoute(path: '/library', builder: (_, __) => const LibraryScreen()), GoRoute(path: '/discover', builder: (_, __) => const DiscoverScreen()), From c08f4ace80be9e0bd3eb71ed09a86cb1029198b8 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 17:41:07 -0400 Subject: [PATCH 03/16] fix(flutter): cache usage display reflects actual disk, not just index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Storage card has been showing "0 B" for users who only stream (never explicitly pin or download an album). usageBytes() summed SUM(size_bytes) from audio_cache_index — but the streaming path through audio_handler writes files via LockCachingAudioSource without ever inserting an index row, so the index undercounts (often to zero) for normal use. Walk the cache directory instead. Catches everything on disk: manually pinned tracks (registered in the index), stream-cached tracks (LockCaching), partial downloads. Falls back gracefully when a file is racing against concurrent writes / deletes. Eviction still operates on the index (it needs the source/recency metadata to pick eviction order). Stream-cached files aren't subject to eviction today — separate problem; addressed when we wire a download-complete hook from LockCaching back into the index. --- .../lib/cache/audio_cache_manager.dart | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/flutter_client/lib/cache/audio_cache_manager.dart b/flutter_client/lib/cache/audio_cache_manager.dart index 9733eba9..34a4a061 100644 --- a/flutter_client/lib/cache/audio_cache_manager.dart +++ b/flutter_client/lib/cache/audio_cache_manager.dart @@ -96,13 +96,25 @@ class AudioCacheManager { .go(); } - /// Total bytes used by the cache. + /// Total bytes used by the cache. Walks the cache directory directly + /// instead of summing the index, because the streaming-as-you-play + /// path (audio_handler's LockCachingAudioSource) writes files + /// without registering an index row. The index sum would always + /// understate (often to zero) for users who only stream. Future usageBytes() async { - final result = await _db.customSelect( - 'SELECT COALESCE(SUM(size_bytes), 0) AS total FROM audio_cache_index', - readsFrom: {_db.audioCacheIndex}, - ).getSingle(); - return result.read('total'); + final dir = Directory(await _tracksDir()); + if (!await dir.exists()) return 0; + var total = 0; + await for (final entity in dir.list(followLinks: false)) { + if (entity is File) { + try { + total += await entity.length(); + } catch (_) { + // Race against concurrent writes / deletes — just skip. + } + } + } + return total; } /// Evicts files until usage ≤ targetBytes. Eviction order: From 9cac66467961fe7f8757c9a1ee44b99d8442f09a Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 17:46:32 -0400 Subject: [PATCH 04/16] feat(flutter): register stream-cached files in the audio cache index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the gap where LockCachingAudioSource wrote files to disk but never told AudioCacheManager about them — meaning evict() couldn't reclaim stream-cached files when usage exceeded the cap, only explicitly-pinned downloads. Wire just_audio's bufferedPositionStream as the "download complete" signal: when bufferedPosition reaches duration (with 200ms slack for header bytes), look up the on-disk file at the LockCaching path, read its size, and insert an audio_cache_index row via the new AudioCacheManager.registerStreamCache(). Source defaults to incidental so stream-cached tracks are first to be evicted under pressure. Dedupe via _streamCacheRegistered Set so we don't hit drift on every ~200ms buffered-position emit. Cache the application cache dir path on first use for the same reason. Eviction now sees the full set of files on disk; usageBytes() (which already walks the dir) and evict() (which reads the index) are finally consistent for stream-cached tracks. Pinned tracks keep their existing manual-download flow unchanged. --- .../lib/cache/audio_cache_manager.dart | 21 ++++++++ flutter_client/lib/player/audio_handler.dart | 49 +++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/flutter_client/lib/cache/audio_cache_manager.dart b/flutter_client/lib/cache/audio_cache_manager.dart index 34a4a061..5c4afecb 100644 --- a/flutter_client/lib/cache/audio_cache_manager.dart +++ b/flutter_client/lib/cache/audio_cache_manager.dart @@ -83,6 +83,27 @@ class AudioCacheManager { return path; } + /// Registers a file already on disk in the cache index. Intended for + /// the streaming path (LockCachingAudioSource) which writes the file + /// itself; we need an index row so eviction can find and delete it + /// when usage exceeds the cap. `source` defaults to incidental so + /// stream-cached tracks are first to be evicted. + Future registerStreamCache( + String trackId, + String path, + int sizeBytes, { + CacheSource source = CacheSource.incidental, + }) async { + await _db.into(_db.audioCacheIndex).insertOnConflictUpdate( + AudioCacheIndexCompanion.insert( + trackId: trackId, + path: path, + sizeBytes: sizeBytes, + source: source, + ), + ); + } + /// Removes a track from the index AND deletes the file. Future unpin(String trackId) async { final row = await (_db.select(_db.audioCacheIndex) diff --git a/flutter_client/lib/player/audio_handler.dart b/flutter_client/lib/player/audio_handler.dart index 132d9296..00ec6326 100644 --- a/flutter_client/lib/player/audio_handler.dart +++ b/flutter_client/lib/player/audio_handler.dart @@ -18,6 +18,12 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl // shuffleMode + repeatMode fields stay current for UI subscribers. _player.shuffleModeEnabledStream.listen((_) => _broadcastState(null)); _player.loopModeStream.listen((_) => _broadcastState(null)); + // Watch buffered-position so we can register stream-cached files + // in the audio cache index once they're fully downloaded. Without + // this, files written by LockCachingAudioSource never appear in + // the index and the eviction loop can't reclaim them. + _player.bufferedPositionStream + .listen((_) => unawaited(_maybeRegisterStreamCache())); } final AudioPlayer _player = AudioPlayer(); @@ -26,6 +32,16 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl AlbumCoverCache? _coverCache; AudioCacheManager? _audioCacheManager; + /// Trackers to dedupe registration — once we've inserted an index row + /// for a trackId, don't repeat the work on every buffered-position + /// emit. Cleared on dispose only; surviving across queue rebuilds is + /// fine because the index is itself the source of truth. + final Set _streamCacheRegistered = {}; + + /// Cached on first use so we don't hit the platform channel every + /// time the buffered-position stream emits (~200ms cadence). + String? _cacheDirPath; + /// Volume stream for UI subscribers. Mirrors the just_audio player's /// volume directly; set via setVolume(double). Stream get volumeStream => _player.volumeStream; @@ -184,6 +200,39 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl ); } + /// Once a track is fully buffered (LockCaching has written the whole + /// file to disk), insert an audio_cache_index row so the file shows + /// up to AudioCacheManager.evict() and clearAll(). No-op if the + /// cache manager isn't configured, no current track, the file isn't + /// fully buffered yet, the on-disk file is missing, or we already + /// registered this trackId during this subscription. + Future _maybeRegisterStreamCache() async { + final mgr = _audioCacheManager; + if (mgr == null) return; + final current = mediaItem.value; + if (current == null) return; + final trackId = current.id; + if (_streamCacheRegistered.contains(trackId)) return; + + final dur = _player.duration; + if (dur == null) return; + final buf = _player.bufferedPosition; + // 200ms slack for header bytes / encoding rounding. + if (buf < dur - const Duration(milliseconds: 200)) return; + + _cacheDirPath ??= (await getApplicationCacheDirectory()).path; + final path = '${_cacheDirPath!}/audio_cache/$trackId.mp3'; + final file = File(path); + if (!await file.exists()) return; + final size = await file.length(); + if (size <= 0) return; + + _streamCacheRegistered.add(trackId); + await mgr.registerStreamCache(trackId, path, size); + debugPrint( + 'audio_handler: registered stream cache for $trackId ($size bytes)'); + } + void _onCurrentIndexChanged(int? idx) { if (idx == null) return; // Push the new track's MediaItem onto the mediaItem stream so From c7549bbe48f75bf6c0dbb977bfb1766d41c3fb77 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 18:13:27 -0400 Subject: [PATCH 05/16] perf(api): collapse N+1 in /api/artists/{id} + 1 round-trip in /api/albums/{id} MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two new sqlc queries replace three sequential per-album round trips that were dominating detail-screen latency. GetAlbumWithArtist: handleGetAlbum was doing GetAlbumByID then GetArtistByID — separate round trips for one logical lookup. The new query joins albums + artists with sqlc.embed and returns both in one SELECT. Detail-page DB cost: 3 trips → 2. ListAlbumsByArtistWithTrackCount: handleGetArtist was loading the artist's album list, then issuing one CountTracksByAlbum per album to populate track_count. On a 30-album artist that's 32 sequential queries — each ~5ms over a local DB, ~30ms over a remote one. The new query embeds the album row + a correlated count(*) subquery, so every album's track count comes back in one SELECT regardless of album count. Detail-page DB cost: 1 + N → 1 + 1. Together these account for the bulk of cold-cache navigation latency on the Flutter client. Combined with the existing SWR + nav hydration on the client side, detail screens should render their header instantly and the body within one round trip instead of N+constant. --- internal/api/library.go | 47 +++++++++++-------- internal/db/dbq/albums.sql.go | 85 ++++++++++++++++++++++++++++++++++ internal/db/queries/albums.sql | 20 ++++++++ 3 files changed, 132 insertions(+), 20 deletions(-) diff --git a/internal/api/library.go b/internal/api/library.go index 3d94da35..206c5549 100644 --- a/internal/api/library.go +++ b/internal/api/library.go @@ -40,19 +40,28 @@ func (h *handlers) handleGetTrack(w http.ResponseWriter, r *http.Request) { // handleGetAlbum implements GET /api/albums/{id}. Returns the album plus its // tracks (ordered by disc/track number via the underlying query) with // duration summed from the track list — keeps one source of truth. +// +// Two DB round trips: the album+artist join and the per-user tracks +// list. Down from three (separate album, artist, tracks) before +// GetAlbumWithArtist landed. func (h *handlers) handleGetAlbum(w http.ResponseWriter, r *http.Request) { q := dbq.New(h.pool) - album, apiErr := resolveByID(r, "id", q.GetAlbumByID, "album") - if apiErr != nil { - writeErr(w, apiErr) + id, ok := requireURLUUID(w, r, "id") + if !ok { return } - artist, err := q.GetArtistByID(r.Context(), album.ArtistID) + row, err := q.GetAlbumWithArtist(r.Context(), id) if err != nil { - h.logger.Error("api: get album artist failed", "err", err) + if errors.Is(err, pgx.ErrNoRows) { + writeErr(w, apierror.NotFound("album")) + return + } + h.logger.Error("api: get album+artist failed", "err", err) writeErr(w, apierror.InternalMsg("lookup failed", err)) return } + album := row.Album + artistName := row.ArtistName var userID pgtype.UUID if user, ok := auth.UserFromContext(r.Context()); ok { userID = user.ID @@ -68,20 +77,24 @@ func (h *handlers) handleGetAlbum(w http.ResponseWriter, r *http.Request) { refs := make([]TrackRef, 0, len(tracks)) durSec := 0 for _, t := range tracks { - ref := trackRefFrom(t, album.Title, artist.Name) + ref := trackRefFrom(t, album.Title, artistName) refs = append(refs, ref) durSec += ref.DurationSec } detail := AlbumDetail{ - AlbumRef: albumRefFrom(album, artist.Name, len(tracks), durSec), + AlbumRef: albumRefFrom(album, artistName, len(tracks), durSec), Tracks: refs, } writeJSON(w, http.StatusOK, detail) } // handleGetArtist implements GET /api/artists/{id}. Returns artist + albums; -// each album carries its own track_count (one count query per album, same -// pattern as subsonic). At M1 sizes (albums-per-artist << 100) this is fine. +// each album carries its own track_count. +// +// Down from 1 + 1 + N queries (artist, albums, per-album CountTracksByAlbum) +// to 1 + 1 (artist, albums-with-track-count via correlated subquery). +// On a 30-album artist that's ~32 round trips collapsed to 2 — the +// difference between "feels slow" and "feels instant" on detail nav. func (h *handlers) handleGetArtist(w http.ResponseWriter, r *http.Request) { q := dbq.New(h.pool) artist, apiErr := resolveByID(r, "id", q.GetArtistByID, "artist") @@ -89,25 +102,19 @@ func (h *handlers) handleGetArtist(w http.ResponseWriter, r *http.Request) { writeErr(w, apiErr) return } - albums, err := q.ListAlbumsByArtist(r.Context(), artist.ID) + rows, err := q.ListAlbumsByArtistWithTrackCount(r.Context(), artist.ID) if err != nil { h.logger.Error("api: list albums by artist failed", "err", err) writeErr(w, apierror.InternalMsg("lookup failed", err)) return } - refs := make([]AlbumRef, 0, len(albums)) - for _, a := range albums { - count, cerr := q.CountTracksByAlbum(r.Context(), a.ID) - if cerr != nil { - h.logger.Error("api: count tracks failed", "err", cerr) - writeErr(w, apierror.InternalMsg("lookup failed", cerr)) - return - } + refs := make([]AlbumRef, 0, len(rows)) + for _, row := range rows { // durationSec=0: not aggregated for nested album lists per spec data flow. - refs = append(refs, albumRefFrom(a, artist.Name, int(count), 0)) + refs = append(refs, albumRefFrom(row.Album, artist.Name, int(row.TrackCount), 0)) } detail := ArtistDetail{ - ArtistRef: artistRefFrom(artist, len(albums)), + ArtistRef: artistRefFrom(artist, len(rows)), Albums: refs, } writeJSON(w, http.StatusOK, detail) diff --git a/internal/db/dbq/albums.sql.go b/internal/db/dbq/albums.sql.go index 5168f686..ca990688 100644 --- a/internal/db/dbq/albums.sql.go +++ b/internal/db/dbq/albums.sql.go @@ -168,6 +168,41 @@ func (q *Queries) GetAlbumCoverageRollup(ctx context.Context) (GetAlbumCoverageR return i, err } +const getAlbumWithArtist = `-- name: GetAlbumWithArtist :one +SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, artists.name AS artist_name +FROM albums +JOIN artists ON artists.id = albums.artist_id +WHERE albums.id = $1 +` + +type GetAlbumWithArtistRow struct { + Album Album + ArtistName string +} + +// Combined fetch for /api/albums/{id}: returns the album row + the +// joined artist name in a single round trip. Replaces a sequential +// GetAlbumByID + GetArtistByID pair on the hot detail-page path. +func (q *Queries) GetAlbumWithArtist(ctx context.Context, id pgtype.UUID) (GetAlbumWithArtistRow, error) { + row := q.db.QueryRow(ctx, getAlbumWithArtist, id) + var i GetAlbumWithArtistRow + err := row.Scan( + &i.Album.ID, + &i.Album.Title, + &i.Album.SortTitle, + &i.Album.ArtistID, + &i.Album.ReleaseDate, + &i.Album.Mbid, + &i.Album.CoverArtPath, + &i.Album.CreatedAt, + &i.Album.UpdatedAt, + &i.Album.CoverArtSource, + &i.Album.CoverArtSourcesVersion, + &i.ArtistName, + ) + return i, err +} + const getAlbumsByIDs = `-- name: GetAlbumsByIDs :many SELECT id, title, sort_title, artist_id, release_date, mbid, cover_art_path, created_at, updated_at, cover_art_source, cover_art_sources_version FROM albums WHERE id = ANY($1::uuid[]) ` @@ -389,6 +424,56 @@ func (q *Queries) ListAlbumsByArtist(ctx context.Context, artistID pgtype.UUID) return items, nil } +const listAlbumsByArtistWithTrackCount = `-- name: ListAlbumsByArtistWithTrackCount :many +SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, + (SELECT count(*) FROM tracks t WHERE t.album_id = albums.id)::bigint + AS track_count +FROM albums +WHERE albums.artist_id = $1 +ORDER BY release_date NULLS LAST, sort_title +` + +type ListAlbumsByArtistWithTrackCountRow struct { + Album Album + TrackCount int64 +} + +// Replaces the N+1 pattern in handleGetArtist (1 + N CountTracksByAlbum +// per album). Returns each album joined with its track count via a +// correlated subquery — single round trip regardless of album count. +func (q *Queries) ListAlbumsByArtistWithTrackCount(ctx context.Context, artistID pgtype.UUID) ([]ListAlbumsByArtistWithTrackCountRow, error) { + rows, err := q.db.Query(ctx, listAlbumsByArtistWithTrackCount, artistID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListAlbumsByArtistWithTrackCountRow + for rows.Next() { + var i ListAlbumsByArtistWithTrackCountRow + if err := rows.Scan( + &i.Album.ID, + &i.Album.Title, + &i.Album.SortTitle, + &i.Album.ArtistID, + &i.Album.ReleaseDate, + &i.Album.Mbid, + &i.Album.CoverArtPath, + &i.Album.CreatedAt, + &i.Album.UpdatedAt, + &i.Album.CoverArtSource, + &i.Album.CoverArtSourcesVersion, + &i.TrackCount, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const listAlbumsByGenre = `-- name: ListAlbumsByGenre :many SELECT DISTINCT ON (albums.id) albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version FROM albums diff --git a/internal/db/queries/albums.sql b/internal/db/queries/albums.sql index 38f3c34a..9811a083 100644 --- a/internal/db/queries/albums.sql +++ b/internal/db/queries/albums.sql @@ -14,6 +14,26 @@ RETURNING *; -- name: GetAlbumByID :one SELECT * FROM albums WHERE id = $1; +-- name: GetAlbumWithArtist :one +-- Combined fetch for /api/albums/{id}: returns the album row + the +-- joined artist name in a single round trip. Replaces a sequential +-- GetAlbumByID + GetArtistByID pair on the hot detail-page path. +SELECT sqlc.embed(albums), artists.name AS artist_name +FROM albums +JOIN artists ON artists.id = albums.artist_id +WHERE albums.id = $1; + +-- name: ListAlbumsByArtistWithTrackCount :many +-- Replaces the N+1 pattern in handleGetArtist (1 + N CountTracksByAlbum +-- per album). Returns each album joined with its track count via a +-- correlated subquery — single round trip regardless of album count. +SELECT sqlc.embed(albums), + (SELECT count(*) FROM tracks t WHERE t.album_id = albums.id)::bigint + AS track_count +FROM albums +WHERE albums.artist_id = $1 +ORDER BY release_date NULLS LAST, sort_title; + -- name: GetAlbumByArtistAndTitle :one -- Scanner uses this for the no-mbid dedupe path: resolve-or-create. SELECT * FROM albums WHERE artist_id = $1 AND title = $2 LIMIT 1; From af5744f8ab5fe3800ecd34d7dfe014ccda71e1a9 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 18:26:20 -0400 Subject: [PATCH 06/16] perf(home): aggregate-first rewrites for two scan-the-world queries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit handleGetHome itself is well-architected (5 sections in parallel via goroutines, latency-bound by the slowest single query). The cold- start lag is two of those queries doing wider scans than necessary. ListLastPlayedArtistsForUser was iterating FROM artists a with a LATERAL play_events join per row — O(total_artists in library) plan even for users who've only played a handful. Inverted: aggregate the user's plays by artist_id first via the play_events → tracks join (uses play_events_user_track_idx + tracks pkey), then attach the artist row and lateral cover/count subqueries only for the artists that actually appear. Cost now bounded by play history, not library size. ListMostPlayedTracksForUser was joining tracks/albums/artists for every play_event row before grouping — O(total plays) work for joins. Pre-aggregated play_events into a CTE keyed by track_id + count(*), then joined to tracks/albums/artists only for the distinct-tracks survivors. Order-by uses the pre-computed count. No handler or generated-Go signature changes — both queries return the same rowset shape, just much faster on libraries where total artists/plays >> distinct-played-artists/distinct-played-tracks. --- internal/db/dbq/recommendation.sql.go | 66 ++++++++++++++++---------- internal/db/queries/recommendation.sql | 66 ++++++++++++++++---------- 2 files changed, 82 insertions(+), 50 deletions(-) diff --git a/internal/db/dbq/recommendation.sql.go b/internal/db/dbq/recommendation.sql.go index e5ca6ba6..fc653a78 100644 --- a/internal/db/dbq/recommendation.sql.go +++ b/internal/db/dbq/recommendation.sql.go @@ -12,17 +12,19 @@ import ( ) const listLastPlayedArtistsForUser = `-- name: ListLastPlayedArtistsForUser :many -SELECT a.id, a.name, a.sort_name, a.mbid, a.created_at, a.updated_at, a.artist_thumb_path, a.artist_fanart_path, a.artist_art_source, a.artist_art_sources_version, - cov.id AS cover_album_id, - cnt.album_count::bigint AS album_count, - max_started.started_at::timestamptz AS last_played_at -FROM artists a -JOIN LATERAL ( - SELECT max(pe.started_at) AS started_at +WITH user_plays AS ( + SELECT t.artist_id, max(pe.started_at) AS last_started FROM play_events pe JOIN tracks t ON t.id = pe.track_id - WHERE pe.user_id = $1 AND t.artist_id = a.id -) max_started ON max_started.started_at IS NOT NULL + WHERE pe.user_id = $1 + GROUP BY t.artist_id +) +SELECT a.id, a.name, a.sort_name, a.mbid, a.created_at, a.updated_at, a.artist_thumb_path, a.artist_fanart_path, a.artist_art_source, a.artist_art_sources_version, + cov.id AS cover_album_id, + cnt.album_count::bigint AS album_count, + up.last_started::timestamptz AS last_played_at +FROM user_plays up +JOIN artists a ON a.id = up.artist_id LEFT JOIN LATERAL ( SELECT id FROM albums WHERE artist_id = a.id AND cover_art_path IS NOT NULL @@ -32,7 +34,7 @@ LEFT JOIN LATERAL ( SELECT count(*) AS album_count FROM albums WHERE artist_id = a.id ) cnt ON true -ORDER BY max_started.started_at DESC, a.id +ORDER BY up.last_started DESC, a.id LIMIT $2 ` @@ -52,6 +54,15 @@ type ListLastPlayedArtistsForUserRow struct { // a derived cover_album_id via a representative-album lateral join (most // recent album that has cover_art_path set). album_count joined for the // ArtistRef wire shape. +// +// Earlier shape iterated `FROM artists a` and ran a LATERAL play_events +// subquery per artist — O(total_artists) plan even for users with a +// handful of plays. New shape aggregates the user's plays by artist +// via the play_events → tracks join up front (uses +// play_events_user_track_idx + tracks pkey lookups), then attaches +// the artist row and lateral cover/count subqueries only for the +// artists that actually appear. Distinct-artists set is small for a +// typical user, so cost is bounded by play history not library size. func (q *Queries) ListLastPlayedArtistsForUser(ctx context.Context, arg ListLastPlayedArtistsForUserParams) ([]ListLastPlayedArtistsForUserRow, error) { rows, err := q.db.Query(ctx, listLastPlayedArtistsForUser, arg.UserID, arg.Limit) if err != nil { @@ -87,24 +98,24 @@ func (q *Queries) ListLastPlayedArtistsForUser(ctx context.Context, arg ListLast } const listMostPlayedTracksForUser = `-- name: ListMostPlayedTracksForUser :many +WITH plays AS ( + SELECT track_id, count(*) AS cnt + FROM play_events + WHERE user_id = $1 AND was_skipped = false + GROUP BY track_id +) SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, albums.title AS album_title, artists.name AS artist_name -FROM tracks t -JOIN albums ON albums.id = t.album_id -JOIN artists ON artists.id = t.artist_id -JOIN play_events pe ON pe.track_id = t.id -WHERE pe.user_id = $1 - AND pe.was_skipped = false - AND NOT EXISTS ( - SELECT 1 FROM lidarr_quarantine q - WHERE q.user_id = $1 AND q.track_id = t.id - ) -GROUP BY t.id, t.title, t.album_id, t.artist_id, t.duration_ms, t.file_path, - t.file_format, t.file_size, t.bitrate, t.track_number, t.disc_number, - t.mbid, t.genre, t.added_at, t.updated_at, - albums.title, artists.name -ORDER BY count(*) DESC, t.id +FROM plays p +JOIN tracks t ON t.id = p.track_id +JOIN albums ON albums.id = t.album_id +JOIN artists ON artists.id = t.artist_id +WHERE NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $1 AND q.track_id = t.id +) +ORDER BY p.cnt DESC, t.id LIMIT $2 ` @@ -122,6 +133,11 @@ type ListMostPlayedTracksForUserRow struct { // M6a: top-N tracks by completed-play count for the user. was_skipped // excludes skips so a user spamming next doesn't fabricate a top track. // Quarantined tracks (per-user soft-hide from M5b) are filtered out. +// +// Aggregate play_events first (uses play_events_user_track_idx) and +// join through tracks/albums/artists only for the survivors. Earlier +// shape did the join across every play_event row before grouping — +// O(plays) instead of O(distinct tracks). func (q *Queries) ListMostPlayedTracksForUser(ctx context.Context, arg ListMostPlayedTracksForUserParams) ([]ListMostPlayedTracksForUserRow, error) { rows, err := q.db.Query(ctx, listMostPlayedTracksForUser, arg.UserID, arg.Limit) if err != nil { diff --git a/internal/db/queries/recommendation.sql b/internal/db/queries/recommendation.sql index 34e3cd4e..b4cec84b 100644 --- a/internal/db/queries/recommendation.sql +++ b/internal/db/queries/recommendation.sql @@ -206,24 +206,29 @@ LIMIT $3; -- M6a: top-N tracks by completed-play count for the user. was_skipped -- excludes skips so a user spamming next doesn't fabricate a top track. -- Quarantined tracks (per-user soft-hide from M5b) are filtered out. +-- +-- Aggregate play_events first (uses play_events_user_track_idx) and +-- join through tracks/albums/artists only for the survivors. Earlier +-- shape did the join across every play_event row before grouping — +-- O(plays) instead of O(distinct tracks). +WITH plays AS ( + SELECT track_id, count(*) AS cnt + FROM play_events + WHERE user_id = $1 AND was_skipped = false + GROUP BY track_id +) SELECT sqlc.embed(t), albums.title AS album_title, artists.name AS artist_name -FROM tracks t -JOIN albums ON albums.id = t.album_id -JOIN artists ON artists.id = t.artist_id -JOIN play_events pe ON pe.track_id = t.id -WHERE pe.user_id = $1 - AND pe.was_skipped = false - AND NOT EXISTS ( - SELECT 1 FROM lidarr_quarantine q - WHERE q.user_id = $1 AND q.track_id = t.id - ) -GROUP BY t.id, t.title, t.album_id, t.artist_id, t.duration_ms, t.file_path, - t.file_format, t.file_size, t.bitrate, t.track_number, t.disc_number, - t.mbid, t.genre, t.added_at, t.updated_at, - albums.title, artists.name -ORDER BY count(*) DESC, t.id +FROM plays p +JOIN tracks t ON t.id = p.track_id +JOIN albums ON albums.id = t.album_id +JOIN artists ON artists.id = t.artist_id +WHERE NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $1 AND q.track_id = t.id +) +ORDER BY p.cnt DESC, t.id LIMIT $2; -- name: ListLastPlayedArtistsForUser :many @@ -231,17 +236,28 @@ LIMIT $2; -- a derived cover_album_id via a representative-album lateral join (most -- recent album that has cover_art_path set). album_count joined for the -- ArtistRef wire shape. -SELECT sqlc.embed(a), - cov.id AS cover_album_id, - cnt.album_count::bigint AS album_count, - max_started.started_at::timestamptz AS last_played_at -FROM artists a -JOIN LATERAL ( - SELECT max(pe.started_at) AS started_at +-- +-- Earlier shape iterated `FROM artists a` and ran a LATERAL play_events +-- subquery per artist — O(total_artists) plan even for users with a +-- handful of plays. New shape aggregates the user's plays by artist +-- via the play_events → tracks join up front (uses +-- play_events_user_track_idx + tracks pkey lookups), then attaches +-- the artist row and lateral cover/count subqueries only for the +-- artists that actually appear. Distinct-artists set is small for a +-- typical user, so cost is bounded by play history not library size. +WITH user_plays AS ( + SELECT t.artist_id, max(pe.started_at) AS last_started FROM play_events pe JOIN tracks t ON t.id = pe.track_id - WHERE pe.user_id = $1 AND t.artist_id = a.id -) max_started ON max_started.started_at IS NOT NULL + WHERE pe.user_id = $1 + GROUP BY t.artist_id +) +SELECT sqlc.embed(a), + cov.id AS cover_album_id, + cnt.album_count::bigint AS album_count, + up.last_started::timestamptz AS last_played_at +FROM user_plays up +JOIN artists a ON a.id = up.artist_id LEFT JOIN LATERAL ( SELECT id FROM albums WHERE artist_id = a.id AND cover_art_path IS NOT NULL @@ -251,7 +267,7 @@ LEFT JOIN LATERAL ( SELECT count(*) AS album_count FROM albums WHERE artist_id = a.id ) cnt ON true -ORDER BY max_started.started_at DESC, a.id +ORDER BY up.last_started DESC, a.id LIMIT $2; -- name: ListRediscoverAlbumsForUser :many From 8d466ebdd5809e1d71bda6db846053b1abba1b12 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 18:32:40 -0400 Subject: [PATCH 07/16] fix(flutter): reconcile stale playlists + recover from 404 on tap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 404 on tapping the For-You tile traced to stale drift rows. BuildSystemPlaylists rotates UUIDs on every rebuild, so old For-You / Songs-Like ids accumulate in cachedPlaylists. The list provider's fetchAndPopulate was only doing insertOrReplace, which adds new rows but never removes the obsolete ones — so the home tile renders 8 playlists when the server only knows 4, and 4 of those tiles 404 on tap. Two fixes: playlistsListProvider.fetchAndPopulate now reconciles. After fetching the fresh list, deleteWhere any user-owned drift row whose id isn't in the fresh response, then upsert the fresh set in the same batch. Public-from-others rows are left alone — they're not keyed by ownership and we don't want to drop someone else's public playlist just because the current user's response didn't enumerate it. Operates inside the existing batch so it's atomic. playlistDetailProvider.fetchAndPopulate now treats a DioException 404 as "this row is stale": delete the cachedPlaylists + any cachedPlaylistTracks rows for this id, return false so the UI yields emptyDetail. The next render of the home row sees the row gone and the tile disappears, completing the cleanup. Side note: every system-playlist rebuild discards drift rows for the just-evicted UUIDs and writes the new ones. That cycle's been silently churning since system playlists shipped — this is the first time the cleanup actually runs. --- .../lib/playlists/playlists_provider.dart | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/flutter_client/lib/playlists/playlists_provider.dart b/flutter_client/lib/playlists/playlists_provider.dart index c0164b5b..092bca37 100644 --- a/flutter_client/lib/playlists/playlists_provider.dart +++ b/flutter_client/lib/playlists/playlists_provider.dart @@ -1,5 +1,6 @@ import 'dart:async'; +import 'package:dio/dio.dart'; import 'package:drift/drift.dart' as drift; import 'package:flutter/foundation.dart' show debugPrint; import 'package:flutter_riverpod/flutter_riverpod.dart'; @@ -44,7 +45,21 @@ final playlistsListProvider = 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 + // drift. Tapping one of those stale tiles 404s. Delete every + // owned drift row whose id isn't in the fresh response, then + // upsert the fresh set. + final freshOwnedIds = + fresh.owned.map((p) => p.id).toSet(); await db.batch((b) { + if (user != null) { + b.deleteWhere(db.cachedPlaylists, (t) { + return t.userId.equals(user.id) & + t.id.isNotIn(freshOwnedIds); + }); + } for (final p in fresh.all) { b.insert(db.cachedPlaylists, p.toDrift(), mode: drift.InsertMode.insertOrReplace); @@ -216,6 +231,21 @@ final playlistDetailProvider = 'playlistDetailProvider($id): drift write done; awaiting watch re-emit'); return true; } catch (e, st) { + // 404 = the playlist row in drift is stale (BuildSystemPlaylists + // rotates UUIDs on each rebuild, so old For-You / Songs-Like + // tiles can outlive the actual server-side playlist). Wipe the + // 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)); + b.deleteWhere(db.cachedPlaylists, (t) => t.id.equals(id)); + }); + return false; + } debugPrint('playlistDetailProvider($id): fetch failed: $e\n$st'); return false; } From 572325e23f55c35b71ba4d4b4b3fdc50ca7a2afa Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 18:38:21 -0400 Subject: [PATCH 08/16] fix(flutter): write cachedTracks rows from playlist fetch (was the empty-rows bug) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Detail screen showed empty rows for system playlists because the fetch batch wrote cachedPlaylists, cachedPlaylistTracks (positions), cachedArtists, and cachedAlbums — but never cachedTracks themselves. The detail screen's LEFT OUTER JOIN cachedTracks then returned null on every row, so trackId was null and titles came back empty. PlaylistTrack on the wire carries enough to populate cachedTracks (id, title, albumId, artistId, durationSec). Adds a third dedup map in fetchAndPopulate, batched with the existing artist + album writes. track_number / disc_number aren't on the wire so they default to 0; the detail screen doesn't surface them. Reusing cachedTracks across albumProvider + playlistDetailProvider also means tapping a playlist's track to play it now finds the row in drift instead of triggering another fetch. --- .../lib/playlists/playlists_provider.dart | 58 +++++++++++++++++-- 1 file changed, 54 insertions(+), 4 deletions(-) diff --git a/flutter_client/lib/playlists/playlists_provider.dart b/flutter_client/lib/playlists/playlists_provider.dart index 092bca37..52fa1c51 100644 --- a/flutter_client/lib/playlists/playlists_provider.dart +++ b/flutter_client/lib/playlists/playlists_provider.dart @@ -158,13 +158,29 @@ final playlistDetailProvider = debugPrint( 'playlistDetailProvider($id): got ${fresh.tracks.length} tracks, writing'); - // Collect the artist + album rows referenced by these tracks so - // the JOINs that build artistName/albumTitle in the row view - // have something to bind to. Without this, system-playlist tracks - // surface with empty artist/album columns. + // Collect the track + artist + album rows referenced by these + // playlist entries. Without writing the cachedTracks rows + // themselves, the detail-screen JOIN against cachedTracks + // returns null on every row (only the playlist_tracks join + // succeeds), so the UI shows a list with empty titles and + // unplayable tracks. + final tracks = {}; final artists = {}; final albums = {}; for (final t in fresh.tracks) { + final tId = t.trackId; + if (tId != null && tId.isNotEmpty) { + tracks.putIfAbsent( + tId, + () => _TrackRefRow( + id: tId, + title: t.title, + albumId: t.albumId ?? '', + artistId: t.artistId ?? '', + durationSec: t.durationSec, + ), + ); + } final aId = t.artistId; final aName = t.artistName; if (aId != null && aId.isNotEmpty && aName.isNotEmpty) { @@ -213,6 +229,25 @@ final playlistDetailProvider = .toList(), ); } + if (tracks.isNotEmpty) { + // Insert/update cachedTracks so the detail screen's JOIN + // produces real titles + durations. We don't have + // track_number / disc_number on the wire (PlaylistTrack + // omits them), so they default to 0 — UI doesn't surface + // them on this screen so it's fine. + b.insertAllOnConflictUpdate( + db.cachedTracks, + tracks.values + .map((t) => CachedTracksCompanion.insert( + id: t.id, + albumId: t.albumId, + artistId: t.artistId, + title: t.title, + durationMs: drift.Value(t.durationSec * 1000), + )) + .toList(), + ); + } if (albums.isNotEmpty) { b.insertAllOnConflictUpdate( db.cachedAlbums, @@ -339,6 +374,21 @@ class AlbumRefRow { final String artistId; } +class _TrackRefRow { + _TrackRefRow({ + required this.id, + required this.title, + required this.albumId, + required this.artistId, + required this.durationSec, + }); + final String id; + final String title; + final String albumId; + final String artistId; + final int durationSec; +} + final systemPlaylistsStatusProvider = FutureProvider((ref) async { final dio = await ref.watch(dioProvider.future); From 22152b1ba33f0a67c942afe600637c2750e962c5 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 18:44:09 -0400 Subject: [PATCH 09/16] =?UTF-8?q?feat(flutter):=20metadata=20prefetcher=20?= =?UTF-8?q?=E2=80=94=20pre-warm=20drift=20for=20likely=20tap=20targets?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Across-the-board sluggishness was every cold-cache tap doing one network round trip while the user waits. SWR helps on re-visits but the first time you tap a tile from home you eat the latency. MetadataPrefetcher listens to homeProvider. When /api/home returns, it fires fire-and-forget reads on albumProvider + artistProvider for the top-N items in each home section (recently added, rediscover, most played, last played). Each provider read triggers the existing cold-cache path, which writes to drift. By the time the user actually taps a tile, it's already a drift hit and the detail screen renders instantly. Cap N=8 per section (covers what's visible on a typical phone without scrolling). Set spans dedupe across sections so popular artists don't get fetched five times. Errors are swallowed — a failed prefetch is silent and the tile falls back to its on-tap fetch behavior. Wired alongside the existing audio prefetcher in app.dart's postFrameCallback. --- flutter_client/lib/app.dart | 6 ++ .../lib/cache/metadata_prefetcher.dart | 77 +++++++++++++++++++ 2 files changed, 83 insertions(+) create mode 100644 flutter_client/lib/cache/metadata_prefetcher.dart diff --git a/flutter_client/lib/app.dart b/flutter_client/lib/app.dart index a456dcd3..58073049 100644 --- a/flutter_client/lib/app.dart +++ b/flutter_client/lib/app.dart @@ -1,6 +1,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'cache/metadata_prefetcher.dart'; import 'cache/prefetcher.dart'; import 'cache/sync_controller.dart'; import 'shared/routing.dart'; @@ -27,6 +28,11 @@ class _MinstrelAppState extends ConsumerState { // ignore: unawaited_futures ref.read(syncControllerProvider.notifier).sync(); ref.read(prefetcherProvider); + // Metadata prefetcher: when /api/home returns, fire background + // albumProvider/artistProvider reads for the top-N items in + // each section so subsequent taps are drift hits, not network + // round trips. + ref.read(metadataPrefetcherProvider); }); } diff --git a/flutter_client/lib/cache/metadata_prefetcher.dart b/flutter_client/lib/cache/metadata_prefetcher.dart new file mode 100644 index 00000000..d1bea408 --- /dev/null +++ b/flutter_client/lib/cache/metadata_prefetcher.dart @@ -0,0 +1,77 @@ +import 'package:flutter/foundation.dart' show debugPrint; +import 'package:flutter_riverpod/flutter_riverpod.dart'; + +import '../library/library_providers.dart'; +import '../models/home_data.dart'; + +/// Pre-warms the drift cache for entities the user is most likely to +/// tap. When /api/home returns its sections, fire one albumProvider / +/// artistProvider read per top-of-section item in the background. The +/// reads trigger the existing cold-cache path, which writes drift. +/// By the time the user actually taps a tile, it's a drift hit and +/// the detail screen renders instantly. +/// +/// All reads are unawaited and idempotent — providers that already +/// have a cached value short-circuit. Readers/writers don't conflict +/// because each provider de-duplicates concurrent subscribers. +class MetadataPrefetcher { + MetadataPrefetcher(this._ref) { + _ref.listen>(homeProvider, (_, next) { + next.whenData(_warmHome); + }); + } + + final Ref _ref; + + /// Cap per section so a 50-album recently-added row doesn't fan + /// out 50 fetches the moment the screen appears. Top-N covers what + /// the user can see without scrolling on a typical phone. + static const _topN = 8; + + void _warmHome(HomeData h) { + final albumIds = {}; + final artistIds = {}; + + for (final a in h.recentlyAddedAlbums.take(_topN)) { + if (a.id.isNotEmpty) albumIds.add(a.id); + if (a.artistId.isNotEmpty) artistIds.add(a.artistId); + } + for (final a in h.rediscoverAlbums.take(_topN)) { + if (a.id.isNotEmpty) albumIds.add(a.id); + if (a.artistId.isNotEmpty) artistIds.add(a.artistId); + } + for (final ar in h.rediscoverArtists.take(_topN)) { + if (ar.id.isNotEmpty) artistIds.add(ar.id); + } + for (final ar in h.lastPlayedArtists.take(_topN)) { + if (ar.id.isNotEmpty) artistIds.add(ar.id); + } + for (final t in h.mostPlayedTracks.take(_topN)) { + if (t.albumId.isNotEmpty) albumIds.add(t.albumId); + if (t.artistId.isNotEmpty) artistIds.add(t.artistId); + } + + debugPrint( + 'metadataPrefetcher: warming ${albumIds.length} albums + ${artistIds.length} artists'); + + for (final id in albumIds) { + _swallow(_ref.read(albumProvider(id).future)); + } + for (final id in artistIds) { + _swallow(_ref.read(artistProvider(id).future)); + } + } + + /// Discards the return value and any error from a fire-and-forget + /// provider read. We don't care about the value here — we only want + /// the side effect of writing drift. + void _swallow(Future f) { + f.then((_) {}).onError((_, __) {}); + } +} + +/// Read once at app start to activate the prefetcher (e.g. wire it +/// from a top-level Consumer or main.dart container override). +final metadataPrefetcherProvider = Provider((ref) { + return MetadataPrefetcher(ref); +}); From 4bd069430b0ce5a11bda8e264d2d1574fe8bc61a Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 18:52:37 -0400 Subject: [PATCH 10/16] feat(flutter): extend metadata prefetch to library tabs + artist detail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MetadataPrefetcher gains warmAlbums(ids) / warmArtists(ids) public methods so callers can fan out drift-cache warm-ups for whatever collection just landed. Wired into: - Library Artists tab — warms first 8 artists from the page on every data emit (initial + paginate + refresh). - Library Albums tab — same for first 8 albums. - Artist detail album grid — warms first 8 albums from the artist's album list as soon as it loads, so tapping into any of them is a drift hit. Hard-cap of 8 per call (same as the home prefetch). Set spans across calls aren't deduped at this layer because providers themselves short-circuit on cached values. Also instrument the artist-detail play button: try/catch around the artistTracksProvider read + playTracks call, snackbar on empty-tracks or thrown-error so silent failures stop being silent. The current behavior was an early return on tracks.isEmpty with no visible feedback. --- .../lib/cache/metadata_prefetcher.dart | 31 +++++++++++--- .../lib/library/artist_detail_screen.dart | 42 ++++++++++++++++--- .../lib/library/library_screen.dart | 20 +++++++-- 3 files changed, 78 insertions(+), 15 deletions(-) diff --git a/flutter_client/lib/cache/metadata_prefetcher.dart b/flutter_client/lib/cache/metadata_prefetcher.dart index d1bea408..b60cc08e 100644 --- a/flutter_client/lib/cache/metadata_prefetcher.dart +++ b/flutter_client/lib/cache/metadata_prefetcher.dart @@ -5,11 +5,9 @@ import '../library/library_providers.dart'; import '../models/home_data.dart'; /// Pre-warms the drift cache for entities the user is most likely to -/// tap. When /api/home returns its sections, fire one albumProvider / -/// artistProvider read per top-of-section item in the background. The -/// reads trigger the existing cold-cache path, which writes drift. -/// By the time the user actually taps a tile, it's a drift hit and -/// the detail screen renders instantly. +/// tap. The home prefetch fires when /api/home returns. Library +/// tabs and artist detail expose `warmAlbums` / `warmArtists` so they +/// can fan out reads from their own AsyncNotifier callbacks. /// /// All reads are unawaited and idempotent — providers that already /// have a cached value short-circuit. Readers/writers don't conflict @@ -28,6 +26,29 @@ class MetadataPrefetcher { /// the user can see without scrolling on a typical phone. static const _topN = 8; + /// Pre-warm drift for each album id (no-op past the first dedup). + /// Called from the library Albums tab + artist detail album grid. + void warmAlbums(Iterable ids) { + var n = 0; + for (final id in ids) { + if (id.isEmpty) continue; + if (n++ >= _topN) break; + _swallow(_ref.read(albumProvider(id).future)); + } + if (n > 0) debugPrint('metadataPrefetcher: warming $n albums'); + } + + /// Pre-warm drift for each artist id. + void warmArtists(Iterable ids) { + var n = 0; + for (final id in ids) { + if (id.isEmpty) continue; + if (n++ >= _topN) break; + _swallow(_ref.read(artistProvider(id).future)); + } + if (n > 0) debugPrint('metadataPrefetcher: warming $n artists'); + } + void _warmHome(HomeData h) { final albumIds = {}; final artistIds = {}; diff --git a/flutter_client/lib/library/artist_detail_screen.dart b/flutter_client/lib/library/artist_detail_screen.dart index e13d276d..2430bf78 100644 --- a/flutter_client/lib/library/artist_detail_screen.dart +++ b/flutter_client/lib/library/artist_detail_screen.dart @@ -1,8 +1,10 @@ +import 'package:flutter/foundation.dart' show debugPrint; import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; import '../api/endpoints/likes.dart'; +import '../cache/metadata_prefetcher.dart'; import '../likes/like_button.dart'; import '../models/album.dart'; import '../models/artist.dart'; @@ -92,10 +94,33 @@ class ArtistDetailScreen extends ConsumerWidget { child: IconButton( icon: Icon(Icons.play_arrow, color: fs.parchment), onPressed: () async { - final tracks = await ref.read(artistTracksProvider(id).future); - if (tracks.isEmpty) return; - final shuffled = [...tracks]..shuffle(); - ref.read(playerActionsProvider).playTracks(shuffled); + 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( + const SnackBar( + content: Text( + 'No tracks found for this artist yet.')), + ); + } + return; + } + final shuffled = [...tracks]..shuffle(); + await ref + .read(playerActionsProvider) + .playTracks(shuffled); + } catch (e) { + debugPrint('artist_detail: play failed: $e'); + if (context.mounted) { + ScaffoldMessenger.of(context).showSnackBar( + SnackBar(content: Text("Couldn't start playback: $e")), + ); + } + } }, ), ), @@ -109,7 +134,11 @@ class ArtistDetailScreen extends ConsumerWidget { albums.when( error: (e, _) => Center(child: Text('$e', style: TextStyle(color: fs.error))), loading: () => const Padding(padding: EdgeInsets.all(16), child: Center(child: CircularProgressIndicator())), - data: (list) => LayoutBuilder(builder: (ctx, constraints) { + data: (list) { + ref + .read(metadataPrefetcherProvider) + .warmAlbums(list.map((a) => a.id)); + return LayoutBuilder(builder: (ctx, constraints) { const cols = 3; const sidePad = 8.0; const gap = 8.0; @@ -144,7 +173,8 @@ class ArtistDetailScreen extends ConsumerWidget { ); }, ); - }), + }); + }, ), ]); } diff --git a/flutter_client/lib/library/library_screen.dart b/flutter_client/lib/library/library_screen.dart index 79f47e6e..145e7cbf 100644 --- a/flutter_client/lib/library/library_screen.dart +++ b/flutter_client/lib/library/library_screen.dart @@ -5,6 +5,7 @@ import 'package:go_router/go_router.dart'; import '../api/endpoints/library_lists.dart'; import '../api/endpoints/likes.dart'; import '../api/endpoints/me.dart'; +import '../cache/metadata_prefetcher.dart'; import '../library/library_providers.dart' show dioProvider; import '../models/album.dart'; import '../models/artist.dart'; @@ -207,7 +208,12 @@ class _ArtistsTab extends ConsumerWidget { return ref.watch(_libraryArtistsProvider).when( loading: () => const Center(child: CircularProgressIndicator()), error: (e, _) => Center(child: Text('$e', style: TextStyle(color: fs.error))), - data: (page) => page.items.isEmpty + data: (page) { + // Warm details for the first screenful so taps are instant. + ref + .read(metadataPrefetcherProvider) + .warmArtists(page.items.map((a) => a.id)); + return page.items.isEmpty ? Center(child: Text("No artists yet — scan a library folder via the server's config.", style: TextStyle(color: fs.ash), textAlign: TextAlign.center)) : RefreshIndicator( onRefresh: () async { @@ -247,7 +253,8 @@ class _ArtistsTab extends ConsumerWidget { }, ), ), - ), + ); + }, ); } } @@ -261,7 +268,11 @@ class _AlbumsTab extends ConsumerWidget { return ref.watch(_libraryAlbumsProvider).when( loading: () => const Center(child: CircularProgressIndicator()), error: (e, _) => Center(child: Text('$e', style: TextStyle(color: fs.error))), - data: (page) => page.items.isEmpty + data: (page) { + ref + .read(metadataPrefetcherProvider) + .warmAlbums(page.items.map((a) => a.id)); + return page.items.isEmpty ? Center(child: Text("No albums yet — scan a library folder via the server's config.", style: TextStyle(color: fs.ash), textAlign: TextAlign.center)) : RefreshIndicator( onRefresh: () async { @@ -315,7 +326,8 @@ class _AlbumsTab extends ConsumerWidget { ), ); }), - ), + ); + }, ); } } From 4ede37d9ad0265924832e7352fabe841f826dd9d Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 19:02:16 -0400 Subject: [PATCH 11/16] =?UTF-8?q?fix(flutter):=20stop=20the=20cache=20feed?= =?UTF-8?q?back=20loop=20=E2=80=94=20playback=20no=20longer=20competes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The prefetcher + alwaysRefresh combination was creating a feedback loop visible in the logs as repeated `metadataPrefetcher: warming N albums` cycles, each kicking N parallel getAlbum fetches that then triggered drift writes that triggered re-emits that re-ran the prefetcher. Tap-to-play was queueing behind 14+ in-flight cache fetches. Three structural fixes: 1. Prefetcher hard-dedupes per session via _warmedArtists Set. Re-rendering a screen no longer re-fires fetches for ids we've already seen. 2. Prefetcher only warms artistProvider, not albumProvider. Albums carry track lists; pre-warming N albums fans out N parallel "fetch tracks" round trips for content the user may never visit. Artist rows are single-row lookups — cheap. Album detail loads on tap (still fast: server-side perf work makes it ~one round trip). 3. Drop alwaysRefresh from albumProvider, artistProvider, artistAlbumsProvider, artistTracksProvider. Each was kicking one silent background refresh per first cache hit. With the prefetcher creating many subscriptions in parallel, that meant every prewarmed id triggered an extra fetch even when drift was already populated. playlistsListProvider keeps alwaysRefresh — system playlists genuinely rotate UUIDs and need the catch-up. Pull-to- refresh remains the explicit invalidation path everywhere else. Removed the warmAlbums calls from the library Albums tab and artist detail album grid (the storm sources). Net effect: cold app boot warms ~12-15 artist rows once, period. Tapping a tile still fetches its detail on demand (one round trip, fast). User-initiated playback isn't queued behind cache work. --- .../lib/cache/metadata_prefetcher.dart | 56 ++++++------------- .../lib/library/artist_detail_screen.dart | 3 - .../lib/library/library_providers.dart | 30 ++++------ .../lib/library/library_screen.dart | 3 - .../lib/playlists/playlists_provider.dart | 14 ++--- 5 files changed, 35 insertions(+), 71 deletions(-) diff --git a/flutter_client/lib/cache/metadata_prefetcher.dart b/flutter_client/lib/cache/metadata_prefetcher.dart index b60cc08e..2eef60cb 100644 --- a/flutter_client/lib/cache/metadata_prefetcher.dart +++ b/flutter_client/lib/cache/metadata_prefetcher.dart @@ -4,14 +4,13 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import '../library/library_providers.dart'; import '../models/home_data.dart'; -/// Pre-warms the drift cache for entities the user is most likely to -/// tap. The home prefetch fires when /api/home returns. Library -/// tabs and artist detail expose `warmAlbums` / `warmArtists` so they -/// can fan out reads from their own AsyncNotifier callbacks. -/// -/// All reads are unawaited and idempotent — providers that already -/// have a cached value short-circuit. Readers/writers don't conflict -/// because each provider de-duplicates concurrent subscribers. +/// Pre-warms the drift cache for likely-tap targets. Conservative on +/// purpose: only warms artistProvider rows (single row, single round +/// trip per id) and only ever fires once per id per session. Album +/// detail is NOT prewarmed — the albumProvider auto-fetches its track +/// list when missing, and pre-warming N albums fans out N parallel +/// "fetch tracks" round trips that compete with the user's actual +/// playback request for bandwidth. class MetadataPrefetcher { MetadataPrefetcher(this._ref) { _ref.listen>(homeProvider, (_, next) { @@ -21,28 +20,21 @@ class MetadataPrefetcher { final Ref _ref; - /// Cap per section so a 50-album recently-added row doesn't fan - /// out 50 fetches the moment the screen appears. Top-N covers what - /// the user can see without scrolling on a typical phone. + /// Per-session dedupe so re-rendering a screen (every UI rebuild + /// fires the data: callback) doesn't trigger N more fetches for + /// ids we've already warmed. + final Set _warmedArtists = {}; + + /// Cap per section. Covers what fits on screen without scrolling. static const _topN = 8; - /// Pre-warm drift for each album id (no-op past the first dedup). - /// Called from the library Albums tab + artist detail album grid. - void warmAlbums(Iterable ids) { - var n = 0; - for (final id in ids) { - if (id.isEmpty) continue; - if (n++ >= _topN) break; - _swallow(_ref.read(albumProvider(id).future)); - } - if (n > 0) debugPrint('metadataPrefetcher: warming $n albums'); - } - - /// Pre-warm drift for each artist id. + /// Pre-warm artists. Albums are intentionally not pre-warmed — + /// see class comment. void warmArtists(Iterable ids) { var n = 0; for (final id in ids) { if (id.isEmpty) continue; + if (!_warmedArtists.add(id)) continue; // already warmed if (n++ >= _topN) break; _swallow(_ref.read(artistProvider(id).future)); } @@ -50,15 +42,11 @@ class MetadataPrefetcher { } void _warmHome(HomeData h) { - final albumIds = {}; final artistIds = {}; - for (final a in h.recentlyAddedAlbums.take(_topN)) { - if (a.id.isNotEmpty) albumIds.add(a.id); if (a.artistId.isNotEmpty) artistIds.add(a.artistId); } for (final a in h.rediscoverAlbums.take(_topN)) { - if (a.id.isNotEmpty) albumIds.add(a.id); if (a.artistId.isNotEmpty) artistIds.add(a.artistId); } for (final ar in h.rediscoverArtists.take(_topN)) { @@ -68,19 +56,9 @@ class MetadataPrefetcher { if (ar.id.isNotEmpty) artistIds.add(ar.id); } for (final t in h.mostPlayedTracks.take(_topN)) { - if (t.albumId.isNotEmpty) albumIds.add(t.albumId); if (t.artistId.isNotEmpty) artistIds.add(t.artistId); } - - debugPrint( - 'metadataPrefetcher: warming ${albumIds.length} albums + ${artistIds.length} artists'); - - for (final id in albumIds) { - _swallow(_ref.read(albumProvider(id).future)); - } - for (final id in artistIds) { - _swallow(_ref.read(artistProvider(id).future)); - } + warmArtists(artistIds); } /// Discards the return value and any error from a fire-and-forget diff --git a/flutter_client/lib/library/artist_detail_screen.dart b/flutter_client/lib/library/artist_detail_screen.dart index 2430bf78..004ea9e1 100644 --- a/flutter_client/lib/library/artist_detail_screen.dart +++ b/flutter_client/lib/library/artist_detail_screen.dart @@ -135,9 +135,6 @@ class ArtistDetailScreen extends ConsumerWidget { error: (e, _) => Center(child: Text('$e', style: TextStyle(color: fs.error))), loading: () => const Padding(padding: EdgeInsets.all(16), child: Center(child: CircularProgressIndicator())), data: (list) { - ref - .read(metadataPrefetcherProvider) - .warmAlbums(list.map((a) => a.id)); return LayoutBuilder(builder: (ctx, constraints) { const cols = 3; const sidePad = 8.0; diff --git a/flutter_client/lib/library/library_providers.dart b/flutter_client/lib/library/library_providers.dart index 202b5bf4..28e0a296 100644 --- a/flutter_client/lib/library/library_providers.dart +++ b/flutter_client/lib/library/library_providers.dart @@ -66,9 +66,10 @@ final artistProvider = isOnline: () async => (await ref .read(connectivityProvider.future) .timeout(const Duration(seconds: 3), onTimeout: () => true)), - // SWR: yield cache instantly on hit, refresh in the background so - // the row catches up to server state without making the user wait. - alwaysRefresh: true, + // No alwaysRefresh: artist rows don't change frequently, and the + // metadata prefetcher creates many subscriptions in parallel — + // each silently re-fetching once would saturate the request + // pipeline behind the user's actual playback request. tag: 'artist($id)', ); }); @@ -100,7 +101,6 @@ final artistAlbumsProvider = isOnline: () async => (await ref .read(connectivityProvider.future) .timeout(const Duration(seconds: 3), onTimeout: () => true)), - alwaysRefresh: true, tag: 'artistAlbums($artistId)', ); }); @@ -137,7 +137,6 @@ final artistTracksProvider = isOnline: () async => (await ref .read(connectivityProvider.future) .timeout(const Duration(seconds: 3), onTimeout: () => true)), - alwaysRefresh: true, tag: 'artistTracks($artistId)', ); }); @@ -169,10 +168,7 @@ final albumProvider = StreamProvider.family< // Once-per-subscription guard so we don't re-fetch in a loop if the // server genuinely returns zero tracks (or if the fetch fails). var fetchAttempted = false; - // SWR revalidation guard — fire one background refresh per - // subscription on the first complete cache hit, so the displayed - // data catches up to server state without making the user wait. - var revalidated = false; + // (revalidated flag removed; see SWR note below the yield.) Future isOnline() async { try { @@ -303,15 +299,13 @@ final albumProvider = StreamProvider.family< ); }).toList(); - // SWR: first complete cache hit triggers one background refresh - // so we don't show stale data indefinitely. Drift watch re-emits - // on success and the next iteration yields the fresh data. - if (!revalidated && trackRows.isNotEmpty) { - revalidated = true; - if (await isOnline()) { - unawaited(fetchAndPopulate()); - } - } + // Note: NO SWR here on purpose. Prior code kicked a background + // refresh on every first cache hit, which combined with the + // metadata prefetcher meant every prewarmed album id triggered + // an extra fetch even when drift was already populated. Tracks + // and album metadata don't change on the same timescale as + // playlists; a stale read is fine until the user invalidates + // (pull-to-refresh) or the album is genuinely re-fetched. yield (album: album, tracks: tracks); } diff --git a/flutter_client/lib/library/library_screen.dart b/flutter_client/lib/library/library_screen.dart index 145e7cbf..80b4a762 100644 --- a/flutter_client/lib/library/library_screen.dart +++ b/flutter_client/lib/library/library_screen.dart @@ -269,9 +269,6 @@ class _AlbumsTab extends ConsumerWidget { loading: () => const Center(child: CircularProgressIndicator()), error: (e, _) => Center(child: Text('$e', style: TextStyle(color: fs.error))), data: (page) { - ref - .read(metadataPrefetcherProvider) - .warmAlbums(page.items.map((a) => a.id)); return page.items.isEmpty ? Center(child: Text("No albums yet — scan a library folder via the server's config.", style: TextStyle(color: fs.ash), textAlign: TextAlign.center)) : RefreshIndicator( diff --git a/flutter_client/lib/playlists/playlists_provider.dart b/flutter_client/lib/playlists/playlists_provider.dart index 52fa1c51..dc4105d0 100644 --- a/flutter_client/lib/playlists/playlists_provider.dart +++ b/flutter_client/lib/playlists/playlists_provider.dart @@ -289,7 +289,6 @@ final playlistDetailProvider = // Once-per-subscription guard so an empty server response doesn't // cause repeated re-fetches. var fetchAttempted = false; - var revalidated = false; await for (final playlistRows in playlistQuery.watch()) { if (playlistRows.isEmpty) { @@ -348,13 +347,12 @@ final playlistDetailProvider = yield PlaylistDetail(playlist: playlist, tracks: tracks); - // SWR: first complete cache hit kicks one background refresh. - if (!revalidated && trackRows.isNotEmpty) { - revalidated = true; - if (await isOnline()) { - unawaited(fetchAndPopulate()); - } - } + // No SWR refresh here. The aggregate playlistsListProvider does + // alwaysRefresh (system playlists rotate UUIDs), but per-detail + // refresh on every visit was multiplying with the prefetcher's + // parallel fetches and starving user-initiated playback. Pull- + // to-refresh on the detail page invalidates the provider, which + // is the right path for an explicit refresh. } }); From acc7149537e8f1e199fad1dbe2a47159cfc8bdca Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 19:11:44 -0400 Subject: [PATCH 12/16] diag(flutter): instrument playTracks + cache appdir; fix CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI fixes: - artist_detail_screen.dart: drop unnecessary foundation import (debugPrint comes from material) and unused metadata_prefetcher import. Playback timing visibility (so we can stop guessing where the lag lives): - playTracks now logs serverUrl / token / configure / setQueue / play() returned, each stage in milliseconds. The next time you tap play, we'll see exactly where the seconds go. - setQueueFromTracks adds two more measurements: total source-build time across all tracks, and setAudioSources duration. Small concrete win: - audio_handler caches the application cache dir path on first use (already cached in _maybeRegisterStreamCache; now also used in _buildAudioSource for the LockCachingAudioSource path). One less platform channel hit per track on cache-miss queue builds. Once we see real numbers we can decide whether the fix is to build sources lazily (initial source first → play → background-add the rest), pre-warm the audio handler at app start so playTracks skips serverUrl + token reads entirely, or something else. --- .../lib/library/artist_detail_screen.dart | 2 -- flutter_client/lib/player/audio_handler.dart | 10 ++++++++-- flutter_client/lib/player/player_provider.dart | 17 +++++++++++++++++ 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/flutter_client/lib/library/artist_detail_screen.dart b/flutter_client/lib/library/artist_detail_screen.dart index 004ea9e1..784776f1 100644 --- a/flutter_client/lib/library/artist_detail_screen.dart +++ b/flutter_client/lib/library/artist_detail_screen.dart @@ -1,10 +1,8 @@ -import 'package:flutter/foundation.dart' show debugPrint; import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; import '../api/endpoints/likes.dart'; -import '../cache/metadata_prefetcher.dart'; import '../likes/like_button.dart'; import '../models/album.dart'; import '../models/artist.dart'; diff --git a/flutter_client/lib/player/audio_handler.dart b/flutter_client/lib/player/audio_handler.dart index 00ec6326..0490f5ae 100644 --- a/flutter_client/lib/player/audio_handler.dart +++ b/flutter_client/lib/player/audio_handler.dart @@ -80,12 +80,18 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl 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 ' + '${sw.elapsedMilliseconds}ms'); + sw.reset(); + sw.start(); await _player.setAudioSources( sources, initialIndex: initialIndex, ); + debugPrint('audio_handler: setAudioSources ${sw.elapsedMilliseconds}ms'); // Kick the cover fetch for the initial item — async, doesn't block // playback. Subsequent track changes are handled by the @@ -143,8 +149,8 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl // pin / Download buttons cover the index path; LockCaching handles // the network optimization. if (mgr != null) { - final cacheDir = await getApplicationCacheDirectory(); - final cacheFile = File('${cacheDir.path}/audio_cache/${t.id}.mp3'); + _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 diff --git a/flutter_client/lib/player/player_provider.dart b/flutter_client/lib/player/player_provider.dart index baca1af3..77d82319 100644 --- a/flutter_client/lib/player/player_provider.dart +++ b/flutter_client/lib/player/player_provider.dart @@ -53,8 +53,22 @@ 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) @@ -64,8 +78,11 @@ 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 { From e8a515dac467c861cf1c2c2a84761d1311874891 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 19:35:12 -0400 Subject: [PATCH 13/16] fix(flutter): import debugPrint in player_provider for the timing logs --- flutter_client/lib/player/player_provider.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/flutter_client/lib/player/player_provider.dart b/flutter_client/lib/player/player_provider.dart index 77d82319..47e51879 100644 --- a/flutter_client/lib/player/player_provider.dart +++ b/flutter_client/lib/player/player_provider.dart @@ -1,4 +1,5 @@ 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'; From 2d5f0691c2170de20e4f60a285296e79bed6eb6f Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 19:43:37 -0400 Subject: [PATCH 14/16] diag(flutter): surface player errors + state transitions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tap→audio measured at ~370ms — that path's fast. Real symptom: a few seconds of audio, then silence with no event surfaced to Flutter. ExoPlayer is failing/completing somewhere and we have no log to act on. Three changes, all log-only: - Add onError to playbackEventStream so stream failures (404, range- request bugs, decoder errors, network drops) print instead of silently halting playback. - Subscribe to playerStateStream and log playing + processingState on every transition. Silent stops will now show as a state shift to completed / idle / buffering with no resumption. - Subscribe to processingStateStream separately to catch fine-grained state transitions ExoPlayer reports between source advances. After hot-restart, tap-then-go-quiet should produce a sequence we can read — most likely either "processingState=completed" partway through (server returning premature EOS or wrong Content-Length) or a thrown error from ExoPlayer's source-loading path. --- flutter_client/lib/player/audio_handler.dart | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/flutter_client/lib/player/audio_handler.dart b/flutter_client/lib/player/audio_handler.dart index 0490f5ae..3bade142 100644 --- a/flutter_client/lib/player/audio_handler.dart +++ b/flutter_client/lib/player/audio_handler.dart @@ -12,7 +12,16 @@ import 'album_cover_cache.dart'; class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandler { MinstrelAudioHandler() { - _player.playbackEventStream.listen(_broadcastState); + _player.playbackEventStream.listen( + _broadcastState, + // ExoPlayer surfaces stream errors (404, premature EOS, decoder + // failure, network drop) here. Without an error sink, the + // player just goes quiet — exactly the "starts then stops" + // symptom we hit. + onError: (Object e, StackTrace st) { + debugPrint('audio_handler: playbackEventStream error: $e\n$st'); + }, + ); _player.currentIndexStream.listen(_onCurrentIndexChanged); // Re-broadcast on shuffle/repeat changes so the PlaybackState's // shuffleMode + repeatMode fields stay current for UI subscribers. @@ -24,6 +33,15 @@ 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(); From 6f20a75f9b5a0ebbf17f74a8712adb6434adaba0 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 20:15:38 -0400 Subject: [PATCH 15/16] feat(flutter): playlist cover art via deterministic /api/playlists/{id}/cover MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same fix shape as albums: drift's CachedPlaylistAdapter.toRef was returning coverUrl: '' because the cache table doesn't persist the server-derived URL. Set it to /api/playlists//cover in the adapter — handleGetPlaylistCover serves the cached collage from disk, so the URL is deterministic and the round-trip through drift no longer drops it. PlaylistCard + PlaylistsListScreen pass the existing queue_music icon as ServerImage's fallback, so when the server hasn't built a collage yet (system playlists with no tracks at build time), the endpoint 404s and the icon shows over the slate background instead of an empty box. --- flutter_client/lib/cache/adapters.dart | 8 +++++++- .../lib/playlists/playlists_list_screen.dart | 6 +++++- .../lib/playlists/widgets/playlist_card.dart | 12 +++++++++++- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/flutter_client/lib/cache/adapters.dart b/flutter_client/lib/cache/adapters.dart index ba19be86..0785284f 100644 --- a/flutter_client/lib/cache/adapters.dart +++ b/flutter_client/lib/cache/adapters.dart @@ -88,6 +88,12 @@ extension TrackRefDriftWrite on TrackRef { extension CachedPlaylistAdapter on CachedPlaylist { /// `ownerUsername` is server-derived; cache stores the userId only. /// Pass empty unless a join supplies it. + /// `coverUrl` is reconstructed deterministically from the playlist + /// id — the server serves the cached collage at this path + /// (handleGetPlaylistCover). Mirrors the album-cover trick. When + /// the server hasn't built a collage yet (system playlists with no + /// tracks at build time), the endpoint 404s and PlaylistCard's + /// ServerImage falls back to its slate placeholder. Playlist toRef({String ownerUsername = ''}) => Playlist( id: id, userId: userId, @@ -96,7 +102,7 @@ extension CachedPlaylistAdapter on CachedPlaylist { isPublic: isPublic, systemVariant: systemVariant, trackCount: trackCount, - coverUrl: '', // server-derived; cache doesn't persist + coverUrl: '/api/playlists/$id/cover', ownerUsername: ownerUsername, createdAt: '', updatedAt: '', diff --git a/flutter_client/lib/playlists/playlists_list_screen.dart b/flutter_client/lib/playlists/playlists_list_screen.dart index 217599b6..7cf4172a 100644 --- a/flutter_client/lib/playlists/playlists_list_screen.dart +++ b/flutter_client/lib/playlists/playlists_list_screen.dart @@ -83,7 +83,11 @@ class _PlaylistTile extends StatelessWidget { color: fs.slate, child: playlist.coverUrl.isEmpty ? Icon(Icons.queue_music, color: fs.ash) - : ServerImage(url: playlist.coverUrl, fit: BoxFit.cover), + : ServerImage( + url: playlist.coverUrl, + fit: BoxFit.cover, + fallback: Icon(Icons.queue_music, color: fs.ash), + ), ), ), const SizedBox(width: 12), diff --git a/flutter_client/lib/playlists/widgets/playlist_card.dart b/flutter_client/lib/playlists/widgets/playlist_card.dart index e6781f37..060351a4 100644 --- a/flutter_client/lib/playlists/widgets/playlist_card.dart +++ b/flutter_client/lib/playlists/widgets/playlist_card.dart @@ -31,9 +31,19 @@ class PlaylistCard extends StatelessWidget { width: 144, height: 144, color: fs.slate, + // coverUrl is deterministic per playlist (see + // CachedPlaylistAdapter.toRef). When the server's + // collage isn't built yet, ServerImage's + // errorBuilder shows the queue_music icon over the + // slate background. child: playlist.coverUrl.isEmpty ? Icon(Icons.queue_music, color: fs.ash, size: 56) - : ServerImage(url: playlist.coverUrl, fit: BoxFit.cover), + : ServerImage( + url: playlist.coverUrl, + fit: BoxFit.cover, + fallback: + Icon(Icons.queue_music, color: fs.ash, size: 56), + ), ), ), const SizedBox(height: 8), From 261b44522d4c9ebe7276d79918b517d51252ff73 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 11 May 2026 20:27:54 -0400 Subject: [PATCH 16/16] =?UTF-8?q?fix(flutter):=20bump=20album-grid=20cellH?= =?UTF-8?q?=20slack=20=E2=80=94=20silence=201px=20overflow=20warnings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Logs were spitting "RenderFlex overflowed by 1.00 pixels on the bottom" from album_card.dart whenever the library Albums tab or artist detail album grid rendered. Cell height was computed as cover + gap + title + artist + 4px slack, which assumes pixel-perfect 14sp/12sp line heights — Flutter's actual rendering with ascent/ descent + line-height multipliers wants one more pixel. Bump slack from 4 to 8 in both grids. AlbumCard layout unchanged; the warnings stop. Otherwise the log shape is healthy now: prefetcher fires once per library page emit (expected, one batch per pagination), cache misses fetch sequentially with clean drift-write → re-emit cycles, and album taps are single-round-trip cold-fetches followed by drift hits. No more cycles of duplicate fetches. --- flutter_client/lib/library/artist_detail_screen.dart | 7 ++++--- flutter_client/lib/library/library_screen.dart | 7 +++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/flutter_client/lib/library/artist_detail_screen.dart b/flutter_client/lib/library/artist_detail_screen.dart index 784776f1..3f624418 100644 --- a/flutter_client/lib/library/artist_detail_screen.dart +++ b/flutter_client/lib/library/artist_detail_screen.dart @@ -141,10 +141,11 @@ class ArtistDetailScreen extends ConsumerWidget { (constraints.maxWidth - sidePad * 2 - gap * (cols - 1)) / cols; // Card content: cover (cellW - 16) + 8 + title (≤2 lines - // ≈ 36) + small fudge. Artist line is suppressed in this + // ≈ 36) + slack. Artist line is suppressed in this // grid (showArtist: false) since the page header already - // names the artist. - final cellH = (cellW - 16) + 8 + 36 + 4; + // names the artist. Slack is generous on purpose — line- + // height variations would otherwise overflow by 1px. + final cellH = (cellW - 16) + 8 + 36 + 8; return GridView.builder( shrinkWrap: true, physics: const NeverScrollableScrollPhysics(), diff --git a/flutter_client/lib/library/library_screen.dart b/flutter_client/lib/library/library_screen.dart index 80b4a762..b3e85380 100644 --- a/flutter_client/lib/library/library_screen.dart +++ b/flutter_client/lib/library/library_screen.dart @@ -288,8 +288,11 @@ class _AlbumsTab extends ConsumerWidget { gap * (cols - 1)) / cols; // cover (cellW - 16) + gap (8) + 2-line title (~36) - // + artist line (~16) + small fudge. - final cellH = (cellW - 16) + 8 + 36 + 16 + 4; + // + artist line (~16) + slack. Slack is generous on + // purpose — line-height + font scaling variations + // would otherwise overflow the cell by a pixel + // (logged as a noisy RenderFlex warning). + final cellH = (cellW - 16) + 8 + 36 + 16 + 8; return NotificationListener( onNotification: (n) { if (n.metrics.pixels >=