feat(flutter): #415 stage 3 — play-events reporter + rotation wiring
The Flutter client previously reported NO plays — mobile listening never reached play_events, so history, recommendation scoring, ListenBrainz scrobbles, and #415 rotation all missed mobile entirely. Operator chose to close that gap properly as part of Stage 3. New: - EventsApi (api/endpoints/events.dart): play_started/ended/skipped. - PlayEventsReporter (player/play_events_reporter.dart): state machine over (track id, playing) mirroring the web dispatcher. Persists an opaque client_id in secure storage. Deliberate divergence from web: a track change inside a queue is classified ended-vs-skipped by whether the prior track reached ~its duration (3s tolerance), instead of web's blanket "track change = skip" which would mark every naturally-finished in-queue track a skip and dilute recommendation skip-ratios — the exact failure mode that motivated doing this properly. Fail-safe: no-ops when there's no audio handler (tests / no-audio env). App-lifecycle paused/ detached closes an open row as a best-effort skip (web pagehide parity). Wired in app.dart postFrame. - PlaylistsApi.systemShuffle(variant): GET the rotation-aware order. Wiring: - audio_handler: _queueSource carried through setQueueFromTracks (source param); preserved across internal skipToQueueItem rebuild. - player_provider.playTracks: source param → setQueueFromTracks. - PlaylistCard: system playlists fetch systemShuffle and play as-is tagged with source (no client shuffle — server already ordered). - playlist_detail_screen: header Play + per-track tap tag source for system playlists so rotation advances from any entry point. Known/flagged separately: the web dispatcher likely has the same false-skip-on-advance issue; not fixed here to keep #415 scoped and clients' wire behavior comparable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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<String?> playStarted({
|
||||
required String trackId,
|
||||
required String clientId,
|
||||
String? source,
|
||||
}) async {
|
||||
final r = await _dio.post<Map<String, dynamic>>(
|
||||
'/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<void> playEnded({
|
||||
required String playEventId,
|
||||
required int durationPlayedMs,
|
||||
}) async {
|
||||
await _dio.post<void>(
|
||||
'/api/events',
|
||||
data: {
|
||||
'type': 'play_ended',
|
||||
'play_event_id': playEventId,
|
||||
'duration_played_ms': durationPlayedMs,
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
Future<void> playSkipped({
|
||||
required String playEventId,
|
||||
required int positionMs,
|
||||
}) async {
|
||||
await _dio.post<void>(
|
||||
'/api/events',
|
||||
data: {
|
||||
'type': 'play_skipped',
|
||||
'play_event_id': playEventId,
|
||||
'position_ms': positionMs,
|
||||
},
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -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<PlaylistDetail> systemShuffle(String variant) async {
|
||||
final seg = variant == 'for_you' ? 'for-you' : variant;
|
||||
final r = await _dio.get<Map<String, dynamic>>(
|
||||
'/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<void> appendTracks(String playlistId, List<String> trackIds) async {
|
||||
|
||||
@@ -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<MinstrelApp> {
|
||||
// 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);
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -82,6 +82,15 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl
|
||||
/// past the fill front needs to rebuild from the stored tracks.
|
||||
List<TrackRef> _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<double> get volumeStream => _player.volumeStream;
|
||||
@@ -109,8 +118,13 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl
|
||||
if (audioCacheManager != null) _audioCacheManager = audioCacheManager;
|
||||
}
|
||||
|
||||
Future<void> setQueueFromTracks(List<TrackRef> tracks, {int initialIndex = 0}) async {
|
||||
Future<void> setQueueFromTracks(
|
||||
List<TrackRef> 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<void> 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();
|
||||
}
|
||||
|
||||
|
||||
@@ -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 = <StreamSubscription<dynamic>>[];
|
||||
bool _disposed = false;
|
||||
|
||||
String? _clientId;
|
||||
EventsApi? _api;
|
||||
|
||||
String? _openPlayEventId;
|
||||
String? _openTrackId;
|
||||
String? _prevTrackId;
|
||||
int _lastPositionMs = 0;
|
||||
int _openDurationMs = 0;
|
||||
|
||||
Future<void> 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<void> _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<String> _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<int>.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<PlayEventsReporter>((ref) {
|
||||
final r = PlayEventsReporter(ref);
|
||||
ref.onDispose(r.dispose);
|
||||
// ignore: unawaited_futures
|
||||
r.start();
|
||||
return r;
|
||||
});
|
||||
@@ -71,10 +71,13 @@ class PlayerActions {
|
||||
List<TrackRef> 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);
|
||||
}
|
||||
|
||||
@@ -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'),
|
||||
|
||||
@@ -155,7 +155,13 @@ class PlaylistCard extends ConsumerWidget {
|
||||
/// index 0. Mirrors the web PlaylistCard's onPlayClick.
|
||||
Future<void> _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 = <TrackRef>[];
|
||||
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,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user