fix(server): apply skip rule on auto-close instead of force-hiding orphans
autoClosePriorOpen hardcoded was_skipped=true for every orphaned play_event (a play_started whose play_ended never arrived, e.g. the client backgrounded mid-track). That hid fully-listened tracks from History — a play that sat open past its own length was capped to the track duration (ratio ~1) yet still flagged skipped. Observed live: History showed 3 plays for a day of listening because most rows were auto-closed orphans marked skipped. Now the auto-close applies the same skip rule as RecordPlayEnded to the duration-capped elapsed estimate: ratio >= threshold OR elapsed >= the duration floor -> a real play that lands in History; a genuine quick-abandon still classifies as a skip. Still writes no skip_events row, so the ambiguous auto-close never feeds the skip-ratio / recommendation signal. This is the server half. The client-side root cause (backgrounded track transitions never closed, orphaning the rows in the first place) is tracked separately. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -471,10 +471,15 @@ func (w *Writer) captureSessionVector(
|
||||
}
|
||||
|
||||
// autoClosePriorOpen closes any open (ended_at IS NULL) play_event for the
|
||||
// user. Sets ended_at = at, duration_played_ms = min(at - started_at, track
|
||||
// duration), was_skipped = true. Skip rule is NOT applied — auto-closed
|
||||
// rows are flagged skipped regardless because we don't know what the user
|
||||
// actually heard.
|
||||
// user. Sets ended_at = at and duration_played_ms = min(at - started_at,
|
||||
// track duration) — our best estimate of how long the orphan played when
|
||||
// the client never sent play_ended (e.g. backgrounded mid-track). The same
|
||||
// skip rule as RecordPlayEnded is then applied to that estimate: an orphan
|
||||
// that sat open past the track's length reads as ratio ~1 → a real play, so
|
||||
// a fully-listened track that lost its close still lands in history instead
|
||||
// of being force-hidden. Deliberately writes no skip_events row — an
|
||||
// auto-close is an ambiguous signal, not a deliberate skip, and must not
|
||||
// feed the skip-ratio / recommendation signal.
|
||||
func (w *Writer) autoClosePriorOpen(
|
||||
ctx context.Context,
|
||||
q *dbq.Queries,
|
||||
@@ -503,6 +508,7 @@ func (w *Writer) autoClosePriorOpen(
|
||||
if track.DurationMs > 0 {
|
||||
ratio = float64(elapsedMs) / float64(track.DurationMs)
|
||||
}
|
||||
isSkip := ratio < w.skipMaxCompletionRatio && elapsedMs < w.skipMaxDurationPlayedMs
|
||||
ratioPtr := ratio
|
||||
durPtr := elapsedMs
|
||||
if _, err := q.UpdatePlayEventEnded(ctx, dbq.UpdatePlayEventEndedParams{
|
||||
@@ -510,11 +516,11 @@ func (w *Writer) autoClosePriorOpen(
|
||||
EndedAt: pgtype.Timestamptz{Time: at, Valid: true},
|
||||
DurationPlayedMs: &durPtr,
|
||||
CompletionRatio: &ratioPtr,
|
||||
WasSkipped: true,
|
||||
WasSkipped: isSkip,
|
||||
}); err != nil {
|
||||
return err
|
||||
}
|
||||
w.logger.Info("playevents: auto-closed prior open row",
|
||||
"play_event_id", prior.ID, "elapsed_ms", elapsedMs)
|
||||
"play_event_id", prior.ID, "elapsed_ms", elapsedMs, "was_skipped", isSkip)
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -122,14 +122,32 @@ func TestRecordPlayStarted_AutoClosesPriorOpenRow(t *testing.T) {
|
||||
if !prior.EndedAt.Valid {
|
||||
t.Errorf("prior should be closed")
|
||||
}
|
||||
if !prior.WasSkipped {
|
||||
t.Errorf("prior should be marked was_skipped=true (auto-close convention)")
|
||||
// 45s elapsed on a 200s track clears the 30s duration threshold, so the
|
||||
// skip rule's AND fails -> NOT a skip. An orphan that played long enough
|
||||
// to count must still land in history rather than being force-hidden.
|
||||
if prior.WasSkipped {
|
||||
t.Errorf("auto-close at 45s should apply the skip rule -> was_skipped=false")
|
||||
}
|
||||
if prior.DurationPlayedMs == nil || *prior.DurationPlayedMs != 45_000 {
|
||||
t.Errorf("duration_played_ms = %v, want 45000", prior.DurationPlayedMs)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRecordPlayStarted_AutoCloseShortPlayIsSkip(t *testing.T) {
|
||||
f := newFixture(t, 200_000)
|
||||
t1 := time.Now().UTC()
|
||||
first, _ := f.w.RecordPlayStarted(context.Background(), f.user, f.track, "c", t1)
|
||||
// 5s elapsed: ratio 0.025 < 0.5 AND 5000 < 30000 -> both fail -> a skip.
|
||||
t2 := t1.Add(5 * time.Second)
|
||||
if _, err := f.w.RecordPlayStarted(context.Background(), f.user, f.track, "c", t2); err != nil {
|
||||
t.Fatalf("second: %v", err)
|
||||
}
|
||||
prior, _ := f.q.GetPlayEventByID(context.Background(), first.PlayEventID)
|
||||
if !prior.WasSkipped {
|
||||
t.Errorf("auto-close at 5s should be was_skipped=true")
|
||||
}
|
||||
}
|
||||
|
||||
func TestRecordPlayStarted_AutoCloseCapsAtTrackDuration(t *testing.T) {
|
||||
f := newFixture(t, 60_000) // 60s track
|
||||
t1 := time.Now().UTC()
|
||||
|
||||
Reference in New Issue
Block a user