diff --git a/.forgejo/workflows/flutter.yml b/.forgejo/workflows/flutter.yml index b677f62b..5f375e69 100644 --- a/.forgejo/workflows/flutter.yml +++ b/.forgejo/workflows/flutter.yml @@ -1,9 +1,13 @@ name: flutter -# Analyze + test + build the Flutter mobile client. Only runs when the -# diff touches flutter_client/ or one of the shared web inputs the -# sync_shared.sh script copies. Other pushes skip — this workflow is -# independent from the Go/web test.yml and release.yml. +# Analyze + test the Flutter mobile client; build an APK only where +# it's actually consumed. dev pushes: analyze+test+codegen only +# (~3min) — the operator dev-tests via a local Android Studio build, +# not the CI artifact. main pushes: also build a debug APK as the +# native-build safety net (Gradle/manifest/plugin breakage that +# analyze+test can't catch) before any release tag. tags: signed +# release APK. Only runs when the diff touches flutter_client/ or a +# shared web input sync_shared.sh copies. on: push: @@ -69,7 +73,9 @@ jobs: run: flutter test --reporter compact - name: Build debug APK - if: github.event_name == 'push' && !startsWith(github.ref, 'refs/tags/') + # main-only: native-build safety net before release tags. + # dev skips this (~7min) — operator dev-tests locally. + if: github.event_name == 'push' && github.ref == 'refs/heads/main' run: flutter build apk --debug - name: Decode signing keystore @@ -112,7 +118,7 @@ jobs: flutter build apk --release --build-name="${TAG}" - name: Upload debug APK artifact - if: github.event_name == 'push' && !startsWith(github.ref, 'refs/tags/') + if: github.event_name == 'push' && github.ref == 'refs/heads/main' uses: forgejo/upload-artifact@v3 with: name: minstrel-debug-${{ github.sha }} diff --git a/flutter_client/lib/api/endpoints/events.dart b/flutter_client/lib/api/endpoints/events.dart new file mode 100644 index 00000000..c5ab0fc0 --- /dev/null +++ b/flutter_client/lib/api/endpoints/events.dart @@ -0,0 +1,91 @@ +import 'package:dio/dio.dart'; + +/// Thin client for POST /api/events — the play-event lifecycle the +/// server uses for history, recommendation scoring, ListenBrainz +/// scrobbles, and (since #415) system-playlist rotation. +/// +/// Mirrors the web events dispatcher's three calls. Best-effort by +/// contract: callers swallow errors — a missed event is acceptable +/// per the server spec's v1 stance, and the server's +/// auto-close-prior-open keeps history sane even if an ended/skipped +/// is lost. +class EventsApi { + EventsApi(this._dio); + final Dio _dio; + + /// POST play_started. Returns the server's play_event_id (used to + /// close the row later), or null if the call failed / response was + /// malformed. `source` tags the originating system playlist + /// ('for_you' | 'discover') so the server advances that rotation; + /// omit for library / user-playlist / radio plays. + Future playStarted({ + required String trackId, + required String clientId, + String? source, + }) async { + final r = await _dio.post>( + '/api/events', + data: { + 'type': 'play_started', + 'track_id': trackId, + 'client_id': clientId, + if (source != null && source.isNotEmpty) 'source': source, + }, + ); + return (r.data ?? const {})['play_event_id'] as String?; + } + + Future playEnded({ + required String playEventId, + required int durationPlayedMs, + }) async { + await _dio.post( + '/api/events', + data: { + 'type': 'play_ended', + 'play_event_id': playEventId, + 'duration_played_ms': durationPlayedMs, + }, + ); + } + + Future playSkipped({ + required String playEventId, + required int positionMs, + }) async { + await _dio.post( + '/api/events', + data: { + 'type': 'play_skipped', + 'play_event_id': playEventId, + 'position_ms': positionMs, + }, + ); + } + + /// Replays a complete play that happened offline / on a flaky + /// connection (#426 part B). One call: the server records start+end + /// from `atIso` (the original play-start time) + durationPlayedMs, + /// applies the canonical skip rule, and advances #415 rotation when + /// source is a system playlist. Driven by the offline mutation + /// queue, never the live path. + Future playOffline({ + required String trackId, + required String clientId, + required String atIso, + required int durationPlayedMs, + String? source, + }) async { + await _dio.post( + '/api/events', + data: { + 'type': 'play_offline', + 'track_id': trackId, + 'client_id': clientId, + 'at': atIso, + 'duration_played_ms': durationPlayedMs, + if (source != null && source.isNotEmpty) 'source': source, + }, + ); + } +} diff --git a/flutter_client/lib/api/endpoints/library.dart b/flutter_client/lib/api/endpoints/library.dart index e5cfff71..53895b7f 100644 --- a/flutter_client/lib/api/endpoints/library.dart +++ b/flutter_client/lib/api/endpoints/library.dart @@ -85,6 +85,19 @@ class LibraryApi { .toList(growable: false); } + /// GET /api/library/shuffle?limit=N (#427 S4). N random library + /// tracks — the online source for "Shuffle all". Bare JSON array. + Future> shuffle({int limit = 100}) async { + final r = await _dio.get>( + '/api/library/shuffle', + queryParameters: {'limit': limit}, + ); + final raw = r.data ?? const []; + return raw + .map((e) => TrackRef.fromJson((e as Map).cast())) + .toList(growable: false); + } + /// GET /api/artists/{id}/tracks. Server emits a bare JSON array, so we /// type the response as `List` rather than a Map envelope. Future> getArtistTracks(String id) async { diff --git a/flutter_client/lib/api/endpoints/playlists.dart b/flutter_client/lib/api/endpoints/playlists.dart index 9d772dd8..3455aaed 100644 --- a/flutter_client/lib/api/endpoints/playlists.dart +++ b/flutter_client/lib/api/endpoints/playlists.dart @@ -45,6 +45,17 @@ class PlaylistsApi { return PlaylistDetail.fromJson(r.data ?? const {}); } + /// GET /api/playlists/system/{kind}/shuffle (#415 / #411 R2). + /// Same shape as get() but tracks are server-ordered rotation-aware + /// (unplayed-this-rotation first; resets when exhausted). {kind} is + /// the raw system_variant. Intentionally uncached — varies per play. + Future systemShuffle(String variant) async { + final r = await _dio.get>( + '/api/playlists/system/$variant/shuffle', + ); + return PlaylistDetail.fromJson(r.data ?? const {}); + } + /// POST /api/playlists/{id}/tracks. Owner only; server returns the /// playlist detail with the new rows. Future appendTracks(String playlistId, List trackIds) async { @@ -53,4 +64,17 @@ class PlaylistsApi { data: {'track_ids': trackIds}, ); } + + /// POST /api/playlists/system/{kind}/refresh (#411 R2). Rebuilds + /// the caller's system playlists and returns the named kind's new + /// playlist id, or null when the library is empty. {kind} is the + /// raw system_variant — the server routes generically off the + /// kind registry, no hyphen mapping. + Future refreshSystem(String variant) async { + final r = await _dio.post>( + '/api/playlists/system/$variant/refresh', + data: const {}, + ); + return (r.data ?? const {})['playlist_id'] as String?; + } } diff --git a/flutter_client/lib/app.dart b/flutter_client/lib/app.dart index 0c39d806..93a6684c 100644 --- a/flutter_client/lib/app.dart +++ b/flutter_client/lib/app.dart @@ -3,9 +3,11 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'cache/cache_filler.dart'; import 'cache/metadata_prefetcher.dart'; +import 'cache/offline_provider.dart'; import 'cache/mutation_queue.dart'; import 'cache/prefetcher.dart'; import 'cache/sync_controller.dart'; +import 'player/play_events_reporter.dart'; import 'shared/live_events_dispatcher.dart'; import 'shared/routing.dart'; import 'theme/theme_data.dart'; @@ -55,6 +57,15 @@ class _MinstrelAppState extends ConsumerState { // enqueue on REST failure so user intent persists across // network loss instead of getting rolled back. ref.read(mutationReplayerProvider); + // Play-events reporter (#415): the Flutter client otherwise + // reports no plays at all — this feeds history, recommendation + // scoring, scrobbles, and system-playlist rotation, and is the + // path that carries the `source` tag for #415. + ref.read(playEventsReporterProvider); + // Offline marker (#427 S1): periodic /healthz reachability + // probe → offlineProvider. Read here to start the poller; S4 + // gates system-playlist play + Shuffle-all on it. + ref.read(offlineProvider); }); } diff --git a/flutter_client/lib/cache/audio_cache_manager.dart b/flutter_client/lib/cache/audio_cache_manager.dart index 5c4afecb..a26aa665 100644 --- a/flutter_client/lib/cache/audio_cache_manager.dart +++ b/flutter_client/lib/cache/audio_cache_manager.dart @@ -8,15 +8,20 @@ import 'package:path_provider/path_provider.dart'; import '../library/library_providers.dart' show dioProvider; import 'db.dart'; +/// Per-bucket cache usage (#427 S2). Rolling includes orphan files — +/// partials written by LockCachingAudioSource that were never indexed +/// (skip-before-fully-buffered) — so the rolling cap actually bounds +/// disk. +typedef BucketUsage = ({int liked, int rolling}); + /// Owns the audio cache directory + drift index. -/// API: -/// - isCached(trackId) -/// - pathFor(trackId) -/// - pin(trackId, source) — downloads + indexes -/// - unpin(trackId) -/// - evict(targetBytes) — tiered LRU -/// - usageBytes() -/// - clearAll() +/// +/// #427 S2: two storage buckets keyed by liked-ness, not by +/// CacheSource. A cached track currently in the user's liked set is +/// charged to (and evicted under) the Liked budget; everything else +/// is Rolling. This dedup is storage/eviction-only — it never filters +/// what the offline surfaces can play (S4). `CacheSource` is retained +/// on the API for callers but is now informational. class AudioCacheManager { AudioCacheManager({ required AppDb db, @@ -37,7 +42,6 @@ class AudioCacheManager { return d.path; } - /// True if the trackId has a complete file on disk. Future isCached(String trackId) async { final row = await (_db.select(_db.audioCacheIndex) ..where((t) => t.trackId.equals(trackId))) @@ -46,7 +50,6 @@ class AudioCacheManager { return File(row.path).existsSync(); } - /// Returns the local file path if cached, else null. Future pathFor(String trackId) async { final row = await (_db.select(_db.audioCacheIndex) ..where((t) => t.trackId.equals(trackId))) @@ -55,12 +58,14 @@ class AudioCacheManager { return File(row.path).existsSync() ? row.path : null; } - /// Downloads the track's stream to disk and indexes it. Idempotent — - /// returns the existing path if already cached. + /// Downloads the track's stream to disk and indexes it. Idempotent. + /// lastPlayedAt is set now — pinning is play-intent. Future pin(String trackId, {required CacheSource source}) async { final existing = await pathFor(trackId); - if (existing != null) return existing; - + if (existing != null) { + await touch(trackId); + return existing; + } final dir = await _tracksDir(); final path = '$dir/$trackId.mp3'; final dio = await _dioFactory(); @@ -78,16 +83,15 @@ class AudioCacheManager { path: path, sizeBytes: size, source: source, + lastPlayedAt: Value(DateTime.now()), ), ); 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. + /// Registers a file LockCachingAudioSource wrote itself. Called + /// once a track is fully buffered; it's being played, so stamp + /// lastPlayedAt. Future registerStreamCache( String trackId, String path, @@ -100,11 +104,20 @@ class AudioCacheManager { path: path, sizeBytes: sizeBytes, source: source, + lastPlayedAt: Value(DateTime.now()), ), ); } - /// Removes a track from the index AND deletes the file. + /// Bumps lastPlayedAt for an already-indexed track so the rolling + /// LRU + the offline "Recently played" view reflect real plays + /// (not just download time). No-op if not indexed. + Future touch(String trackId) async { + await (_db.update(_db.audioCacheIndex) + ..where((t) => t.trackId.equals(trackId))) + .write(AudioCacheIndexCompanion(lastPlayedAt: Value(DateTime.now()))); + } + Future unpin(String trackId) async { final row = await (_db.select(_db.audioCacheIndex) ..where((t) => t.trackId.equals(trackId))) @@ -117,66 +130,150 @@ class AudioCacheManager { .go(); } - /// 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. + /// Total bytes on disk (directory walk — authoritative; catches + /// orphan partials the index misses). Future usageBytes() async { 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) { + await for (final e in dir.list(followLinks: false)) { + if (e is File) { try { - total += await entity.length(); - } catch (_) { - // Race against concurrent writes / deletes — just skip. - } + total += await e.length(); + } catch (_) {} } } return total; } - /// Evicts files until usage ≤ targetBytes. Eviction order: - /// incidental → autoPrefetch → autoPlaylist → autoLiked. - /// `manual` never evicts via this path; only clearAll() removes them. - Future evict({required int targetBytes}) async { - final used = await usageBytes(); - if (used <= targetBytes) return; - final order = [ - CacheSource.incidental, - CacheSource.autoPrefetch, - CacheSource.autoPlaylist, - CacheSource.autoLiked, - ]; - int remaining = used - targetBytes; - for (final src in order) { - if (remaining <= 0) break; - final rows = await (_db.select(_db.audioCacheIndex) - ..where((t) => t.source.equalsValue(src)) - ..orderBy([(t) => OrderingTerm.asc(t.cachedAt)])) + /// Bytes per bucket. Indexed rows split by liked-ness; on-disk + /// files with no index row (orphan partials) count as Rolling so + /// the rolling cap genuinely bounds disk. + Future bucketUsage(Set liked) async { + final rows = await _db.select(_db.audioCacheIndex).get(); + final indexed = {}; + var likedB = 0; + var rollingB = 0; + for (final r in rows) { + indexed.add(r.trackId); + if (liked.contains(r.trackId)) { + likedB += r.sizeBytes; + } else { + rollingB += r.sizeBytes; + } + } + final dir = Directory(await _tracksDir()); + if (await dir.exists()) { + await for (final e in dir.list(followLinks: false)) { + if (e is! File) continue; + final name = e.uri.pathSegments.last; + final id = name.endsWith('.mp3') + ? name.substring(0, name.length - 4) + : name; + if (indexed.contains(id)) continue; + try { + rollingB += await e.length(); + } catch (_) {} + } + } + return (liked: likedB, rolling: rollingB); + } + + /// Enforces both budgets independently (0 = unlimited). + /// + /// Rolling: evict non-liked indexed rows LRU (oldest lastPlayedAt + /// /cachedAt first), then sweep orphan files oldest-by-mtime, until + /// rolling usage ≤ rollingCap. Liked: evict liked indexed rows LRU + /// until ≤ likedCap. Liked is only ever touched by its own (large) + /// cap, so normal use never evicts the user's liked library. + Future evictBuckets({ + required int likedCap, + required int rollingCap, + required Set liked, + }) async { + final usage = await bucketUsage(liked); + + if (rollingCap > 0 && usage.rolling > rollingCap) { + var over = usage.rolling - rollingCap; + final rolling = await (_db.select(_db.audioCacheIndex) + ..where((t) => t.trackId.isNotIn(liked.toList())) + ..orderBy([ + (t) => OrderingTerm.asc(t.lastPlayedAt), + (t) => OrderingTerm.asc(t.cachedAt), + ])) .get(); - for (final row in rows) { - if (remaining <= 0) break; - final f = File(row.path); + for (final r in rolling) { + if (over <= 0) break; + final f = File(r.path); if (f.existsSync()) await f.delete(); await (_db.delete(_db.audioCacheIndex) - ..where((t) => t.trackId.equals(row.trackId))) + ..where((t) => t.trackId.equals(r.trackId))) .go(); - remaining -= row.sizeBytes; + over -= r.sizeBytes; + } + if (over > 0) await _sweepOrphans(over); + } + + if (likedCap > 0 && usage.liked > likedCap) { + var over = usage.liked - likedCap; + final likedRows = await (_db.select(_db.audioCacheIndex) + ..where((t) => t.trackId.isIn(liked.toList())) + ..orderBy([ + (t) => OrderingTerm.asc(t.lastPlayedAt), + (t) => OrderingTerm.asc(t.cachedAt), + ])) + .get(); + for (final r in likedRows) { + if (over <= 0) break; + final f = File(r.path); + if (f.existsSync()) await f.delete(); + await (_db.delete(_db.audioCacheIndex) + ..where((t) => t.trackId.equals(r.trackId))) + .go(); + over -= r.sizeBytes; } } } - /// Clears EVERY row + EVERY file. Wired to the operator's "Clear cache" - /// button — `manual` source rows are removed here but only here. + /// Deletes orphan files (on disk, no index row) oldest-mtime-first + /// until `over` bytes are reclaimed. These are unindexed partials, + /// always Rolling, always evict-first. + Future _sweepOrphans(int over) async { + final indexed = { + for (final r in await _db.select(_db.audioCacheIndex).get()) r.trackId + }; + final dir = Directory(await _tracksDir()); + if (!await dir.exists()) return; + final orphans = <({File f, int size, DateTime mtime})>[]; + await for (final e in dir.list(followLinks: false)) { + if (e is! File) continue; + final name = e.uri.pathSegments.last; + final id = + name.endsWith('.mp3') ? name.substring(0, name.length - 4) : name; + if (indexed.contains(id)) continue; + try { + final st = e.statSync(); + orphans.add((f: e, size: st.size, mtime: st.modified)); + } catch (_) {} + } + orphans.sort((a, b) => a.mtime.compareTo(b.mtime)); + var remaining = over; + for (final o in orphans) { + if (remaining <= 0) break; + try { + await o.f.delete(); + remaining -= o.size; + } catch (_) {} + } + } + + /// Clears EVERY row + EVERY file. Wired to "Clear cache". Future clearAll() async { final dir = await _tracksDir(); final d = Directory(dir); if (d.existsSync()) { - for (final entity in d.listSync()) { - if (entity is File) await entity.delete(); + for (final e in d.listSync()) { + if (e is File) await e.delete(); } } await _db.delete(_db.audioCacheIndex).go(); diff --git a/flutter_client/lib/cache/cache_settings_provider.dart b/flutter_client/lib/cache/cache_settings_provider.dart index 6a17e609..517ab195 100644 --- a/flutter_client/lib/cache/cache_settings_provider.dart +++ b/flutter_client/lib/cache/cache_settings_provider.dart @@ -3,17 +3,29 @@ import 'package:flutter_secure_storage/flutter_secure_storage.dart'; import '../auth/auth_provider.dart' show secureStorageProvider; -/// Operator-tunable cache settings (#357 plan B). Persisted via +/// Operator-tunable cache settings. Persisted via /// flutter_secure_storage on the same device. +/// +/// #427 S2: the single `capBytes` is replaced by two independent +/// budgets — Liked and Rolling-recent — each defaulting to 5GB. +/// Bucketing is by liked-ness (a cached track currently in the +/// user's liked set is charged to Liked; everything else to +/// Rolling), so the dedup is storage-only and never filters +/// playback. The old `cache_cap_bytes` key is intentionally not +/// migrated; defaults reapply (a one-time re-tune, not data loss). class CacheSettings { const CacheSettings({ - required this.capBytes, + required this.likedCapBytes, + required this.rollingCapBytes, required this.prefetchWindow, required this.cacheLikedTracks, }); - /// 0 = unlimited. - final int capBytes; + /// Liked-bucket budget. 0 = unlimited. + final int likedCapBytes; + + /// Rolling (recently-played) budget. 0 = unlimited. + final int rollingCapBytes; /// 1..10. Number of next-tracks the prefetcher pre-downloads. final int prefetchWindow; @@ -22,25 +34,31 @@ class CacheSettings { final bool cacheLikedTracks; CacheSettings copyWith({ - int? capBytes, + int? likedCapBytes, + int? rollingCapBytes, int? prefetchWindow, bool? cacheLikedTracks, }) => CacheSettings( - capBytes: capBytes ?? this.capBytes, + likedCapBytes: likedCapBytes ?? this.likedCapBytes, + rollingCapBytes: rollingCapBytes ?? this.rollingCapBytes, prefetchWindow: prefetchWindow ?? this.prefetchWindow, cacheLikedTracks: cacheLikedTracks ?? this.cacheLikedTracks, ); + static const _fiveGiB = 5 * 1024 * 1024 * 1024; + static const defaults = CacheSettings( - capBytes: 5 * 1024 * 1024 * 1024, + likedCapBytes: _fiveGiB, + rollingCapBytes: _fiveGiB, prefetchWindow: 5, cacheLikedTracks: true, ); } class CacheSettingsController extends AsyncNotifier { - static const _kCap = 'cache_cap_bytes'; + static const _kLikedCap = 'cache_liked_cap_bytes'; + static const _kRollingCap = 'cache_rolling_cap_bytes'; static const _kPrefetch = 'cache_prefetch_window'; static const _kCacheLiked = 'cache_liked_tracks'; @@ -49,13 +67,17 @@ class CacheSettingsController extends AsyncNotifier { @override Future build() async { _storage = ref.read(secureStorageProvider); - final cap = await _storage.read(key: _kCap); + final likedCap = await _storage.read(key: _kLikedCap); + final rollingCap = await _storage.read(key: _kRollingCap); final pre = await _storage.read(key: _kPrefetch); final liked = await _storage.read(key: _kCacheLiked); return CacheSettings( - capBytes: cap == null - ? CacheSettings.defaults.capBytes - : int.tryParse(cap) ?? CacheSettings.defaults.capBytes, + likedCapBytes: likedCap == null + ? CacheSettings.defaults.likedCapBytes + : int.tryParse(likedCap) ?? CacheSettings.defaults.likedCapBytes, + rollingCapBytes: rollingCap == null + ? CacheSettings.defaults.rollingCapBytes + : int.tryParse(rollingCap) ?? CacheSettings.defaults.rollingCapBytes, prefetchWindow: pre == null ? CacheSettings.defaults.prefetchWindow : (int.tryParse(pre) ?? 5).clamp(1, 10), @@ -65,9 +87,14 @@ class CacheSettingsController extends AsyncNotifier { ); } - Future setCapBytes(int bytes) async { - await _storage.write(key: _kCap, value: bytes.toString()); - state = AsyncData(state.value!.copyWith(capBytes: bytes)); + Future setLikedCapBytes(int bytes) async { + await _storage.write(key: _kLikedCap, value: bytes.toString()); + state = AsyncData(state.value!.copyWith(likedCapBytes: bytes)); + } + + Future setRollingCapBytes(int bytes) async { + await _storage.write(key: _kRollingCap, value: bytes.toString()); + state = AsyncData(state.value!.copyWith(rollingCapBytes: bytes)); } Future setPrefetchWindow(int n) async { diff --git a/flutter_client/lib/cache/db.dart b/flutter_client/lib/cache/db.dart index 26b79754..824adcb9 100644 --- a/flutter_client/lib/cache/db.dart +++ b/flutter_client/lib/cache/db.dart @@ -101,6 +101,11 @@ class AudioCacheIndex extends Table { TextColumn get path => text()(); IntColumn get sizeBytes => integer()(); DateTimeColumn get cachedAt => dateTime().withDefault(currentDateAndTime)(); + /// When the track was last played. Drives the offline "Recently + /// played" ordering and rolling-bucket LRU eviction. Distinct from + /// cachedAt (download time). Nullable for pre-schema-9 rows until + /// the next play touches them (migration backfills to cachedAt). + DateTimeColumn get lastPlayedAt => dateTime().nullable()(); TextColumn get source => textEnum()(); @override Set get primaryKey => {trackId}; @@ -247,7 +252,7 @@ class AppDb extends _$AppDb { AppDb([QueryExecutor? e]) : super(e ?? driftDatabase(name: 'minstrel_cache')); @override - int get schemaVersion => 8; + int get schemaVersion => 9; @override MigrationStrategy get migration => MigrationStrategy( @@ -300,6 +305,17 @@ class AppDb extends _$AppDb { // upgrade; populated by controllers when REST calls fail. await m.createTable(cachedMutations); } + if (from < 9) { + // Schema 9 (#427 S2): two-bucket cache. lastPlayedAt + // gives the offline "Recently played" view a real + // recency signal (cachedAt is download time, not play + // time). Nullable — backfilled to cachedAt so existing + // rows order sensibly until next play touches them. + await m.addColumn(audioCacheIndex, audioCacheIndex.lastPlayedAt); + await customStatement( + 'UPDATE audio_cache_index SET last_played_at = cached_at', + ); + } }, ); } diff --git a/flutter_client/lib/cache/mutation_queue.dart b/flutter_client/lib/cache/mutation_queue.dart index 7e7bb354..5077f892 100644 --- a/flutter_client/lib/cache/mutation_queue.dart +++ b/flutter_client/lib/cache/mutation_queue.dart @@ -30,6 +30,7 @@ import 'package:flutter/foundation.dart' show debugPrint; import 'package:flutter_riverpod/flutter_riverpod.dart'; import '../api/endpoints/discover.dart'; +import '../api/endpoints/events.dart'; import '../api/endpoints/likes.dart'; import '../library/library_providers.dart' show dioProvider; import '../likes/likes_provider.dart' show likesApiProvider; @@ -52,6 +53,7 @@ class MutationKinds { static const playlistAppend = 'playlist.append'; static const requestCreate = 'request.create'; static const requestCancel = 'request.cancel'; + static const playOffline = 'play.offline'; } class MutationQueue { @@ -237,6 +239,8 @@ typedef _Handler = Future Function(Ref, Map); /// * playlist.append: {'playlistId': uuid, 'trackIds': [uuid, …]} /// * request.create: {full createRequest args; see DiscoverApi} /// * request.cancel: {'id': uuid} +/// * play.offline: {'trackId': uuid, 'clientId': str, 'at': iso8601, +/// 'durationPlayedMs': int, 'source'?: 'for_you'|'discover'} final Map _handlers = { MutationKinds.likeAdd: (ref, p) async { final api = await ref.read(likesApiProvider.future); @@ -281,6 +285,17 @@ final Map _handlers = { final api = await ref.read(requestsApiProvider.future); await api.cancel(p['id'] as String); }, + MutationKinds.playOffline: (ref, p) async { + final dio = await ref.read(dioProvider.future); + final api = EventsApi(dio); + await api.playOffline( + trackId: p['trackId'] as String, + clientId: p['clientId'] as String, + atIso: p['at'] as String, + durationPlayedMs: (p['durationPlayedMs'] as num).toInt(), + source: p['source'] as String?, + ); + }, }; LikeKind _likeKindFromString(String s) => switch (s) { diff --git a/flutter_client/lib/cache/offline_provider.dart b/flutter_client/lib/cache/offline_provider.dart new file mode 100644 index 00000000..0ef4bcc4 --- /dev/null +++ b/flutter_client/lib/cache/offline_provider.dart @@ -0,0 +1,105 @@ +// Reachability-based offline marker (#427 S1). +// +// `connectivityProvider` only knows whether a network *interface* is +// up — not whether the Minstrel server is actually reachable (captive +// portals, server down, DNS, VPN-only routes all read "online"). This +// is the single source of truth other features gate on: offline = +// the server failed its /healthz probe N times in a row; recovery on +// the first success. +// +// Deliberately NOT coupled to connectivityProvider: subscribing to +// that StreamProvider eagerly mounts its 2s checkConnectivity timeout +// and leaks a pending Timer through widget tests that never reach the +// auth state (the bug fixed for MutationReplayer). /healthz failing +// already covers interface-down — it just fails the probe. +// +// Optimistic: assume online until proven offline, so a cold launch +// behaves normally and only flips after sustained unreachability. + +import 'dart:async'; + +import 'package:dio/dio.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; + +import '../api/endpoints/health.dart'; +import '../library/library_providers.dart' show dioProvider; + +class OfflineMonitor extends Notifier { + Timer? _initial; + Timer? _periodic; + int _consecutiveFails = 0; + + /// Consecutive failed probes before we declare offline. Small + /// enough to react within ~half a minute, large enough that a + /// single dropped request doesn't flip the whole UI. + static const _threshold = 3; + + /// Probe cadence. Slower when believed-online (cheap heartbeat); + /// faster when offline so recovery is noticed quickly. + static const _onlineInterval = Duration(seconds: 30); + static const _offlineInterval = Duration(seconds: 10); + + /// Let auth + server-url + dio providers finish async warmup + /// before the first probe so a cold start doesn't false-positive. + static const _initialDelay = Duration(seconds: 5); + + @override + bool build() { + ref.onDispose(() { + _initial?.cancel(); + _periodic?.cancel(); + }); + _initial = Timer(_initialDelay, () { + _check(); + _arm(); + }); + return false; // optimistic until a probe says otherwise + } + + void _arm() { + _periodic?.cancel(); + _periodic = Timer.periodic( + state ? _offlineInterval : _onlineInterval, + (_) => _check(), + ); + } + + Future _check() async { + final Dio dio; + try { + // Not configured yet (no server URL) → don't flip; the app is + // still on the connect screen and "offline" is meaningless. + dio = await ref.read(dioProvider.future); + } catch (_) { + return; + } + try { + // dio's own connect/receive timeouts bound this — no extra + // Timer (which would leak in widget tests). + await HealthApi(dio).check(); + _consecutiveFails = 0; + _set(false); + } catch (_) { + _consecutiveFails++; + if (_consecutiveFails >= _threshold) { + _set(true); + } + } + } + + void _set(bool offline) { + if (state == offline) return; + state = offline; + _arm(); // cadence follows the new state + } + + /// Force an immediate probe — e.g. a user-initiated retry. Public + /// so S4's offline surfaces can offer a "try again" affordance. + Future recheck() => _check(); +} + +/// `true` when the server is unreachable. Watch this to gate +/// online-only affordances; read it once at app start to activate +/// the poller. +final offlineProvider = + NotifierProvider(OfflineMonitor.new); diff --git a/flutter_client/lib/cache/prefetcher.dart b/flutter_client/lib/cache/prefetcher.dart index 785d05f6..2abeaf8e 100644 --- a/flutter_client/lib/cache/prefetcher.dart +++ b/flutter_client/lib/cache/prefetcher.dart @@ -3,6 +3,7 @@ import 'dart:async'; import 'package:audio_service/audio_service.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; +import '../likes/likes_provider.dart' show likedIdsProvider; import '../player/album_color_extractor.dart'; import '../player/player_provider.dart'; import 'audio_cache_manager.dart'; @@ -86,10 +87,17 @@ class Prefetcher { } } - // Eviction pass after pinning new files. - if (settings.capBytes > 0) { - await mgr.evict(targetBytes: settings.capBytes); - } + // Eviction pass after pinning new files. Per-bucket (#427 S2): + // liked-ness decides the bucket, so a track that's both liked + // and recently played is protected by the Liked cap, not the + // rolling LRU. + final liked = + _ref.read(likedIdsProvider).value?.tracks ?? const {}; + await mgr.evictBuckets( + likedCap: settings.likedCapBytes, + rollingCap: settings.rollingCapBytes, + liked: liked, + ); } } diff --git a/flutter_client/lib/cache/shuffle_source.dart b/flutter_client/lib/cache/shuffle_source.dart new file mode 100644 index 00000000..e42d9767 --- /dev/null +++ b/flutter_client/lib/cache/shuffle_source.dart @@ -0,0 +1,104 @@ +// Offline play sources over the local cache index (#427 S4). +// +// "Shuffle all" is always-present and degrades with reachability: +// online → GET /api/library/shuffle (random over the whole library) +// offline → a client shuffle over the entire local cache index +// +// Offline-only pools surfaced on Home beside the (disabled) system +// playlists: "Recently played" (cache by lastPlayedAt desc) and +// "Liked" (cache ∩ liked set). All of these are UNIONs over the +// cache regardless of storage bucket — liked AND recently-played +// both included. The two-bucket split (S2) is storage/eviction-only +// and never filters playback, which is exactly what this relies on. + +import 'dart:math'; + +import 'package:flutter_riverpod/flutter_riverpod.dart'; + +import '../library/library_providers.dart' show libraryApiProvider; +import '../likes/likes_provider.dart' show likedIdsProvider; +import '../models/track.dart'; +import 'adapters.dart'; +import 'audio_cache_manager.dart' show appDbProvider; +import 'offline_provider.dart'; + +class ShuffleSource { + ShuffleSource(this._ref); + final Ref _ref; + + /// Shuffle-all. Online: server-random. Offline: every cached + /// track, shuffled. Empty offline → nothing cached yet. + Future> tracks({int limit = 100}) async { + if (_ref.read(offlineProvider)) { + final ids = await _cachedIdsByRecency(); + final list = await _refs(ids); + list.shuffle(Random()); + return list.length > limit ? list.sublist(0, limit) : list; + } + final api = await _ref.read(libraryApiProvider.future); + return api.shuffle(limit: limit); + } + + /// Cached tracks, most-recently-played first (liked included). + /// Cache-only — surfaced on Home only when offline. + Future> recentlyPlayed({int limit = 100}) async { + final ids = await _cachedIdsByRecency(); + final list = await _refs(ids); + return list.length > limit ? list.sublist(0, limit) : list; + } + + /// Cached tracks that are in the user's liked set. Cache-only — + /// surfaced on Home only when offline. + Future> liked({int limit = 100}) async { + final likedSet = + _ref.read(likedIdsProvider).value?.tracks ?? const {}; + final ids = + (await _cachedIdsByRecency()).where(likedSet.contains).toList(); + final list = await _refs(ids); + return list.length > limit ? list.sublist(0, limit) : list; + } + + /// Cached track ids ordered by lastPlayedAt desc (nulls last so + /// never-touched downloads sort after real plays). + Future> _cachedIdsByRecency() async { + final db = _ref.read(appDbProvider); + final rows = await db.select(db.audioCacheIndex).get(); + rows.sort((a, b) { + final av = a.lastPlayedAt, bv = b.lastPlayedAt; + if (av == null && bv == null) return 0; + if (av == null) return 1; + if (bv == null) return -1; + return bv.compareTo(av); + }); + return rows.map((r) => r.trackId).toList(); + } + + /// Materializes ordered track ids into TrackRefs from the cached + /// metadata tables, preserving the given order. + Future> _refs(List orderedIds) async { + if (orderedIds.isEmpty) return const []; + final db = _ref.read(appDbProvider); + final meta = await (db.select(db.cachedTracks) + ..where((t) => t.id.isIn(orderedIds))) + .get(); + if (meta.isEmpty) return const []; + final byId = {for (final t in meta) t.id: t}; + final artistName = { + for (final a in await db.select(db.cachedArtists).get()) a.id: a.name + }; + final albumTitle = { + for (final a in await db.select(db.cachedAlbums).get()) a.id: a.title + }; + return [ + for (final id in orderedIds) + if (byId[id] case final t?) + t.toRef( + artistName: artistName[t.artistId] ?? '', + albumTitle: albumTitle[t.albumId] ?? '', + ) + ]; + } +} + +final shuffleSourceProvider = + Provider((ref) => ShuffleSource(ref)); diff --git a/flutter_client/lib/library/album_detail_screen.dart b/flutter_client/lib/library/album_detail_screen.dart index 8e11844e..9b628906 100644 --- a/flutter_client/lib/library/album_detail_screen.dart +++ b/flutter_client/lib/library/album_detail_screen.dart @@ -3,8 +3,6 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import '../api/endpoints/likes.dart'; import '../models/album.dart'; -import '../cache/audio_cache_manager.dart'; -import '../cache/db.dart'; import '../likes/like_button.dart'; import '../player/player_provider.dart'; import '../shared/widgets/server_image.dart'; @@ -113,21 +111,6 @@ class AlbumDetailScreen extends ConsumerWidget { Text(r.album.artistName, style: TextStyle(color: fs.ash)), ], )), - IconButton( - key: const Key('download_album_button'), - icon: Icon(Icons.download, color: fs.ash), - tooltip: 'Download album', - onPressed: () { - final mgr = ref.read(audioCacheManagerProvider); - for (final t in r.tracks) { - // ignore: unawaited_futures - mgr.pin(t.id, source: CacheSource.autoPlaylist); - } - ScaffoldMessenger.of(context).showSnackBar( - SnackBar(content: Text('Downloading ${r.tracks.length} tracks…')), - ); - }, - ), Container( width: 48, height: 48, decoration: BoxDecoration(color: fs.accent, shape: BoxShape.circle), diff --git a/flutter_client/lib/library/home_screen.dart b/flutter_client/lib/library/home_screen.dart index 968ed772..a3eff3dc 100644 --- a/flutter_client/lib/library/home_screen.dart +++ b/flutter_client/lib/library/home_screen.dart @@ -4,10 +4,13 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; import '../api/errors.dart'; +import '../cache/offline_provider.dart'; +import '../cache/shuffle_source.dart'; import '../cache/tile_providers.dart'; import '../models/playlist.dart'; import '../models/system_playlists_status.dart'; import '../models/track.dart'; +import '../player/player_provider.dart'; import '../playlists/playlists_provider.dart'; import '../playlists/widgets/playlist_card.dart'; import '../playlists/widgets/playlist_placeholder_card.dart'; @@ -63,6 +66,7 @@ class HomeScreen extends ConsumerWidget { _PlaylistsSection( playlists: allPlaylists.value?.owned ?? const [], status: status.value ?? SystemPlaylistsStatus.empty(), + offline: ref.watch(offlineProvider), ), _RecentlyAddedSection(ids: h.recentlyAddedAlbums), _RediscoverSection( @@ -222,23 +226,124 @@ class _CompactTrackSkeleton extends StatelessWidget { // ─── Sections ──────────────────────────────────────────────────────── class _PlaylistsSection extends StatelessWidget { - const _PlaylistsSection({required this.playlists, required this.status}); + const _PlaylistsSection({ + required this.playlists, + required this.status, + required this.offline, + }); final List playlists; final SystemPlaylistsStatus status; + final bool offline; @override Widget build(BuildContext context) { final items = _buildPlaylistsRow(playlists, status); + final children = [ + // Offline: surface the cache-backed pools where the (now + // play-disabled) system playlists sit, so there's something + // to play. #427 S4b. + if (offline) ...const [ + _OfflinePoolCard( + label: 'Recently played', + icon: Icons.history, + kind: _OfflinePoolKind.recentlyPlayed, + ), + _OfflinePoolCard( + label: 'Liked', + icon: Icons.favorite, + kind: _OfflinePoolKind.liked, + ), + ], + for (final item in items) + if (item is _RealPlaylist) + PlaylistCard(playlist: item.playlist) + else + PlaylistPlaceholderCard( + label: (item as _PlaceholderPlaylist).label, + variant: item.variant, + ), + ]; return HorizontalScrollRow( title: 'Playlists', height: 220, - children: items.map((item) { - if (item is _RealPlaylist) { - return PlaylistCard(playlist: item.playlist); - } - final ph = item as _PlaceholderPlaylist; - return PlaylistPlaceholderCard(label: ph.label, variant: ph.variant); - }).toList(), + children: children, + ); + } +} + +enum _OfflinePoolKind { recentlyPlayed, liked } + +/// Home tile for an offline cache-backed pool. Tapping shuffles + +/// plays that pool from the local cache. Sized to match +/// PlaylistCard so the row stays visually consistent. +class _OfflinePoolCard extends ConsumerWidget { + const _OfflinePoolCard({ + required this.label, + required this.icon, + required this.kind, + }); + final String label; + final IconData icon; + final _OfflinePoolKind kind; + + @override + Widget build(BuildContext context, WidgetRef ref) { + final fs = Theme.of(context).extension()!; + return SizedBox( + width: 176, + child: Material( + color: Colors.transparent, + child: InkWell( + onTap: () async { + final messenger = ScaffoldMessenger.of(context); + final src = ref.read(shuffleSourceProvider); + final refs = await switch (kind) { + _OfflinePoolKind.recentlyPlayed => src.recentlyPlayed(), + _OfflinePoolKind.liked => src.liked(), + }; + if (refs.isEmpty) { + messenger.showSnackBar( + SnackBar(content: Text('No cached $label tracks yet')), + ); + return; + } + await ref + .read(playerActionsProvider) + .playTracks(refs, shuffle: true); + }, + child: Padding( + padding: const EdgeInsets.symmetric(horizontal: 8), + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Container( + width: 144, + height: 144, + decoration: BoxDecoration( + color: fs.slate, + borderRadius: BorderRadius.circular(6), + ), + child: Icon(icon, color: fs.accent, size: 56), + ), + const SizedBox(height: 8), + Text( + label, + maxLines: 1, + overflow: TextOverflow.ellipsis, + style: TextStyle(color: fs.parchment, fontSize: 14), + ), + Padding( + padding: const EdgeInsets.only(top: 2), + child: Text( + 'Offline', + style: TextStyle(color: fs.ash, fontSize: 11), + ), + ), + ], + ), + ), + ), + ), ); } } diff --git a/flutter_client/lib/library/library_screen.dart b/flutter_client/lib/library/library_screen.dart index b3e64948..d6cb3a93 100644 --- a/flutter_client/lib/library/library_screen.dart +++ b/flutter_client/lib/library/library_screen.dart @@ -15,6 +15,7 @@ import '../cache/cache_first.dart'; import '../cache/connectivity_provider.dart'; import '../cache/db.dart'; import '../cache/metadata_prefetcher.dart'; +import '../cache/shuffle_source.dart'; import '../cache/tile_providers.dart'; import '../library/library_providers.dart' show dioProvider; import '../models/album.dart'; @@ -377,7 +378,27 @@ class _LibraryScreenState extends ConsumerState backgroundColor: fs.obsidian, elevation: 0, title: Text('Library', style: TextStyle(color: fs.parchment)), - actions: const [MainAppBarActions(currentRoute: '/library')], + actions: [ + IconButton( + key: const Key('shuffle_all_button'), + tooltip: 'Shuffle all', + icon: Icon(Icons.shuffle, color: fs.parchment), + onPressed: () async { + final messenger = ScaffoldMessenger.of(context); + final refs = await ref.read(shuffleSourceProvider).tracks(); + if (refs.isEmpty) { + messenger.showSnackBar(const SnackBar( + content: Text('Nothing to shuffle yet'), + )); + return; + } + await ref + .read(playerActionsProvider) + .playTracks(refs, shuffle: true); + }, + ), + const MainAppBarActions(currentRoute: '/library'), + ], bottom: TabBar( controller: _ctrl, isScrollable: true, diff --git a/flutter_client/lib/models/playlist.dart b/flutter_client/lib/models/playlist.dart index 7916fde7..1ae01860 100644 --- a/flutter_client/lib/models/playlist.dart +++ b/flutter_client/lib/models/playlist.dart @@ -38,6 +38,16 @@ class Playlist { bool get isSystem => systemVariant != null; + /// Whether this playlist supports the generic by-kind refresh/ + /// shuffle endpoints (#411 R2) — i.e. a singleton system kind. + /// The server exposes a `refreshable` flag for JSON-sourced + /// playlists, but the list tiles are drift-cache-sourced (no + /// migration just for this), so derive it: every system kind is a + /// singleton except songs_like_artist (multi-per-user). This rule + /// holds for For-You/Discover and all planned discovery mixes; if + /// a future non-singleton kind appears, extend the exclusion. + bool get refreshable => isSystem && systemVariant != 'songs_like_artist'; + factory Playlist.fromJson(Map j) => Playlist( id: j['id'] as String, userId: j['user_id'] as String? ?? '', diff --git a/flutter_client/lib/player/audio_handler.dart b/flutter_client/lib/player/audio_handler.dart index 6217ae7d..b8156f08 100644 --- a/flutter_client/lib/player/audio_handler.dart +++ b/flutter_client/lib/player/audio_handler.dart @@ -82,6 +82,15 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl /// past the fill front needs to rebuild from the stored tracks. List _lastTracks = const []; + /// #415: which system playlist this queue was seeded from + /// ('for_you' | 'discover'), or null for library / user-playlist / + /// radio. The play-events reporter reads this so play_started + /// carries `source` and the server advances that rotation. A fresh + /// setQueueFromTracks from a non-system surface clears it; internal + /// rebuilds (skipToQueueItem) preserve it. + String? _queueSource; + String? get queueSource => _queueSource; + /// Volume stream for UI subscribers. Mirrors the just_audio player's /// volume directly; set via setVolume(double). Stream get volumeStream => _player.volumeStream; @@ -109,8 +118,13 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl if (audioCacheManager != null) _audioCacheManager = audioCacheManager; } - Future setQueueFromTracks(List tracks, {int initialIndex = 0}) async { + Future setQueueFromTracks( + List tracks, { + int initialIndex = 0, + String? source, + }) async { if (tracks.isEmpty) return; + _queueSource = source; final clampedInitial = initialIndex.clamp(0, tracks.length - 1); // Bump the generation FIRST. Any in-flight _fillRemainingSources @@ -190,7 +204,9 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl @override Future skipToQueueItem(int index) async { if (index < 0 || index >= _lastTracks.length) return; - await setQueueFromTracks(_lastTracks, initialIndex: index); + // Preserve the system-playlist source across an internal rebuild + // — a queue-item skip is still playing from the same playlist. + await setQueueFromTracks(_lastTracks, initialIndex: index, source: _queueSource); await play(); } diff --git a/flutter_client/lib/player/play_events_reporter.dart b/flutter_client/lib/player/play_events_reporter.dart new file mode 100644 index 00000000..a06d2630 --- /dev/null +++ b/flutter_client/lib/player/play_events_reporter.dart @@ -0,0 +1,277 @@ +// Flutter play-event lifecycle reporter (#415 stage 3). +// +// The Flutter client previously reported NO plays — listening on +// mobile never reached the server's play_events, so history, +// recommendation scoring, ListenBrainz scrobbles, and (since #415) +// system-playlist rotation all missed mobile activity entirely. This +// closes that gap and is the path that carries the #415 `source` tag. +// +// State machine over (current track id, playing). Mirrors the web +// events dispatcher, with one deliberate divergence: when a track +// changes inside a queue we classify ended-vs-skipped by whether the +// prior track reached (near) its duration, instead of the web +// dispatcher's blanket "track change = skip". Blanket-skip would mark +// every naturally-finished in-queue track as a skip and dilute the +// recommendation skip-ratio — the exact failure mode that motivated +// doing this properly. (The web dispatcher likely has the same +// false-skip issue; flagged separately, not fixed here.) + +import 'dart:async'; +import 'dart:math'; + +import 'package:audio_service/audio_service.dart'; +import 'package:flutter/widgets.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; + +import '../api/endpoints/events.dart'; +import '../auth/auth_provider.dart' show secureStorageProvider; +import '../cache/mutation_queue.dart' + show MutationKinds, mutationQueueProvider; +import '../library/library_providers.dart' show dioProvider; +import 'audio_handler.dart' show MinstrelAudioHandler; +import 'player_provider.dart' show audioHandlerProvider; + +const _clientIdKey = 'play_events_client_id'; + +/// Tolerance for "the track basically finished": within 3s of the +/// known duration counts as a natural completion, not a skip. +const _completionToleranceMs = 3000; + +class PlayEventsReporter with WidgetsBindingObserver { + PlayEventsReporter(this._ref); + final Ref _ref; + + final _subs = >[]; + bool _disposed = false; + + String? _clientId; + EventsApi? _api; + + // Server play_event_id when the live play_started succeeded; null + // if start failed / offline — then the close is captured into the + // offline mutation queue instead of a live ended/skipped call. + String? _openPlayEventId; + + // The play currently being tracked, captured independently of + // connectivity so an offline play is still a complete record. + String? _curTrackId; + DateTime? _curStartedAt; + String? _curSource; + int _curLastPositionMs = 0; + int _curDurationMs = 0; + bool _curReachedEnd = false; + + String? _prevTrackId; + + Future start() async { + final MinstrelAudioHandler handler; + try { + // audioHandlerProvider throws until main() overrides it (real + // app always does). In tests / no-audio environments there's + // nothing to report — fail safe and stay inert rather than + // surfacing an unhandled async error. + handler = _ref.read(audioHandlerProvider); + _clientId = await _resolveClientId(); + final dio = await _ref.read(dioProvider.future); + _api = EventsApi(dio); + } catch (_) { + return; + } + if (_disposed) return; + WidgetsBinding.instance.addObserver(this); + + _subs.add(handler.positionStream.listen((p) { + final ms = p.inMilliseconds; + // Advance the tracked play's progress ONLY while its track is + // the current one. A track change resets position to 0; gating + // on _curTrackId keeps the finishing track's last-known values + // intact for the close branch. + final mi = handler.mediaItem.value; + if (mi != null && mi.id == _curTrackId) { + _curLastPositionMs = ms; + final d = mi.duration; + if (d != null && d.inMilliseconds > 0) { + _curDurationMs = d.inMilliseconds; + if (ms >= _curDurationMs - _completionToleranceMs) { + _curReachedEnd = true; + } + } + } + })); + _subs.add(handler.mediaItem.listen((_) => _evaluate(handler))); + _subs.add(handler.playbackState.listen((_) => _evaluate(handler))); + } + + void _evaluate(MinstrelAudioHandler handler) { + if (_disposed) return; + final mi = handler.mediaItem.value; + final st = handler.playbackState.value; + final tid = mi?.id; + final playing = st.playing; + final completed = st.processingState == AudioProcessingState.completed; + + // Track changed → close the prior tracked play. + if (tid != _prevTrackId && _curTrackId != null) { + _closeCurrent(viaOffline: false); + } + + // Entered playing for a new track → begin tracking it. + if (tid != null && playing && _curTrackId != tid) { + _beginTrack(handler, tid); + } + + // Whole-queue natural end (just_audio only emits `completed` at + // the end of the sequence, not between items) → close it. + if (completed && _curTrackId != null) { + _curReachedEnd = true; + _closeCurrent(viaOffline: false); + } + + _prevTrackId = tid; + } + + void _beginTrack(MinstrelAudioHandler handler, String trackId) { + _curTrackId = trackId; + _curStartedAt = DateTime.now().toUtc(); + _curSource = handler.queueSource; + _curLastPositionMs = 0; + _curReachedEnd = false; + final d = handler.mediaItem.value?.duration; + _curDurationMs = d?.inMilliseconds ?? 0; + _openPlayEventId = null; + // Fire the live play_started; adopt the server id only if we're + // still on this track when the response lands. Failure is fine — + // the close path captures the whole play into the offline queue. + _startLive(handler, trackId); + } + + Future _startLive(MinstrelAudioHandler handler, String trackId) async { + final api = _api; + final cid = _clientId; + if (api == null || cid == null) return; + try { + final id = await api.playStarted( + trackId: trackId, + clientId: cid, + source: _curSource, + ); + if (id != null && _curTrackId == trackId) { + _openPlayEventId = id; + } + } catch (_) { + // Offline / flaky — _openPlayEventId stays null; the close path + // enqueues the completed play for replay. + } + } + + /// Closes the currently-tracked play. `finished` is derived from + /// whether it reached ~its duration. If the live start registered a + /// server id we attempt the live ended/skipped close and fall back + /// to the offline queue on failure; with no server id (offline + /// start) — or viaOffline (app teardown, must be durable) — the + /// completed play is enqueued directly. The server's RecordOffline + /// Play applies the canonical skip rule, so the offline payload + /// only needs duration, not our finished/skipped guess. + void _closeCurrent({required bool viaOffline}) { + final trackId = _curTrackId; + final startedAt = _curStartedAt; + if (trackId == null || startedAt == null) { + _resetCurrent(); + return; + } + final reached = _curReachedEnd; + final lastPos = _curLastPositionMs; + final durationMs = (reached && _curDurationMs > 0) + ? _curDurationMs + : lastPos; + final source = _curSource; + final id = _openPlayEventId; + + if (!viaOffline && id != null) { + // Live close; on failure, fall back to the durable offline path + // so a transient blip at close time doesn't lose the play. + final fut = reached + ? _api?.playEnded(playEventId: id, durationPlayedMs: durationMs) + : _api?.playSkipped(playEventId: id, positionMs: lastPos); + fut?.catchError((_) { + _enqueueOffline(trackId, startedAt, source, durationMs); + }); + } else { + _enqueueOffline(trackId, startedAt, source, durationMs); + } + _resetCurrent(); + } + + void _resetCurrent() { + _curTrackId = null; + _curStartedAt = null; + _curSource = null; + _curLastPositionMs = 0; + _curDurationMs = 0; + _curReachedEnd = false; + _openPlayEventId = null; + } + + void _enqueueOffline( + String trackId, + DateTime startedAt, + String? source, + int durationPlayedMs, + ) { + final cid = _clientId; + if (cid == null) return; + // ignore: unawaited_futures + _ref.read(mutationQueueProvider).enqueue(MutationKinds.playOffline, { + 'trackId': trackId, + 'clientId': cid, + 'at': startedAt.toIso8601String(), + 'durationPlayedMs': durationPlayedMs, + if (source != null && source.isNotEmpty) 'source': source, + }); + } + + @override + void didChangeAppLifecycleState(AppLifecycleState state) { + // App backgrounded / killed mid-play: close durably via the + // offline queue (a fire-and-forget POST during teardown is + // unreliable; the queue survives a process kill and drains on + // next launch). Mirrors the intent of web's pagehide beacon. + if (state == AppLifecycleState.paused || + state == AppLifecycleState.detached) { + if (_curTrackId != null) { + _closeCurrent(viaOffline: true); + } + } + } + + Future _resolveClientId() async { + final storage = _ref.read(secureStorageProvider); + final existing = await storage.read(key: _clientIdKey); + if (existing != null && existing.isNotEmpty) return existing; + final rnd = Random.secure(); + final bytes = List.generate(16, (_) => rnd.nextInt(256)); + final id = + bytes.map((b) => b.toRadixString(16).padLeft(2, '0')).join(); + await storage.write(key: _clientIdKey, value: id); + return id; + } + + void dispose() { + _disposed = true; + WidgetsBinding.instance.removeObserver(this); + for (final s in _subs) { + s.cancel(); + } + _subs.clear(); + } +} + +/// Read once at app start (app.dart postFrame) to activate reporting. +/// Disposed via ref.onDispose when the scope tears down. +final playEventsReporterProvider = Provider((ref) { + final r = PlayEventsReporter(ref); + ref.onDispose(r.dispose); + // ignore: unawaited_futures + r.start(); + return r; +}); diff --git a/flutter_client/lib/player/player_provider.dart b/flutter_client/lib/player/player_provider.dart index 14f4afe5..c1134aba 100644 --- a/flutter_client/lib/player/player_provider.dart +++ b/flutter_client/lib/player/player_provider.dart @@ -1,3 +1,5 @@ +import 'dart:math' show Random; + import 'package:audio_service/audio_service.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; @@ -65,7 +67,21 @@ class PlayerActions { } final Ref _ref; - Future playTracks(List tracks, {int initialIndex = 0}) async { + Future playTracks( + List tracks, { + int initialIndex = 0, + bool shuffle = false, + String? source, + }) async { + // shuffle=true means "play this pool randomly, starting at a + // random track" (client-side; used for non-system surfaces that + // want shuffle). System playlists instead fetch a server-ordered + // list and pass source — no client shuffle, the order is already + // rotation-aware (#415). + var startAt = initialIndex; + if (shuffle && tracks.length > 1) { + startAt = Random().nextInt(tracks.length); + } final url = await _ref.read(serverUrlProvider.future); final token = await _ref.read(secureStorageProvider).read(key: 'session_token'); final cache = _ref.read(albumCoverCacheProvider); @@ -78,7 +94,10 @@ class PlayerActions { audioCacheManager: audioCache, likeBridge: _buildLikeBridge(), ); - await h.setQueueFromTracks(tracks, initialIndex: initialIndex); + await h.setQueueFromTracks(tracks, initialIndex: startAt, source: source); + if (shuffle) { + await h.setShuffleMode(AudioServiceShuffleMode.all); + } await h.play(); } diff --git a/flutter_client/lib/playlists/playlist_detail_screen.dart b/flutter_client/lib/playlists/playlist_detail_screen.dart index 50c4d12b..ddb19d77 100644 --- a/flutter_client/lib/playlists/playlist_detail_screen.dart +++ b/flutter_client/lib/playlists/playlist_detail_screen.dart @@ -2,8 +2,6 @@ import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; -import '../cache/audio_cache_manager.dart'; -import '../cache/db.dart'; import '../models/playlist.dart'; import '../models/track.dart'; import '../player/player_provider.dart'; @@ -212,6 +210,9 @@ class _Body extends ConsumerWidget { ref.read(playerActionsProvider).playTracks( playableRefs, initialIndex: startIdx >= 0 ? startIdx : 0, + source: detail.playlist.refreshable + ? detail.playlist.systemVariant + : null, ); // Keep liveTrack referenced to avoid an unused-variable // warning while we leave hooks for menu wiring later. @@ -263,29 +264,22 @@ class _Header extends ConsumerWidget { ), const Spacer(), if (playable.isNotEmpty) ...[ - OutlinedButton.icon( - key: const Key('download_playlist_button'), - onPressed: () { - final mgr = ref.read(audioCacheManagerProvider); - for (final t in playable) { - final id = t.trackId ?? ''; - if (id.isEmpty) continue; - // Fire-and-forget; downloads happen in background. - // ignore: unawaited_futures - mgr.pin(id, source: CacheSource.autoPlaylist); - } - ScaffoldMessenger.of(context).showSnackBar( - SnackBar(content: Text('Downloading ${playable.length} tracks…')), - ); - }, - icon: const Icon(Icons.download, size: 16), - label: const Text('Download'), - ), - const SizedBox(width: 8), + if (p.refreshable) ...[ + OutlinedButton.icon( + key: const Key('regenerate_playlist_button'), + onPressed: () => _regenerate(context, ref), + icon: const Icon(Icons.refresh, size: 16), + label: const Text('Regenerate'), + ), + const SizedBox(width: 8), + ], FilledButton.icon( onPressed: () { final refs = playable.map(_toTrackRef).toList(growable: false); - ref.read(playerActionsProvider).playTracks(refs); + ref.read(playerActionsProvider).playTracks( + refs, + source: p.refreshable ? p.systemVariant : null, + ); }, icon: const Icon(Icons.play_arrow), label: const Text('Play'), @@ -299,6 +293,37 @@ class _Header extends ConsumerWidget { ]), ); } + + /// Forces a server-side rebuild of this system playlist. The + /// rebuild rotates the playlist UUID, so the old detail route now + /// 404s — rebind by pushReplacement-ing to the new id. The + /// aggregate list is invalidated too so the home row's tile points + /// at the fresh UUID. ScaffoldMessenger + router captured before + /// the await so we don't touch a stale BuildContext after. + Future _regenerate(BuildContext context, WidgetRef ref) async { + final messenger = ScaffoldMessenger.of(context); + final router = GoRouter.of(context); + final p = detail.playlist; + try { + final api = await ref.read(playlistsApiProvider.future); + final newId = await api.refreshSystem(p.systemVariant!); + ref.invalidate(playlistsListProvider); + if (newId == null) { + messenger.showSnackBar(const SnackBar( + content: Text('Nothing to build yet — library is empty.'), + )); + return; + } + messenger.showSnackBar( + SnackBar(content: Text('${p.name} regenerated')), + ); + router.pushReplacement('/playlists/$newId'); + } catch (e) { + messenger.showSnackBar( + SnackBar(content: Text("Couldn't regenerate: $e")), + ); + } + } } class _PlaylistTrackRow extends ConsumerWidget { diff --git a/flutter_client/lib/playlists/widgets/playlist_card.dart b/flutter_client/lib/playlists/widgets/playlist_card.dart index 47e75033..c7a0a2c5 100644 --- a/flutter_client/lib/playlists/widgets/playlist_card.dart +++ b/flutter_client/lib/playlists/widgets/playlist_card.dart @@ -2,6 +2,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; +import '../../cache/offline_provider.dart'; import '../../library/widgets/play_circle_button.dart'; import '../../models/playlist.dart'; import '../../models/track.dart'; @@ -23,7 +24,12 @@ class PlaylistCard extends ConsumerWidget { @override Widget build(BuildContext context, WidgetRef ref) { final fs = Theme.of(context).extension()!; - final hasTracks = playlist.trackCount > 0; + // #427 S4: refreshable system playlists need the live build/ + // shuffle endpoints — unavailable offline. Disable their play + // (use Shuffle all instead). User playlists play from cache. + final offline = ref.watch(offlineProvider); + final hasTracks = playlist.trackCount > 0 && + !(offline && playlist.refreshable); return SizedBox( width: 176, child: Material( @@ -66,6 +72,31 @@ class PlaylistCard extends ConsumerWidget { onPressed: () => _playPlaylist(ref), ), ), + // System playlists get a refresh affordance so the + // user can force a fresh mix instead of waiting for + // the daily 03:00 rebuild. Mirrors the web kebab. + if (playlist.refreshable) + Positioned( + top: 2, + right: 2, + child: PopupMenuButton( + icon: Icon(Icons.more_vert, color: fs.parchment, size: 18), + tooltip: 'Playlist actions', + color: fs.iron, + onSelected: (_) => _refresh(context, ref), + itemBuilder: (_) => [ + PopupMenuItem( + value: 'refresh', + child: Row(children: [ + Icon(Icons.refresh, color: fs.parchment, size: 18), + const SizedBox(width: 8), + Text(_refreshLabel, + style: TextStyle(color: fs.parchment)), + ]), + ), + ], + ), + ), ], ), const SizedBox(height: 8), @@ -91,6 +122,16 @@ class PlaylistCard extends ConsumerWidget { ), ), ), + if (playlist.isSystem && _refreshedLabel(playlist.createdAt) != '') + Padding( + padding: const EdgeInsets.only(top: 2), + child: Text( + _refreshedLabel(playlist.createdAt), + maxLines: 1, + overflow: TextOverflow.ellipsis, + style: TextStyle(color: fs.ash, fontSize: 11), + ), + ), ]), ), ), @@ -98,13 +139,63 @@ class PlaylistCard extends ConsumerWidget { ); } + String get _refreshLabel => 'Refresh ${playlist.name}'; + + /// #417: system playlists atomic-replace on rebuild, so createdAt + /// is the last-rotated time. Friendly wording mirrors the web + /// PlaylistCard. Empty string when the timestamp is unparseable. + String _refreshedLabel(String iso) { + final t = DateTime.tryParse(iso); + if (t == null) return ''; + final now = DateTime.now(); + final mins = now.difference(t).inMinutes; + if (mins < 5) return 'Refreshed just now'; + final startOfToday = DateTime(now.year, now.month, now.day); + if (!t.isBefore(startOfToday)) return 'Refreshed today'; + final days = startOfToday.difference(DateTime(t.year, t.month, t.day)).inDays; + if (days <= 1) return 'Refreshed yesterday'; + if (days < 7) return 'Refreshed $days days ago'; + const months = [ + 'Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', + 'Jul', 'Aug', 'Sep', 'Oct', 'Nov', 'Dec', + ]; + return 'Refreshed ${months[t.month - 1]} ${t.day}'; + } + + /// Forces a server-side rebuild of this system playlist, then + /// invalidates the aggregate list so the rotated UUID + new tracks + /// land on the next read. ScaffoldMessenger is captured before the + /// await so we don't touch a stale BuildContext afterward. + Future _refresh(BuildContext context, WidgetRef ref) async { + final messenger = ScaffoldMessenger.of(context); + try { + final api = await ref.read(playlistsApiProvider.future); + await api.refreshSystem(playlist.systemVariant!); + ref.invalidate(playlistsListProvider); + messenger.showSnackBar( + SnackBar(content: Text('${playlist.name} refreshed')), + ); + } catch (e) { + messenger.showSnackBar( + SnackBar(content: Text("Couldn't refresh: $e")), + ); + } + } + /// Fetches the playlist via /api/playlists/{id}, materializes each /// PlaylistTrack into a TrackRef (filtering out unavailable rows /// whose `trackId` is null after a track-delete), and plays from /// index 0. Mirrors the web PlaylistCard's onPlayClick. Future _playPlaylist(WidgetRef ref) async { final api = await ref.read(playlistsApiProvider.future); - final detail = await api.get(playlist.id); + // Refreshable (singleton) system playlists: fetch the server's + // rotation-aware order (#415) and play as-is, tagged with the + // source so the reporter advances rotation. User playlists AND + // non-singleton system kinds (songs_like_artist — no by-kind + // endpoint) play the stored order via get(), untagged. + final detail = playlist.refreshable + ? await api.systemShuffle(playlist.systemVariant!) + : await api.get(playlist.id); final refs = []; for (final t in detail.tracks) { if (t.trackId == null) continue; @@ -120,6 +211,13 @@ class PlaylistCard extends ConsumerWidget { )); } if (refs.isEmpty) return; - await ref.read(playerActionsProvider).playTracks(refs, initialIndex: 0); + // Rotation-aware kinds arrive pre-ordered from the server — play + // as-is, tagged so the reporter advances rotation. No client + // shuffle. Everything else plays stored order, untagged. + await ref.read(playerActionsProvider).playTracks( + refs, + initialIndex: 0, + source: playlist.refreshable ? playlist.systemVariant : null, + ); } } diff --git a/flutter_client/lib/settings/about_section.dart b/flutter_client/lib/settings/about_section.dart new file mode 100644 index 00000000..efd5ada1 --- /dev/null +++ b/flutter_client/lib/settings/about_section.dart @@ -0,0 +1,227 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:package_info_plus/package_info_plus.dart'; + +import '../theme/theme_extension.dart'; +import '../update/client_update_provider.dart'; +import '../update/update_info.dart'; + +final _packageInfoProvider = FutureProvider((_) { + return PackageInfo.fromPlatform(); +}); + +class AboutSection extends ConsumerStatefulWidget { + const AboutSection({super.key}); + + @override + ConsumerState createState() => _AboutSectionState(); +} + +enum _InstallStage { idle, downloading, error } + +class _AboutSectionState extends ConsumerState { + DateTime? _lastChecked; + bool _checking = false; + + _InstallStage _installStage = _InstallStage.idle; + double _installProgress = 0; + String? _installError; + + Future _checkNow() async { + setState(() => _checking = true); + ref.invalidate(clientUpdateProvider); + try { + await ref.read(clientUpdateProvider.future); + } finally { + if (mounted) { + setState(() { + _checking = false; + _lastChecked = DateTime.now(); + }); + } + } + } + + Future _install(UpdateInfo info) async { + setState(() { + _installStage = _InstallStage.downloading; + _installProgress = 0; + _installError = null; + }); + try { + final installer = await ref.read(updateInstallerProvider.future); + final path = await installer.download( + info.apkUrl, + onProgress: (p) { + if (mounted) setState(() => _installProgress = p); + }, + ); + await installer.install(path); + } catch (e) { + if (!mounted) return; + setState(() { + _installStage = _InstallStage.error; + _installError = '$e'; + }); + } + } + + @override + Widget build(BuildContext context) { + final fs = Theme.of(context).extension()!; + final pkg = ref.watch(_packageInfoProvider); + final update = ref.watch(clientUpdateProvider); + + final installed = pkg.value == null + ? '…' + : '${pkg.value!.version}+${pkg.value!.buildNumber}'; + + final UpdateInfo? available = update.value; + final hasUpdate = available != null; + + return Column(crossAxisAlignment: CrossAxisAlignment.start, children: [ + const Padding( + padding: EdgeInsets.fromLTRB(16, 4, 16, 8), + child: _SectionHeader('About'), + ), + Padding( + padding: const EdgeInsets.fromLTRB(16, 0, 16, 4), + child: Row( + children: [ + Icon(Icons.info_outline, color: fs.parchment, size: 18), + const SizedBox(width: 8), + Text('Installed version', + style: TextStyle(color: fs.parchment, fontSize: 14)), + const Spacer(), + Text(installed, + style: TextStyle( + color: fs.ash, + fontSize: 13, + fontFamily: 'JetBrainsMono')), + ], + ), + ), + Padding( + padding: const EdgeInsets.fromLTRB(16, 4, 16, 4), + child: Row( + children: [ + Icon(Icons.cloud_outlined, color: fs.parchment, size: 18), + const SizedBox(width: 8), + Text('Latest version', + style: TextStyle(color: fs.parchment, fontSize: 14)), + const Spacer(), + Text( + hasUpdate ? available.version : _statusFor(update, installed), + style: TextStyle( + color: hasUpdate ? fs.accent : fs.ash, + fontSize: 13, + fontFamily: 'JetBrainsMono'), + ), + ], + ), + ), + if (_lastChecked != null) + Padding( + padding: const EdgeInsets.fromLTRB(16, 4, 16, 0), + child: Text( + 'Last checked ${_formatTime(_lastChecked!)}', + style: TextStyle(color: fs.ash, fontSize: 11), + ), + ), + Padding( + padding: const EdgeInsets.fromLTRB(16, 12, 16, 0), + child: Row( + children: [ + FilledButton.icon( + key: const Key('about_check_for_updates'), + onPressed: _checking ? null : _checkNow, + style: FilledButton.styleFrom( + backgroundColor: fs.accent, + foregroundColor: fs.parchment, + ), + icon: _checking + ? SizedBox( + width: 14, + height: 14, + child: CircularProgressIndicator( + strokeWidth: 2, + valueColor: AlwaysStoppedAnimation(fs.parchment), + ), + ) + : const Icon(Icons.refresh, size: 18), + label: Text(_checking ? 'Checking…' : 'Check for updates'), + ), + if (hasUpdate) ...[ + const SizedBox(width: 12), + FilledButton.icon( + key: const Key('about_install_update'), + onPressed: _installStage == _InstallStage.downloading + ? null + : () => _install(available), + style: FilledButton.styleFrom( + backgroundColor: fs.moss, + foregroundColor: fs.parchment, + ), + icon: _installStage == _InstallStage.downloading + ? SizedBox( + width: 14, + height: 14, + child: CircularProgressIndicator( + strokeWidth: 2, + value: _installProgress > 0 ? _installProgress : null, + valueColor: AlwaysStoppedAnimation(fs.parchment), + ), + ) + : const Icon(Icons.system_update, size: 18), + label: Text( + _installStage == _InstallStage.downloading + ? 'Downloading…' + : _installStage == _InstallStage.error + ? 'Retry install' + : 'Install ${available.version}', + ), + ), + ], + ], + ), + ), + if (_installStage == _InstallStage.error && _installError != null) + Padding( + padding: const EdgeInsets.fromLTRB(16, 8, 16, 0), + child: Text(_installError!, + style: TextStyle(color: fs.error, fontSize: 12), + maxLines: 2, + overflow: TextOverflow.ellipsis), + ), + ]); + } + + String _statusFor(AsyncValue update, String installed) { + if (update.isLoading) return 'Checking…'; + if (update.hasError) return 'Check failed'; + return 'Up to date'; + } + + String _formatTime(DateTime t) { + final h = t.hour.toString().padLeft(2, '0'); + final m = t.minute.toString().padLeft(2, '0'); + return '$h:$m'; + } +} + +class _SectionHeader extends StatelessWidget { + const _SectionHeader(this.label); + final String label; + @override + Widget build(BuildContext context) { + final fs = Theme.of(context).extension()!; + return Text( + label, + style: TextStyle( + color: fs.parchment, + fontSize: 16, + fontWeight: FontWeight.w500, + ), + ); + } +} diff --git a/flutter_client/lib/settings/settings_screen.dart b/flutter_client/lib/settings/settings_screen.dart index d1bdbab9..e8810062 100644 --- a/flutter_client/lib/settings/settings_screen.dart +++ b/flutter_client/lib/settings/settings_screen.dart @@ -9,6 +9,7 @@ import '../library/library_providers.dart' show dioProvider; import '../models/my_profile.dart'; import '../shared/widgets/main_app_bar_actions.dart'; import '../theme/theme_extension.dart'; +import 'about_section.dart'; import 'storage_section.dart'; import '../theme/theme_mode_provider.dart'; @@ -56,6 +57,8 @@ class SettingsScreen extends ConsumerWidget { _PasswordSection(), _Divider(), _ListenBrainzSection(), + _Divider(), + AboutSection(), _AdminSection(), SizedBox(height: 96), ], diff --git a/flutter_client/lib/settings/storage_section.dart b/flutter_client/lib/settings/storage_section.dart index f5488195..68e6b84c 100644 --- a/flutter_client/lib/settings/storage_section.dart +++ b/flutter_client/lib/settings/storage_section.dart @@ -4,10 +4,12 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import '../cache/audio_cache_manager.dart'; import '../cache/cache_settings_provider.dart'; import '../cache/sync_controller.dart'; +import '../likes/likes_provider.dart' show likedIdsProvider; import '../theme/theme_extension.dart'; -/// Settings card: usage display, cap selector, prefetch window selector, -/// cache-liked toggle, Clear cache + Sync now buttons. +/// Settings card: per-bucket usage, two cap selectors (Liked + +/// Recently-played), prefetch window, cache-liked toggle, Clear +/// cache + Sync now. #427 S2/S3: two independent budgets. class StorageSection extends ConsumerStatefulWidget { const StorageSection({super.key}); @@ -16,7 +18,7 @@ class StorageSection extends ConsumerStatefulWidget { } class _StorageSectionState extends ConsumerState { - int? _usageBytes; + BucketUsage? _usage; bool _syncing = false; @override @@ -25,10 +27,13 @@ class _StorageSectionState extends ConsumerState { _refreshUsage(); } + Set _likedSet() => + ref.read(likedIdsProvider).value?.tracks ?? const {}; + Future _refreshUsage() async { final mgr = ref.read(audioCacheManagerProvider); - final used = await mgr.usageBytes(); - if (mounted) setState(() => _usageBytes = used); + final u = await mgr.bucketUsage(_likedSet()); + if (mounted) setState(() => _usage = u); } String _fmtBytes(int? n) { @@ -36,10 +41,14 @@ class _StorageSectionState extends ConsumerState { if (n == 0) return '0 B'; if (n < 1024) return '$n B'; if (n < 1024 * 1024) return '${(n / 1024).toStringAsFixed(1)} KB'; - if (n < 1024 * 1024 * 1024) return '${(n / 1024 / 1024).toStringAsFixed(1)} MB'; + if (n < 1024 * 1024 * 1024) { + return '${(n / 1024 / 1024).toStringAsFixed(1)} MB'; + } return '${(n / 1024 / 1024 / 1024).toStringAsFixed(2)} GB'; } + String _cap(int bytes) => bytes == 0 ? 'unlimited' : _fmtBytes(bytes); + @override Widget build(BuildContext context) { final fs = Theme.of(context).extension()!; @@ -65,17 +74,32 @@ class _StorageSectionState extends ConsumerState { fontSize: 18, fontWeight: FontWeight.w500)), const SizedBox(height: 12), - Row(children: [ - Text('Cache usage', style: TextStyle(color: fs.ash)), - const Spacer(), - Text( - '${_fmtBytes(_usageBytes)} / ' - '${s.capBytes == 0 ? "unlimited" : _fmtBytes(s.capBytes)}', - style: TextStyle(color: fs.parchment), - ), - ]), + _usageRow('Liked', _usage?.liked, s.likedCapBytes, fs), + const SizedBox(height: 4), + _usageRow('Recently played', _usage?.rolling, + s.rollingCapBytes, fs), const SizedBox(height: 16), - _capSelector(s, fs), + _capSelector( + 'Liked cache limit', + const Key('liked_cap_selector'), + s.likedCapBytes, + (v) => ref + .read(cacheSettingsProvider.notifier) + .setLikedCapBytes(v), + s, + fs, + ), + const SizedBox(height: 8), + _capSelector( + 'Recently-played cache limit', + const Key('rolling_cap_selector'), + s.rollingCapBytes, + (v) => ref + .read(cacheSettingsProvider.notifier) + .setRollingCapBytes(v), + s, + fs, + ), const SizedBox(height: 8), _prefetchSelector(s, fs), const SizedBox(height: 8), @@ -128,29 +152,52 @@ class _StorageSectionState extends ConsumerState { ); } - Widget _capSelector(CacheSettings s, FabledSwordTheme fs) { - const options = [ - (1024 * 1024 * 1024, '1 GB'), - (5 * 1024 * 1024 * 1024, '5 GB'), - (10 * 1024 * 1024 * 1024, '10 GB'), - (25 * 1024 * 1024 * 1024, '25 GB'), - (0, 'Unlimited'), - ]; + Widget _usageRow(String label, int? used, int cap, FabledSwordTheme fs) { return Row(children: [ - Expanded(child: Text('Cache size limit', style: TextStyle(color: fs.ash))), + Text(label, style: TextStyle(color: fs.ash)), + const Spacer(), + Text('${_fmtBytes(used)} / ${_cap(cap)}', + style: TextStyle(color: fs.parchment)), + ]); + } + + static const _capOptions = [ + (1024 * 1024 * 1024, '1 GB'), + (5 * 1024 * 1024 * 1024, '5 GB'), + (10 * 1024 * 1024 * 1024, '10 GB'), + (25 * 1024 * 1024 * 1024, '25 GB'), + (0, 'Unlimited'), + ]; + + Widget _capSelector( + String label, + Key key, + int current, + Future Function(int) setCap, + CacheSettings s, + FabledSwordTheme fs, + ) { + return Row(children: [ + Expanded(child: Text(label, style: TextStyle(color: fs.ash))), DropdownButton( - key: const Key('cap_selector'), - value: options.any((o) => o.$1 == s.capBytes) - ? s.capBytes + key: key, + value: _capOptions.any((o) => o.$1 == current) + ? current : 5 * 1024 * 1024 * 1024, - items: options + items: _capOptions .map((o) => DropdownMenuItem(value: o.$1, child: Text(o.$2))) .toList(), onChanged: (v) async { if (v == null) return; - await ref.read(cacheSettingsProvider.notifier).setCapBytes(v); - if (v > 0) { - await ref.read(audioCacheManagerProvider).evict(targetBytes: v); + await setCap(v); + // Enforce immediately against the freshest settings. + final fresh = ref.read(cacheSettingsProvider).value; + if (fresh != null) { + await ref.read(audioCacheManagerProvider).evictBuckets( + likedCap: fresh.likedCapBytes, + rollingCap: fresh.rollingCapBytes, + liked: _likedSet(), + ); } await _refreshUsage(); }, @@ -161,7 +208,8 @@ class _StorageSectionState extends ConsumerState { Widget _prefetchSelector(CacheSettings s, FabledSwordTheme fs) { const options = [1, 3, 5, 7, 10]; return Row(children: [ - Expanded(child: Text('Pre-fetch ahead', style: TextStyle(color: fs.ash))), + Expanded( + child: Text('Pre-fetch ahead', style: TextStyle(color: fs.ash))), DropdownButton( key: const Key('prefetch_selector'), value: options.contains(s.prefetchWindow) ? s.prefetchWindow : 5, @@ -183,8 +231,8 @@ class _StorageSectionState extends ConsumerState { builder: (ctx) => AlertDialog( title: const Text('Clear cache?'), content: const Text( - 'This deletes all cached audio files (including manually downloaded ones). ' - 'The next play of any track will re-download from the server.', + 'This deletes all cached audio (liked and recently-played). ' + 'The next play of any track re-downloads from the server.', ), actions: [ TextButton( diff --git a/flutter_client/pubspec.yaml b/flutter_client/pubspec.yaml index 4a03b811..03c19c18 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.14+5 +version: 2026.5.15+6 environment: sdk: '>=3.5.0 <4.0.0' diff --git a/flutter_client/test/cache/audio_cache_manager_test.dart b/flutter_client/test/cache/audio_cache_manager_test.dart index 669279f1..433f5ae1 100644 --- a/flutter_client/test/cache/audio_cache_manager_test.dart +++ b/flutter_client/test/cache/audio_cache_manager_test.dart @@ -4,6 +4,7 @@ library; import 'dart:io'; import 'package:dio/dio.dart'; +import 'package:drift/drift.dart' show Value; import 'package:drift/native.dart' show NativeDatabase; import 'package:flutter_test/flutter_test.dart'; @@ -54,7 +55,8 @@ void main() { expect(await mgr.usageBytes(), 350); }); - test('eviction order: incidental first, manual never', skip: _skipDrift, () async { + test('rolling cap evicts non-liked LRU; liked protected', + skip: _skipDrift, () async { final db = _testDb(); addTearDown(db.close); final tmp = Directory.systemTemp.createTempSync(); @@ -63,30 +65,47 @@ void main() { dioFactory: () async => Dio(), cacheDirFactory: () async => tmp, ); - final f1 = File('${tmp.path}/audio_cache/inc.mp3'); - await f1.create(recursive: true); - await f1.writeAsBytes(List.filled(100, 0)); - final f2 = File('${tmp.path}/audio_cache/man.mp3'); - await f2.create(recursive: true); - await f2.writeAsBytes(List.filled(100, 0)); + Future mk(String id) async { + final f = File('${tmp.path}/audio_cache/$id.mp3'); + await f.create(recursive: true); + await f.writeAsBytes(List.filled(100, 0)); + } + await mk('old'); + await mk('new'); + await mk('lik'); await db.batch((b) { b.insertAll(db.audioCacheIndex, [ + // 'old' has the older lastPlayedAt → evicted first. AudioCacheIndexCompanion.insert( - trackId: 'inc', - path: f1.path, + trackId: 'old', + path: '${tmp.path}/audio_cache/old.mp3', sizeBytes: 100, - source: CacheSource.incidental), + source: CacheSource.incidental, + lastPlayedAt: Value(DateTime(2020))), AudioCacheIndexCompanion.insert( - trackId: 'man', - path: f2.path, + trackId: 'new', + path: '${tmp.path}/audio_cache/new.mp3', sizeBytes: 100, - source: CacheSource.manual), + source: CacheSource.incidental, + lastPlayedAt: Value(DateTime(2024))), + AudioCacheIndexCompanion.insert( + trackId: 'lik', + path: '${tmp.path}/audio_cache/lik.mp3', + sizeBytes: 100, + source: CacheSource.incidental, + lastPlayedAt: Value(DateTime(2019))), ]); }); - expect(await mgr.usageBytes(), 200); - await mgr.evict(targetBytes: 100); - expect(await mgr.isCached('inc'), false); - expect(await mgr.isCached('man'), true); + final liked = {'lik'}; + final u = await mgr.bucketUsage(liked); + expect(u.liked, 100); + expect(u.rolling, 200); + // Rolling cap fits one 100-byte file; Liked cap huge. + await mgr.evictBuckets( + likedCap: 1 << 30, rollingCap: 100, liked: liked); + expect(await mgr.isCached('old'), false); // oldest rolling, evicted + expect(await mgr.isCached('new'), true); // newer rolling, kept + expect(await mgr.isCached('lik'), true); // liked, protected }); test('clearAll removes everything including manual', skip: _skipDrift, () async { diff --git a/flutter_client/test/cache/cache_settings_provider_test.dart b/flutter_client/test/cache/cache_settings_provider_test.dart index 0df3681d..284800e4 100644 --- a/flutter_client/test/cache/cache_settings_provider_test.dart +++ b/flutter_client/test/cache/cache_settings_provider_test.dart @@ -27,7 +27,9 @@ void main() { addTearDown(container.dispose); final s = await container.read(cacheSettingsProvider.future); - expect(s.capBytes, CacheSettings.defaults.capBytes); + expect(s.likedCapBytes, CacheSettings.defaults.likedCapBytes); + expect(s.rollingCapBytes, CacheSettings.defaults.rollingCapBytes); + expect(s.likedCapBytes, 5 * 1024 * 1024 * 1024); expect(s.prefetchWindow, 5); expect(s.cacheLikedTracks, true); }); diff --git a/flutter_client/test/playlists/widgets/playlist_card_test.dart b/flutter_client/test/playlists/widgets/playlist_card_test.dart index 2d04a589..69086907 100644 --- a/flutter_client/test/playlists/widgets/playlist_card_test.dart +++ b/flutter_client/test/playlists/widgets/playlist_card_test.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:minstrel/models/playlist.dart'; @@ -35,24 +36,48 @@ const _forYou = Playlist( void main() { testWidgets('renders name and no badge for user playlist', (tester) async { - await tester.pumpWidget(MaterialApp( + await tester.pumpWidget(ProviderScope( + child: MaterialApp( theme: buildThemeData(), home: const Scaffold( body: PlaylistCard(playlist: _userPlaylist), ), - )); + ))); expect(find.text('Road trip'), findsOneWidget); expect(find.text('for you'), findsNothing); }); testWidgets('renders system badge for system playlist', (tester) async { - await tester.pumpWidget(MaterialApp( + await tester.pumpWidget(ProviderScope( + child: MaterialApp( theme: buildThemeData(), home: const Scaffold( body: PlaylistCard(playlist: _forYou), ), - )); + ))); expect(find.text('For You'), findsOneWidget); expect(find.text('for you'), findsOneWidget); }); + + testWidgets('shows refresh kebab on system playlists', (tester) async { + await tester.pumpWidget(ProviderScope( + child: MaterialApp( + theme: buildThemeData(), + home: const Scaffold( + body: PlaylistCard(playlist: _forYou), + ), + ))); + expect(find.byIcon(Icons.more_vert), findsOneWidget); + }); + + testWidgets('no refresh kebab on user playlists', (tester) async { + await tester.pumpWidget(ProviderScope( + child: MaterialApp( + theme: buildThemeData(), + home: const Scaffold( + body: PlaylistCard(playlist: _userPlaylist), + ), + ))); + expect(find.byIcon(Icons.more_vert), findsNothing); + }); } diff --git a/flutter_client/test/settings/storage_section_test.dart b/flutter_client/test/settings/storage_section_test.dart index 816d2815..f4711bb9 100644 --- a/flutter_client/test/settings/storage_section_test.dart +++ b/flutter_client/test/settings/storage_section_test.dart @@ -46,12 +46,14 @@ void main() { await t.pumpAndSettle(); expect(find.text('Storage'), findsOneWidget); - expect(find.text('Cache size limit'), findsOneWidget); + expect(find.text('Liked cache limit'), findsOneWidget); + expect(find.text('Recently-played cache limit'), findsOneWidget); expect(find.text('Pre-fetch ahead'), findsOneWidget); expect(find.byKey(const Key('clear_cache_button')), findsOneWidget); expect(find.byKey(const Key('sync_now_button')), findsOneWidget); expect(find.byKey(const Key('cache_liked_toggle')), findsOneWidget); - expect(find.byKey(const Key('cap_selector')), findsOneWidget); + expect(find.byKey(const Key('liked_cap_selector')), findsOneWidget); + expect(find.byKey(const Key('rolling_cap_selector')), findsOneWidget); expect(find.byKey(const Key('prefetch_selector')), findsOneWidget); }); diff --git a/internal/api/api.go b/internal/api/api.go index 6e039e5d..4f724493 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -74,6 +74,7 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev authed.Get("/artists/{id}/tracks", h.handleGetArtistTracks) authed.Get("/albums/{id}", h.handleGetAlbum) authed.Get("/albums/{id}/cover", h.handleGetCover) + authed.Get("/library/shuffle", h.handleLibraryShuffle) authed.Get("/library/albums", h.handleListLibraryAlbums) authed.Get("/library/sync", h.handleLibrarySync) authed.Get("/tracks/{id}", h.handleGetTrack) @@ -172,8 +173,8 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev authed.Delete("/playlists/{id}/tracks/{position}", h.handleRemovePlaylistTrack) authed.Put("/playlists/{id}/tracks", h.handleReorderPlaylist) authed.Get("/playlists/{id}/cover", h.handleGetPlaylistCover) - authed.Post("/playlists/system/discover/refresh", h.handleDiscoverRefresh) - authed.Post("/playlists/system/for-you/refresh", h.handleForYouRefresh) + authed.Post("/playlists/system/{kind}/refresh", h.handleSystemPlaylistRefresh) + authed.Get("/playlists/system/{kind}/shuffle", h.handleSystemPlaylistShuffle) }) }) } diff --git a/internal/api/events.go b/internal/api/events.go index 2e95ee36..c7e49f16 100644 --- a/internal/api/events.go +++ b/internal/api/events.go @@ -20,6 +20,10 @@ type eventRequest struct { PositionMs *int32 `json:"position_ms"` At *string `json:"at"` ClientID *string `json:"client_id"` + // Source tags which surface a play_started came from. "for_you" + // / "discover" feed the system-playlist rotation dedup (#415); + // absent / "" for library, user-playlist, radio, Subsonic. + Source *string `json:"source"` } type playStartedResponse struct { @@ -61,6 +65,8 @@ func (h *handlers) handleEvents(w http.ResponseWriter, r *http.Request) { h.handleEventPlayEnded(w, r, user, req, at) case "play_skipped": h.handleEventPlaySkipped(w, r, user, req, at) + case "play_offline": + h.handleEventPlayOffline(w, r, user, req, at, clientID) default: writeErr(w, apierror.BadRequest("bad_request", "unknown event type")) } @@ -84,7 +90,11 @@ func (h *handlers) handleEventPlayStarted( writeErr(w, apierror.InternalMsg("lookup failed", err)) return } - res, err := h.events.RecordPlayStarted(r.Context(), user.ID, trackID, clientID, at) + source := "" + if req.Source != nil { + source = *req.Source + } + res, err := h.events.RecordPlayStartedWithSource(r.Context(), user.ID, trackID, clientID, source, at) if err != nil { h.logger.Error("api: events: play_started", "err", err) writeErr(w, apierror.InternalMsg("record failed", err)) @@ -96,6 +106,48 @@ func (h *handlers) handleEventPlayStarted( }) } +// handleEventPlayOffline records a complete play that happened with +// no/flaky connectivity, replayed from the Flutter offline mutation +// queue (#426 part B). `at` is the original (client) play-start time +// — parsed by handleEvents from req.At — so history lands on the +// real timeline, not replay time. duration_played_ms drives the skip +// rule; source advances #415 rotation for system-playlist plays. +func (h *handlers) handleEventPlayOffline( + w http.ResponseWriter, r *http.Request, + user dbq.User, req eventRequest, at time.Time, clientID string, +) { + trackID, ok := parseUUID(req.TrackID) + if !ok { + writeErr(w, apierror.BadRequest("bad_request", "invalid track_id")) + return + } + if req.DurationPlayedMs == nil || *req.DurationPlayedMs < 0 { + writeErr(w, apierror.BadRequest("bad_request", "duration_played_ms required and must be >= 0")) + return + } + if _, err := dbq.New(h.pool).GetTrackByID(r.Context(), trackID); err != nil { + if errors.Is(err, pgx.ErrNoRows) { + writeErr(w, &apierror.Error{Status: 404, Code: "not_found", Message: "track not found"}) + return + } + h.logger.Error("api: events: offline lookup track", "err", err) + writeErr(w, apierror.InternalMsg("lookup failed", err)) + return + } + source := "" + if req.Source != nil { + source = *req.Source + } + if err := h.events.RecordOfflinePlay( + r.Context(), user.ID, trackID, clientID, source, at, *req.DurationPlayedMs, + ); err != nil { + h.logger.Error("api: events: play_offline", "err", err) + writeErr(w, apierror.InternalMsg("record failed", err)) + return + } + writeJSON(w, http.StatusOK, okResponse{OK: true}) +} + func (h *handlers) handleEventPlayEnded( w http.ResponseWriter, r *http.Request, user dbq.User, req eventRequest, at time.Time, diff --git a/internal/api/library.go b/internal/api/library.go index 206c5549..1bfc7523 100644 --- a/internal/api/library.go +++ b/internal/api/library.go @@ -3,6 +3,7 @@ package api import ( "errors" "net/http" + "strconv" "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgtype" @@ -158,6 +159,43 @@ func (h *handlers) handleGetArtistTracks(w http.ResponseWriter, r *http.Request) writeJSON(w, http.StatusOK, out) } +// handleLibraryShuffle implements GET /api/library/shuffle?limit=N — +// the online source for the client's always-present "Shuffle all" +// (#427 S4). N random tracks across the whole library, per-user +// quarantine filtered. limit defaults to 100, clamped to 1..500. +// Offline, the client shuffles its local cache instead and never +// calls this. +func (h *handlers) handleLibraryShuffle(w http.ResponseWriter, r *http.Request) { + user, ok := requireUser(w, r) + if !ok { + return + } + limit := 100 + if v := r.URL.Query().Get("limit"); v != "" { + if n, err := strconv.Atoi(v); err == nil { + limit = n + } + } + if limit < 1 { + limit = 1 + } + if limit > 500 { + limit = 500 + } + rows, err := dbq.New(h.pool).ListRandomTracksForUser(r.Context(), + dbq.ListRandomTracksForUserParams{UserID: user.ID, Limit: int32(limit)}) + if err != nil { + h.logger.Error("api: library shuffle", "err", err) + writeErr(w, apierror.InternalMsg("shuffle failed", err)) + return + } + out := make([]TrackRef, 0, len(rows)) + for _, row := range rows { + out = append(out, trackRefFrom(row.Track, row.AlbumTitle, row.ArtistName)) + } + writeJSON(w, http.StatusOK, out) +} + // handleListArtists implements GET /api/artists with two sort modes // (alpha|newest) and enveloped pagination. The alpha branch uses // ListArtistsAlphaWithCovers to ship cover_url + album_count in a diff --git a/internal/api/playlists.go b/internal/api/playlists.go index 1a94f5a9..e74291d3 100644 --- a/internal/api/playlists.go +++ b/internal/api/playlists.go @@ -32,12 +32,17 @@ type playlistRowView struct { IsPublic bool `json:"is_public"` Kind string `json:"kind"` SystemVariant *string `json:"system_variant,omitempty"` - SeedArtistID *string `json:"seed_artist_id,omitempty"` - CoverURL string `json:"cover_url,omitempty"` - TrackCount int32 `json:"track_count"` - DurationSec int32 `json:"duration_sec"` - CreatedAt string `json:"created_at"` - UpdatedAt string `json:"updated_at"` + // Refreshable: a singleton system kind addressable by the generic + // /system/{kind}/{refresh,shuffle} endpoints. Lets clients show + // the per-tile refresh affordance generically without hardcoding + // which kinds support it (false for user + songs_like_artist). + Refreshable bool `json:"refreshable"` + SeedArtistID *string `json:"seed_artist_id,omitempty"` + CoverURL string `json:"cover_url,omitempty"` + TrackCount int32 `json:"track_count"` + DurationSec int32 `json:"duration_sec"` + CreatedAt string `json:"created_at"` + UpdatedAt string `json:"updated_at"` } // playlistTrackView is one row of a playlist's track list on the wire. @@ -435,6 +440,7 @@ func playlistRowToView(r *playlists.PlaylistRow) playlistRowView { IsPublic: r.IsPublic, Kind: r.Kind, SystemVariant: r.SystemVariant, + Refreshable: r.SystemVariant != nil && playlists.RefreshableSystemKind(*r.SystemVariant), TrackCount: r.TrackCount, DurationSec: r.DurationSec, CreatedAt: formatTimestamp(r.CreatedAt), diff --git a/internal/api/playlists_discover_refresh.go b/internal/api/playlists_discover_refresh.go deleted file mode 100644 index 7ec2cf9f..00000000 --- a/internal/api/playlists_discover_refresh.go +++ /dev/null @@ -1,69 +0,0 @@ -package api - -import ( - "errors" - "net/http" - "time" - - "github.com/jackc/pgx/v5" - - "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" - "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" - "git.fabledsword.com/bvandeusen/minstrel/internal/playlists" -) - -// discoverRefreshResp is the wire shape for POST -// /api/playlists/system/discover/refresh. playlist_id is null when -// the build succeeded but the user's library yielded no eligible -// tracks (degenerate empty-library); track_count is 0 in that case. -type discoverRefreshResp struct { - PlaylistID *string `json:"playlist_id"` - TrackCount int32 `json:"track_count"` -} - -// handleDiscoverRefresh re-runs BuildSystemPlaylists synchronously -// for the calling user, then returns the resulting Discover row's -// id + track_count. Used by the frontend's refresh affordances on -// the detail page header and the home tile kebab. -// -// Authenticated user only (gated by the authed sub-router's -// RequireUser middleware). Each user only ever refreshes their own -// Discover — no admin route, no cross-user impersonation. -func (h *handlers) handleDiscoverRefresh(w http.ResponseWriter, r *http.Request) { - user, ok := requireUser(w, r) - if !ok { - return - } - if err := playlists.BuildSystemPlaylists(r.Context(), h.pool, h.logger, user.ID, time.Now(), h.dataDir); err != nil { - h.logger.Error("discover refresh: build failed", - "user_id", uuidToString(user.ID), "err", err) - writeErr(w, apierror.InternalMsg("build failed", err)) - return - } - - sysVariant := "discover" - pl, err := dbq.New(h.pool).GetSystemPlaylistByVariantForUser(r.Context(), - dbq.GetSystemPlaylistByVariantForUserParams{ - UserID: user.ID, - SystemVariant: &sysVariant, - }) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - // Build succeeded but no Discover row produced — empty library. - writeJSON(w, http.StatusOK, discoverRefreshResp{ - PlaylistID: nil, TrackCount: 0, - }) - return - } - h.logger.Error("discover refresh: lookup failed", - "user_id", uuidToString(user.ID), "err", err) - writeErr(w, apierror.InternalMsg("lookup failed", err)) - return - } - - idStr := uuidToString(pl.ID) - writeJSON(w, http.StatusOK, discoverRefreshResp{ - PlaylistID: &idStr, - TrackCount: pl.TrackCount, - }) -} diff --git a/internal/api/playlists_discover_refresh_test.go b/internal/api/playlists_discover_refresh_test.go deleted file mode 100644 index 0134c622..00000000 --- a/internal/api/playlists_discover_refresh_test.go +++ /dev/null @@ -1,64 +0,0 @@ -package api - -import ( - "encoding/json" - "net/http" - "net/http/httptest" - "os" - "testing" - - "github.com/go-chi/chi/v5" - - "git.fabledsword.com/bvandeusen/minstrel/internal/auth" -) - -func newDiscoverRefreshRouter(h *handlers) chi.Router { - r := chi.NewRouter() - r.Route("/api", func(api chi.Router) { - api.Use(auth.RequireUser(h.pool)) - api.Post("/playlists/system/discover/refresh", h.handleDiscoverRefresh) - }) - return r -} - -func TestDiscoverRefresh_AuthenticatedUser_Returns200(t *testing.T) { - if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" { - t.Skip("MINSTREL_TEST_DATABASE_URL not set") - } - h, pool := testHandlers(t) - user := seedUser(t, pool, "discrefresh", "pw", false) - - req := httptest.NewRequest(http.MethodPost, "/api/playlists/system/discover/refresh", nil) - req = withUser(req, user) - rec := httptest.NewRecorder() - newDiscoverRefreshRouter(h).ServeHTTP(rec, req) - - if rec.Code != http.StatusOK { - t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String()) - } - var resp discoverRefreshResp - if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { - t.Fatalf("decode: %v", err) - } - // On a fresh user with no library, the build runs and emits a - // Discover playlist (or no row if library is truly empty). - // Either case is OK; we only assert the response shape decoded. - if resp.TrackCount < 0 { - t.Errorf("track_count = %d, want >= 0", resp.TrackCount) - } -} - -func TestDiscoverRefresh_NoAuth_Returns401(t *testing.T) { - if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" { - t.Skip("MINSTREL_TEST_DATABASE_URL not set") - } - h, _ := testHandlers(t) - - req := httptest.NewRequest(http.MethodPost, "/api/playlists/system/discover/refresh", nil) - rec := httptest.NewRecorder() - newDiscoverRefreshRouter(h).ServeHTTP(rec, req) - - if rec.Code != http.StatusUnauthorized { - t.Errorf("status = %d, want 401", rec.Code) - } -} diff --git a/internal/api/playlists_foryou_refresh.go b/internal/api/playlists_foryou_refresh.go deleted file mode 100644 index eee83f8f..00000000 --- a/internal/api/playlists_foryou_refresh.go +++ /dev/null @@ -1,98 +0,0 @@ -package api - -import ( - "errors" - "net/http" - "time" - - "github.com/jackc/pgx/v5" - - "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" - "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" - "git.fabledsword.com/bvandeusen/minstrel/internal/playlists" -) - -// foryouRefreshResp is the wire shape for POST -// /api/playlists/system/for-you/refresh. track_ids is the playlist's -// tracks in playlist position order — the frontend enqueues this -// list directly to start playback without a second roundtrip. -// -// playlist_id is null + track_ids empty when the build succeeded but -// the user's library yielded no eligible candidates (degenerate -// empty-library case). -type foryouRefreshResp struct { - PlaylistID *string `json:"playlist_id"` - TrackCount int32 `json:"track_count"` - TrackIDs []string `json:"track_ids"` -} - -// handleForYouRefresh re-runs BuildSystemPlaylists synchronously for -// the calling user and returns their freshly-built For-You playlist. -// Used by the home Playlists row's For-You tile play button: click -// triggers refresh, the response carries the track IDs, the -// frontend enqueues + auto-plays. -// -// Authenticated user only — each user refreshes only their own -// For-You. -func (h *handlers) handleForYouRefresh(w http.ResponseWriter, r *http.Request) { - user, ok := requireUser(w, r) - if !ok { - return - } - if err := playlists.BuildSystemPlaylists(r.Context(), h.pool, h.logger, user.ID, time.Now(), h.dataDir); err != nil { - h.logger.Error("for-you refresh: build failed", - "user_id", uuidToString(user.ID), "err", err) - writeErr(w, apierror.InternalMsg("build failed", err)) - return - } - - q := dbq.New(h.pool) - sysVariant := "for_you" - pl, err := q.GetSystemPlaylistByVariantForUser(r.Context(), - dbq.GetSystemPlaylistByVariantForUserParams{ - UserID: user.ID, - SystemVariant: &sysVariant, - }) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - // Build succeeded but no For-You row — empty library. - writeJSON(w, http.StatusOK, foryouRefreshResp{ - PlaylistID: nil, - TrackCount: 0, - TrackIDs: []string{}, - }) - return - } - h.logger.Error("for-you refresh: lookup failed", - "user_id", uuidToString(user.ID), "err", err) - writeErr(w, apierror.InternalMsg("lookup failed", err)) - return - } - - // Fetch tracks in playlist position order so the frontend can - // enqueue them directly. - rows, err := q.ListPlaylistTracks(r.Context(), pl.ID) - if err != nil { - h.logger.Error("for-you refresh: list tracks failed", - "playlist_id", uuidToString(pl.ID), "err", err) - writeErr(w, apierror.InternalMsg("list tracks failed", err)) - return - } - trackIDs := make([]string, 0, len(rows)) - for _, row := range rows { - // Skip rows where the track was removed from the library - // (LiveTrackID won't be Valid). The snapshot row still - // exists in playlist_tracks, but there's nothing to enqueue. - if !row.LiveTrackID.Valid { - continue - } - trackIDs = append(trackIDs, uuidToString(row.LiveTrackID)) - } - - idStr := uuidToString(pl.ID) - writeJSON(w, http.StatusOK, foryouRefreshResp{ - PlaylistID: &idStr, - TrackCount: pl.TrackCount, - TrackIDs: trackIDs, - }) -} diff --git a/internal/api/playlists_foryou_refresh_test.go b/internal/api/playlists_foryou_refresh_test.go deleted file mode 100644 index 76b6122a..00000000 --- a/internal/api/playlists_foryou_refresh_test.go +++ /dev/null @@ -1,67 +0,0 @@ -package api - -import ( - "encoding/json" - "net/http" - "net/http/httptest" - "os" - "testing" - - "github.com/go-chi/chi/v5" - - "git.fabledsword.com/bvandeusen/minstrel/internal/auth" -) - -func newForYouRefreshRouter(h *handlers) chi.Router { - r := chi.NewRouter() - r.Route("/api", func(api chi.Router) { - api.Use(auth.RequireUser(h.pool)) - api.Post("/playlists/system/for-you/refresh", h.handleForYouRefresh) - }) - return r -} - -func TestForYouRefresh_AuthenticatedUser_Returns200(t *testing.T) { - if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" { - t.Skip("MINSTREL_TEST_DATABASE_URL not set") - } - h, pool := testHandlers(t) - user := seedUser(t, pool, "foryourefresh", "pw", false) - - req := httptest.NewRequest(http.MethodPost, "/api/playlists/system/for-you/refresh", nil) - req = withUser(req, user) - rec := httptest.NewRecorder() - newForYouRefreshRouter(h).ServeHTTP(rec, req) - - if rec.Code != http.StatusOK { - t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String()) - } - var resp foryouRefreshResp - if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { - t.Fatalf("decode: %v", err) - } - if resp.TrackIDs == nil { - t.Errorf("track_ids = nil, want []string (empty array, not null)") - } - if int(resp.TrackCount) != len(resp.TrackIDs) { - // Acceptable mismatch only when some tracks were removed from - // the library between build and response. For a fresh test - // user the counts should match. - t.Errorf("track_count=%d, len(track_ids)=%d — mismatch", resp.TrackCount, len(resp.TrackIDs)) - } -} - -func TestForYouRefresh_NoAuth_Returns401(t *testing.T) { - if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" { - t.Skip("MINSTREL_TEST_DATABASE_URL not set") - } - h, _ := testHandlers(t) - - req := httptest.NewRequest(http.MethodPost, "/api/playlists/system/for-you/refresh", nil) - rec := httptest.NewRecorder() - newForYouRefreshRouter(h).ServeHTTP(rec, req) - - if rec.Code != http.StatusUnauthorized { - t.Errorf("status = %d, want 401", rec.Code) - } -} diff --git a/internal/api/playlists_system_shuffle.go b/internal/api/playlists_system_shuffle.go new file mode 100644 index 00000000..bca19717 --- /dev/null +++ b/internal/api/playlists_system_shuffle.go @@ -0,0 +1,202 @@ +package api + +import ( + "errors" + "math/rand" + "net/http" + "time" + + "github.com/go-chi/chi/v5" + "github.com/jackc/pgx/v5" + + "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/playlists" +) + +// Generic registry-driven system-playlist endpoints (#411 R2), +// replacing the former per-kind handlers. {kind} is the raw +// system_variant ('for_you' | 'discover' | future singleton kinds); +// only singleton kinds (playlists.RefreshableSystemKind) are +// addressable here — songs_like_artist is multi-per-user and played +// by playlist id, so it 404s. +// +// GET /api/playlists/system/{kind}/shuffle — the caller's playlist +// for that kind with tracks in rotation-aware order (#415): unheard +// this rotation first (shuffled), then heard; resets when exhausted. +// Same JSON shape as GET /api/playlists/{id} so clients reuse track +// parsing. Intentionally uncached — it varies per play, unlike the +// cached detail GET. +// +// POST /api/playlists/system/{kind}/refresh — rebuilds the caller's +// system playlists and returns that kind's fresh row. + +func systemKindFromURL(w http.ResponseWriter, r *http.Request) (string, bool) { + kind := chi.URLParam(r, "kind") + if !playlists.RefreshableSystemKind(kind) { + writeErr(w, &apierror.Error{Status: 404, Code: "not_found", Message: "unknown system playlist"}) + return "", false + } + return kind, true +} + +func (h *handlers) handleSystemPlaylistShuffle(w http.ResponseWriter, r *http.Request) { + kind, ok := systemKindFromURL(w, r) + if !ok { + return + } + h.serveSystemPlaylistShuffle(w, r, kind) +} + +// systemRefreshResp unifies the prior for-you/discover shapes +// (for-you carried track_ids, discover didn't); clients ignore +// extra fields. playlist_id null + empty when the build produced no +// row for that kind (empty library). +type systemRefreshResp struct { + PlaylistID *string `json:"playlist_id"` + TrackCount int32 `json:"track_count"` + TrackIDs []string `json:"track_ids"` +} + +func (h *handlers) handleSystemPlaylistRefresh(w http.ResponseWriter, r *http.Request) { + user, ok := requireUser(w, r) + if !ok { + return + } + kind, ok := systemKindFromURL(w, r) + if !ok { + return + } + if err := playlists.BuildSystemPlaylists(r.Context(), h.pool, h.logger, user.ID, time.Now(), h.dataDir); err != nil { + h.logger.Error("system refresh: build failed", + "kind", kind, "user_id", uuidToString(user.ID), "err", err) + writeErr(w, apierror.InternalMsg("build failed", err)) + return + } + q := dbq.New(h.pool) + v := kind + pl, err := q.GetSystemPlaylistByVariantForUser(r.Context(), + dbq.GetSystemPlaylistByVariantForUserParams{UserID: user.ID, SystemVariant: &v}) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + writeJSON(w, http.StatusOK, systemRefreshResp{TrackIDs: []string{}}) + return + } + h.logger.Error("system refresh: lookup failed", + "kind", kind, "user_id", uuidToString(user.ID), "err", err) + writeErr(w, apierror.InternalMsg("lookup failed", err)) + return + } + rows, err := q.ListPlaylistTracks(r.Context(), pl.ID) + if err != nil { + h.logger.Error("system refresh: list tracks failed", + "kind", kind, "playlist_id", uuidToString(pl.ID), "err", err) + writeErr(w, apierror.InternalMsg("list tracks failed", err)) + return + } + trackIDs := make([]string, 0, len(rows)) + for _, row := range rows { + if !row.LiveTrackID.Valid { + continue + } + trackIDs = append(trackIDs, uuidToString(row.LiveTrackID)) + } + idStr := uuidToString(pl.ID) + writeJSON(w, http.StatusOK, systemRefreshResp{ + PlaylistID: &idStr, + TrackCount: pl.TrackCount, + TrackIDs: trackIDs, + }) +} + +func (h *handlers) serveSystemPlaylistShuffle(w http.ResponseWriter, r *http.Request, variant string) { + user, ok := requireUser(w, r) + if !ok { + return + } + q := dbq.New(h.pool) + + v := variant + pl, err := q.GetSystemPlaylistByVariantForUser(r.Context(), + dbq.GetSystemPlaylistByVariantForUserParams{ + UserID: user.ID, + SystemVariant: &v, + }) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + // No snapshot yet (empty library / not built). Empty list, + // not an error — the client just has nothing to play. + writeJSON(w, http.StatusOK, playlistDetailView{ + Tracks: []playlistTrackView{}, + }) + return + } + h.logger.Error("system shuffle: lookup failed", "variant", variant, "err", err) + writeErr(w, apierror.InternalMsg("lookup failed", err)) + return + } + + detail, err := h.playlists.Get(r.Context(), user.ID, pl.ID) + if err != nil { + h.writePlaylistErr(w, err, "system shuffle") + return + } + + // Played set for the current rotation. ErrNoRows = nothing played + // from this playlist yet → empty set, everything counts unplayed. + played := map[[16]byte]bool{} + st, rerr := q.GetRotationState(r.Context(), dbq.GetRotationStateParams{ + UserID: user.ID, PlaylistKind: variant, + }) + if rerr != nil && !errors.Is(rerr, pgx.ErrNoRows) { + h.logger.Error("system shuffle: rotation lookup failed", "variant", variant, "err", rerr) + writeErr(w, apierror.InternalMsg("rotation lookup failed", rerr)) + return + } + if rerr == nil { + for _, id := range st.PlayedTrackIds { + if id.Valid { + played[id.Bytes] = true + } + } + } + + // Partition playable tracks. Rows whose upstream track was deleted + // (TrackID nil) are dropped — nothing to enqueue. + unplayed := make([]playlists.PlaylistTrack, 0, len(detail.Tracks)) + heard := make([]playlists.PlaylistTrack, 0) + for _, t := range detail.Tracks { + if t.TrackID == nil || !t.TrackID.Valid { + continue + } + if played[t.TrackID.Bytes] { + heard = append(heard, t) + } else { + unplayed = append(unplayed, t) + } + } + + // Whole snapshot heard → reset the rotation and treat everything + // as fresh. Reset failure is non-fatal: we still return a + // playable reshuffled list; the next play just re-resets. + if len(unplayed) == 0 && len(heard) > 0 { + if err := q.ResetRotationState(r.Context(), dbq.ResetRotationStateParams{ + UserID: user.ID, PlaylistKind: variant, + }); err != nil { + h.logger.Warn("system shuffle: rotation reset failed", + "variant", variant, "err", err) + } + unplayed = heard + heard = nil + } + + rand.Shuffle(len(unplayed), func(i, j int) { + unplayed[i], unplayed[j] = unplayed[j], unplayed[i] + }) + rand.Shuffle(len(heard), func(i, j int) { + heard[i], heard[j] = heard[j], heard[i] + }) + + detail.Tracks = append(unplayed, heard...) + writeJSON(w, http.StatusOK, playlistDetailToView(detail)) +} diff --git a/internal/api/playlists_system_test.go b/internal/api/playlists_system_test.go new file mode 100644 index 00000000..1a1570e1 --- /dev/null +++ b/internal/api/playlists_system_test.go @@ -0,0 +1,111 @@ +package api + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "testing" + + "github.com/go-chi/chi/v5" + + "git.fabledsword.com/bvandeusen/minstrel/internal/auth" +) + +// Generic registry-driven system-playlist endpoints (#411 R2). +// Replaces the former per-kind refresh handler tests. +func newSystemPlaylistRouter(h *handlers) chi.Router { + r := chi.NewRouter() + r.Route("/api", func(api chi.Router) { + api.Use(auth.RequireUser(h.pool)) + api.Post("/playlists/system/{kind}/refresh", h.handleSystemPlaylistRefresh) + api.Get("/playlists/system/{kind}/shuffle", h.handleSystemPlaylistShuffle) + }) + return r +} + +func TestSystemRefresh_ForYou_Returns200(t *testing.T) { + if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" { + t.Skip("MINSTREL_TEST_DATABASE_URL not set") + } + h, pool := testHandlers(t) + user := seedUser(t, pool, "sysforyou", "pw", false) + + req := httptest.NewRequest(http.MethodPost, "/api/playlists/system/for_you/refresh", nil) + req = withUser(req, user) + rec := httptest.NewRecorder() + newSystemPlaylistRouter(h).ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String()) + } + var resp systemRefreshResp + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v", err) + } + if resp.TrackIDs == nil { + t.Errorf("track_ids = nil, want [] (empty array, not null)") + } + if int(resp.TrackCount) != len(resp.TrackIDs) { + t.Errorf("track_count=%d, len(track_ids)=%d — mismatch", resp.TrackCount, len(resp.TrackIDs)) + } +} + +func TestSystemRefresh_Discover_Returns200(t *testing.T) { + if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" { + t.Skip("MINSTREL_TEST_DATABASE_URL not set") + } + h, pool := testHandlers(t) + user := seedUser(t, pool, "sysdiscover", "pw", false) + + req := httptest.NewRequest(http.MethodPost, "/api/playlists/system/discover/refresh", nil) + req = withUser(req, user) + rec := httptest.NewRecorder() + newSystemPlaylistRouter(h).ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String()) + } + var resp systemRefreshResp + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v", err) + } + if resp.TrackCount < 0 { + t.Errorf("track_count = %d, want >= 0", resp.TrackCount) + } +} + +// A non-singleton (songs_like_artist) or unknown kind isn't +// addressable by the generic endpoints → 404, not a build. +func TestSystemRefresh_NonSingletonKind_Returns404(t *testing.T) { + if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" { + t.Skip("MINSTREL_TEST_DATABASE_URL not set") + } + h, pool := testHandlers(t) + user := seedUser(t, pool, "sysbadkind", "pw", false) + + for _, kind := range []string{"songs_like_artist", "bogus"} { + req := httptest.NewRequest(http.MethodPost, "/api/playlists/system/"+kind+"/refresh", nil) + req = withUser(req, user) + rec := httptest.NewRecorder() + newSystemPlaylistRouter(h).ServeHTTP(rec, req) + if rec.Code != http.StatusNotFound { + t.Errorf("kind=%s: status = %d, want 404", kind, rec.Code) + } + } +} + +func TestSystemRefresh_NoAuth_Returns401(t *testing.T) { + if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" { + t.Skip("MINSTREL_TEST_DATABASE_URL not set") + } + h, _ := testHandlers(t) + + req := httptest.NewRequest(http.MethodPost, "/api/playlists/system/for_you/refresh", nil) + rec := httptest.NewRecorder() + newSystemPlaylistRouter(h).ServeHTTP(rec, req) + + if rec.Code != http.StatusUnauthorized { + t.Errorf("status = %d, want 401", rec.Code) + } +} diff --git a/internal/db/dbq/events.sql.go b/internal/db/dbq/events.sql.go index 83a04dee..59f08d04 100644 --- a/internal/db/dbq/events.sql.go +++ b/internal/db/dbq/events.sql.go @@ -54,7 +54,7 @@ func (q *Queries) GetMostRecentPlaySessionForUser(ctx context.Context, userID pg } const getOpenPlayEventForUser = `-- name: GetOpenPlayEventForUser :one -SELECT id, user_id, track_id, session_id, started_at, ended_at, duration_played_ms, completion_ratio, was_skipped, client_id, session_vector_at_play, scrobbled_at FROM play_events +SELECT id, user_id, track_id, session_id, started_at, ended_at, duration_played_ms, completion_ratio, was_skipped, client_id, session_vector_at_play, scrobbled_at, source FROM play_events WHERE user_id = $1 AND ended_at IS NULL ORDER BY started_at DESC LIMIT 1 @@ -78,12 +78,13 @@ func (q *Queries) GetOpenPlayEventForUser(ctx context.Context, userID pgtype.UUI &i.ClientID, &i.SessionVectorAtPlay, &i.ScrobbledAt, + &i.Source, ) return i, err } const getPlayEventByID = `-- name: GetPlayEventByID :one -SELECT id, user_id, track_id, session_id, started_at, ended_at, duration_played_ms, completion_ratio, was_skipped, client_id, session_vector_at_play, scrobbled_at FROM play_events WHERE id = $1 +SELECT id, user_id, track_id, session_id, started_at, ended_at, duration_played_ms, completion_ratio, was_skipped, client_id, session_vector_at_play, scrobbled_at, source FROM play_events WHERE id = $1 ` func (q *Queries) GetPlayEventByID(ctx context.Context, id pgtype.UUID) (PlayEvent, error) { @@ -102,15 +103,16 @@ func (q *Queries) GetPlayEventByID(ctx context.Context, id pgtype.UUID) (PlayEve &i.ClientID, &i.SessionVectorAtPlay, &i.ScrobbledAt, + &i.Source, ) return i, err } const insertPlayEvent = `-- name: InsertPlayEvent :one INSERT INTO play_events ( - user_id, track_id, session_id, started_at, client_id -) VALUES ($1, $2, $3, $4, $5) -RETURNING id, user_id, track_id, session_id, started_at, ended_at, duration_played_ms, completion_ratio, was_skipped, client_id, session_vector_at_play, scrobbled_at + user_id, track_id, session_id, started_at, client_id, source +) VALUES ($1, $2, $3, $4, $5, $6) +RETURNING id, user_id, track_id, session_id, started_at, ended_at, duration_played_ms, completion_ratio, was_skipped, client_id, session_vector_at_play, scrobbled_at, source ` type InsertPlayEventParams struct { @@ -119,6 +121,7 @@ type InsertPlayEventParams struct { SessionID pgtype.UUID StartedAt pgtype.Timestamptz ClientID *string + Source *string } func (q *Queries) InsertPlayEvent(ctx context.Context, arg InsertPlayEventParams) (PlayEvent, error) { @@ -128,6 +131,7 @@ func (q *Queries) InsertPlayEvent(ctx context.Context, arg InsertPlayEventParams arg.SessionID, arg.StartedAt, arg.ClientID, + arg.Source, ) var i PlayEvent err := row.Scan( @@ -143,6 +147,7 @@ func (q *Queries) InsertPlayEvent(ctx context.Context, arg InsertPlayEventParams &i.ClientID, &i.SessionVectorAtPlay, &i.ScrobbledAt, + &i.Source, ) return i, err } @@ -286,7 +291,7 @@ SET ended_at = $2, completion_ratio = $4, was_skipped = $5 WHERE id = $1 -RETURNING id, user_id, track_id, session_id, started_at, ended_at, duration_played_ms, completion_ratio, was_skipped, client_id, session_vector_at_play, scrobbled_at +RETURNING id, user_id, track_id, session_id, started_at, ended_at, duration_played_ms, completion_ratio, was_skipped, client_id, session_vector_at_play, scrobbled_at, source ` type UpdatePlayEventEndedParams struct { @@ -322,6 +327,7 @@ func (q *Queries) UpdatePlayEventEnded(ctx context.Context, arg UpdatePlayEventE &i.ClientID, &i.SessionVectorAtPlay, &i.ScrobbledAt, + &i.Source, ) return i, err } diff --git a/internal/db/dbq/models.go b/internal/db/dbq/models.go index f99894a5..3b772075 100644 --- a/internal/db/dbq/models.go +++ b/internal/db/dbq/models.go @@ -369,6 +369,7 @@ type PlayEvent struct { ClientID *string SessionVectorAtPlay []byte ScrobbledAt pgtype.Timestamptz + Source *string } type PlaySession struct { @@ -475,6 +476,14 @@ type SmtpConfig struct { UpdatedAt pgtype.Timestamptz } +type SystemPlaylistRotationState struct { + UserID pgtype.UUID + PlaylistKind string + PlayedTrackIds []pgtype.UUID + RotationStartedAt pgtype.Timestamptz + UpdatedAt pgtype.Timestamptz +} + type SystemPlaylistRun struct { UserID pgtype.UUID LastRunAt pgtype.Timestamptz diff --git a/internal/db/dbq/system_mixes.sql.go b/internal/db/dbq/system_mixes.sql.go new file mode 100644 index 00000000..5557b28b --- /dev/null +++ b/internal/db/dbq/system_mixes.sql.go @@ -0,0 +1,303 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 +// source: system_mixes.sql + +package dbq + +import ( + "context" + + "github.com/jackc/pgx/v5/pgtype" +) + +const listDeepCutsTracks = `-- name: ListDeepCutsTracks :many + +WITH affinity_artists AS ( + SELECT artist_id FROM general_likes_artists WHERE user_id = $1 + UNION + SELECT t.artist_id + FROM play_events pe JOIN tracks t ON t.id = pe.track_id + WHERE pe.user_id = $1 AND pe.was_skipped = false + GROUP BY t.artist_id + HAVING COUNT(*) >= 5 +), +play_counts AS ( + SELECT track_id, COUNT(*) AS c + FROM play_events + WHERE user_id = $1 AND was_skipped = false + GROUP BY track_id +) +SELECT t.id, t.album_id, t.artist_id + FROM tracks t + JOIN affinity_artists aa ON aa.artist_id = t.artist_id + LEFT JOIN play_counts pc ON pc.track_id = t.id + WHERE COALESCE(pc.c, 0) <= 2 + AND NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $1 AND q.track_id = t.id + ) + ORDER BY md5(t.id::text || $2::text) + LIMIT 200 +` + +type ListDeepCutsTracksParams struct { + UserID pgtype.UUID + Column2 string +} + +type ListDeepCutsTracksRow struct { + ID pgtype.UUID + AlbumID pgtype.UUID + ArtistID pgtype.UUID +} + +// Candidate queries for the discovery system-playlist mixes +// (#419-423). Each returns (id, album_id, artist_id) rows; the Go +// producer caps + truncates and converts to rankedCandidate. Daily- +// deterministic md5(id||dateStr) ordering (where used) keeps a mix +// stable within a day while rotating across days; shuffle-on-play +// gives per-play variety on top. +// #419 Deep Cuts: low-play tracks (<=2 plays) from artists the user +// has liked OR played heavily (>=5 non-skip plays across the artist). +// $1 user_id, $2 date string. +func (q *Queries) ListDeepCutsTracks(ctx context.Context, arg ListDeepCutsTracksParams) ([]ListDeepCutsTracksRow, error) { + rows, err := q.db.Query(ctx, listDeepCutsTracks, arg.UserID, arg.Column2) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListDeepCutsTracksRow + for rows.Next() { + var i ListDeepCutsTracksRow + if err := rows.Scan(&i.ID, &i.AlbumID, &i.ArtistID); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const listFirstListensTracks = `-- name: ListFirstListensTracks :many +WITH heard_albums AS ( + SELECT DISTINCT t.album_id + FROM play_events pe JOIN tracks t ON t.id = pe.track_id + WHERE pe.user_id = $1 +), +played_artists AS ( + SELECT DISTINCT t.artist_id + FROM play_events pe JOIN tracks t ON t.id = pe.track_id + WHERE pe.user_id = $1 +) +SELECT t.id, t.album_id, t.artist_id + FROM tracks t + JOIN albums al ON al.id = t.album_id + WHERE NOT EXISTS (SELECT 1 FROM heard_albums h WHERE h.album_id = al.id) + AND NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $1 AND q.track_id = t.id + ) + ORDER BY + (CASE + WHEN EXISTS ( + SELECT 1 FROM general_likes_artists gla + WHERE gla.user_id = $1 AND gla.artist_id = al.artist_id + ) THEN 0 + WHEN al.artist_id IN (SELECT artist_id FROM played_artists) THEN 1 + ELSE 2 + END), + al.id, t.disc_number NULLS FIRST, t.track_number NULLS FIRST + LIMIT 300 +` + +type ListFirstListensTracksRow struct { + ID pgtype.UUID + AlbumID pgtype.UUID + ArtistID pgtype.UUID +} + +// #423 First Listens: albums the user has never played any track of. +// Tiered: liked-artist albums first, then played-artist albums, then +// the rest — album-coherent within each tier. Not diversity-capped +// (whole-album discovery). $1 user_id. +func (q *Queries) ListFirstListensTracks(ctx context.Context, userID pgtype.UUID) ([]ListFirstListensTracksRow, error) { + rows, err := q.db.Query(ctx, listFirstListensTracks, userID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListFirstListensTracksRow + for rows.Next() { + var i ListFirstListensTracksRow + if err := rows.Scan(&i.ID, &i.AlbumID, &i.ArtistID); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const listNewForYouTracks = `-- name: ListNewForYouTracks :many +WITH affinity_artists AS ( + SELECT artist_id FROM general_likes_artists WHERE user_id = $1 + UNION + SELECT t.artist_id + FROM play_events pe JOIN tracks t ON t.id = pe.track_id + WHERE pe.user_id = $1 AND pe.was_skipped = false + GROUP BY t.artist_id + HAVING COUNT(*) >= 3 +) +SELECT t.id, t.album_id, t.artist_id + FROM tracks t + JOIN albums al ON al.id = t.album_id + JOIN affinity_artists aa ON aa.artist_id = al.artist_id + WHERE al.created_at >= now() - interval '30 days' + AND NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $1 AND q.track_id = t.id + ) + ORDER BY al.created_at DESC, t.disc_number NULLS FIRST, t.track_number NULLS FIRST + LIMIT 200 +` + +type ListNewForYouTracksRow struct { + ID pgtype.UUID + AlbumID pgtype.UUID + ArtistID pgtype.UUID +} + +// #421 New for you: tracks from albums added in the last 30 days +// whose artist the user has liked OR played (>=3 non-skip). Album- +// coherent (newest album first, then disc/track) — the producer does +// NOT diversity-cap these; they're meant as whole-album discovery. +// $1 user_id. +func (q *Queries) ListNewForYouTracks(ctx context.Context, userID pgtype.UUID) ([]ListNewForYouTracksRow, error) { + rows, err := q.db.Query(ctx, listNewForYouTracks, userID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListNewForYouTracksRow + for rows.Next() { + var i ListNewForYouTracksRow + if err := rows.Scan(&i.ID, &i.AlbumID, &i.ArtistID); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const listOnThisDayTracks = `-- name: ListOnThisDayTracks :many +WITH windowed AS ( + SELECT track_id, COUNT(*) AS c + FROM play_events + WHERE user_id = $1 AND was_skipped = false + AND started_at < now() - interval '60 days' + AND ABS(EXTRACT(DOY FROM started_at) - EXTRACT(DOY FROM now())) <= 7 + GROUP BY track_id +) +SELECT t.id, t.album_id, t.artist_id + FROM tracks t + JOIN windowed w ON w.track_id = t.id + WHERE NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $1 AND q.track_id = t.id + ) + ORDER BY w.c DESC, md5(t.id::text || $2::text) + LIMIT 200 +` + +type ListOnThisDayTracksParams struct { + UserID pgtype.UUID + Column2 string +} + +type ListOnThisDayTracksRow struct { + ID pgtype.UUID + AlbumID pgtype.UUID + ArtistID pgtype.UUID +} + +// #422 On This Day: tracks the user played around this calendar date +// (±7 day-of-year) in the past, excluding the very recent (>60 days +// ago) so it's nostalgic, not just "last week". Weighted by how much +// they were played in those windows. +// $1 user_id, $2 date string. +func (q *Queries) ListOnThisDayTracks(ctx context.Context, arg ListOnThisDayTracksParams) ([]ListOnThisDayTracksRow, error) { + rows, err := q.db.Query(ctx, listOnThisDayTracks, arg.UserID, arg.Column2) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListOnThisDayTracksRow + for rows.Next() { + var i ListOnThisDayTracksRow + if err := rows.Scan(&i.ID, &i.AlbumID, &i.ArtistID); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const listRediscoverTracks = `-- name: ListRediscoverTracks :many +WITH stats AS ( + SELECT track_id, COUNT(*) AS c, MAX(started_at) AS last_at + FROM play_events + WHERE user_id = $1 AND was_skipped = false + GROUP BY track_id +) +SELECT t.id, t.album_id, t.artist_id + FROM tracks t + JOIN stats s ON s.track_id = t.id + WHERE s.c >= 5 + AND s.last_at <= now() - interval '6 months' + AND NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $1 AND q.track_id = t.id + ) + ORDER BY s.c DESC, t.id + LIMIT 200 +` + +type ListRediscoverTracksRow struct { + ID pgtype.UUID + AlbumID pgtype.UUID + ArtistID pgtype.UUID +} + +// #420 Rediscover: tracks the user played a lot (>=5 non-skip) but +// not in the last 6 months. Ordered by historical affection. +// $1 user_id. +func (q *Queries) ListRediscoverTracks(ctx context.Context, userID pgtype.UUID) ([]ListRediscoverTracksRow, error) { + rows, err := q.db.Query(ctx, listRediscoverTracks, userID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListRediscoverTracksRow + for rows.Next() { + var i ListRediscoverTracksRow + if err := rows.Scan(&i.ID, &i.AlbumID, &i.ArtistID); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} diff --git a/internal/db/dbq/system_playlist_rotation.sql.go b/internal/db/dbq/system_playlist_rotation.sql.go new file mode 100644 index 00000000..6aeff689 --- /dev/null +++ b/internal/db/dbq/system_playlist_rotation.sql.go @@ -0,0 +1,91 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 +// source: system_playlist_rotation.sql + +package dbq + +import ( + "context" + + "github.com/jackc/pgx/v5/pgtype" +) + +const appendRotationPlayed = `-- name: AppendRotationPlayed :exec + +INSERT INTO system_playlist_rotation_state ( + user_id, playlist_kind, played_track_ids, rotation_started_at, updated_at +) VALUES ( + $1, $2, + array_append('{}'::uuid[], $3::uuid), now(), now() +) +ON CONFLICT (user_id, playlist_kind) DO UPDATE +SET played_track_ids = + CASE WHEN $3::uuid = ANY(system_playlist_rotation_state.played_track_ids) + THEN system_playlist_rotation_state.played_track_ids + ELSE array_append(system_playlist_rotation_state.played_track_ids, $3::uuid) + END, + updated_at = now() +` + +type AppendRotationPlayedParams struct { + UserID pgtype.UUID + PlaylistKind string + TrackID pgtype.UUID +} + +// Rotation state for system-playlist sample-history dedup (#415). +// One row per (user, playlist_kind). played_track_ids accumulates +// tracks heard from that playlist in the current rotation; shuffle +// prefers ids NOT in this set, then resets when the snapshot is +// exhausted. +// Records that the given track was played from the (user, kind) +// system playlist. Creates the row on first play; on conflict +// appends only if not already present so the array stays a set. +func (q *Queries) AppendRotationPlayed(ctx context.Context, arg AppendRotationPlayedParams) error { + _, err := q.db.Exec(ctx, appendRotationPlayed, arg.UserID, arg.PlaylistKind, arg.TrackID) + return err +} + +const getRotationState = `-- name: GetRotationState :one +SELECT user_id, playlist_kind, played_track_ids, rotation_started_at, updated_at FROM system_playlist_rotation_state +WHERE user_id = $1 AND playlist_kind = $2 +` + +type GetRotationStateParams struct { + UserID pgtype.UUID + PlaylistKind string +} + +func (q *Queries) GetRotationState(ctx context.Context, arg GetRotationStateParams) (SystemPlaylistRotationState, error) { + row := q.db.QueryRow(ctx, getRotationState, arg.UserID, arg.PlaylistKind) + var i SystemPlaylistRotationState + err := row.Scan( + &i.UserID, + &i.PlaylistKind, + &i.PlayedTrackIds, + &i.RotationStartedAt, + &i.UpdatedAt, + ) + return i, err +} + +const resetRotationState = `-- name: ResetRotationState :exec +UPDATE system_playlist_rotation_state +SET played_track_ids = '{}', + rotation_started_at = now(), + updated_at = now() +WHERE user_id = $1 AND playlist_kind = $2 +` + +type ResetRotationStateParams struct { + UserID pgtype.UUID + PlaylistKind string +} + +// Clears the played set and advances the rotation window. Called +// when the playlist snapshot has been fully sampled (Stage 2). +func (q *Queries) ResetRotationState(ctx context.Context, arg ResetRotationStateParams) error { + _, err := q.db.Exec(ctx, resetRotationState, arg.UserID, arg.PlaylistKind) + return err +} diff --git a/internal/db/dbq/tracks.sql.go b/internal/db/dbq/tracks.sql.go index 2f522c3a..7e21a3f3 100644 --- a/internal/db/dbq/tracks.sql.go +++ b/internal/db/dbq/tracks.sql.go @@ -255,6 +255,73 @@ func (q *Queries) ListArtistTracksForUser(ctx context.Context, arg ListArtistTra return items, nil } +const listRandomTracksForUser = `-- name: ListRandomTracksForUser :many +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 +WHERE NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $1 AND q.track_id = t.id + ) +ORDER BY random() +LIMIT $2 +` + +type ListRandomTracksForUserParams struct { + UserID pgtype.UUID + Limit int32 +} + +type ListRandomTracksForUserRow struct { + Track Track + AlbumTitle string + ArtistName string +} + +// #427 S4: backing query for GET /api/library/shuffle — the online +// source for "Shuffle all". N random tracks across the whole +// library, per-user-quarantine filtered. $1 user_id, $2 limit. +func (q *Queries) ListRandomTracksForUser(ctx context.Context, arg ListRandomTracksForUserParams) ([]ListRandomTracksForUserRow, error) { + rows, err := q.db.Query(ctx, listRandomTracksForUser, arg.UserID, arg.Limit) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListRandomTracksForUserRow + for rows.Next() { + var i ListRandomTracksForUserRow + if err := rows.Scan( + &i.Track.ID, + &i.Track.Title, + &i.Track.AlbumID, + &i.Track.ArtistID, + &i.Track.TrackNumber, + &i.Track.DiscNumber, + &i.Track.DurationMs, + &i.Track.FilePath, + &i.Track.FileSize, + &i.Track.FileFormat, + &i.Track.Bitrate, + &i.Track.Mbid, + &i.Track.Genre, + &i.Track.AddedAt, + &i.Track.UpdatedAt, + &i.AlbumTitle, + &i.ArtistName, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const listTracksByAlbum = `-- name: ListTracksByAlbum :many SELECT id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at FROM tracks WHERE album_id = $1 diff --git a/internal/db/migrations/0027_system_playlist_rotation.down.sql b/internal/db/migrations/0027_system_playlist_rotation.down.sql new file mode 100644 index 00000000..c52a8f5f --- /dev/null +++ b/internal/db/migrations/0027_system_playlist_rotation.down.sql @@ -0,0 +1,5 @@ +-- 0027_system_playlist_rotation.down.sql +DROP TABLE IF EXISTS system_playlist_rotation_state; + +ALTER TABLE play_events + DROP COLUMN IF EXISTS source; diff --git a/internal/db/migrations/0027_system_playlist_rotation.up.sql b/internal/db/migrations/0027_system_playlist_rotation.up.sql new file mode 100644 index 00000000..3314a4ea --- /dev/null +++ b/internal/db/migrations/0027_system_playlist_rotation.up.sql @@ -0,0 +1,26 @@ +-- 0027_system_playlist_rotation.up.sql — sample-history dedup for +-- system playlists (Fable #415). Two parts: +-- +-- 1. play_events.source: which system playlist a play originated +-- from ('for_you' | 'discover' | future kinds), or NULL for any +-- play not launched from a system-playlist surface (library, +-- user playlist, radio, Subsonic). Lets the ingest path know a +-- play should count against that playlist's rotation. +-- +-- 2. system_playlist_rotation_state: per (user, playlist_kind) set +-- of track ids already played in the current rotation window. +-- Shuffle-on-play prefers the unplayed tail; when the snapshot +-- is exhausted the set resets and rotation_started_at advances. +-- Centralized server-side so phone / web / watch share one +-- rotation, mirroring how radio dedup already lives server-side. +ALTER TABLE play_events + ADD COLUMN source text; + +CREATE TABLE system_playlist_rotation_state ( + user_id uuid NOT NULL REFERENCES users(id) ON DELETE CASCADE, + playlist_kind text NOT NULL, + played_track_ids uuid[] NOT NULL DEFAULT '{}', + rotation_started_at timestamptz NOT NULL DEFAULT now(), + updated_at timestamptz NOT NULL DEFAULT now(), + PRIMARY KEY (user_id, playlist_kind) +); diff --git a/internal/db/queries/events.sql b/internal/db/queries/events.sql index 60e42f4d..db0d4829 100644 --- a/internal/db/queries/events.sql +++ b/internal/db/queries/events.sql @@ -25,8 +25,8 @@ LIMIT 1; -- name: InsertPlayEvent :one INSERT INTO play_events ( - user_id, track_id, session_id, started_at, client_id -) VALUES ($1, $2, $3, $4, $5) + user_id, track_id, session_id, started_at, client_id, source +) VALUES ($1, $2, $3, $4, $5, $6) RETURNING *; -- name: UpdatePlayEventEnded :one diff --git a/internal/db/queries/system_mixes.sql b/internal/db/queries/system_mixes.sql new file mode 100644 index 00000000..62074620 --- /dev/null +++ b/internal/db/queries/system_mixes.sql @@ -0,0 +1,145 @@ +-- Candidate queries for the discovery system-playlist mixes +-- (#419-423). Each returns (id, album_id, artist_id) rows; the Go +-- producer caps + truncates and converts to rankedCandidate. Daily- +-- deterministic md5(id||dateStr) ordering (where used) keeps a mix +-- stable within a day while rotating across days; shuffle-on-play +-- gives per-play variety on top. + +-- name: ListDeepCutsTracks :many +-- #419 Deep Cuts: low-play tracks (<=2 plays) from artists the user +-- has liked OR played heavily (>=5 non-skip plays across the artist). +-- $1 user_id, $2 date string. +WITH affinity_artists AS ( + SELECT artist_id FROM general_likes_artists WHERE user_id = $1 + UNION + SELECT t.artist_id + FROM play_events pe JOIN tracks t ON t.id = pe.track_id + WHERE pe.user_id = $1 AND pe.was_skipped = false + GROUP BY t.artist_id + HAVING COUNT(*) >= 5 +), +play_counts AS ( + SELECT track_id, COUNT(*) AS c + FROM play_events + WHERE user_id = $1 AND was_skipped = false + GROUP BY track_id +) +SELECT t.id, t.album_id, t.artist_id + FROM tracks t + JOIN affinity_artists aa ON aa.artist_id = t.artist_id + LEFT JOIN play_counts pc ON pc.track_id = t.id + WHERE COALESCE(pc.c, 0) <= 2 + AND NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $1 AND q.track_id = t.id + ) + ORDER BY md5(t.id::text || $2::text) + LIMIT 200; + +-- name: ListRediscoverTracks :many +-- #420 Rediscover: tracks the user played a lot (>=5 non-skip) but +-- not in the last 6 months. Ordered by historical affection. +-- $1 user_id. +WITH stats AS ( + SELECT track_id, COUNT(*) AS c, MAX(started_at) AS last_at + FROM play_events + WHERE user_id = $1 AND was_skipped = false + GROUP BY track_id +) +SELECT t.id, t.album_id, t.artist_id + FROM tracks t + JOIN stats s ON s.track_id = t.id + WHERE s.c >= 5 + AND s.last_at <= now() - interval '6 months' + AND NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $1 AND q.track_id = t.id + ) + ORDER BY s.c DESC, t.id + LIMIT 200; + +-- name: ListNewForYouTracks :many +-- #421 New for you: tracks from albums added in the last 30 days +-- whose artist the user has liked OR played (>=3 non-skip). Album- +-- coherent (newest album first, then disc/track) — the producer does +-- NOT diversity-cap these; they're meant as whole-album discovery. +-- $1 user_id. +WITH affinity_artists AS ( + SELECT artist_id FROM general_likes_artists WHERE user_id = $1 + UNION + SELECT t.artist_id + FROM play_events pe JOIN tracks t ON t.id = pe.track_id + WHERE pe.user_id = $1 AND pe.was_skipped = false + GROUP BY t.artist_id + HAVING COUNT(*) >= 3 +) +SELECT t.id, t.album_id, t.artist_id + FROM tracks t + JOIN albums al ON al.id = t.album_id + JOIN affinity_artists aa ON aa.artist_id = al.artist_id + WHERE al.created_at >= now() - interval '30 days' + AND NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $1 AND q.track_id = t.id + ) + ORDER BY al.created_at DESC, t.disc_number NULLS FIRST, t.track_number NULLS FIRST + LIMIT 200; + +-- name: ListOnThisDayTracks :many +-- #422 On This Day: tracks the user played around this calendar date +-- (±7 day-of-year) in the past, excluding the very recent (>60 days +-- ago) so it's nostalgic, not just "last week". Weighted by how much +-- they were played in those windows. +-- $1 user_id, $2 date string. +WITH windowed AS ( + SELECT track_id, COUNT(*) AS c + FROM play_events + WHERE user_id = $1 AND was_skipped = false + AND started_at < now() - interval '60 days' + AND ABS(EXTRACT(DOY FROM started_at) - EXTRACT(DOY FROM now())) <= 7 + GROUP BY track_id +) +SELECT t.id, t.album_id, t.artist_id + FROM tracks t + JOIN windowed w ON w.track_id = t.id + WHERE NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $1 AND q.track_id = t.id + ) + ORDER BY w.c DESC, md5(t.id::text || $2::text) + LIMIT 200; + +-- name: ListFirstListensTracks :many +-- #423 First Listens: albums the user has never played any track of. +-- Tiered: liked-artist albums first, then played-artist albums, then +-- the rest — album-coherent within each tier. Not diversity-capped +-- (whole-album discovery). $1 user_id. +WITH heard_albums AS ( + SELECT DISTINCT t.album_id + FROM play_events pe JOIN tracks t ON t.id = pe.track_id + WHERE pe.user_id = $1 +), +played_artists AS ( + SELECT DISTINCT t.artist_id + FROM play_events pe JOIN tracks t ON t.id = pe.track_id + WHERE pe.user_id = $1 +) +SELECT t.id, t.album_id, t.artist_id + FROM tracks t + JOIN albums al ON al.id = t.album_id + WHERE NOT EXISTS (SELECT 1 FROM heard_albums h WHERE h.album_id = al.id) + AND NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $1 AND q.track_id = t.id + ) + ORDER BY + (CASE + WHEN EXISTS ( + SELECT 1 FROM general_likes_artists gla + WHERE gla.user_id = $1 AND gla.artist_id = al.artist_id + ) THEN 0 + WHEN al.artist_id IN (SELECT artist_id FROM played_artists) THEN 1 + ELSE 2 + END), + al.id, t.disc_number NULLS FIRST, t.track_number NULLS FIRST + LIMIT 300; diff --git a/internal/db/queries/system_playlist_rotation.sql b/internal/db/queries/system_playlist_rotation.sql new file mode 100644 index 00000000..3f95050e --- /dev/null +++ b/internal/db/queries/system_playlist_rotation.sql @@ -0,0 +1,36 @@ +-- Rotation state for system-playlist sample-history dedup (#415). +-- One row per (user, playlist_kind). played_track_ids accumulates +-- tracks heard from that playlist in the current rotation; shuffle +-- prefers ids NOT in this set, then resets when the snapshot is +-- exhausted. + +-- name: AppendRotationPlayed :exec +-- Records that the given track was played from the (user, kind) +-- system playlist. Creates the row on first play; on conflict +-- appends only if not already present so the array stays a set. +INSERT INTO system_playlist_rotation_state ( + user_id, playlist_kind, played_track_ids, rotation_started_at, updated_at +) VALUES ( + sqlc.arg(user_id), sqlc.arg(playlist_kind), + array_append('{}'::uuid[], sqlc.arg(track_id)::uuid), now(), now() +) +ON CONFLICT (user_id, playlist_kind) DO UPDATE +SET played_track_ids = + CASE WHEN sqlc.arg(track_id)::uuid = ANY(system_playlist_rotation_state.played_track_ids) + THEN system_playlist_rotation_state.played_track_ids + ELSE array_append(system_playlist_rotation_state.played_track_ids, sqlc.arg(track_id)::uuid) + END, + updated_at = now(); + +-- name: GetRotationState :one +SELECT * FROM system_playlist_rotation_state +WHERE user_id = $1 AND playlist_kind = $2; + +-- name: ResetRotationState :exec +-- Clears the played set and advances the rotation window. Called +-- when the playlist snapshot has been fully sampled (Stage 2). +UPDATE system_playlist_rotation_state +SET played_track_ids = '{}', + rotation_started_at = now(), + updated_at = now() +WHERE user_id = $1 AND playlist_kind = $2; diff --git a/internal/db/queries/tracks.sql b/internal/db/queries/tracks.sql index 1e661ce0..4019b684 100644 --- a/internal/db/queries/tracks.sql +++ b/internal/db/queries/tracks.sql @@ -81,6 +81,23 @@ WHERE t.artist_id = $1 ORDER BY albums.release_date NULLS LAST, albums.sort_title, t.disc_number NULLS FIRST, t.track_number NULLS FIRST, t.id; +-- name: ListRandomTracksForUser :many +-- #427 S4: backing query for GET /api/library/shuffle — the online +-- source for "Shuffle all". N random tracks across the whole +-- library, per-user-quarantine filtered. $1 user_id, $2 limit. +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 +WHERE NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $1 AND q.track_id = t.id + ) +ORDER BY random() +LIMIT $2; + -- name: CountTracksByArtist :one -- Used by request-progress reporting to count tracks ingested under a -- matched artist (sum across all the artist's albums) while a request diff --git a/internal/playevents/writer.go b/internal/playevents/writer.go index 00bff645..f523a3b4 100644 --- a/internal/playevents/writer.go +++ b/internal/playevents/writer.go @@ -57,12 +57,36 @@ type StartedResult struct { // RecordPlayStarted: in one transaction, auto-close any prior open row for // the user, FindOrCreate the play_session, insert the new play_event, -// return its id and session id. +// return its id and session id. Source-agnostic — kept for the Subsonic +// shim (frozen API), which never originates from a system playlist. func (w *Writer) RecordPlayStarted( ctx context.Context, userID, trackID pgtype.UUID, clientID string, at time.Time, +) (StartedResult, error) { + return w.RecordPlayStartedWithSource(ctx, userID, trackID, clientID, "", at) +} + +// systemPlaylistSources are the play_events.source values that count +// against a system playlist's rotation (Fable #415). Add future +// system-playlist kinds here as they ship (deep_cuts, rediscover, …). +var systemPlaylistSources = map[string]bool{ + "for_you": true, + "discover": true, +} + +// RecordPlayStartedWithSource is RecordPlayStarted plus a `source` +// tag identifying which surface the play came from. When source is a +// known system-playlist kind the track is appended to that user's +// rotation set so shuffle-on-play can prefer the unplayed tail on a +// re-play within the day. Empty source → behaves exactly like the +// pre-#415 path (NULL column, no rotation write). +func (w *Writer) RecordPlayStartedWithSource( + ctx context.Context, + userID, trackID pgtype.UUID, + clientID, source string, + at time.Time, ) (StartedResult, error) { var out StartedResult err := pgx.BeginFunc(ctx, w.pool, func(tx pgx.Tx) error { @@ -78,12 +102,17 @@ func (w *Writer) RecordPlayStarted( if clientID != "" { clientIDPtr = &clientID } + var sourcePtr *string + if source != "" { + sourcePtr = &source + } ev, err := q.InsertPlayEvent(ctx, dbq.InsertPlayEventParams{ UserID: userID, TrackID: trackID, SessionID: sessionID, StartedAt: pgtype.Timestamptz{Time: at, Valid: true}, ClientID: clientIDPtr, + Source: sourcePtr, }) if err != nil { return err @@ -95,6 +124,16 @@ func (w *Writer) RecordPlayStarted( if err := w.captureSessionVector(ctx, q, ev.ID, sessionID, at); err != nil { return err } + + if systemPlaylistSources[source] { + if err := q.AppendRotationPlayed(ctx, dbq.AppendRotationPlayedParams{ + UserID: userID, + PlaylistKind: source, + TrackID: trackID, + }); err != nil { + return err + } + } return nil }) return out, err @@ -267,6 +306,119 @@ func (w *Writer) RecordSyntheticCompletedPlay( return nil } +// RecordOfflinePlay writes a complete start+end play in one call from +// a caller-supplied timestamp + duration. This is the replay path for +// the Flutter offline mutation queue (#426 part B): a play that +// happened with no/flaky connectivity is captured locally as a +// completed unit and replayed here once back online, preserving the +// original `at`. +// +// Generalizes RecordSyntheticCompletedPlay: instead of assuming full +// completion it applies the spec §6 skip rule from the real +// durationPlayed vs track length (same AND-of-thresholds as +// RecordPlayEnded), and threads `source` so #415 rotation advances +// for system-playlist plays just like the live path. +// +// durationPlayedMs is clamped to [0, track duration] to absorb +// client clock / position drift. +func (w *Writer) RecordOfflinePlay( + ctx context.Context, + userID, trackID pgtype.UUID, + clientID, source string, + at time.Time, + durationPlayedMs int32, +) error { + var playEventID pgtype.UUID + if err := pgx.BeginFunc(ctx, w.pool, func(tx pgx.Tx) error { + q := dbq.New(tx) + track, err := q.GetTrackByID(ctx, trackID) + if err != nil { + return err + } + if err := w.autoClosePriorOpen(ctx, q, userID, at); err != nil { + return err + } + sessionID, err := playsessions.FindOrCreate(ctx, q, userID, at, clientID, w.sessionTimeout) + if err != nil { + return err + } + var clientIDPtr *string + if clientID != "" { + clientIDPtr = &clientID + } + var sourcePtr *string + if source != "" { + sourcePtr = &source + } + ev, err := q.InsertPlayEvent(ctx, dbq.InsertPlayEventParams{ + UserID: userID, + TrackID: trackID, + SessionID: sessionID, + StartedAt: pgtype.Timestamptz{Time: at, Valid: true}, + ClientID: clientIDPtr, + Source: sourcePtr, + }) + if err != nil { + return err + } + playEventID = ev.ID + if err := w.captureSessionVector(ctx, q, ev.ID, sessionID, at); err != nil { + return err + } + + played := durationPlayedMs + if played < 0 { + played = 0 + } + if track.DurationMs > 0 && played > track.DurationMs { + played = track.DurationMs + } + ratio := 0.0 + if track.DurationMs > 0 { + ratio = float64(played) / float64(track.DurationMs) + } + isSkip := ratio < w.skipMaxCompletionRatio && played < w.skipMaxDurationPlayedMs + ratioPtr := ratio + durPtr := played + if _, err := q.UpdatePlayEventEnded(ctx, dbq.UpdatePlayEventEndedParams{ + ID: ev.ID, + EndedAt: pgtype.Timestamptz{Time: at.Add(time.Duration(played) * time.Millisecond), Valid: true}, + DurationPlayedMs: &durPtr, + CompletionRatio: &ratioPtr, + WasSkipped: isSkip, + }); err != nil { + return err + } + if isSkip { + if _, err := q.InsertSkipEvent(ctx, dbq.InsertSkipEventParams{ + UserID: userID, + TrackID: trackID, + SessionID: sessionID, + SkippedAt: pgtype.Timestamptz{Time: at.Add(time.Duration(played) * time.Millisecond), Valid: true}, + PositionMs: played, + }); err != nil { + return err + } + } + if systemPlaylistSources[source] { + if err := q.AppendRotationPlayed(ctx, dbq.AppendRotationPlayedParams{ + UserID: userID, + PlaylistKind: source, + TrackID: trackID, + }); err != nil { + return err + } + } + return nil + }); err != nil { + return err + } + if err := scrobble.MaybeEnqueue(ctx, dbq.New(w.pool), playEventID); err != nil { + w.logger.Warn("playevents: scrobble enqueue failed", "play_event_id", playEventID, "err", err) + } + return nil +} + // captureSessionVector queries the user's prior plays in the given session // (before `at`), builds the session vector, and UPDATEs the just-inserted // play_event with it. Runs inside the caller's transaction (q is the diff --git a/internal/playevents/writer_test.go b/internal/playevents/writer_test.go index 6f9d8233..bfb30dc7 100644 --- a/internal/playevents/writer_test.go +++ b/internal/playevents/writer_test.go @@ -372,3 +372,52 @@ func TestRecordPlayEnded_SubThreshold_NoEnqueue(t *testing.T) { t.Errorf("expected 0 queue rows for sub-threshold play, got %d", n) } } + +func TestRecordPlayStartedWithSource_AppendsRotation(t *testing.T) { + f := newFixture(t, 200_000) + now := time.Now().UTC() + if _, err := f.w.RecordPlayStartedWithSource( + context.Background(), f.user, f.track, "c", "for_you", now, + ); err != nil { + t.Fatalf("RecordPlayStartedWithSource: %v", err) + } + st, err := f.q.GetRotationState(context.Background(), dbq.GetRotationStateParams{ + UserID: f.user, PlaylistKind: "for_you", + }) + if err != nil { + t.Fatalf("GetRotationState: %v", err) + } + if len(st.PlayedTrackIds) != 1 || st.PlayedTrackIds[0] != f.track { + t.Errorf("played_track_ids = %v, want [%v]", st.PlayedTrackIds, f.track) + } + + // Re-playing the same track stays a set (no duplicate append). + if _, err := f.w.RecordPlayStartedWithSource( + context.Background(), f.user, f.track, "c", "for_you", now.Add(time.Minute), + ); err != nil { + t.Fatalf("RecordPlayStartedWithSource 2: %v", err) + } + st, err = f.q.GetRotationState(context.Background(), dbq.GetRotationStateParams{ + UserID: f.user, PlaylistKind: "for_you", + }) + if err != nil { + t.Fatalf("GetRotationState 2: %v", err) + } + if len(st.PlayedTrackIds) != 1 { + t.Errorf("played_track_ids should dedupe, got %v", st.PlayedTrackIds) + } +} + +func TestRecordPlayStarted_NoRotationWithoutSource(t *testing.T) { + f := newFixture(t, 200_000) + now := time.Now().UTC() + if _, err := f.w.RecordPlayStarted(context.Background(), f.user, f.track, "c", now); err != nil { + t.Fatalf("RecordPlayStarted: %v", err) + } + _, err := f.q.GetRotationState(context.Background(), dbq.GetRotationStateParams{ + UserID: f.user, PlaylistKind: "for_you", + }) + if err == nil { + t.Errorf("expected no rotation row for a source-less play") + } +} diff --git a/internal/playlists/system.go b/internal/playlists/system.go index 63c1f800..c0610dad 100644 --- a/internal/playlists/system.go +++ b/internal/playlists/system.go @@ -151,14 +151,22 @@ var systemMixWeights = recommendation.ScoringWeights{ // forYouHeadN is the number of top-scored tracks that anchor the For-You // playlist; forYouTailN is the number sampled from positions 2*headN -// onward, daily-deterministic via tieBreakHash. The 12 + 13 split +// onward, daily-deterministic via tieBreakHash. The 50 + 50 split // (~50% from the tail) gives the user a substantially different mix // day-to-day while preserving a recognizable anchor of strong -// similarity matches. Songs-like-X keeps simple pickTopN (the +// similarity matches. +// +// Sized to 100 to match Discover so the shuffle-on-play default +// (system playlists shuffle when launched from a tile) has a deep +// enough pool that re-plays within a day feel varied. Libraries with +// a thin For-You candidate pool degrade gracefully: pickHeadAndTail +// returns top-N-by-score when the capped pool can't support a full +// head/tail split, exactly as Discover returns fewer than 100 when +// its buckets are thin. Songs-like-X keeps simple pickTopN (the // seed-artist context already frames the "you'll like this" promise). const ( - forYouHeadN = 12 - forYouTailN = 13 + forYouHeadN = 50 + forYouTailN = 50 ) // scoreAndSortCandidates scores every candidate with recommendation.Score @@ -217,6 +225,191 @@ func capCandidatesByAlbumAndArtist(cands []recommendation.Candidate) []recommend return out } +// builtPlaylist is what a producer hands back: one playlist to +// materialize. A producer may return zero (no eligible tracks), +// one (For-You / Discover / each new kind), or many (Songs-like-X, +// one per seed artist) of these. +type builtPlaylist struct { + Name string + Variant string + SeedArtistID pgtype.UUID // zero value for non-seeded kinds + Tracks []rankedCandidate +} + +// systemPlaylistProducer computes the playlists for one kind for a +// given user/day. A returned error is fatal to the whole build +// (e.g. a required base query failed); per-item issues should be +// logged and yield fewer playlists, not an error. +type systemPlaylistProducer func( + ctx context.Context, q *dbq.Queries, logger *slog.Logger, + userID pgtype.UUID, dateStr string, now time.Time, +) ([]builtPlaylist, error) + +type systemPlaylistKind struct { + Key string + // Singleton kinds produce exactly one playlist per user (For-You, + // Discover, the discovery mixes) and so can be addressed by kind: + // the generic /system/{kind}/{refresh,shuffle} endpoints + the + // per-tile refresh affordance only apply to these. Non-singleton + // kinds (songs_like_artist → up to 3 per user) are played by + // playlist id and have no by-kind endpoint. + Singleton bool + Produce systemPlaylistProducer +} + +// RefreshableSystemKind reports whether `key` is a known singleton +// system-playlist kind — i.e. addressable by the generic by-kind +// refresh/shuffle endpoints and eligible for the per-tile refresh +// affordance. The API layer + clients use this so neither hardcodes +// the kind list. +func RefreshableSystemKind(key string) bool { + for _, k := range systemPlaylistRegistry { + if k.Key == key { + return k.Singleton + } + } + return false +} + +// systemPlaylistRegistry is the single source of truth for which +// system playlists exist. BuildSystemPlaylists iterates it; the +// generic /api/playlists/system/{kind}/{refresh,shuffle} endpoints +// validate against it. Adding a new mix = a producer + one entry +// here (plus its candidate query). Order is the materialize order; +// it has no functional effect (atomic replace + per-playlist +// collage are order-independent). +var systemPlaylistRegistry = []systemPlaylistKind{ + {Key: "for_you", Singleton: true, Produce: produceForYou}, + {Key: "songs_like_artist", Singleton: false, Produce: produceSeedMixes}, + {Key: "discover", Singleton: true, Produce: produceDiscover}, + {Key: "deep_cuts", Singleton: true, Produce: produceDeepCuts}, + {Key: "rediscover", Singleton: true, Produce: produceRediscover}, + {Key: "new_for_you", Singleton: true, Produce: produceNewForYou}, + {Key: "on_this_day", Singleton: true, Produce: produceOnThisDay}, + {Key: "first_listens", Singleton: true, Produce: produceFirstListens}, +} + +// produceForYou: today's seed from the user's top-5 played tracks +// (rotates daily via userIDHash), similarity candidate pool, head+ +// tail composition. The base seed query failing is fatal; a +// candidate-load failure is logged and yields no For-You. +func produceForYou( + ctx context.Context, q *dbq.Queries, logger *slog.Logger, + userID pgtype.UUID, dateStr string, now time.Time, +) ([]builtPlaylist, error) { + forYouSeeds, err := q.PickTopPlayedTracksForUser(ctx, userID) + if err != nil { + return nil, fmt.Errorf("pick for-you seed candidates: %w", err) + } + forYouSeed := pickForYouSeedForDay(forYouSeeds, userID, dateStr) + if !forYouSeed.Valid { + return nil, nil + } + zeroVec := recommendation.SessionVector{Seed: true} + cands, cerr := recommendation.LoadCandidatesFromSimilarity( + ctx, q, userID, forYouSeed, + 1, // recentlyPlayedHours — small to avoid filtering the seed's recent neighbourhood + zeroVec, + []pgtype.UUID{forYouSeed}, + recommendation.DefaultCandidateSourceLimits(), + ) + if cerr != nil { + logger.Warn("system playlist: for-you candidates load failed; skipping", + "user_id", uuidStringPL(userID), "err", cerr) + return nil, nil + } + tracks := pickHeadAndTail(cands, userID, dateStr, now, forYouHeadN, forYouTailN) + if len(tracks) == 0 { + return nil, nil + } + return []builtPlaylist{{Name: "For You", Variant: "for_you", Tracks: tracks}}, nil +} + +// produceSeedMixes: up to 3 "Songs like {artist}" mixes. Seed +// artists rotate daily-deterministically. The base seed-artist +// query failing is fatal; per-artist failures are logged + skipped. +func produceSeedMixes( + ctx context.Context, q *dbq.Queries, logger *slog.Logger, + userID pgtype.UUID, dateStr string, now time.Time, +) ([]builtPlaylist, error) { + seedRows, err := q.PickSeedArtists(ctx, userID) + if err != nil { + return nil, fmt.Errorf("pick seed artists: %w", err) + } + seedRowsLocal := make([]seedArtistRow, 0, len(seedRows)) + for _, r := range seedRows { + seedRowsLocal = append(seedRowsLocal, seedArtistRow{ + ArtistID: r.ArtistID, Score: r.Score, + }) + } + seedPool := pickSeedArtistsFromRows(seedRowsLocal) + seeds := pickSeedArtistsForDay(seedPool, userID, dateStr) + + out := make([]builtPlaylist, 0, len(seeds)) + for _, artistID := range seeds { + artistRow, aerr := q.GetArtistByID(ctx, artistID) + if aerr != nil { + logger.Warn("system playlist: seed artist load failed; skipping", + "artist_id", uuidStringPL(artistID), "err", aerr) + continue + } + seedTrack, terr := q.PickTopPlayedTrackForArtistByUser(ctx, dbq.PickTopPlayedTrackForArtistByUserParams{ + UserID: userID, ArtistID: artistID, + }) + if terr != nil || !seedTrack.Valid { + continue + } + zeroVec := recommendation.SessionVector{Seed: true} + cands, cerr := recommendation.LoadCandidatesFromSimilarity( + ctx, q, userID, seedTrack, 1, zeroVec, []pgtype.UUID{seedTrack}, + recommendation.DefaultCandidateSourceLimits(), + ) + if cerr != nil { + logger.Warn("system playlist: seed candidates load failed; skipping", + "artist_id", uuidStringPL(artistID), "err", cerr) + continue + } + // "Songs like X" excludes X's own songs. + filtered := make([]recommendation.Candidate, 0, len(cands)) + for _, c := range cands { + if !pgtypeUUIDEqual(c.Track.ArtistID, artistID) { + filtered = append(filtered, c) + } + } + tracks := pickTopN(filtered, userID, dateStr, now, systemMixLength) + if len(tracks) == 0 { + continue + } + out = append(out, builtPlaylist{ + Name: fmt.Sprintf("Songs like %s", artistRow.Name), + Variant: "songs_like_artist", + SeedArtistID: artistID, + Tracks: tracks, + }) + } + return out, nil +} + +// produceDiscover: unheard tracks biased toward dormant artists. +// Bucket failures are handled inside buildDiscoverCandidates and +// redistribute as deficit; a returned error is non-fatal here +// (logged, yields no Discover) to match the prior behavior. +func produceDiscover( + ctx context.Context, q *dbq.Queries, logger *slog.Logger, + userID pgtype.UUID, dateStr string, _ time.Time, +) ([]builtPlaylist, error) { + tracks, derr := buildDiscoverCandidates(ctx, q, logger, userID, dateStr) + if derr != nil { + logger.Warn("system playlist: discover candidates load failed; skipping", + "user_id", uuidStringPL(userID), "err", derr) + return nil, nil + } + if len(tracks) == 0 { + return nil, nil + } + return []builtPlaylist{{Name: "Discover", Variant: "discover", Tracks: tracks}}, nil +} + // BuildSystemPlaylists builds the user's daily system mixes (one For-You + // up to 3 Songs-like-{seed} mixes). Atomic-replace inside one tx; // concurrency-guarded via system_playlist_runs.in_flight; deterministic @@ -265,112 +458,22 @@ func BuildSystemPlaylists(ctx context.Context, pool *pgxpool.Pool, logger *slog. } }() - // 1. For-You: pick today's seed from the user's top-5 played tracks. - // Seed rotates daily via userIDHash so the candidate pool shifts - // across days while staying stable within a day. - forYouSeeds, err := q.PickTopPlayedTracksForUser(ctx, userID) - if err != nil { - buildErr = fmt.Errorf("pick for-you seed candidates: %w", err) - return buildErr - } - forYouSeed := pickForYouSeedForDay(forYouSeeds, userID, dateStr) - var forYouTracks []rankedCandidate - if forYouSeed.Valid { - zeroVec := recommendation.SessionVector{Seed: true} - cands, cerr := recommendation.LoadCandidatesFromSimilarity( - ctx, q, userID, forYouSeed, - 1, // recentlyPlayedHours — small to avoid filtering the seed's recent neighbourhood - zeroVec, - []pgtype.UUID{forYouSeed}, - recommendation.DefaultCandidateSourceLimits(), - ) - if cerr == nil { - // For-You uses head+tail composition: forYouHeadN top-similarity - // tracks + forYouTailN tail-sampled to inject daily-deterministic - // surprise. Songs-like-X keeps pickTopN (top-25 with caps) since - // the seed-artist context already provides the "you'll like this" - // framing. - forYouTracks = pickHeadAndTail(cands, userID, dateStr, now, forYouHeadN, forYouTailN) - } else { - logger.Warn("system playlist: for-you candidates load failed; skipping", - "user_id", uuidStringPL(userID), "err", cerr) + // Run every registered system-playlist producer. Each returns the + // playlists it wants materialized for this user/day (zero or more); + // a returned error is fatal to the whole build (matches the prior + // behavior where a seed-query failure aborted). Per-item issues are + // logged inside the producer and just yield fewer playlists. + var built []builtPlaylist + for _, kind := range systemPlaylistRegistry { + out, perr := kind.Produce(ctx, q, logger, userID, dateStr, now) + if perr != nil { + buildErr = fmt.Errorf("produce %s: %w", kind.Key, perr) + return buildErr } + built = append(built, out...) } - // 2. Seed artists for "Songs like {X}". SQL returns up to 5 candidates - // in score order; pickSeedArtistsForDay shuffles them daily- - // deterministically and takes 3 so the set of mixes rotates day-to-day. - seedRows, err := q.PickSeedArtists(ctx, userID) - if err != nil { - buildErr = fmt.Errorf("pick seed artists: %w", err) - return buildErr - } - seedRowsLocal := make([]seedArtistRow, 0, len(seedRows)) - for _, r := range seedRows { - seedRowsLocal = append(seedRowsLocal, seedArtistRow{ - ArtistID: r.ArtistID, Score: r.Score, - }) - } - seedPool := pickSeedArtistsFromRows(seedRowsLocal) - seeds := pickSeedArtistsForDay(seedPool, userID, dateStr) - - type seedMix struct { - ArtistID pgtype.UUID - ArtistName string - Tracks []rankedCandidate - } - seedMixes := make([]seedMix, 0, len(seeds)) - for _, artistID := range seeds { - artistRow, aerr := q.GetArtistByID(ctx, artistID) - if aerr != nil { - logger.Warn("system playlist: seed artist load failed; skipping", - "artist_id", uuidStringPL(artistID), "err", aerr) - continue - } - seedTrack, terr := q.PickTopPlayedTrackForArtistByUser(ctx, dbq.PickTopPlayedTrackForArtistByUserParams{ - UserID: userID, ArtistID: artistID, - }) - if terr != nil || !seedTrack.Valid { - continue - } - zeroVec := recommendation.SessionVector{Seed: true} - cands, cerr := recommendation.LoadCandidatesFromSimilarity( - ctx, q, userID, seedTrack, 1, zeroVec, []pgtype.UUID{seedTrack}, - recommendation.DefaultCandidateSourceLimits(), - ) - if cerr != nil { - logger.Warn("system playlist: seed candidates load failed; skipping", - "artist_id", uuidStringPL(artistID), "err", cerr) - continue - } - // Keep only tracks NOT by the seed artist (we want "songs like X", - // not X's own songs). - filtered := make([]recommendation.Candidate, 0, len(cands)) - for _, c := range cands { - if !pgtypeUUIDEqual(c.Track.ArtistID, artistID) { - filtered = append(filtered, c) - } - } - tracks := pickTopN(filtered, userID, dateStr, now, systemMixLength) - seedMixes = append(seedMixes, seedMix{ - ArtistID: artistID, - ArtistName: artistRow.Name, - Tracks: tracks, - }) - } - - // 4. Discover: surface unheard tracks, biased toward dormant artists. - // Individual bucket failures are logged inside buildDiscoverCandidates - // and redistribute as deficit; the function only returns an error for - // unrecoverable conditions, so a remaining derr here is genuine. - discoverTracks, derr := buildDiscoverCandidates(ctx, q, logger, userID, dateStr) - if derr != nil { - logger.Warn("system playlist: discover candidates load failed; skipping", - "user_id", uuidStringPL(userID), "err", derr) - discoverTracks = nil - } - - // 3. Atomic replace inside a transaction. + // Atomic replace inside a transaction. tx, err := pool.Begin(ctx) if err != nil { buildErr = fmt.Errorf("begin tx: %w", err) @@ -388,37 +491,16 @@ func BuildSystemPlaylists(ctx context.Context, pool *pgxpool.Pool, logger *slog. // for each one after the transaction commits. Collage generation // reads the playlist's tracks via a separate query that won't see // uncommitted rows, so the work happens post-commit. - createdIDs := make([]pgtype.UUID, 0, 1+len(seedMixes)) + createdIDs := make([]pgtype.UUID, 0, len(built)) - if len(forYouTracks) > 0 { - id, err := insertSystemPlaylist(ctx, qtx, userID, "For You", "for_you", - pgtype.UUID{}, forYouTracks) - if err != nil { - buildErr = fmt.Errorf("insert for-you: %w", err) - return buildErr - } - createdIDs = append(createdIDs, id) - } - - for _, m := range seedMixes { - if len(m.Tracks) == 0 { + for _, bp := range built { + if len(bp.Tracks) == 0 { continue } - name := fmt.Sprintf("Songs like %s", m.ArtistName) - id, err := insertSystemPlaylist(ctx, qtx, userID, name, "songs_like_artist", - m.ArtistID, m.Tracks) + id, err := insertSystemPlaylist(ctx, qtx, userID, bp.Name, bp.Variant, + bp.SeedArtistID, bp.Tracks) if err != nil { - buildErr = fmt.Errorf("insert songs-like %s: %w", uuidStringPL(m.ArtistID), err) - return buildErr - } - createdIDs = append(createdIDs, id) - } - - if len(discoverTracks) > 0 { - id, err := insertSystemPlaylist(ctx, qtx, userID, "Discover", "discover", - pgtype.UUID{}, discoverTracks) - if err != nil { - buildErr = fmt.Errorf("insert discover: %w", err) + buildErr = fmt.Errorf("insert %s (%s): %w", bp.Variant, bp.Name, err) return buildErr } createdIDs = append(createdIDs, id) diff --git a/internal/playlists/system_mixes.go b/internal/playlists/system_mixes.go new file mode 100644 index 00000000..e57233ea --- /dev/null +++ b/internal/playlists/system_mixes.go @@ -0,0 +1,145 @@ +package playlists + +import ( + "context" + "log/slog" + "time" + + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// Discovery mixes (#419-423). Each is one candidate query + a thin +// producer registered in systemPlaylistRegistry. They follow the +// Discover model: a SQL query returns ordered (id, album_id, +// artist_id) rows; we optionally diversity-cap, truncate, and emit +// rankedCandidate (Score unused — SQL gave the ranking). All are +// singleton kinds, so the generic by-kind refresh/shuffle endpoints +// and per-tile refresh affordance work with zero client changes. + +// discoveryMixLen caps each mix at the same depth as For-You / +// Discover so shuffle-on-play has a varied pool within a day. +const discoveryMixLen = 100 + +// finishMix caps (per-album<=2 / per-artist<=3) when diversify is +// set, truncates to discoveryMixLen, and converts to the insert +// type. Album-coherent mixes (New for you, First Listens) pass +// diversify=false so whole albums survive. +func finishMix(rows []discoverTrack, diversify bool) []rankedCandidate { + pool := rows + if diversify { + pool = capByAlbumAndArtist(pool) + } + if len(pool) > discoveryMixLen { + pool = pool[:discoveryMixLen] + } + if len(pool) == 0 { + return nil + } + tracks := make([]rankedCandidate, len(pool)) + for i, t := range pool { + tracks[i] = rankedCandidate{TrackID: t.ID} + } + return tracks +} + +// emit wraps the finished track list in a single builtPlaylist (the +// discovery mixes are all singletons). nil tracks → no playlist. +func emit(name, variant string, tracks []rankedCandidate) []builtPlaylist { + if len(tracks) == 0 { + return nil + } + return []builtPlaylist{{Name: name, Variant: variant, Tracks: tracks}} +} + +func produceDeepCuts( + ctx context.Context, q *dbq.Queries, logger *slog.Logger, + userID pgtype.UUID, dateStr string, _ time.Time, +) ([]builtPlaylist, error) { + rows, err := q.ListDeepCutsTracks(ctx, dbq.ListDeepCutsTracksParams{ + UserID: userID, Column2: dateStr, + }) + if err != nil { + logger.Warn("system playlist: deep-cuts query failed; skipping", + "user_id", uuidStringPL(userID), "err", err) + return nil, nil + } + dt := make([]discoverTrack, len(rows)) + for i, r := range rows { + dt[i] = discoverTrack{ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID} + } + return emit("Deep Cuts", "deep_cuts", finishMix(dt, true)), nil +} + +func produceRediscover( + ctx context.Context, q *dbq.Queries, logger *slog.Logger, + userID pgtype.UUID, _ string, _ time.Time, +) ([]builtPlaylist, error) { + rows, err := q.ListRediscoverTracks(ctx, userID) + if err != nil { + logger.Warn("system playlist: rediscover query failed; skipping", + "user_id", uuidStringPL(userID), "err", err) + return nil, nil + } + dt := make([]discoverTrack, len(rows)) + for i, r := range rows { + dt[i] = discoverTrack{ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID} + } + return emit("Rediscover", "rediscover", finishMix(dt, true)), nil +} + +func produceNewForYou( + ctx context.Context, q *dbq.Queries, logger *slog.Logger, + userID pgtype.UUID, _ string, _ time.Time, +) ([]builtPlaylist, error) { + rows, err := q.ListNewForYouTracks(ctx, userID) + if err != nil { + logger.Warn("system playlist: new-for-you query failed; skipping", + "user_id", uuidStringPL(userID), "err", err) + return nil, nil + } + dt := make([]discoverTrack, len(rows)) + for i, r := range rows { + dt[i] = discoverTrack{ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID} + } + // Album-coherent: no diversity cap so whole new albums survive. + return emit("New for you", "new_for_you", finishMix(dt, false)), nil +} + +func produceOnThisDay( + ctx context.Context, q *dbq.Queries, logger *slog.Logger, + userID pgtype.UUID, dateStr string, _ time.Time, +) ([]builtPlaylist, error) { + rows, err := q.ListOnThisDayTracks(ctx, dbq.ListOnThisDayTracksParams{ + UserID: userID, Column2: dateStr, + }) + if err != nil { + logger.Warn("system playlist: on-this-day query failed; skipping", + "user_id", uuidStringPL(userID), "err", err) + return nil, nil + } + dt := make([]discoverTrack, len(rows)) + for i, r := range rows { + dt[i] = discoverTrack{ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID} + } + return emit("On this day", "on_this_day", finishMix(dt, true)), nil +} + +func produceFirstListens( + ctx context.Context, q *dbq.Queries, logger *slog.Logger, + userID pgtype.UUID, _ string, _ time.Time, +) ([]builtPlaylist, error) { + rows, err := q.ListFirstListensTracks(ctx, userID) + if err != nil { + logger.Warn("system playlist: first-listens query failed; skipping", + "user_id", uuidStringPL(userID), "err", err) + return nil, nil + } + dt := make([]discoverTrack, len(rows)) + for i, r := range rows { + dt[i] = discoverTrack{ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID} + } + // Album-coherent (tiered by liked/played artist in SQL): no cap. + return emit("First listens", "first_listens", finishMix(dt, false)), nil +} diff --git a/web/src/lib/api/playlists.refresh-discover.test.ts b/web/src/lib/api/playlists.refresh-discover.test.ts deleted file mode 100644 index 98b72e43..00000000 --- a/web/src/lib/api/playlists.refresh-discover.test.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { refreshDiscover } from './playlists'; - -vi.mock('./client', () => ({ - api: { post: vi.fn() } -})); - -import { api } from './client'; - -describe('refreshDiscover', () => { - beforeEach(() => vi.clearAllMocks()); - - it('POSTs the correct path and returns the response', async () => { - const sample = { playlist_id: 'abc-123', track_count: 100 }; - (api.post as unknown as ReturnType).mockResolvedValueOnce(sample); - const got = await refreshDiscover(); - expect(api.post).toHaveBeenCalledWith('/api/playlists/system/discover/refresh', {}); - expect(got).toEqual(sample); - }); - - it('handles empty-library response', async () => { - const sample = { playlist_id: null, track_count: 0 }; - (api.post as unknown as ReturnType).mockResolvedValueOnce(sample); - const got = await refreshDiscover(); - expect(got.playlist_id).toBe(null); - expect(got.track_count).toBe(0); - }); -}); diff --git a/web/src/lib/api/playlists.refresh-foryou.test.ts b/web/src/lib/api/playlists.refresh-foryou.test.ts deleted file mode 100644 index de715232..00000000 --- a/web/src/lib/api/playlists.refresh-foryou.test.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { refreshForYou } from './playlists'; - -vi.mock('./client', () => ({ - api: { post: vi.fn() } -})); - -import { api } from './client'; - -describe('refreshForYou', () => { - beforeEach(() => vi.clearAllMocks()); - - it('POSTs the correct path and returns the response', async () => { - const sample = { playlist_id: 'pl-abc', track_count: 25, track_ids: ['t1', 't2'] }; - (api.post as unknown as ReturnType).mockResolvedValueOnce(sample); - const got = await refreshForYou(); - expect(api.post).toHaveBeenCalledWith('/api/playlists/system/for-you/refresh', {}); - expect(got).toEqual(sample); - }); - - it('handles empty-library response', async () => { - const sample = { playlist_id: null, track_count: 0, track_ids: [] }; - (api.post as unknown as ReturnType).mockResolvedValueOnce(sample); - const got = await refreshForYou(); - expect(got.playlist_id).toBe(null); - expect(got.track_ids).toEqual([]); - }); -}); diff --git a/web/src/lib/api/playlists.ts b/web/src/lib/api/playlists.ts index 2b442068..bd340d74 100644 --- a/web/src/lib/api/playlists.ts +++ b/web/src/lib/api/playlists.ts @@ -19,6 +19,32 @@ export async function getPlaylist(id: string): Promise { return api.get(`/api/playlists/${encodeURIComponent(id)}`); } +// #415 stage 2/3: rotation-aware play order for a system playlist. +// Same shape as getPlaylist but tracks are server-ordered (unplayed +// this rotation first, then heard; resets when exhausted). +// Intentionally uncached — it varies per play, unlike getPlaylist. +// {variant} is the raw system_variant (#411 R2: generic by-kind). +export async function systemShuffle(variant: string): Promise { + return api.get( + `/api/playlists/system/${encodeURIComponent(variant)}/shuffle`, + ); +} + +// #411 R2: generic by-kind refresh. Rebuilds the caller's system +// playlists and returns the named kind's fresh row. Replaces the +// former per-kind refreshForYou/refreshDiscover. +export type RefreshSystemResponse = { + playlist_id: string | null; + track_count: number; + track_ids: string[]; +}; +export async function refreshSystem(variant: string): Promise { + return api.post( + `/api/playlists/system/${encodeURIComponent(variant)}/refresh`, + {}, + ); +} + export type CreatePlaylistInput = { name: string; description?: string; @@ -79,35 +105,3 @@ export function createPlaylistQuery(id: string) { }); } -// System playlist refresh ---------------------------------------------------- - -export type RefreshDiscoverResponse = { - playlist_id: string | null; - track_count: number; -}; - -// Re-runs the daily system playlist build for the calling user and -// returns the resulting Discover playlist's id + track count. -// Used by the detail-page Refresh button and the home tile kebab. -export async function refreshDiscover(): Promise { - return api.post('/api/playlists/system/discover/refresh', {}); -} - -// For-You refresh ------------------------------------------------------------ - -export type RefreshForYouResponse = { - playlist_id: string | null; - track_count: number; - track_ids: string[]; -}; - -// POST /api/playlists/system/for-you/refresh — synchronously -// rebuilds the calling user's For-You playlist and returns the new -// id, track count, and track id list. -// -// Used by the home Playlists row's For-You tile play button: click -// triggers refresh, the frontend then calls getPlaylist(new_id) to -// get full TrackRef snapshots and enqueues for playback. -export async function refreshForYou(): Promise { - return api.post('/api/playlists/system/for-you/refresh', {}); -} diff --git a/web/src/lib/api/types.ts b/web/src/lib/api/types.ts index c58ded2f..d4235b42 100644 --- a/web/src/lib/api/types.ts +++ b/web/src/lib/api/types.ts @@ -59,7 +59,11 @@ export type Playlist = { description: string; is_public: boolean; kind: 'user' | 'system'; - system_variant: 'for_you' | 'songs_like_artist' | 'discover' | null; + system_variant: string | null; + // Server-derived: a singleton system kind addressable by the + // generic /system/{kind}/{refresh,shuffle} endpoints. Drives the + // per-tile refresh affordance without the client hardcoding kinds. + refreshable: boolean; seed_artist_id: string | null; cover_url: string; // empty string when no cover; UI renders glyph track_count: number; diff --git a/web/src/lib/components/AddToPlaylistMenu.test.ts b/web/src/lib/components/AddToPlaylistMenu.test.ts index 89ae38e5..8f0a123b 100644 --- a/web/src/lib/components/AddToPlaylistMenu.test.ts +++ b/web/src/lib/components/AddToPlaylistMenu.test.ts @@ -14,6 +14,7 @@ const playlistsData = vi.hoisted(() => ({ is_public: false, kind: 'user', system_variant: null, + refreshable: false, seed_artist_id: null, cover_url: '', track_count: 3, @@ -30,6 +31,7 @@ const playlistsData = vi.hoisted(() => ({ is_public: false, kind: 'user', system_variant: null, + refreshable: false, seed_artist_id: null, cover_url: '', track_count: 5, @@ -58,6 +60,7 @@ vi.mock('$lib/api/playlists', () => ({ is_public: false, kind: 'user', system_variant: null, + refreshable: false, seed_artist_id: null, cover_url: '', track_count: 0, diff --git a/web/src/lib/components/PlaylistCard.svelte b/web/src/lib/components/PlaylistCard.svelte index 45f58e22..cd401158 100644 --- a/web/src/lib/components/PlaylistCard.svelte +++ b/web/src/lib/components/PlaylistCard.svelte @@ -3,7 +3,7 @@ import { useQueryClient } from '@tanstack/svelte-query'; import type { Playlist, PlaylistTrack, TrackRef } from '$lib/api/types'; import { user } from '$lib/auth/store.svelte'; - import { getPlaylist, refreshDiscover, refreshForYou } from '$lib/api/playlists'; + import { getPlaylist, systemShuffle, refreshSystem } from '$lib/api/playlists'; import { playlistTrackToRef } from '$lib/playlists/playlistTrackToRef'; import { errCode } from '$lib/api/errors'; import { qk } from '$lib/api/queries'; @@ -13,7 +13,30 @@ let { playlist }: { playlist: Playlist } = $props(); const isOwn = $derived(user.value?.id === playlist.user_id); - const isDiscover = $derived(playlist.system_variant === 'discover'); + // Server tells us which system playlists support by-kind refresh + // (#411 R2) — no hardcoded kind list here. + const refreshable = $derived(playlist.refreshable === true); + + // #417: system playlists atomic-replace on rebuild, so created_at + // is the last-rotated time. Surface it so users know how fresh the + // mix is. Friendly wording, not the HistoryRow "m/h ago" style. + function refreshedLabel(iso: string): string { + const t = new Date(iso); + if (Number.isNaN(t.getTime())) return ''; + const now = new Date(); + const mins = Math.floor((now.getTime() - t.getTime()) / 60000); + if (mins < 5) return 'Refreshed just now'; + const startOfToday = new Date(now); + startOfToday.setHours(0, 0, 0, 0); + const days = Math.floor((startOfToday.getTime() - t.getTime()) / 86400000) + 1; + if (t.getTime() >= startOfToday.getTime()) return 'Refreshed today'; + if (days <= 1) return 'Refreshed yesterday'; + if (days < 7) return `Refreshed ${days} days ago`; + return `Refreshed ${t.toLocaleDateString(undefined, { month: 'short', day: 'numeric' })}`; + } + const refreshed = $derived( + playlist.system_variant != null ? refreshedLabel(playlist.created_at) : '' + ); const queryClient = useQueryClient(); @@ -40,38 +63,37 @@ if (starting) return; starting = true; try { - let detailID = playlist.id; - // For-You: refresh first so click means "give me a fresh mix - // and play it." Atomic replace creates a new playlist row, so - // the response's playlist_id is what we follow up with. - if (playlist.system_variant === 'for_you') { - const refreshed = await refreshForYou(); - if (!refreshed.playlist_id) { - // Empty library — nothing to play. - return; + const variant = playlist.system_variant; + if (variant != null) { + // #415: server returns the rotation-aware order (unplayed + // this rotation first). Play it as-is — no client shuffle — + // and tag the queue so play_started carries `source` and + // the server advances the rotation. + const detail = await systemShuffle(variant); + const refs = toTrackRefs(detail.tracks); + if (refs.length > 0) { + playQueue(refs, 0, { source: variant }); + } + } else { + const detail = await getPlaylist(playlist.id); + const refs = toTrackRefs(detail.tracks); + if (refs.length > 0) { + playQueue(refs, 0); } - detailID = refreshed.playlist_id; - // Invalidate caches so the home + detail views see the new - // playlist on their next read. - await queryClient.invalidateQueries({ queryKey: qk.playlists() }); - } - const detail = await getPlaylist(detailID); - const refs = toTrackRefs(detail.tracks); - if (refs.length > 0) { - playQueue(refs, 0); } } finally { starting = false; } } - async function onRefreshDiscover(e: MouseEvent) { + async function onRefresh(e: MouseEvent) { e.preventDefault(); e.stopPropagation(); menuOpen = false; + if (!refreshable || playlist.system_variant == null) return; try { - await refreshDiscover(); - pushToast('Discover refreshed.'); + await refreshSystem(playlist.system_variant); + pushToast(`${playlist.name} refreshed.`); await queryClient.invalidateQueries({ queryKey: qk.playlist(playlist.id) }); await queryClient.invalidateQueries({ queryKey: qk.playlists() }); } catch (err: unknown) { @@ -83,7 +105,7 @@ { menuOpen = false; }} />
- {#if isDiscover} + {#if refreshable}
{/if} @@ -152,6 +174,9 @@ · by {playlist.owner_username} {/if}
+ {#if refreshed} +
{refreshed}
+ {/if} diff --git a/web/src/lib/components/PlaylistCard.test.ts b/web/src/lib/components/PlaylistCard.test.ts index 1213c5f9..c5d3bf3c 100644 --- a/web/src/lib/components/PlaylistCard.test.ts +++ b/web/src/lib/components/PlaylistCard.test.ts @@ -17,6 +17,7 @@ vi.mock('$lib/api/playlists', () => ({ is_public: false, kind: 'user', system_variant: null, + refreshable: false, seed_artist_id: null, cover_url: '', track_count: 1, @@ -38,8 +39,38 @@ vi.mock('$lib/api/playlists', () => ({ } ] } as PlaylistDetail), - refreshDiscover: vi.fn().mockResolvedValue({ playlist_id: 'p-1', track_count: 5 }), - refreshForYou: vi.fn().mockResolvedValue({ playlist_id: 'p-new', track_count: 25, track_ids: ['t-1'] }) + systemShuffle: vi.fn().mockResolvedValue({ + id: 'p-sys', + user_id: 'u-self', + owner_username: 'me', + name: 'For You', + description: '', + is_public: false, + kind: 'system', + system_variant: 'for_you', + refreshable: true, + seed_artist_id: null, + cover_url: '', + track_count: 1, + duration_sec: 60, + created_at: '2026-01-01T00:00:00Z', + updated_at: '2026-01-01T00:00:00Z', + tracks: [ + { + position: 0, + track_id: 't-9', + album_id: 'al-1', + artist_id: 'ar-1', + title: 'Rotation Track', + artist_name: 'Test Artist', + album_title: 'Test Album', + duration_sec: 60, + stream_url: '/s/t-9', + added_at: '2026-01-01T00:00:00Z' + } + ] + } as PlaylistDetail), + refreshSystem: vi.fn().mockResolvedValue({ playlist_id: 'p-1', track_count: 5, track_ids: ['t-1'] }) })); vi.mock('$lib/player/store.svelte', () => ({ @@ -62,6 +93,7 @@ const base: Playlist = { is_public: false, kind: 'user', system_variant: null, + refreshable: false, seed_artist_id: null, cover_url: '', track_count: 12, @@ -143,151 +175,124 @@ describe('PlaylistCard', () => { expect(playQueue).toHaveBeenCalled(); }); - test('shows kebab button only on Discover playlists', () => { + test('shows kebab on refreshable (singleton system) playlists', () => { const discoverPlaylist: Playlist = { ...base, kind: 'system', system_variant: 'discover', + refreshable: true, name: 'Discover' }; render(PlaylistCard, { props: { playlist: discoverPlaylist } }); expect(screen.getByLabelText(/playlist actions/i)).toBeInTheDocument(); }); - test('does not show kebab button on for_you playlists', () => { - const forYouPlaylist: Playlist = { - ...base, - kind: 'system', - system_variant: 'for_you', - name: 'For You' - }; - render(PlaylistCard, { props: { playlist: forYouPlaylist } }); - expect(screen.queryByLabelText(/playlist actions/i)).not.toBeInTheDocument(); - }); - - test('does not show kebab button on user playlists', () => { + test('does not show kebab on user playlists', () => { render(PlaylistCard, { props: { playlist: base } }); expect(screen.queryByLabelText(/playlist actions/i)).not.toBeInTheDocument(); }); - test('Refresh Discover item appears in kebab menu when opened', async () => { + test('does not show kebab on a non-refreshable system playlist', () => { + // e.g. songs_like_artist — system but multi-per-user, server + // reports refreshable:false. + const songsLike: Playlist = { + ...base, + kind: 'system', + system_variant: 'songs_like_artist', + refreshable: false, + name: 'Songs like X' + }; + render(PlaylistCard, { props: { playlist: songsLike } }); + expect(screen.queryByLabelText(/playlist actions/i)).not.toBeInTheDocument(); + }); + + test('Refresh item appears in the kebab, labelled by name', async () => { const discoverPlaylist: Playlist = { ...base, kind: 'system', system_variant: 'discover', + refreshable: true, name: 'Discover' }; render(PlaylistCard, { props: { playlist: discoverPlaylist } }); - const kebabBtn = screen.getByLabelText(/playlist actions/i); - await fireEvent.click(kebabBtn); + await fireEvent.click(screen.getByLabelText(/playlist actions/i)); expect(screen.getByRole('menuitem', { name: /refresh discover/i })).toBeInTheDocument(); }); - test('Refresh Discover item is absent from for_you card kebab', () => { + test('clicking Refresh calls refreshSystem with the raw variant', async () => { + const { refreshSystem } = await import('$lib/api/playlists'); + const discoverPlaylist: Playlist = { + ...base, + kind: 'system', + system_variant: 'discover', + refreshable: true, + name: 'Discover' + }; + render(PlaylistCard, { props: { playlist: discoverPlaylist } }); + await fireEvent.click(screen.getByLabelText(/playlist actions/i)); + await fireEvent.click(screen.getByRole('menuitem', { name: /refresh discover/i })); + await waitFor(() => expect(refreshSystem).toHaveBeenCalledWith('discover')); + }); + + test('For-You play calls systemShuffle, not getPlaylist', async () => { + const { getPlaylist, systemShuffle } = await import('$lib/api/playlists'); + const { playQueue } = await import('$lib/player/store.svelte'); + const forYouPlaylist: Playlist = { + ...base, + kind: 'system', + system_variant: 'for_you', + refreshable: true, + name: 'For You', + track_count: 25 + }; + + render(PlaylistCard, { props: { playlist: forYouPlaylist } }); + const playButton = screen.getByLabelText(/Play For You/i); + await fireEvent.click(playButton); + await new Promise(resolve => setTimeout(resolve, 10)); + + // #415/#411: system play hits the rotation-aware shuffle endpoint + // (raw variant), not the cached detail GET, and never refreshes. + expect(systemShuffle).toHaveBeenCalledWith('for_you'); + expect(getPlaylist).not.toHaveBeenCalled(); + expect(playQueue).toHaveBeenCalled(); + }); + + test('For-You play passes source:for_you to playQueue (no client shuffle)', async () => { + const { playQueue } = await import('$lib/player/store.svelte'); const forYouPlaylist: Playlist = { ...base, kind: 'system', system_variant: 'for_you', name: 'For You' }; - render(PlaylistCard, { props: { playlist: forYouPlaylist } }); - expect(screen.queryByRole('menuitem', { name: /refresh discover/i })).not.toBeInTheDocument(); - }); - - test('clicking Refresh Discover calls refreshDiscover', async () => { - const { refreshDiscover } = await import('$lib/api/playlists'); - const discoverPlaylist: Playlist = { - ...base, - kind: 'system', - system_variant: 'discover', - name: 'Discover' - }; - render(PlaylistCard, { props: { playlist: discoverPlaylist } }); - const kebabBtn = screen.getByLabelText(/playlist actions/i); - await fireEvent.click(kebabBtn); - const refreshItem = screen.getByRole('menuitem', { name: /refresh discover/i }); - await fireEvent.click(refreshItem); - await waitFor(() => expect(refreshDiscover).toHaveBeenCalled()); - }); - - test('For-You play button calls refreshForYou before getPlaylist', async () => { - const { refreshForYou, getPlaylist } = await import('$lib/api/playlists'); - const { playQueue } = await import('$lib/player/store.svelte'); - const forYouPlaylist: Playlist = { - ...base, - kind: 'system', - system_variant: 'for_you', - name: 'For You', - track_count: 25 - }; - // getPlaylist should return a detail with the NEW id - (getPlaylist as ReturnType).mockResolvedValueOnce({ - id: 'p-new', - user_id: 'u-self', - owner_username: 'me', - name: 'For You', - description: '', - is_public: false, - kind: 'system', - system_variant: 'for_you', - seed_artist_id: null, - cover_url: '', - track_count: 1, - duration_sec: 60, - created_at: '2026-01-01T00:00:00Z', - updated_at: '2026-01-01T00:00:00Z', - tracks: [ - { - position: 0, - track_id: 't-1', - album_id: 'al-1', - artist_id: 'ar-1', - title: 'Test Track', - artist_name: 'Test Artist', - album_title: 'Test Album', - duration_sec: 60, - stream_url: '/s/t-1', - added_at: '2026-01-01T00:00:00Z' - } - ] - } as PlaylistDetail); - render(PlaylistCard, { props: { playlist: forYouPlaylist } }); const playButton = screen.getByLabelText(/Play For You/i); await fireEvent.click(playButton); await new Promise(resolve => setTimeout(resolve, 10)); - expect(refreshForYou).toHaveBeenCalled(); - expect(getPlaylist).toHaveBeenCalledWith('p-new'); - expect(playQueue).toHaveBeenCalled(); + expect(playQueue).toHaveBeenCalledWith( + expect.any(Array), + 0, + expect.objectContaining({ source: 'for_you' }) + ); }); - test('For-You play with empty-library response does not call playQueue', async () => { - const { refreshForYou } = await import('$lib/api/playlists'); + test('user-playlist play uses getPlaylist + plain playQueue (no source)', async () => { + const { getPlaylist, systemShuffle } = await import('$lib/api/playlists'); const { playQueue } = await import('$lib/player/store.svelte'); - (refreshForYou as ReturnType).mockResolvedValueOnce({ - playlist_id: null, - track_count: 0, - track_ids: [] - }); - const forYouPlaylist: Playlist = { - ...base, - kind: 'system', - system_variant: 'for_you', - name: 'For You', - track_count: 1 - }; - render(PlaylistCard, { props: { playlist: forYouPlaylist } }); - const playButton = screen.getByLabelText(/Play For You/i); + render(PlaylistCard, { props: { playlist: base } }); + const playButton = screen.getByLabelText(/Play Saturday morning/i); await fireEvent.click(playButton); await new Promise(resolve => setTimeout(resolve, 10)); - expect(refreshForYou).toHaveBeenCalled(); - expect(playQueue).not.toHaveBeenCalled(); + expect(systemShuffle).not.toHaveBeenCalled(); + expect(getPlaylist).toHaveBeenCalledWith('p-1'); + expect(playQueue).toHaveBeenCalledWith(expect.any(Array), 0); }); - test('non-For-You play button does NOT call refreshForYou', async () => { - const { refreshForYou, getPlaylist } = await import('$lib/api/playlists'); + test('user-playlist play does not refresh anything', async () => { + const { refreshSystem, getPlaylist } = await import('$lib/api/playlists'); const { playQueue } = await import('$lib/player/store.svelte'); render(PlaylistCard, { props: { playlist: base } }); @@ -295,7 +300,7 @@ describe('PlaylistCard', () => { await fireEvent.click(playButton); await new Promise(resolve => setTimeout(resolve, 10)); - expect(refreshForYou).not.toHaveBeenCalled(); + expect(refreshSystem).not.toHaveBeenCalled(); expect(getPlaylist).toHaveBeenCalledWith('p-1'); expect(playQueue).toHaveBeenCalled(); }); diff --git a/web/src/lib/player/events.svelte.test.ts b/web/src/lib/player/events.svelte.test.ts index 74e324f8..d7ba8cee 100644 --- a/web/src/lib/player/events.svelte.test.ts +++ b/web/src/lib/player/events.svelte.test.ts @@ -92,6 +92,35 @@ describe('useEventsDispatcher', () => { cleanup(); }); + test('auto-advance after reaching duration POSTs play_ended, not skip (#426)', async () => { + const cleanup = $effect.root(() => useEventsDispatcher()); + + playQueue([track('1', 200), track('2', 200)]); + flushSync(); + reportDuration(200); + reportStateFromAudio('playing'); + flushSync(); + await Promise.resolve(); + await Promise.resolve(); + // Track 1 plays through to its end... + reportTimeUpdate(200); + flushSync(); + // ...then the queue advances to track 2 (track id changes, store + // resets position/duration to 0 synchronously). + skipNext(); + flushSync(); + await Promise.resolve(); + + const closesForPe1 = (api.post as ReturnType).mock.calls + .map((c) => c[1] as { type: string; play_event_id?: string }) + .filter((b) => b.play_event_id === 'pe-1'); + // The finished track must close as ended, never skipped. + expect(closesForPe1.some((b) => b.type === 'play_ended')).toBe(true); + expect(closesForPe1.some((b) => b.type === 'play_skipped')).toBe(false); + + cleanup(); + }); + test('user-initiated skip mid-track POSTs play_skipped', async () => { const cleanup = $effect.root(() => useEventsDispatcher()); diff --git a/web/src/lib/player/events.svelte.ts b/web/src/lib/player/events.svelte.ts index 60d82236..44f1d148 100644 --- a/web/src/lib/player/events.svelte.ts +++ b/web/src/lib/player/events.svelte.ts @@ -9,21 +9,49 @@ import type { PlayStartedResponse, EventOkResponse } from '$lib/api/types'; // The dispatcher is a small state machine over (current track id, state). // Per spec data-flow: // - First transition into 'playing' for a new track → POST play_started. -// - Track id changes while a play is open → POST play_skipped on the prior. +// - Track id changes while a play is open → close the prior row: +// play_ended if it reached ~its duration, else play_skipped (#426). +// Without this every auto-advanced in-queue track was reported as a +// skip, force-flagging was_skipped=true server-side and poisoning +// the recommendation skip-ratio. Mirrors the Flutter reporter. // - Natural completion (state moves from 'playing' to 'paused' AND -// position >= duration > 0) → POST play_ended on the open row. +// position >= duration > 0) → POST play_ended on the open row +// (covers the last track / explicit pause-at-end). // - pagehide with a live row → navigator.sendBeacon a play_skipped. + +// Within this much of the known duration counts as "finished", not a +// skip. Matches the Flutter PlayEventsReporter tolerance. +const COMPLETION_TOLERANCE_MS = 3000; + export function useEventsDispatcher(): void { let openPlayEventId: string | null = null; let openTrackId: string | null = null; + // lastPositionMs tracks the *current* track's position (used by the + // pagehide beacon). openLastPositionMs / openReachedEnd track the + // *open row's* track specifically and are updated ONLY while that + // track is current — so a track change (which synchronously resets + // the store's position to 0 and duration to 0) can't clobber them + // before the close branch reads them. This is the #426 race fix. let lastPositionMs = 0; + let openLastPositionMs = 0; + let openDurationMs = 0; + let openReachedEnd = false; let prevTrackId: string | null = null; let prevState: string | null = null; const clientId = getOrCreateClientId(); - // Track the latest position for use by close events. Cheap; no API call. $effect(() => { - lastPositionMs = Math.round(player.position * 1000); + const posMs = Math.round(player.position * 1000); + lastPositionMs = posMs; + if (openTrackId && player.current?.id === openTrackId) { + openLastPositionMs = posMs; + if (player.duration > 0) { + openDurationMs = Math.round(player.duration * 1000); + if (posMs >= openDurationMs - COMPLETION_TOLERANCE_MS) { + openReachedEnd = true; + } + } + } }); // Main state machine. Reacts only to (current track id, state) transitions. @@ -32,17 +60,25 @@ export function useEventsDispatcher(): void { const s = player.state; const tid = t?.id ?? null; - // Track changed: close any prior open play as a skip. + // Track changed: close the prior open row. If that track reached + // ~its duration it finished naturally (auto-advance) → play_ended + // with its full duration; otherwise the user moved on early → + // play_skipped at the last position it actually reached (#426). if (tid !== prevTrackId && openPlayEventId) { const id = openPlayEventId; - const pos = lastPositionMs; + const finished = openReachedEnd; + const endedMs = openDurationMs > 0 ? openDurationMs : openLastPositionMs; + const skipMs = openLastPositionMs; openPlayEventId = null; openTrackId = null; - api.post('/api/events', { - type: 'play_skipped', - play_event_id: id, - position_ms: pos - }).catch(() => {}); + openDurationMs = 0; + openLastPositionMs = 0; + openReachedEnd = false; + api.post('/api/events', + finished + ? { type: 'play_ended', play_event_id: id, duration_played_ms: endedMs } + : { type: 'play_skipped', play_event_id: id, position_ms: skipMs } + ).catch(() => {}); } // Entered playing for a new track (no open row yet for this track). @@ -62,6 +98,9 @@ export function useEventsDispatcher(): void { const dur = lastPositionMs; openPlayEventId = null; openTrackId = null; + openDurationMs = 0; + openLastPositionMs = 0; + openReachedEnd = false; api.post('/api/events', { type: 'play_ended', play_event_id: id, @@ -92,7 +131,11 @@ export function useEventsDispatcher(): void { const res = await api.post('/api/events', { type: 'play_started', track_id: trackId, - client_id: clientId + client_id: clientId, + // #415: tag the play with the system playlist it came from + // (null for library / user-playlist / radio) so the server + // can advance that playlist's rotation. + source: player.queueSource ?? undefined }); // The user may have moved on by the time the response arrives. Only // adopt the id if we're still on the same track and still playing. diff --git a/web/src/lib/player/store.svelte.ts b/web/src/lib/player/store.svelte.ts index 6a0d9d82..156d2b49 100644 --- a/web/src/lib/player/store.svelte.ts +++ b/web/src/lib/player/store.svelte.ts @@ -41,6 +41,13 @@ let _pendingRestorePosition: number | null = null; let _radioSeedId = $state(null); let _radioRefreshInFlight = false; +// #415: which system playlist (if any) this queue was seeded from. +// 'for_you' | 'discover' | null. Read by the events dispatcher so +// play_started carries `source` and the play counts against that +// playlist's rotation. Re-set on every playQueue (a fresh non-system +// queue clears it). +let _queueSource = $state(null); + let _audioEl: HTMLAudioElement | null = null; export const player = { @@ -55,6 +62,7 @@ export const player = { get shuffle() { return _shuffle; }, get repeat() { return _repeat; }, get error() { return _error; }, + get queueSource() { return _queueSource; }, get queueDrawerOpen() { return _queueDrawerOpen; } }; @@ -62,15 +70,38 @@ export function registerAudioEl(el: HTMLAudioElement | null): void { _audioEl = el; } -export function playQueue(tracks: TrackRef[], startIndex = 0): void { +export function playQueue( + tracks: TrackRef[], + startIndex = 0, + opts: { shuffle?: boolean; source?: string | null } = {}, +): void { _radioSeedId = null; // M4c: non-radio enqueue clears the radio refresh state - _queue = tracks; - if (tracks.length === 0) { + // #415: a fresh queue resets the system-playlist source. Set only + // when seeded from a system playlist (server already returned the + // rotation-aware order, so no client shuffle in that path). + _queueSource = opts.source ?? null; + if (opts.shuffle && tracks.length > 1) { + // Fisher-Yates over the whole list. startIndex is ignored — the + // caller is asking for "random play from this pool," so the first + // track should also be random, not the one at startIndex. + const arr = tracks.slice(); + for (let j = arr.length - 1; j > 0; j--) { + const r = Math.floor(_rng() * (j + 1)); + [arr[j], arr[r]] = [arr[r], arr[j]]; + } + _queue = arr; _index = 0; - _state = 'idle'; - } else { - _index = Math.max(0, Math.min(startIndex, tracks.length - 1)); + _shuffle = true; _state = 'loading'; + } else { + _queue = tracks; + if (tracks.length === 0) { + _index = 0; + _state = 'idle'; + } else { + _index = Math.max(0, Math.min(startIndex, tracks.length - 1)); + _state = 'loading'; + } } _position = 0; _duration = 0; diff --git a/web/src/routes/page.test.ts b/web/src/routes/page.test.ts index 884dd067..a4d6b76f 100644 --- a/web/src/routes/page.test.ts +++ b/web/src/routes/page.test.ts @@ -84,6 +84,7 @@ describe('home Playlists section', () => { is_public: false, kind: 'system', system_variant: 'for_you', + refreshable: false, seed_artist_id: null, cover_url: '', track_count: 25, diff --git a/web/src/routes/playlists/[id]/+page.svelte b/web/src/routes/playlists/[id]/+page.svelte index 8182ad39..0767e0f4 100644 --- a/web/src/routes/playlists/[id]/+page.svelte +++ b/web/src/routes/playlists/[id]/+page.svelte @@ -12,7 +12,7 @@ deletePlaylist, removePlaylistTrack, reorderPlaylist, - refreshDiscover + refreshSystem } from '$lib/api/playlists'; import { qk } from '$lib/api/queries'; import { user } from '$lib/auth/store.svelte'; @@ -130,20 +130,20 @@ } } - // --- Discover refresh --- - let refreshingDiscover = $state(false); + // --- System-playlist refresh (#411 R2: generic by-kind) --- + let refreshingSystem = $state(false); - async function onRefreshDiscover() { - refreshingDiscover = true; + async function onRefreshSystem(variant: string) { + refreshingSystem = true; try { - await refreshDiscover(); - pushToast('Discover refreshed.'); + await refreshSystem(variant); + pushToast('Refreshed.'); await queryClient.invalidateQueries({ queryKey: qk.playlist(id) }); await queryClient.invalidateQueries({ queryKey: qk.playlists() }); } catch (e: unknown) { pushToast(`Refresh failed: ${errCode(e)}`, 'error'); } finally { - refreshingDiscover = false; + refreshingSystem = false; } } @@ -177,15 +177,15 @@ {#if !isOwner}· by {pl.owner_username}{/if}

- {#if pl.system_variant === 'discover'} + {#if pl.refreshable && pl.system_variant} {/if} {#if isOwner} diff --git a/web/src/routes/playlists/[id]/playlist.test.ts b/web/src/routes/playlists/[id]/playlist.test.ts index 4eac7aef..031d170c 100644 --- a/web/src/routes/playlists/[id]/playlist.test.ts +++ b/web/src/routes/playlists/[id]/playlist.test.ts @@ -19,7 +19,7 @@ vi.mock('$lib/api/playlists', () => ({ deletePlaylist: vi.fn().mockResolvedValue(undefined), removePlaylistTrack: vi.fn().mockResolvedValue(undefined), reorderPlaylist: vi.fn().mockResolvedValue(undefined), - refreshDiscover: vi.fn().mockResolvedValue({ playlist_id: 'p1', track_count: 5 }) + refreshSystem: vi.fn().mockResolvedValue({ playlist_id: 'p1', track_count: 5, track_ids: [] }) })); vi.mock('$lib/auth/store.svelte', () => ({ @@ -44,13 +44,13 @@ vi.mock('$lib/api/quarantine', () => emptyQuarantineMock()); vi.mock('$lib/api/admin/tracks', () => ({ removeTrack: vi.fn() })); import PlaylistDetailPage from './+page.svelte'; -import { createPlaylistQuery, deletePlaylist, refreshDiscover } from '$lib/api/playlists'; +import { createPlaylistQuery, deletePlaylist, refreshSystem } from '$lib/api/playlists'; const mockedQuery = createPlaylistQuery as ReturnType; const ownDetail: PlaylistDetail = { id: 'p1', user_id: 'u-self', owner_username: 'me', name: 'Mine', - description: '', is_public: false, kind: 'user', system_variant: null, seed_artist_id: null, cover_url: '', track_count: 2, duration_sec: 274, + description: '', is_public: false, kind: 'user', system_variant: null, refreshable: false, seed_artist_id: null, cover_url: '', track_count: 2, duration_sec: 274, created_at: '', updated_at: '', tracks: [ { position: 0, track_id: 't1', album_id: 'a1', artist_id: 'ar1', title: 'A', artist_name: 'X', album_title: 'Y', duration_sec: 137, stream_url: '/api/tracks/t1/stream', added_at: '' }, @@ -87,43 +87,58 @@ describe('Playlist detail page', () => { confirmSpy.mockRestore(); }); - test('renders Refresh button only on Discover playlists', () => { + test('renders Refresh button on a refreshable system playlist', () => { const discoverDetail: PlaylistDetail = { ...ownDetail, kind: 'system', - system_variant: 'discover' + system_variant: 'discover', + refreshable: true }; mockedQuery.mockReturnValue(mockQuery({ data: discoverDetail })); render(PlaylistDetailPage); - expect(screen.getByLabelText(/refresh discover/i)).toBeInTheDocument(); + expect(screen.getByLabelText(/refresh playlist/i)).toBeInTheDocument(); }); - test('does not render Refresh button on for_you playlists', () => { + test('renders Refresh button on For You too (also refreshable now)', () => { const forYouDetail: PlaylistDetail = { ...ownDetail, kind: 'system', - system_variant: 'for_you' + system_variant: 'for_you', + refreshable: true }; mockedQuery.mockReturnValue(mockQuery({ data: forYouDetail })); render(PlaylistDetailPage); - expect(screen.queryByLabelText(/refresh discover/i)).not.toBeInTheDocument(); + expect(screen.getByLabelText(/refresh playlist/i)).toBeInTheDocument(); + }); + + test('does not render Refresh on a non-refreshable system playlist', () => { + const songsLike: PlaylistDetail = { + ...ownDetail, + kind: 'system', + system_variant: 'songs_like_artist', + refreshable: false + }; + mockedQuery.mockReturnValue(mockQuery({ data: songsLike })); + render(PlaylistDetailPage); + expect(screen.queryByLabelText(/refresh playlist/i)).not.toBeInTheDocument(); }); test('does not render Refresh button on user playlists', () => { mockedQuery.mockReturnValue(mockQuery({ data: ownDetail })); render(PlaylistDetailPage); - expect(screen.queryByLabelText(/refresh discover/i)).not.toBeInTheDocument(); + expect(screen.queryByLabelText(/refresh playlist/i)).not.toBeInTheDocument(); }); - test('Refresh button calls refreshDiscover when clicked', async () => { + test('Refresh button calls refreshSystem with the variant', async () => { const discoverDetail: PlaylistDetail = { ...ownDetail, kind: 'system', - system_variant: 'discover' + system_variant: 'discover', + refreshable: true }; mockedQuery.mockReturnValue(mockQuery({ data: discoverDetail })); render(PlaylistDetailPage); - await fireEvent.click(screen.getByLabelText(/refresh discover/i)); - await waitFor(() => expect(refreshDiscover).toHaveBeenCalled()); + await fireEvent.click(screen.getByLabelText(/refresh playlist/i)); + await waitFor(() => expect(refreshSystem).toHaveBeenCalledWith('discover')); }); }); diff --git a/web/src/routes/playlists/playlists.test.ts b/web/src/routes/playlists/playlists.test.ts index e54247ee..a9fe445e 100644 --- a/web/src/routes/playlists/playlists.test.ts +++ b/web/src/routes/playlists/playlists.test.ts @@ -19,7 +19,7 @@ const mockedCreatePlaylist = createPlaylist as ReturnType; function p(over: Partial): Playlist { return { id: 'p1', user_id: 'u-self', owner_username: 'me', name: 'A', - description: '', is_public: false, kind: 'user', system_variant: null, seed_artist_id: null, cover_url: '', track_count: 0, duration_sec: 0, + description: '', is_public: false, kind: 'user', system_variant: null, refreshable: false, seed_artist_id: null, cover_url: '', track_count: 0, duration_sec: 0, created_at: '', updated_at: '', ...over }; }