feat(android): move skip classification from client to server
android / Build + lint + test (push) Successful in 3m36s
android / Build signed release APK (push) Has been skipped

PlayEventsReporter used to make the skip-vs-ended call client-side:
if the play head reached within 3s of duration -> playEnded with
full duration; otherwise -> playSkipped (forces was_skipped=true on
the server regardless of actual play time). That left the server's
configurable skip rule (skip_max_completion_ratio +
skip_max_duration_played_ms, default 50%/30s) dead code and meant
any "next" tap registered as a skip even after most of the track
played - too strict per operator 2026-06-01.

PlayEventsReporter now always calls playEnded with the actual
position played to. Server's RecordPlayEnded applies the rule and
decides was_skipped. If the play ran to completion, the last
observed position is approximately the track duration -> ratio ~1
-> not skipped. If the user hit next mid-track, the duration the
server sees is the real listen time and the rule fires normally.

Drops the curReachedEnd field and COMPLETION_TOLERANCE_MS constant
that backed the prior classification. playSkipped wire endpoint
stays in place for a future explicit-dislike affordance (not wired
to track-change transitions any more).

Parity-map row updated. Web backflow tracked as task #57 (Scribe
521). Flutter not touched - deprecated per M8.
This commit is contained in:
2026-06-01 08:32:05 -04:00
parent eb4537c013
commit d82e744d87
@@ -9,7 +9,6 @@ import com.fabledsword.minstrel.cache.mutations.MutationQueue
import com.fabledsword.minstrel.cache.mutations.PlayOfflinePayload
import com.fabledsword.minstrel.di.ApplicationScope
import com.fabledsword.minstrel.models.wire.PlayEndedRequest
import com.fabledsword.minstrel.models.wire.PlaySkippedRequest
import com.fabledsword.minstrel.models.wire.PlayStartedRequest
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
@@ -21,8 +20,6 @@ import java.util.UUID
import javax.inject.Inject
import javax.inject.Singleton
private const val COMPLETION_TOLERANCE_MS = 3_000L
/**
* Mobile play-event lifecycle reporter. Closes the parity gap where
* Android-played tracks never reached the server — without this,
@@ -31,11 +28,18 @@ private const val COMPLETION_TOLERANCE_MS = 3_000L
* and system-playlist rotation didn't advance on mobile activity.
*
* State machine over (current track id, playing) observed against
* [PlayerController.uiState]. Mirrors Flutter's
* `play_events_reporter.dart`. The skip-vs-ended classification uses
* a 3-second tolerance to avoid marking naturally-finished in-queue
* tracks as skipped (which would dilute the recommendation
* skip-ratio).
* [PlayerController.uiState]. The client always calls `playEnded`
* with the actual position played to; the server applies its
* configurable skip rule (`skip_max_completion_ratio` AND
* `skip_max_duration_played_ms`, default 50% AND 30s) to decide
* was_skipped. Earlier shape made the classification client-side via
* a "within 3s of duration" rule, which left the server's threshold
* dead code and treated any "next" tap as a skip — too strict
* (operator 2026-06-01).
*
* `playSkipped` endpoint stays on the wire for a future explicit-
* dislike affordance, but isn't called from normal track-change
* transitions any more.
*
* Live calls go through [EventsApi]; failures fall through to the
* offline [MutationQueue] so a flaky connection never loses a play.
@@ -59,7 +63,6 @@ class PlayEventsReporter @Inject constructor(
private var curSource: String? = null
private var curLastPositionMs: Long = 0
private var curDurationMs: Long = 0
private var curReachedEnd: Boolean = false
// Server play_event_id when the live play_started succeeded;
// null if start failed/offline — then the close is captured into
@@ -106,12 +109,7 @@ class PlayEventsReporter @Inject constructor(
// change (where currentTrack is briefly the new one).
if (curTrackId != null && trackId == curTrackId) {
curLastPositionMs = positionMs
if (durationMs > 0) {
curDurationMs = durationMs
if (positionMs >= curDurationMs - COMPLETION_TOLERANCE_MS) {
curReachedEnd = true
}
}
if (durationMs > 0) curDurationMs = durationMs
}
prevTrackId = trackId
@@ -123,7 +121,6 @@ class PlayEventsReporter @Inject constructor(
curSource = source
curLastPositionMs = 0
curDurationMs = durationMs
curReachedEnd = false
openPlayEventId = null
startLive(trackId = trackId, source = source)
}
@@ -158,30 +155,26 @@ class PlayEventsReporter @Inject constructor(
resetCurrent()
return
}
val reached = curReachedEnd
val lastPos = curLastPositionMs
val durationMs = if (reached && curDurationMs > 0) curDurationMs else lastPos
// Always send the actual position played to; the server's
// skip rule (50%/30s defaults, configurable) classifies
// was_skipped from this. If the track ran to completion the
// last observed position is ~= duration, which gives ratio ~1
// and the server records a non-skip. The COMPLETION_TOLERANCE
// rounding the prior shape did is no longer needed — the
// server's threshold is the source of truth.
val durationMs = curLastPositionMs
val source = curSource
val id = openPlayEventId
if (!viaOffline && id != null) {
scope.launch {
try {
if (reached) {
api.playEnded(
PlayEndedRequest(
playEventId = id,
durationPlayedMs = durationMs,
),
)
} else {
api.playSkipped(
PlaySkippedRequest(
playEventId = id,
positionMs = lastPos,
),
)
}
api.playEnded(
PlayEndedRequest(
playEventId = id,
durationPlayedMs = durationMs,
),
)
} catch (
@Suppress("TooGenericExceptionCaught", "SwallowedException") e: Throwable,
) {
@@ -200,7 +193,6 @@ class PlayEventsReporter @Inject constructor(
curSource = null
curLastPositionMs = 0
curDurationMs = 0
curReachedEnd = false
openPlayEventId = null
}