bvandeusen/minstrel
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 38 Wiki Activity

Merge pull request 'v2026.05.19.3 — playback stall resilience + legacy home cleanup' (#56) from dev into main

Browse Source
This commit was merged in pull request #56.
This commit is contained in:
bvandeusen
2026-05-19 15:48:08 -04:00
parent 19de0c2874 5c1a5f5e8b
commit ccbd3b62a0
9 changed files with 180 additions and 193 deletions
Download Patch File Download Diff File Expand all files Collapse all files
flutter_client/lib/cache/db.dart
Vendored
+20 -19
View File
@@ -120,19 +120,6 @@ class SyncMetadata extends Table {
Set<Column> get primaryKey => {id};
}
/// Single-row cache of the /api/home response (#357 follow-up). The
/// home screen is the first thing the user sees on app open; storing
/// the last successful HomeData as JSON lets us yield it immediately
/// before the REST round-trip resolves, eliminating the cold-start
/// blank on slow / remote connections. Schema 3+.
class CachedHomeSnapshot extends Table {
IntColumn get id => integer().withDefault(const Constant(1))();
TextColumn get json => text()();
DateTimeColumn get updatedAt => dateTime().withDefault(currentDateAndTime)();
@override
Set<Column> get primaryKey => {id};
}
/// Single-row cache of the /api/me/history response. Mirrors the
/// CachedHomeSnapshot pattern: opening the History tab in the Library
/// screen yields the last-known page immediately while a fresh REST
@@ -255,7 +242,6 @@ enum CacheSource { manual, autoLiked, autoPlaylist, autoPrefetch, incidental }
CachedPlaylistTracks,
AudioCacheIndex,
SyncMetadata,
CachedHomeSnapshot,
CachedHistorySnapshot,
CachedQuarantineMine,
CachedHomeIndex,
@@ -267,7 +253,7 @@ class AppDb extends _$AppDb {
AppDb([QueryExecutor? e]) : super(e ?? driftDatabase(name: 'minstrel_cache'));
@override
int get schemaVersion => 10;
int get schemaVersion => 11;
@override
MigrationStrategy get migration => MigrationStrategy(
@@ -284,10 +270,16 @@ class AppDb extends _$AppDb {
await customStatement('UPDATE sync_metadata SET cursor = 0');
}
if (from < 3) {
// Schema 3: cached_home_snapshot for #357 drift-first
// home page. No existing rows to migrate — table starts
// empty; the first /api/home fetch populates it.
await m.createTable(cachedHomeSnapshot);
// Schema 3: cached_home_snapshot (legacy drift-first home).
// The table + its homeProvider/JSON encoders were removed
// in #406; recreated here as raw SQL so this historical
// step still compiles without the generated table symbol.
// The from<11 step below drops it.
await customStatement(
'CREATE TABLE IF NOT EXISTS "cached_home_snapshot" '
'("id" INTEGER NOT NULL DEFAULT 1, "json" TEXT, '
'"updated_at" INTEGER, PRIMARY KEY ("id"));',
);
}
if (from < 4) {
// Schema 4: cached_history_snapshot for drift-first
@@ -337,6 +329,15 @@ class AppDb extends _$AppDb {
// first persist populates it.
await m.createTable(cachedResumeState);
}
if (from < 11) {
// Schema 11 (#406): drop the legacy cached_home_snapshot
// table. The home screen now renders solely from the
// per-item cached_home_index path; the legacy homeProvider
// + its JSON encoders were removed.
await customStatement(
'DROP TABLE IF EXISTS "cached_home_snapshot";',
);
}
},
);
}
flutter_client/lib/cache/metadata_prefetcher.dart
Vendored
+15 -22
View File
@@ -1,7 +1,7 @@
import 'package:flutter_riverpod/flutter_riverpod.dart';
import '../library/library_providers.dart';
import '../models/home_data.dart';
import '../models/home_index.dart';
/// Pre-warms the drift cache for likely-tap targets. Conservative on
/// purpose: only warms artistProvider rows (single row, single round
@@ -10,10 +10,15 @@ import '../models/home_data.dart';
/// 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.
///
/// Driven off the per-item home path (homeIndexProvider). Only the
/// artist-typed sections (rediscover / last-played artists) carry
/// artist ids directly; album/track tiles hydrate their own artist on
/// render via the per-item path, so warming those here is unnecessary.
class MetadataPrefetcher {
MetadataPrefetcher(this._ref) {
_ref.listen<AsyncValue<HomeData>>(homeProvider, (_, next) {
next.whenData(_warmHome);
_ref.listen<AsyncValue<HomeIndex>>(homeIndexProvider, (_, next) {
next.whenData(_warmIndex);
});
}
@@ -24,7 +29,8 @@ class MetadataPrefetcher {
/// ids we've already warmed.
final Set<String> _warmedArtists = {};
/// Cap per section. Covers what fits on screen without scrolling.
/// Cap warmed per emission. Covers what fits on screen without
/// scrolling.
static const _topN = 8;
/// Pre-warm artists. Albums are intentionally not pre-warmed —
@@ -39,24 +45,11 @@ class MetadataPrefetcher {
}
}
void _warmHome(HomeData h) {
final artistIds = <String>{};
for (final a in h.recentlyAddedAlbums.take(_topN)) {
if (a.artistId.isNotEmpty) artistIds.add(a.artistId);
}
for (final a in h.rediscoverAlbums.take(_topN)) {
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.artistId.isNotEmpty) artistIds.add(t.artistId);
}
warmArtists(artistIds);
void _warmIndex(HomeIndex h) {
warmArtists(<String>{
...h.rediscoverArtists.take(_topN),
...h.lastPlayedArtists.take(_topN),
});
}
/// Discards the return value and any error from a fire-and-forget
flutter_client/lib/library/library_providers.dart
-96
View File
@@ -1,5 +1,4 @@
import 'dart:async';
import 'dart:convert';
import 'package:dio/dio.dart';
import 'package:drift/drift.dart' as drift;
@@ -16,7 +15,6 @@ import '../cache/connectivity_provider.dart';
import '../cache/db.dart';
import '../models/album.dart';
import '../models/artist.dart';
import '../models/home_data.dart';
import '../models/home_index.dart';
import '../models/track.dart';
@@ -44,57 +42,6 @@ final libraryApiProvider = FutureProvider<LibraryApi>((ref) async {
return LibraryApi(await ref.watch(dioProvider.future));
});
/// Drift-first home data (#357 follow-up). The home screen is the first
/// view the user sees on app open; without a local cache the cold-start
/// blocks on the /api/home round-trip, which is the dominant felt-
/// latency on slow / remote connections.
///
/// Cache layout: a single row in `cached_home_snapshot` storing the
/// last successful HomeData as JSON. On stream subscription:
///
/// - If a row exists, yield it immediately (parsed from JSON) and
/// kick off a background revalidation against /api/home (SWR).
/// The drift watch() re-emits when the new row is written.
/// - If no row exists yet (cold cache), fetch /api/home synchronously,
/// upsert into drift, and emit. Subsequent emissions come from the
/// drift watch as background revalidations land.
///
/// The HomeData JSON encodes back to the same wire shape /api/home
/// emits, so the existing HomeData.fromJson handles both sources.
final homeProvider = StreamProvider<HomeData>((ref) {
final db = ref.watch(appDbProvider);
return cacheFirst<CachedHomeSnapshotData, HomeData>(
driftStream: db.select(db.cachedHomeSnapshot).watch(),
fetchAndPopulate: () async {
final api = await ref.read(libraryApiProvider.future);
final fresh = await api.getHome();
await db.into(db.cachedHomeSnapshot).insertOnConflictUpdate(
CachedHomeSnapshotCompanion.insert(
json: _encodeHomeData(fresh),
updatedAt: drift.Value(DateTime.now()),
),
);
},
toResult: (rows) => rows.isEmpty
? const HomeData(
recentlyAddedAlbums: [],
rediscoverAlbums: [],
rediscoverArtists: [],
mostPlayedTracks: [],
lastPlayedArtists: [],
)
: HomeData.fromJson(jsonDecode(rows.first.json) as Map<String, dynamic>),
isOnline: () async => (await ref
.read(connectivityProvider.future)
.timeout(const Duration(seconds: 3), onTimeout: () => true)),
// SWR: when cached snapshot exists, still hit /api/home in the
// background so the user sees the freshest curation. The cache
// mostly serves the first frame, not the canonical state.
alwaysRefresh: true,
tag: 'home',
);
});
/// Drift-first per-item home index. Reads from cached_home_index
/// (populated by /api/home/index discovery) and yields a HomeIndex
/// the screen consumes to lay out section→tile slots. Each tile is
@@ -184,49 +131,6 @@ final homeIndexProvider = StreamProvider<HomeIndex>((ref) {
);
});
/// Encodes HomeData back to the wire-format JSON shape /api/home
/// emits, so HomeData.fromJson can round-trip through the drift cache.
String _encodeHomeData(HomeData h) => jsonEncode({
'recently_added_albums': h.recentlyAddedAlbums.map(_albumToJson).toList(),
'rediscover_albums': h.rediscoverAlbums.map(_albumToJson).toList(),
'rediscover_artists': h.rediscoverArtists.map(_artistToJson).toList(),
'most_played_tracks': h.mostPlayedTracks.map(_trackToJson).toList(),
'last_played_artists': h.lastPlayedArtists.map(_artistToJson).toList(),
});
Map<String, dynamic> _albumToJson(AlbumRef a) => {
'id': a.id,
'title': a.title,
'sort_title': a.sortTitle,
'artist_id': a.artistId,
'artist_name': a.artistName,
'year': a.year,
'track_count': a.trackCount,
'duration_sec': a.durationSec,
'cover_url': a.coverUrl,
};
Map<String, dynamic> _artistToJson(ArtistRef a) => {
'id': a.id,
'name': a.name,
'sort_name': a.sortName,
'album_count': a.albumCount,
'cover_url': a.coverUrl,
};
Map<String, dynamic> _trackToJson(TrackRef t) => {
'id': t.id,
'title': t.title,
'album_id': t.albumId,
'album_title': t.albumTitle,
'artist_id': t.artistId,
'artist_name': t.artistName,
'track_number': t.trackNumber,
'disc_number': t.discNumber,
'duration_sec': t.durationSec,
'stream_url': t.streamUrl,
};
/// Drift-first per #357 plan C. Watches cached_artists for the row;
/// when empty + online, fetches via REST + populates drift, which
/// re-emits via watch().
flutter_client/lib/player/audio_handler.dart
+116 -26
View File
@@ -76,6 +76,24 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl
/// need this. Active only while playing; cancelled otherwise.
Timer? _positionBroadcastTimer;
/// Buffering-stall watchdog. On poor coverage a stream hangs in
/// ProcessingState.buffering with NO error event, so the onError
/// recovery never fires. Armed while playing+buffering; on fire, if
/// buffered position hasn't advanced it's a dead stream → recover.
/// Re-arms a fresh window while buffering-but-progressing (slow-but-
/// downloading is not a stall).
static const _stallTimeout = Duration(seconds: 15);
Timer? _stallTimer;
Duration _stallMarkBuffered = Duration.zero;
/// Per-track recovery budget. Reset when a track reaches
/// ready+playing. One retry of the same source (rebuilt fresh), then
/// skip to the next cached track (or pause) instead of thrashing
/// through unreachable streams.
static const _maxRecoverRetries = 1;
String? _recoveringTrackId;
int _recoverAttempts = 0;
/// Player volume captured when an OS duck interruption begins,
/// restored when it ends. Null when not currently ducked.
double? _volumeBeforeDuck;
@@ -492,33 +510,12 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl
///
/// If we're at the last track, seekToNext is a no-op; the state
/// drops to idle and _broadcastState reflects that.
/// onError entrypoint (404 / decoder / premature EOS / network drop).
/// Routes into the unified recovery path (retry the track once, then
/// skip to the next cached track or pause) instead of immediately
/// skipping the literal next — possibly also-unreachable — source.
Future<void> _handlePlaybackError() async {
// mediaItem is the correctly-mapped current track (post #49 logical
// index), so it's the one that just failed — no player-index math.
final failed = mediaItem.value?.title;
if (failed != null && failed.isNotEmpty) {
_playbackErrors.add(failed);
}
final currentIdx = _player.currentIndex;
final queueLen = queue.value.length;
if (currentIdx == null || currentIdx + 1 >= queueLen) {
// Last track or no queue context — just pause; _broadcastState
// already reflects the idle/error processingState.
try {
await _player.pause();
} catch (_) {}
return;
}
try {
await _player.seekToNext();
} catch (_) {
// If seekToNext fails too (next source not built yet, etc.),
// there's nothing safe to do — pause and let the user recover
// manually.
try {
await _player.pause();
} catch (_) {}
}
await _recoverPlayback();
}
void _onCurrentIndexChanged(int? idx) {
@@ -573,6 +570,8 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl
_idleStopTimer = null;
_positionBroadcastTimer?.cancel();
_positionBroadcastTimer = null;
_stallTimer?.cancel();
_stallTimer = null;
try {
await _player.stop();
} catch (_) {}
@@ -786,6 +785,7 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl
));
_reconcileIdleTimer();
_reconcilePositionBroadcast();
_reconcileStallWatchdog();
}
/// Arms (or cancels) the idle-cleanup timer based on the current
@@ -842,6 +842,96 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl
);
}
/// Buffering-stall watchdog. Armed while playing+buffering/loading.
/// On fire: still stuck with no >=1s buffered progress → dead stream,
/// recover; progressing → re-arm a fresh window (slow-but-downloading
/// is fine). When the track is happily ready+playing, clears the
/// per-track recovery budget. Driven from _broadcastState; mirrors
/// the other reconcile helpers.
void _reconcileStallWatchdog() {
final ps = _player.processingState;
if (ps == ProcessingState.ready && _player.playing) {
_recoveringTrackId = null;
_recoverAttempts = 0;
}
final buffering = _player.playing &&
(ps == ProcessingState.buffering || ps == ProcessingState.loading);
if (!buffering) {
_stallTimer?.cancel();
_stallTimer = null;
return;
}
if (_stallTimer != null) return; // window already running
_stallMarkBuffered = _player.bufferedPosition;
_stallTimer = Timer(_stallTimeout, () {
_stallTimer = null;
final ps2 = _player.processingState;
final stillBuffering = _player.playing &&
(ps2 == ProcessingState.buffering ||
ps2 == ProcessingState.loading);
if (!stillBuffering) return;
final progressed = (_player.bufferedPosition - _stallMarkBuffered) >=
const Duration(seconds: 1);
if (progressed) {
_reconcileStallWatchdog(); // slow but downloading — re-arm
} else {
unawaited(_recoverPlayback());
}
});
}
/// Unified recovery for a failed OR stalled stream. Retries the
/// current track once (rebuilt from scratch via skipToQueueItem so a
/// transient blip doesn't lose it); on exhaustion, surfaces the
/// failure (#58 SnackBar) and skips to the next cached track — or
/// pauses — instead of blindly walking into more unreachable streams.
Future<void> _recoverPlayback() async {
_stallTimer?.cancel();
_stallTimer = null;
final media = mediaItem.value;
if (media == null) return;
final trackId = media.id;
final curIdx = _lastTracks.indexWhere((t) => t.id == trackId);
if (curIdx < 0) {
try {
await _player.pause();
} catch (_) {}
return;
}
if (_recoveringTrackId != trackId) {
_recoveringTrackId = trackId;
_recoverAttempts = 0;
}
if (_recoverAttempts < _maxRecoverRetries) {
_recoverAttempts++;
// Rebuild + restart the SAME track (fresh source / HTTP).
await skipToQueueItem(curIdx);
return;
}
if (media.title.isNotEmpty) _playbackErrors.add(media.title);
_recoveringTrackId = null;
_recoverAttempts = 0;
await _skipToNextCachedOrPause(curIdx);
}
/// Skips to the first track after [fromIdx] that is fully cached on
/// disk (plays with zero network). If none, pauses rather than
/// thrashing through unreachable streams on a dead connection.
Future<void> _skipToNextCachedOrPause(int fromIdx) async {
final mgr = _audioCacheManager;
if (mgr != null) {
for (var i = fromIdx + 1; i < _lastTracks.length; i++) {
if (await mgr.pathFor(_lastTracks[i].id) != null) {
await skipToQueueItem(i);
return;
}
}
}
try {
await _player.pause();
} catch (_) {}
}
/// Configures the OS audio session for music playback and wires
/// interruption + becoming-noisy handling. just_audio auto-activates /
/// deactivates the session around playback once it's configured, but
flutter_client/lib/shared/live_events_dispatcher.dart
+5 -5
View File
@@ -20,7 +20,7 @@
import 'package:flutter/widgets.dart';
import 'package:flutter_riverpod/flutter_riverpod.dart';
import '../library/library_providers.dart' show homeProvider;
import '../library/library_providers.dart' show homeIndexProvider;
import '../quarantine/quarantine_provider.dart';
import 'live_events_provider.dart';
@@ -62,8 +62,8 @@ class _LiveEventsDispatcher with WidgetsBindingObserver {
case 'quarantine.deleted_via_lidarr':
_ref.invalidate(myQuarantineProvider);
// Hidden / liked / album rows referencing a deleted track may
// now be stale — homeProvider re-fetch refreshes the cards.
_ref.invalidate(homeProvider);
// now be stale — homeIndexProvider re-fetch refreshes the cards.
_ref.invalidate(homeIndexProvider);
case 'playlist.created':
case 'playlist.updated':
case 'playlist.deleted':
@@ -72,7 +72,7 @@ class _LiveEventsDispatcher with WidgetsBindingObserver {
// is reflected immediately. Detail screens that need the
// mutated row will get a separate invalidate from their own
// listener (filed as a follow-up).
_ref.invalidate(homeProvider);
_ref.invalidate(homeIndexProvider);
case 'scan.run_started':
case 'scan.run_finished':
// Admin scan card provider lives in the admin feature folder
@@ -93,7 +93,7 @@ class _LiveEventsDispatcher with WidgetsBindingObserver {
// own, but the re-invalidate flushes any stale data that
// landed while the app was backgrounded.
_ref.invalidate(myQuarantineProvider);
_ref.invalidate(homeProvider);
_ref.invalidate(homeIndexProvider);
}
}
flutter_client/pubspec.yaml
+1 -1
View File
@@ -1,7 +1,7 @@
name: minstrel
description: Minstrel mobile client
publish_to: 'none'
version: 2026.5.19+12
version: 2026.5.19+13
environment:
sdk: '>=3.5.0 <4.0.0'
web/src/routes/discover/discover.test.ts
+11 -7
View File
@@ -3,6 +3,16 @@ import { render, screen, fireEvent, waitFor } from '@testing-library/svelte';
import { mockQuery } from '../../test-utils/query';
import { apiClientMock } from '../../test-utils/mocks/client';
import type { LidarrSearchResult } from '$lib/api/types';
import { pageUrlModule } from '../../test-utils/mocks/appState';
// $app/state / $app/navigation must be mocked or rendering the page
// pulls in SvelteKit's client runtime (`notifiable_store is not a
// function`) at module load. Same pattern as the other route tests.
const pageState = vi.hoisted(() => ({
pageUrl: new URL('http://localhost/discover')
}));
vi.mock('$app/state', () => pageUrlModule(pageState));
vi.mock('$app/navigation', () => ({ goto: vi.fn() }));
// Lidarr search query factory and createRequest are mocked at the module
// level so each test can shape what the page sees without standing up a
@@ -56,13 +66,7 @@ afterEach(() => {
vi.clearAllMocks();
});
// FIXME(M7 #374): suite errors at module-load with
// `TypeError: notifiable_store is not a function` from SvelteKit's
// internal client.js. Surfaces only on the new dev-push CI; PR-to-main
// runs were green. Probably a vitest module-resolution interaction
// with a recent SvelteKit; needs targeted triage. Skipped to unblock
// CI for #372 — the page itself isn't broken, the test harness is.
describe.skip('Discover page', () => {
describe('Discover page', () => {
test('initial state (no query) shows the suggestion feed', () => {
render(DiscoverPage);
expect(screen.getByText(/suggested for you/i)).toBeInTheDocument();
web/src/routes/requests/requests.test.ts
+11 -4
View File
@@ -3,6 +3,16 @@ import { render, screen, fireEvent, waitFor } from '@testing-library/svelte';
import { mockQuery } from '../../test-utils/query';
import { apiClientMock } from '../../test-utils/mocks/client';
import type { LidarrRequest } from '$lib/api/types';
import { pageUrlModule } from '../../test-utils/mocks/appState';
// $app/state / $app/navigation must be mocked or rendering the page
// pulls in SvelteKit's client runtime (`notifiable_store is not a
// function`) at module load. Same pattern as the other route tests.
const pageState = vi.hoisted(() => ({
pageUrl: new URL('http://localhost/requests')
}));
vi.mock('$app/state', () => pageUrlModule(pageState));
vi.mock('$app/navigation', () => ({ goto: vi.fn() }));
// Capture invalidateQueries on the mocked QueryClient so the cancel test can
// assert against the same instance the page consumed. Per-file mock OVERRIDES
@@ -66,10 +76,7 @@ afterEach(() => {
vi.clearAllMocks();
});
// FIXME(M7 #374): same SvelteKit `notifiable_store is not a function`
// failure as discover.test.ts. Skipped to unblock CI for #372 — needs
// triage as a separate task.
describe.skip('Requests page', () => {
describe('Requests page', () => {
test('renders one row per request from the API', () => {
const rows: LidarrRequest[] = [
req({ id: 'r1', kind: 'artist', artist_name: 'Boards of Canada' }),
web/vitest.config.ts
+1 -13
View File
@@ -7,19 +7,7 @@ export default defineConfig({
test: {
environment: 'jsdom',
include: ['src/**/*.test.ts', 'src/**/*.svelte.test.ts', 'scripts/**/*.test.js'],
// M7 #374: these two test files explode at module-load with
// `TypeError: notifiable_store is not a function` from SvelteKit's
// internal client.js — the import chain triggers SvelteKit's client
// runtime init in a way the dev-push vitest invocation can't satisfy.
// describe.skip inside the files doesn't help because vitest must
// load the file before it can see the describe block. Excluded here
// until the triage in #374 lands a real fix (probably a $app/paths
// mock in vitest.setup.ts).
exclude: [
...configDefaults.exclude,
'src/routes/discover/discover.test.ts',
'src/routes/requests/requests.test.ts'
],
exclude: [...configDefaults.exclude],
setupFiles: ['./vitest.setup.ts']
}
});