diff --git a/flutter_client/lib/api/endpoints/events.dart b/flutter_client/lib/api/endpoints/events.dart new file mode 100644 index 00000000..a20d9376 --- /dev/null +++ b/flutter_client/lib/api/endpoints/events.dart @@ -0,0 +1,65 @@ +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, + }, + ); + } +} diff --git a/flutter_client/lib/api/endpoints/playlists.dart b/flutter_client/lib/api/endpoints/playlists.dart index 4e841b0f..92f6d49b 100644 --- a/flutter_client/lib/api/endpoints/playlists.dart +++ b/flutter_client/lib/api/endpoints/playlists.dart @@ -45,6 +45,19 @@ class PlaylistsApi { return PlaylistDetail.fromJson(r.data ?? const {}); } + /// GET /api/playlists/system/{variant}/shuffle (#415 stage 2/3). + /// Same shape as get() but tracks are server-ordered rotation-aware + /// (unplayed-this-rotation first; resets when exhausted). Model + /// variant uses underscores; the route segment is hyphenated. + /// Intentionally uncached — varies per play. + Future systemShuffle(String variant) async { + final seg = variant == 'for_you' ? 'for-you' : variant; + final r = await _dio.get>( + '/api/playlists/system/$seg/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 { diff --git a/flutter_client/lib/app.dart b/flutter_client/lib/app.dart index 0c39d806..d4a1150e 100644 --- a/flutter_client/lib/app.dart +++ b/flutter_client/lib/app.dart @@ -6,6 +6,7 @@ import 'cache/metadata_prefetcher.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 +56,11 @@ 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); }); } 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..3c3fb9dd --- /dev/null +++ b/flutter_client/lib/player/play_events_reporter.dart @@ -0,0 +1,198 @@ +// 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 '../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; + + String? _openPlayEventId; + String? _openTrackId; + String? _prevTrackId; + int _lastPositionMs = 0; + int _openDurationMs = 0; + + 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) { + _lastPositionMs = p.inMilliseconds; + // Keep the open track's duration current while it's the one + // playing, so on a track change we still know how long the + // (now-prior) track was. + final mi = handler.mediaItem.value; + if (mi != null && mi.id == _openTrackId && mi.duration != null) { + _openDurationMs = mi.duration!.inMilliseconds; + } + })); + _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 with an open row → close the prior row. Natural + // completion (reached ~duration) ends it; otherwise it's a skip. + if (tid != _prevTrackId && _openPlayEventId != null) { + final id = _openPlayEventId!; + final pos = _lastPositionMs; + _openPlayEventId = null; + _openTrackId = null; + final finished = _openDurationMs > 0 && + pos >= _openDurationMs - _completionToleranceMs; + if (finished) { + _api?.playEnded(playEventId: id, durationPlayedMs: pos) + .catchError((_) {}); + } else { + _api?.playSkipped(playEventId: id, positionMs: pos) + .catchError((_) {}); + } + } + + // Entered playing for a new track → open a row. + if (tid != null && playing && _openTrackId != tid) { + _startNew(handler, tid); + } + + // Whole-queue natural end (just_audio only emits `completed` at + // the end of the sequence, not between items) → end the open row. + if (completed && _openPlayEventId != null) { + final id = _openPlayEventId!; + final dur = _lastPositionMs; + _openPlayEventId = null; + _openTrackId = null; + _api?.playEnded(playEventId: id, durationPlayedMs: dur) + .catchError((_) {}); + } + + _prevTrackId = tid; + } + + Future _startNew(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: handler.queueSource, + ); + // The user may have moved on by the time the response lands — + // only adopt the id if we're still on the same track. + if (id != null && handler.mediaItem.value?.id == trackId) { + _openPlayEventId = id; + _openTrackId = trackId; + final d = handler.mediaItem.value?.duration; + _openDurationMs = d?.inMilliseconds ?? 0; + } + } catch (_) { + // Best-effort; a missed event is acceptable per spec v1. + } + } + + @override + void didChangeAppLifecycleState(AppLifecycleState state) { + // App backgrounded / killed with a live row: best-effort skip + // close so the row doesn't dangle. Mirrors web's pagehide beacon. + if (state == AppLifecycleState.paused || + state == AppLifecycleState.detached) { + final id = _openPlayEventId; + if (id != null) { + _api + ?.playSkipped(playEventId: id, positionMs: _lastPositionMs) + .catchError((_) {}); + } + } + } + + 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 cc14d645..c1134aba 100644 --- a/flutter_client/lib/player/player_provider.dart +++ b/flutter_client/lib/player/player_provider.dart @@ -71,10 +71,13 @@ class PlayerActions { List tracks, { int initialIndex = 0, bool shuffle = false, + String? source, }) async { // shuffle=true means "play this pool randomly, starting at a - // random track." Used for system playlists so the first track of - // a re-play within the day is different from the prior one. + // 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); @@ -91,7 +94,7 @@ class PlayerActions { audioCacheManager: audioCache, likeBridge: _buildLikeBridge(), ); - await h.setQueueFromTracks(tracks, initialIndex: startAt); + await h.setQueueFromTracks(tracks, initialIndex: startAt, source: source); if (shuffle) { await h.setShuffleMode(AudioServiceShuffleMode.all); } diff --git a/flutter_client/lib/playlists/playlist_detail_screen.dart b/flutter_client/lib/playlists/playlist_detail_screen.dart index 957abd9d..7f0f59d8 100644 --- a/flutter_client/lib/playlists/playlist_detail_screen.dart +++ b/flutter_client/lib/playlists/playlist_detail_screen.dart @@ -212,6 +212,9 @@ class _Body extends ConsumerWidget { ref.read(playerActionsProvider).playTracks( playableRefs, initialIndex: startIdx >= 0 ? startIdx : 0, + source: detail.playlist.isSystem + ? detail.playlist.systemVariant + : null, ); // Keep liveTrack referenced to avoid an unused-variable // warning while we leave hooks for menu wiring later. @@ -293,7 +296,10 @@ class _Header extends ConsumerWidget { FilledButton.icon( onPressed: () { final refs = playable.map(_toTrackRef).toList(growable: false); - ref.read(playerActionsProvider).playTracks(refs); + ref.read(playerActionsProvider).playTracks( + refs, + source: p.isSystem ? p.systemVariant : null, + ); }, icon: const Icon(Icons.play_arrow), label: const Text('Play'), diff --git a/flutter_client/lib/playlists/widgets/playlist_card.dart b/flutter_client/lib/playlists/widgets/playlist_card.dart index ccb25613..0026834b 100644 --- a/flutter_client/lib/playlists/widgets/playlist_card.dart +++ b/flutter_client/lib/playlists/widgets/playlist_card.dart @@ -155,7 +155,13 @@ class PlaylistCard extends ConsumerWidget { /// 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); + // System playlists: fetch the server's rotation-aware order + // (#415) and play it as-is, tagged with the source so the + // play-events reporter advances that playlist's rotation. User + // playlists keep stored order, untagged. + final detail = playlist.isSystem + ? await api.systemShuffle(playlist.systemVariant!) + : await api.get(playlist.id); final refs = []; for (final t in detail.tracks) { if (t.trackId == null) continue; @@ -171,13 +177,14 @@ class PlaylistCard extends ConsumerWidget { )); } if (refs.isEmpty) return; - // System playlists default to shuffle so replaying within a day - // doesn't deliver the identical experience. User playlists keep - // stored order. + // System playlists already arrive in rotation-aware order from + // the server (#415) — play as-is, tagged with the variant so the + // reporter advances rotation. No client shuffle. User playlists + // play in stored order, untagged. await ref.read(playerActionsProvider).playTracks( refs, initialIndex: 0, - shuffle: playlist.isSystem, + source: playlist.isSystem ? playlist.systemVariant : null, ); } }