From 3e7b2582a20bf4fc9496875dceb655fdc4fbb860 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 14 May 2026 07:36:59 -0400 Subject: [PATCH 1/9] fix(player): queue skip + cut over silence on track tap Two bugs in the audio handler caused the playback issues seen on the v2026.05.13.0 build: 1. **Queue button on the now-playing screen did nothing.** MinstrelAudio Handler never overrode skipToQueueItem, so taps in QueueScreen fell through to BaseAudioHandler's empty default. The queue UI updated nothing because the handler did nothing. Now overridden: rebuilds the source list via setQueueFromTracks(_lastTracks, initialIndex) so the targeted item plays cleanly even when its source hadn't been built yet by the background fill. 2. **Tapping a song in a playlist let the previous track bleed through until the new source finished building.** setQueueFromTracks awaits _buildAudioSource before swapping the player's source list, and that wait can be a few hundred ms on a cache miss. During the wait the old source kept playing while the mini player UI had already flipped to the new title/artist. Now pause()ing the player at the start of setQueueFromTracks silences the old source the moment the user taps. Also stashes the most recent TrackRef list as _lastTracks so skipToQueueItem can reconstruct sources without having to peek into just_audio's internal source list. --- flutter_client/lib/player/audio_handler.dart | 35 ++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/flutter_client/lib/player/audio_handler.dart b/flutter_client/lib/player/audio_handler.dart index db622187..9d48ae89 100644 --- a/flutter_client/lib/player/audio_handler.dart +++ b/flutter_client/lib/player/audio_handler.dart @@ -68,6 +68,14 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl /// new queue, leaving the player "locked" to a corrupted state. int _queueGeneration = 0; + /// Tracks the most recent setQueueFromTracks() input so + /// skipToQueueItem can reconstruct the source list. just_audio + /// requires every source to be built before it can be a skip + /// target, but setQueueFromTracks only builds the initial source + /// and fills the rest in the background — so a skip to an index + /// past the fill front needs to rebuild from the stored tracks. + List _lastTracks = const []; + /// Volume stream for UI subscribers. Mirrors the just_audio player's /// volume directly; set via setVolume(double). Stream get volumeStream => _player.volumeStream; @@ -105,6 +113,16 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl // Reset suppress flag in case a prior backward-fill bailed on // gen check before reaching its `finally`. _suppressIndexUpdates = false; + _lastTracks = tracks; + + // Pause the old source immediately so the previous track stops + // audibly the moment the user taps, instead of bleeding through + // until the new source finishes building. setAudioSources below + // will swap the source list cleanly; pause is the simplest way + // to silence the player during the (possibly multi-100ms) build. + if (_player.playing) { + await _player.pause(); + } // Populate the visible queue + current mediaItem immediately so // the player UI reflects the user's tap before any source has @@ -130,6 +148,23 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl unawaited(_fillRemainingSources(tracks, clampedInitial, myGen)); } + /// Switches playback to the [index]th queue item. The full + /// just_audio source list isn't necessarily built yet + /// (_fillRemainingSources runs in the background), so we + /// reconstruct from the stored TrackRef list rather than calling + /// _player.seek(index: ...) on a possibly-missing source. Calling + /// setQueueFromTracks again is the safe path: it bumps the + /// generation, cancels any in-flight fill, rebuilds source[0] as + /// the target, and re-fills around. play() restarts playback so + /// queue taps feel like "jump to this song" rather than "set the + /// pointer and wait for me to press play." + @override + Future skipToQueueItem(int index) async { + if (index < 0 || index >= _lastTracks.length) return; + await setQueueFromTracks(_lastTracks, initialIndex: index); + await play(); + } + /// 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 From 6a08d94255e70469198aab402d4a14e65ce0067f Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 14 May 2026 07:37:42 -0400 Subject: [PATCH 2/9] fix(player): smooth mini bar cover swap across track change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audio handler broadcasts MediaItem twice on every track change: once with artUri=null (the new track's bare metadata), then again with artUri pointing at the AlbumCoverCache file once the cover lands on disk. The mini player's cover element was rebuilding in place: previous track's image → slate placeholder → new track's image. That flash is the flicker reported on the v2026.05.13.0 build. AnimatedSwitcher around the cover (180ms crossfade) keyed by the artUri value makes the swap a smooth crossfade instead of a visible snap. The Hero parent stays — its tag is stable across track changes so the mini→full bar expansion animation keeps working unchanged. --- flutter_client/lib/player/player_bar.dart | 28 ++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/flutter_client/lib/player/player_bar.dart b/flutter_client/lib/player/player_bar.dart index 261e8d8d..d6f31cec 100644 --- a/flutter_client/lib/player/player_bar.dart +++ b/flutter_client/lib/player/player_bar.dart @@ -95,14 +95,25 @@ class _TrackInfo extends StatelessWidget { // artwork from this 48dp footprint to the full-screen size rather // than fade-cutting. Tag is stable per-route (not keyed by media.id) // so the transition works regardless of what's playing. - Widget cover; + // + // AnimatedSwitcher around the cover smooths the artUri null→file:// + // swap that fires on every track change: audio_handler broadcasts + // the new MediaItem without artUri immediately, then re-broadcasts + // with artUri once AlbumCoverCache resolves the path. Without the + // crossfade the cover snaps from the previous track's image to a + // slate placeholder to the new image — visible as a flicker. + final Widget coverChild; if (media.artUri != null) { // CachedNetworkImageProvider for HTTPS art URIs so the mini bar // hits the same disk cache the rest of the UI uses (ServerImage, // discover thumbnails). FileImage stays for the file:// branch // populated by AlbumCoverCache — bytes are already on disk and a // second cache layer would only burn duplicate space. - cover = Image( + coverChild = Image( + // Key includes the artUri so AnimatedSwitcher detects a swap; + // without this it treats successive Image widgets as the same + // tree node and skips the transition. + key: ValueKey('art-${media.artUri}'), image: media.artUri!.isScheme('file') ? FileImage(File.fromUri(media.artUri!)) as ImageProvider : CachedNetworkImageProvider(media.artUri.toString()), @@ -117,8 +128,19 @@ class _TrackInfo extends StatelessWidget { Container(width: 48, height: 48, color: fs.slate), ); } else { - cover = Container(width: 48, height: 48, color: fs.slate); + coverChild = Container( + key: const ValueKey('art-placeholder'), + width: 48, + height: 48, + color: fs.slate, + ); } + final cover = AnimatedSwitcher( + duration: const Duration(milliseconds: 180), + switchInCurve: Curves.easeOut, + switchOutCurve: Curves.easeOut, + child: coverChild, + ); return Row( // Default centering vertically aligns the like/kebab buttons // against the album art (48dp) — visually they span the title + From 2ebe6229b73d6001d237c731dbcf6b6989552a25 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 14 May 2026 07:41:27 -0400 Subject: [PATCH 3/9] fix(player): smooth full-player cover + backdrop on track change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related "snap in" effects on the now-playing screen: 1. **Album art snapped in after the fade.** AnimatedSwitcher cross- fades the new _AlbumArt over 300ms, but FileImage's bytes weren't decoded yet — the widget was visually empty during the fade and the cover landed abruptly after. precacheImage on the new file:// artUri pre-decodes the bytes so by the time AnimatedSwitcher mounts the new tile, the cover paints synchronously inside it. The cross-fade now carries real content end-to-end. 2. **Backdrop color snapped in.** albumColorProvider.family is loading for the new id during the track-change moment, so dominant fell back to fs.obsidian; AnimatedContainer tweened to obsidian and then snapped to the resolved color a beat later. _NowPlayingScreenState now holds _lastDominant across builds: while extraction for the new id is loading, the gradient stays on the previous album's color, then animates straight to the new one once it resolves. No intermediate obsidian stop. Net effect: track changes feel like a single smooth transition instead of fade-out → blank → snap. --- .../lib/player/now_playing_screen.dart | 49 +++++++++++++++++-- 1 file changed, 44 insertions(+), 5 deletions(-) diff --git a/flutter_client/lib/player/now_playing_screen.dart b/flutter_client/lib/player/now_playing_screen.dart index 1b0038ee..18ed3e00 100644 --- a/flutter_client/lib/player/now_playing_screen.dart +++ b/flutter_client/lib/player/now_playing_screen.dart @@ -45,6 +45,17 @@ class _NowPlayingScreenState extends ConsumerState { // dismiss. Reset on each drag start. double _dragOffset = 0; + /// Last resolved dominant color. Held across track changes so the + /// gradient backdrop tweens smoothly from the previous album's color + /// to the next, instead of dropping through fs.obsidian during the + /// brief moment albumColorProvider for the new id is still loading. + Color? _lastDominant; + + /// Dedupe key for the cover precache side-effect. Without this we'd + /// fire precacheImage on every MediaItem rebroadcast (twice per track + /// change, once on artUri arrival). + String? _precachedArtUri; + void _onDragStart(DragStartDetails _) { _dragOffset = 0; } @@ -87,17 +98,45 @@ class _NowPlayingScreenState extends ConsumerState { final albumId = (media.extras?['album_id'] as String?) ?? ''; // Dominant-color gradient backdrop seeded from the current track's - // album cover. While the color resolves (or when no cover exists), - // the gradient collapses to a flat fs.obsidian — same look as - // before the polish landed. Tweens to the new color on track change - // via AnimatedContainer. + // album cover. Tweens to the new color on track change via + // AnimatedContainer. + // + // We hold _lastDominant across builds so a track change doesn't + // briefly tween to fs.obsidian while albumColorProvider for the + // new id is loading — that brief stop produced the "background + // coloring snaps in" effect users saw. With a held color, the + // gradient stays on the previous album's color until the new + // extraction resolves, then animates straight to it. final dominantAsync = ref.watch(albumColorProvider(albumId)); - final dominant = dominantAsync.asData?.value ?? fs.obsidian; + final resolved = dominantAsync.asData?.value; + if (resolved != null) _lastDominant = resolved; + final dominant = _lastDominant ?? fs.obsidian; // 0.55 alpha keeps the color present without overwhelming contrast // on the title / artist text below. The lower half of the screen // stays obsidian for control legibility. final gradientTop = dominant.withValues(alpha: 0.55); + // Precache the cover image bytes the moment a new MediaItem (with + // a file:// artUri) is broadcast. Without this, AnimatedSwitcher's + // cross-fade for the cover would complete before FileImage finished + // decoding the bytes — visible as a "snap in" of the album art + // after the fade. precacheImage decodes in the background; by the + // time the new _AlbumArt mounts inside the switcher, the bytes are + // ready and the cover paints synchronously. + // + // Non-file artUris (the rare moment before AlbumCoverCache has + // written the file) fall back to ServerImage / CachedNetworkImage + // which has its own 120ms fade — no precache benefit there. + final artUri = media.artUri; + final artKey = artUri?.toString(); + if (artUri != null && + artUri.isScheme('file') && + artKey != _precachedArtUri) { + _precachedArtUri = artKey; + // ignore: unawaited_futures + precacheImage(FileImage(File.fromUri(artUri)), context); + } + return Scaffold( backgroundColor: fs.obsidian, // The whole screen accepts vertical drag for dismissal. Buttons From c29d25d1cbec2e081a3bf2762a43135b977bbb33 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 14 May 2026 07:50:35 -0400 Subject: [PATCH 4/9] fix(playlists): dedup tracks across discover buckets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Discover playlists could surface the same track twice with the duplicates landing back-to-back — a "first song plays, then plays again, skip works" symptom user reported on v2026.05.13.0. Root cause: interleaveBuckets rotates one track per pass per bucket but never tracks which IDs it has already emitted, so a track that's both a dormant-artist pick AND a random-unheard pick comes out once from each bucket. On a single-user server the crossUser bucket is empty, so the redistribute step rolls its slots into dormant + random. Their output then interleaves d0, r0, d1, r1, … — and when d0 == r0 (common: a dormant-artist track is also valid for random-unheard) the result is [X, X, …] with adjacent duplicates. Fix: track seen track IDs across all buckets while interleaving; skip already-taken IDs and advance to the next index in that bucket. Dedup priority is bucket order, so a track in both dormant and random comes from dormant. Regression test covers the single-user case directly. The existing round-robin test still passes — no shared IDs in that fixture. Note: stale duplicates already written to drift / served as cached playlists will clear naturally on the next playlist rebuild (the 03:00-local refresh, or any manual /api/me/playlists/refresh). --- internal/playlists/discover.go | 25 ++++++++++++++++++++++++- internal/playlists/discover_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/internal/playlists/discover.go b/internal/playlists/discover.go index f86be276..65f41d48 100644 --- a/internal/playlists/discover.go +++ b/internal/playlists/discover.go @@ -254,18 +254,41 @@ func redistributeSlots(buckets []bucketRequest) []int { // interleaveBuckets round-robins items from the buckets in order. The // first item of each bucket appears before the second of any bucket. +// interleaveBuckets walks each bucket round-robin, emitting one track +// per bucket per pass. Tracks already seen in an earlier bucket (or an +// earlier pass) are skipped — single-user servers hit this often +// because the empty crossUser bucket redistributes slots to dormant + +// random, and a dormant-artist track is also a valid random-unheard +// pick. Without dedup the same track lands at two interleaved +// positions, which on a 2-bucket round-robin (dormant+random) +// produces ADJACENT duplicates in the playlist — exactly the +// "every song has a duplicate right after it" report from v2026.05.13.0. +// +// Dedup priority is bucket order (caller-supplied), so a track in both +// dormant and random is taken from dormant. func interleaveBuckets(buckets ...[]discoverTrack) []discoverTrack { total := 0 for _, b := range buckets { total += len(b) } out := make([]discoverTrack, 0, total) + seen := make(map[pgtype.UUID]struct{}, total) indices := make([]int, len(buckets)) for { anyAppended := false for bi, b := range buckets { + // Advance past tracks already taken from an earlier bucket + // or an earlier pass. + for indices[bi] < len(b) { + if _, dup := seen[b[indices[bi]].ID]; !dup { + break + } + indices[bi]++ + } if indices[bi] < len(b) { - out = append(out, b[indices[bi]]) + t := b[indices[bi]] + out = append(out, t) + seen[t.ID] = struct{}{} indices[bi]++ anyAppended = true } diff --git a/internal/playlists/discover_test.go b/internal/playlists/discover_test.go index e58d93a7..e299cb11 100644 --- a/internal/playlists/discover_test.go +++ b/internal/playlists/discover_test.go @@ -137,3 +137,27 @@ func TestInterleaveBuckets_RoundRobin(t *testing.T) { } } } + +func TestInterleaveBuckets_DedupsAcrossBuckets(t *testing.T) { + // Single-user server scenario: crossUser bucket empty, dormant + + // random share tracks. Without dedup, shared tracks land at + // adjacent interleaved positions — the duplication users reported + // on the Discover playlist in v2026.05.13.0. + dormant := []discoverTrack{{ID: uuidN(1)}, {ID: uuidN(2)}, {ID: uuidN(3)}} + crossUser := []discoverTrack{} // empty bucket, single-user server + random := []discoverTrack{{ID: uuidN(1)}, {ID: uuidN(2)}, {ID: uuidN(4)}} + got := interleaveBuckets(dormant, crossUser, random) + + // Tracks 1 and 2 appear in both dormant and random — must be + // emitted once each (from dormant, the earlier bucket). Track 3 + // is dormant-only, track 4 is random-only. + if len(got) != 4 { + t.Fatalf("len = %d, want 4 (3 unique from dormant + 1 unique from random)", len(got)) + } + wantOrder := []byte{1, 2, 3, 4} + for i, w := range wantOrder { + if got[i].ID.Bytes[15] != w { + t.Errorf("got[%d].ID = %d, want %d", i, got[i].ID.Bytes[15], w) + } + } +} From 8f1bc60757f2c5c9099e424bc04ceb8cdc5590a0 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 14 May 2026 08:30:23 -0400 Subject: [PATCH 5/9] chore(flutter): bump versionCode to 2 for v2026.05.13.1 hotfix --- flutter_client/pubspec.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flutter_client/pubspec.yaml b/flutter_client/pubspec.yaml index b0d8ee18..5ad68b0f 100644 --- a/flutter_client/pubspec.yaml +++ b/flutter_client/pubspec.yaml @@ -1,7 +1,7 @@ name: minstrel description: Minstrel mobile client publish_to: 'none' -version: 2026.5.13+1 +version: 2026.5.13+2 environment: sdk: '>=3.5.0 <4.0.0' From 6efb3159d5c434736eb354aae639847762111a99 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 14 May 2026 08:32:35 -0400 Subject: [PATCH 6/9] fix(flutter): silence 404 log noise from missing playlist covers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Playlist collages aren't generated until the build job runs over a playlist with tracks — system playlists (For-You / Discover / Songs- like) and any newly-created playlist hit a brief window where /api/playlists/:id/cover returns 404. ServerImage's errorWidget already renders the visual fallback (queue_music icon over slate); this fix just keeps cached_network_image from spamming the dev console with HttpExceptionWithStatus stack traces. errorListener filters 404 specifically — auth (401/403) and any 5xx still log so real connectivity / permission issues stay visible. User-visible behavior unchanged; this is a dev-mode log hygiene fix. --- .../lib/shared/widgets/server_image.dart | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/flutter_client/lib/shared/widgets/server_image.dart b/flutter_client/lib/shared/widgets/server_image.dart index 88d07c4d..47af8517 100644 --- a/flutter_client/lib/shared/widgets/server_image.dart +++ b/flutter_client/lib/shared/widgets/server_image.dart @@ -1,5 +1,8 @@ import 'package:cached_network_image/cached_network_image.dart'; +import 'package:flutter/foundation.dart' show debugPrint; import 'package:flutter/material.dart'; +import 'package:flutter_cache_manager/flutter_cache_manager.dart' + show HttpExceptionWithStatus; import 'package:flutter_riverpod/flutter_riverpod.dart'; import '../../auth/auth_provider.dart'; @@ -71,6 +74,19 @@ class ServerImage extends ConsumerWidget { // Keep failures local — a single 401/timeout shouldn't dump // a stack trace or replace the parent Container's background. errorWidget: (_, __, ___) => empty, + // Filter expected 404s out of dev console noise: playlist + // collages aren't built until the playlist has tracks and + // the build job runs (system playlists like For-You / + // Discover hit this on first-render). The errorWidget + // already renders the fallback for the user; this just + // keeps the dev console clean. Non-404 errors still + // surface so auth/connectivity issues remain visible. + errorListener: (err) { + if (err is HttpExceptionWithStatus && err.statusCode == 404) { + return; + } + debugPrint('ServerImage: $err'); + }, ); }, ); From bf0ef5e0c3db3709c27e0d38ae43e6587b4e8d13 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 14 May 2026 08:39:37 -0400 Subject: [PATCH 7/9] fix(flutter): keep artist avatars round in the Library grid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ArtistCard hardcoded its avatar at Container(width: 124, height: 124) inside a ClipOval. In the Library Artists 3-column grid the cell is narrower than the card's nominal 140dp width — on a typical phone the cell is ~109dp, the padded inner content area ~93dp. The parent constrained the Container's width to ~93dp but the explicit height stayed 124dp, so ClipOval clipped a 93×124 rectangle and the avatar rendered as a vertical ellipse. Fix mirrors AlbumCard's pattern: ArtistCard takes an optional `width` parameter (default 140 for horizontal carousels) and derives coverSize = width - 16, so the Container is always square. ArtistsTab now uses LayoutBuilder to compute cell width and passes it through, same as AlbumsTab. Avatar stays a true circle at any cell width. mainAxisExtent on the grid replaces the previous fixed childAspectRatio so cell height tracks cellW + name line, with slack matching AlbumsTab's overflow guard. --- .../lib/library/library_screen.dart | 74 +++++++++++-------- .../lib/library/widgets/artist_card.dart | 31 ++++++-- 2 files changed, 69 insertions(+), 36 deletions(-) diff --git a/flutter_client/lib/library/library_screen.dart b/flutter_client/lib/library/library_screen.dart index 33ceb20a..c300294a 100644 --- a/flutter_client/lib/library/library_screen.dart +++ b/flutter_client/lib/library/library_screen.dart @@ -398,36 +398,52 @@ class _ArtistsTab extends ConsumerWidget { // Listen for scroll-near-bottom and ask the notifier // to fetch the next page. 800px lookahead so the next // page lands before the user reaches the visible end. - child: NotificationListener( - onNotification: (n) { - if (n.metrics.pixels >= - n.metrics.maxScrollExtent - 800) { - ref - .read(_libraryArtistsProvider.notifier) - .loadMore(); - } - return false; - }, - child: GridView.builder( - padding: const EdgeInsets.all(8), - gridDelegate: - const SliverGridDelegateWithFixedCrossAxisCount( - crossAxisCount: 3, - mainAxisSpacing: 8, - crossAxisSpacing: 8, - childAspectRatio: 0.78, - ), - itemCount: page.items.length, - itemBuilder: (ctx, i) { - final artist = page.items[i]; - return ArtistCard( - artist: artist, - onTap: () => ctx.push('/artists/${artist.id}', - extra: artist), - ); + // LayoutBuilder + cell-aware ArtistCard width mirrors + // the AlbumsTab pattern so the circular avatar stays + // a true circle on narrow grid cells. + child: LayoutBuilder(builder: (ctx, constraints) { + const cols = 3; + const sidePad = 8.0; + const gap = 8.0; + final cellW = (constraints.maxWidth - + sidePad * 2 - + gap * (cols - 1)) / + cols; + // avatar (cellW - 16) + gap (8) + 1 line name (~18) + // + slack matched to AlbumsTab's overflow guard. + final cellH = (cellW - 16) + 8 + 18 + 8; + return NotificationListener( + onNotification: (n) { + if (n.metrics.pixels >= + n.metrics.maxScrollExtent - 800) { + ref + .read(_libraryArtistsProvider.notifier) + .loadMore(); + } + return false; }, - ), - ), + child: GridView.builder( + padding: const EdgeInsets.all(sidePad), + gridDelegate: + SliverGridDelegateWithFixedCrossAxisCount( + crossAxisCount: cols, + mainAxisExtent: cellH, + mainAxisSpacing: gap, + crossAxisSpacing: gap, + ), + itemCount: page.items.length, + itemBuilder: (ctx, i) { + final artist = page.items[i]; + return ArtistCard( + artist: artist, + width: cellW, + onTap: () => ctx.push('/artists/${artist.id}', + extra: artist), + ); + }, + ), + ); + }), ); }, ); diff --git a/flutter_client/lib/library/widgets/artist_card.dart b/flutter_client/lib/library/widgets/artist_card.dart index 3d6bd6b2..fed8c74f 100644 --- a/flutter_client/lib/library/widgets/artist_card.dart +++ b/flutter_client/lib/library/widgets/artist_card.dart @@ -12,15 +12,27 @@ import '../library_providers.dart'; import 'play_circle_button.dart'; class ArtistCard extends ConsumerWidget { - const ArtistCard({required this.artist, required this.onTap, super.key}); + const ArtistCard({ + required this.artist, + required this.onTap, + this.width = 140, + super.key, + }); final ArtistRef artist; final VoidCallback onTap; + /// Outer width of the card. Avatar is a circle of width-16 (8dp + /// horizontal padding either side). Default suits horizontal lists; + /// grids should pass the cell width so the avatar shrinks + /// proportionally and stays a true circle. + final double width; + @override Widget build(BuildContext context, WidgetRef ref) { final fs = Theme.of(context).extension()!; + final coverSize = width - 16; return SizedBox( - width: 140, + width: width, child: Material( color: Colors.transparent, child: InkWell( @@ -29,15 +41,20 @@ class ArtistCard extends ConsumerWidget { padding: const EdgeInsets.symmetric(horizontal: 8), child: Column(crossAxisAlignment: CrossAxisAlignment.start, children: [ // Stack: circular avatar + overlaid play button at bottom-right. - // The avatar is a circle (ClipOval) — bottom-right of its - // bounding box still places the button inside the visible area - // because the button is small relative to the radius. + // Avatar size derives from the [width] parameter so the + // Library Artists 3-column grid can pass its cell width + // (~109dp on typical phones) and the avatar stays a + // perfect circle. Previous hardcoded 124×124 got squeezed + // horizontally in the grid (cell narrower than 124+16 + // padding) but kept its 124dp height — ClipOval produced + // a visible vertical ellipse. Mirrors AlbumCard's width- + // parameter convention. Stack( children: [ ClipOval( child: Container( - width: 124, - height: 124, + width: coverSize, + height: coverSize, color: fs.slate, child: artist.coverUrl.isEmpty ? SvgPicture.asset('assets/svg/album-fallback.svg', From 507c532f6de9025e99965f17eaded48bca28d998 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 14 May 2026 08:42:50 -0400 Subject: [PATCH 8/9] fix(flutter): drop redundant foundation import (debugPrint via material) --- flutter_client/lib/shared/widgets/server_image.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/flutter_client/lib/shared/widgets/server_image.dart b/flutter_client/lib/shared/widgets/server_image.dart index 47af8517..92b854e4 100644 --- a/flutter_client/lib/shared/widgets/server_image.dart +++ b/flutter_client/lib/shared/widgets/server_image.dart @@ -1,5 +1,4 @@ import 'package:cached_network_image/cached_network_image.dart'; -import 'package:flutter/foundation.dart' show debugPrint; import 'package:flutter/material.dart'; import 'package:flutter_cache_manager/flutter_cache_manager.dart' show HttpExceptionWithStatus; From 2a18e91c39ee9017ba63cb8f55c5b9a836dafd52 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 14 May 2026 10:14:46 -0400 Subject: [PATCH 9/9] feat(flutter): drift-first Library tabs + systemPlaylistsStatus + tab pre-warm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four-part change to push more surfaces onto the drift cache and eliminate cold-tab-visit latency on the Library screen. * **Library Artists tab** — _libraryArtistsProvider migrates from REST-paginated AsyncNotifier with infinite-scroll loadMore to a drift-first StreamProvider over cached_artists ordered by sortName. Sync already populates the full set; cacheFirst's fetchAndPopulate covers the fresh-install + sync-not-yet-done cold case via /api/artists?limit=1000. SWR refresh on every visit. GridView.builder lazily realizes only visible cells so loading the full list up front is fine for typical libraries. loadMore + NotificationListener gone. * **Library Albums tab** — same migration, drift-first over cached_albums joined with cached_artists for the artistName field. * **systemPlaylistsStatusProvider** — new CachedSystemPlaylistsStatus single-row table (schema 6 → 7, JSON blob like CachedHomeSnapshot) for the home Playlists row's "building / pending / failed" placeholder logic. Drift-first means the row paints with the prior status instantly instead of flickering through SystemPlaylistsStatus.empty() while the REST call resolves. * **Library screen tab pre-warm** — ref.listen on all 5 tab providers in _LibraryScreenState.build subscribes them upfront so swiping between tabs feels instant rather than each tab paying its own cold-cache cost on first visit. cacheFirst handles dedupe of concurrent fetchAndPopulate triggers. Test mock updated for the StreamProvider type change on systemPlaylistsStatusProvider. --- flutter_client/lib/cache/db.dart | 25 +- .../lib/library/library_screen.dart | 295 ++++++++---------- .../lib/playlists/playlists_provider.dart | 40 ++- .../test/library/home_screen_test.dart | 6 +- 4 files changed, 201 insertions(+), 165 deletions(-) diff --git a/flutter_client/lib/cache/db.dart b/flutter_client/lib/cache/db.dart index d2227677..b3fca3b1 100644 --- a/flutter_client/lib/cache/db.dart +++ b/flutter_client/lib/cache/db.dart @@ -142,6 +142,21 @@ class CachedHistorySnapshot extends Table { Set get primaryKey => {id}; } +/// Single-row cache of the /api/me/system-playlists-status response. +/// The home Playlists row reads this every render to pick between +/// real cards and placeholder cards for For-You / Discover / Songs- +/// like slots, so a fresh-mount cold fetch produces a visible +/// "building / pending / failed" flicker. Storing the last result +/// as a JSON blob means the home renders with the prior status +/// instantly, then SWR refreshes underneath. Schema 7+. +class CachedSystemPlaylistsStatus extends Table { + IntColumn get id => integer().withDefault(const Constant(1))(); + TextColumn get json => text()(); + DateTimeColumn get updatedAt => dateTime().withDefault(currentDateAndTime)(); + @override + Set get primaryKey => {id}; +} + /// Section→position→entity-id index for the home screen, populated /// from the per-item discovery endpoint `/api/home/index`. Each row /// pins one tile slot to an entity; the actual entity data lives in @@ -201,12 +216,13 @@ enum CacheSource { manual, autoLiked, autoPlaylist, autoPrefetch, incidental } CachedHistorySnapshot, CachedQuarantineMine, CachedHomeIndex, + CachedSystemPlaylistsStatus, ]) class AppDb extends _$AppDb { AppDb([QueryExecutor? e]) : super(e ?? driftDatabase(name: 'minstrel_cache')); @override - int get schemaVersion => 6; + int get schemaVersion => 7; @override MigrationStrategy get migration => MigrationStrategy( @@ -247,6 +263,13 @@ class AppDb extends _$AppDb { // builds still read from it as a fallback path. await m.createTable(cachedHomeIndex); } + if (from < 7) { + // Schema 7: cached_system_playlists_status. Single-row + // snapshot of /api/me/system-playlists-status used by the + // home Playlists row. Empty on upgrade; first fetch + // populates. + await m.createTable(cachedSystemPlaylistsStatus); + } }, ); } diff --git a/flutter_client/lib/library/library_screen.dart b/flutter_client/lib/library/library_screen.dart index c300294a..8392c728 100644 --- a/flutter_client/lib/library/library_screen.dart +++ b/flutter_client/lib/library/library_screen.dart @@ -9,6 +9,7 @@ import '../api/endpoints/library_lists.dart'; import '../api/endpoints/likes.dart'; import '../api/endpoints/me.dart'; import '../auth/auth_provider.dart' show authControllerProvider; +import '../cache/adapters.dart'; import '../cache/audio_cache_manager.dart' show appDbProvider; import '../cache/cache_first.dart'; import '../cache/connectivity_provider.dart'; @@ -19,9 +20,6 @@ import '../library/library_providers.dart' show dioProvider; import '../models/album.dart'; import '../models/artist.dart'; import '../models/history_event.dart'; -// Aliased: Flutter's Material exports a `Page` class (Navigator 2.0 -// route descriptor) that conflicts with our wire-format Page. -import '../models/page.dart' as wire; import '../models/quarantine_mine.dart'; import '../models/track.dart'; import '../player/player_provider.dart'; @@ -51,89 +49,80 @@ final _meApiProvider = FutureProvider((ref) async { return MeApi(await ref.watch(dioProvider.future)); }); -/// Paginated artist list. AsyncNotifier so the screen can call -/// `loadMore()` when the scroll approaches the bottom; subsequent -/// pages append to the existing items rather than replacing them. -class _LibraryArtistsNotifier extends AsyncNotifier> { - static const _pageSize = 50; - bool _loadingMore = false; - - @override - Future> build() async { - final api = await ref.watch(_libraryListsApiProvider.future); - return api.listArtists(limit: _pageSize, offset: 0); - } - - Future loadMore() async { - if (_loadingMore) return; - final cur = state.value; - if (cur == null) return; - if (cur.items.length >= cur.total) return; - _loadingMore = true; - try { +/// Drift-first all-artists list. Reads cached_artists ordered by +/// sortName; GridView.builder takes care of lazy widget construction +/// so loading the full set up front is fine for typical library sizes. +/// SWR refresh on every subscription hits /api/artists with a generous +/// limit so newly-scanned-but-not-yet-synced rows land soon after. +/// +/// The old AsyncNotifier+loadMore infinite-scroll path went away: the +/// previous "fetch one page at a time as you scroll" felt like the +/// rest of the app was buffering when this tab was the slow surface, +/// and SyncController already populates the full set of artist rows +/// via /api/library/sync. +final _libraryArtistsProvider = StreamProvider>((ref) { + final db = ref.watch(appDbProvider); + final query = db.select(db.cachedArtists) + ..orderBy([(t) => drift.OrderingTerm.asc(t.sortName)]); + return cacheFirst>( + driftStream: query.watch(), + fetchAndPopulate: () async { + // Cold-cache fallback: sync should have populated drift already, + // but a fresh install + first Library visit before sync completes + // will be empty. Fetch a generous chunk via /api/artists and + // persist; subsequent SWR refreshes keep things current. final api = await ref.read(_libraryListsApiProvider.future); - final next = await api.listArtists( - limit: _pageSize, - offset: cur.items.length, - ); - state = AsyncData(wire.Paged( - items: [...cur.items, ...next.items], - total: next.total, - limit: next.limit, - offset: 0, - )); - } catch (_) { - // Swallow — the next near-bottom event will retry. The current - // partial list stays visible. - } finally { - _loadingMore = false; - } - } -} + final fresh = await api.listArtists(limit: 1000, offset: 0); + await db.batch((b) { + b.insertAllOnConflictUpdate( + db.cachedArtists, + fresh.items.map((a) => a.toDrift()).toList(), + ); + }); + }, + toResult: (rows) => rows.map((r) => r.toRef()).toList(), + isOnline: () async => (await ref + .read(connectivityProvider.future) + .timeout(const Duration(seconds: 3), onTimeout: () => true)), + alwaysRefresh: true, + tag: 'libraryArtists', + ); +}); -final _libraryArtistsProvider = AsyncNotifierProvider< - _LibraryArtistsNotifier, - wire.Paged>(_LibraryArtistsNotifier.new); - -class _LibraryAlbumsNotifier extends AsyncNotifier> { - static const _pageSize = 50; - bool _loadingMore = false; - - @override - Future> build() async { - final api = await ref.watch(_libraryListsApiProvider.future); - return api.listAlbums(limit: _pageSize, offset: 0); - } - - Future loadMore() async { - if (_loadingMore) return; - final cur = state.value; - if (cur == null) return; - if (cur.items.length >= cur.total) return; - _loadingMore = true; - try { +/// Drift-first all-albums list. Joins cached_albums with cached_artists +/// for the artistName field. Same cold-cache fallback + SWR refresh +/// pattern as libraryArtistsProvider. +final _libraryAlbumsProvider = StreamProvider>((ref) { + final db = ref.watch(appDbProvider); + final query = db.select(db.cachedAlbums).join([ + drift.leftOuterJoin(db.cachedArtists, + db.cachedArtists.id.equalsExp(db.cachedAlbums.artistId)), + ]) + ..orderBy([drift.OrderingTerm.asc(db.cachedAlbums.sortTitle)]); + return cacheFirst>( + driftStream: query.watch(), + fetchAndPopulate: () async { final api = await ref.read(_libraryListsApiProvider.future); - final next = await api.listAlbums( - limit: _pageSize, - offset: cur.items.length, - ); - state = AsyncData(wire.Paged( - items: [...cur.items, ...next.items], - total: next.total, - limit: next.limit, - offset: 0, - )); - } catch (_) { - // Swallow — next scroll event will retry. - } finally { - _loadingMore = false; - } - } -} - -final _libraryAlbumsProvider = AsyncNotifierProvider< - _LibraryAlbumsNotifier, - wire.Paged>(_LibraryAlbumsNotifier.new); + final fresh = await api.listAlbums(limit: 1000, offset: 0); + await db.batch((b) { + b.insertAllOnConflictUpdate( + db.cachedAlbums, + fresh.items.map((a) => a.toDrift()).toList(), + ); + }); + }, + toResult: (rows) => rows.map((r) { + final album = r.readTable(db.cachedAlbums); + final artist = r.readTableOrNull(db.cachedArtists); + return album.toRef(artistName: artist?.name ?? ''); + }).toList(), + isOnline: () async => (await ref + .read(connectivityProvider.future) + .timeout(const Duration(seconds: 3), onTimeout: () => true)), + alwaysRefresh: true, + tag: 'libraryAlbums', + ); +}); // Drift-first History tab. Mirrors homeProvider's pattern: store the // last /api/me/history page as JSON in a single-row drift table, yield @@ -337,6 +326,19 @@ class _LibraryScreenState extends ConsumerState @override Widget build(BuildContext context) { final fs = Theme.of(context).extension()!; + // Pre-warm every tab's provider on Library mount so swiping + // between tabs feels instant rather than each tab paying its + // own cold-cache cost on first visit. ref.listen subscribes + // without rebuilding LibraryScreen on emit; subscriptions stay + // alive for the lifetime of this widget. cacheFirst handles + // dedupe of concurrent fetchAndPopulate triggers. + ref.listen(_libraryArtistsProvider, (_, __) {}); + ref.listen(_libraryAlbumsProvider, (_, __) {}); + ref.listen(_historyProvider, (_, __) {}); + ref.listen(_likedTrackIdsProvider, (_, __) {}); + ref.listen(_likedAlbumIdsProvider, (_, __) {}); + ref.listen(_likedArtistIdsProvider, (_, __) {}); + ref.listen(myQuarantineProvider, (_, __) {}); return Scaffold( backgroundColor: fs.obsidian, appBar: AppBar( @@ -383,24 +385,24 @@ 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) { + data: (artists) { // Warm details for the first screenful so taps are instant. ref .read(metadataPrefetcherProvider) - .warmArtists(page.items.map((a) => a.id)); - return page.items.isEmpty + .warmArtists(artists.map((a) => a.id)); + return artists.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 { - ref.invalidate(_libraryArtistsProvider); - await ref.read(_libraryArtistsProvider.future); - }, - // Listen for scroll-near-bottom and ask the notifier - // to fetch the next page. 800px lookahead so the next - // page lands before the user reaches the visible end. + onRefresh: () async => ref.refresh(_libraryArtistsProvider.future), // LayoutBuilder + cell-aware ArtistCard width mirrors // the AlbumsTab pattern so the circular avatar stays // a true circle on narrow grid cells. + // + // Drift-first: GridView holds the full sorted list; + // .builder lazily realizes only visible cells, so + // even on large libraries the up-front cost is just + // a sort over cached_artists, not N network round + // trips. child: LayoutBuilder(builder: (ctx, constraints) { const cols = 3; const sidePad = 8.0; @@ -412,36 +414,25 @@ class _ArtistsTab extends ConsumerWidget { // avatar (cellW - 16) + gap (8) + 1 line name (~18) // + slack matched to AlbumsTab's overflow guard. final cellH = (cellW - 16) + 8 + 18 + 8; - return NotificationListener( - onNotification: (n) { - if (n.metrics.pixels >= - n.metrics.maxScrollExtent - 800) { - ref - .read(_libraryArtistsProvider.notifier) - .loadMore(); - } - return false; - }, - child: GridView.builder( - padding: const EdgeInsets.all(sidePad), - gridDelegate: - SliverGridDelegateWithFixedCrossAxisCount( - crossAxisCount: cols, - mainAxisExtent: cellH, - mainAxisSpacing: gap, - crossAxisSpacing: gap, - ), - itemCount: page.items.length, - itemBuilder: (ctx, i) { - final artist = page.items[i]; - return ArtistCard( - artist: artist, - width: cellW, - onTap: () => ctx.push('/artists/${artist.id}', - extra: artist), - ); - }, + return GridView.builder( + padding: const EdgeInsets.all(sidePad), + gridDelegate: + SliverGridDelegateWithFixedCrossAxisCount( + crossAxisCount: cols, + mainAxisExtent: cellH, + mainAxisSpacing: gap, + crossAxisSpacing: gap, ), + itemCount: artists.length, + itemBuilder: (ctx, i) { + final artist = artists[i]; + return ArtistCard( + artist: artist, + width: cellW, + onTap: () => ctx.push('/artists/${artist.id}', + extra: artist), + ); + }, ); }), ); @@ -459,17 +450,16 @@ 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) { - return page.items.isEmpty + data: (albums) { + return albums.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 { - ref.invalidate(_libraryAlbumsProvider); - await ref.read(_libraryAlbumsProvider.future); - }, + onRefresh: () async => ref.refresh(_libraryAlbumsProvider.future), // Same responsive 3-up grid as the artist detail // album list — LayoutBuilder + AlbumCard sized to the // cell, mainAxisExtent matched to actual card height. + // Drift-first; full sorted list, lazy realization via + // GridView.builder. child: LayoutBuilder(builder: (ctx, constraints) { const cols = 3; const sidePad = 8.0; @@ -484,37 +474,26 @@ class _AlbumsTab extends ConsumerWidget { // 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 >= - n.metrics.maxScrollExtent - 800) { - ref - .read(_libraryAlbumsProvider.notifier) - .loadMore(); - } - return false; - }, - child: GridView.builder( - padding: const EdgeInsets.all(sidePad), - gridDelegate: - SliverGridDelegateWithFixedCrossAxisCount( - crossAxisCount: cols, - mainAxisExtent: cellH, - mainAxisSpacing: gap, - crossAxisSpacing: gap, - ), - itemCount: page.items.length, - itemBuilder: (ctx, i) { - final album = page.items[i]; - return AlbumCard( - album: album, - width: cellW, - titleMaxLines: 2, - onTap: () => ctx.push('/albums/${album.id}', - extra: album), - ); - }, + return GridView.builder( + padding: const EdgeInsets.all(sidePad), + gridDelegate: + SliverGridDelegateWithFixedCrossAxisCount( + crossAxisCount: cols, + mainAxisExtent: cellH, + mainAxisSpacing: gap, + crossAxisSpacing: gap, ), + itemCount: albums.length, + itemBuilder: (ctx, i) { + final album = albums[i]; + return AlbumCard( + album: album, + width: cellW, + titleMaxLines: 2, + onTap: () => ctx.push('/albums/${album.id}', + extra: album), + ); + }, ); }), ); diff --git a/flutter_client/lib/playlists/playlists_provider.dart b/flutter_client/lib/playlists/playlists_provider.dart index 9b4e9c64..2c8be2df 100644 --- a/flutter_client/lib/playlists/playlists_provider.dart +++ b/flutter_client/lib/playlists/playlists_provider.dart @@ -1,4 +1,5 @@ import 'dart:async'; +import 'dart:convert'; import 'package:dio/dio.dart'; import 'package:drift/drift.dart' as drift; @@ -362,8 +363,41 @@ class _TrackRefRow { final int durationSec; } +/// Drift-first per #357 pattern. Reads from cached_system_playlists_ +/// status (single-row JSON blob populated by /api/me/system-playlists- +/// status). Home Playlists row reads this every render to choose +/// between real cards and "building / pending / failed" placeholders +/// — drift-first means the row paints with the prior status instantly +/// rather than flickering through SystemPlaylistsStatus.empty() while +/// the REST round-trip resolves. SWR refresh on every visit keeps it +/// current. final systemPlaylistsStatusProvider = - FutureProvider((ref) async { - final dio = await ref.watch(dioProvider.future); - return MeApi(dio).systemPlaylistsStatus(); + StreamProvider((ref) { + final db = ref.watch(appDbProvider); + return cacheFirst( + driftStream: db.select(db.cachedSystemPlaylistsStatus).watch(), + fetchAndPopulate: () async { + final dio = await ref.read(dioProvider.future); + final fresh = await MeApi(dio).systemPlaylistsStatus(); + await db.into(db.cachedSystemPlaylistsStatus).insertOnConflictUpdate( + CachedSystemPlaylistsStatusCompanion.insert( + json: jsonEncode({ + 'in_flight': fresh.inFlight, + 'last_run_at': fresh.lastRunAt, + 'last_error': fresh.lastError, + }), + updatedAt: drift.Value(DateTime.now()), + ), + ); + }, + toResult: (rows) => rows.isEmpty + ? SystemPlaylistsStatus.empty() + : SystemPlaylistsStatus.fromJson( + jsonDecode(rows.first.json) as Map), + isOnline: () async => (await ref + .read(connectivityProvider.future) + .timeout(const Duration(seconds: 3), onTimeout: () => true)), + alwaysRefresh: true, + tag: 'systemPlaylistsStatus', + ); }); diff --git a/flutter_client/test/library/home_screen_test.dart b/flutter_client/test/library/home_screen_test.dart index be869cc6..0212ad81 100644 --- a/flutter_client/test/library/home_screen_test.dart +++ b/flutter_client/test/library/home_screen_test.dart @@ -28,7 +28,7 @@ void main() { (ref) => Stream.value(PlaylistsList.empty()), ), systemPlaylistsStatusProvider.overrideWith( - (ref) async => SystemPlaylistsStatus.empty(), + (ref) => Stream.value(SystemPlaylistsStatus.empty()), ), ], child: MaterialApp(theme: buildThemeData(), home: const HomeScreen()), @@ -49,7 +49,7 @@ void main() { (ref) => Stream.value(PlaylistsList.empty()), ), systemPlaylistsStatusProvider.overrideWith( - (ref) async => SystemPlaylistsStatus.empty(), + (ref) => Stream.value(SystemPlaylistsStatus.empty()), ), ], child: MaterialApp(theme: buildThemeData(), home: const HomeScreen()), @@ -91,7 +91,7 @@ void main() { const PlaylistsList(owned: [forYou], public: [])), ), systemPlaylistsStatusProvider.overrideWith( - (ref) async => SystemPlaylistsStatus.empty(), + (ref) => Stream.value(SystemPlaylistsStatus.empty()), ), ], child: MaterialApp(theme: buildThemeData(), home: const HomeScreen()),