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. } });