diff --git a/internal/playevents/writer.go b/internal/playevents/writer.go index 9759d578..9d3a75f0 100644 --- a/internal/playevents/writer.go +++ b/internal/playevents/writer.go @@ -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 } diff --git a/internal/playevents/writer_test.go b/internal/playevents/writer_test.go index bfb30dc7..e4133922 100644 --- a/internal/playevents/writer_test.go +++ b/internal/playevents/writer_test.go @@ -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()