diff --git a/internal/api/me_recommendation_metrics.go b/internal/api/me_recommendation_metrics.go index 6da64398..20b8604d 100644 --- a/internal/api/me_recommendation_metrics.go +++ b/internal/api/me_recommendation_metrics.go @@ -39,6 +39,11 @@ type surfaceMetric struct { SkipRate float64 `json:"skip_rate"` // skips / plays, [0,1] AvgCompletion float64 `json:"avg_completion"` // mean completion ratio, [0,1] LowConfidence bool `json:"low_confidence"` // plays < recMetricsLowVolume + // Breakdown splits the family into sub-populations. Only For You + // carries one (#1249): taste picks vs fresh picks (the deliberate + // freshness injection), plus earlier plays that predate attribution. + // The parent row remains the sum of its breakdown. + Breakdown []surfaceMetric `json:"breakdown,omitempty"` } // surfaceGroup is one intent band of surface families. @@ -163,6 +168,40 @@ func (h *handlers) handleGetRecommendationMetrics(w http.ResponseWriter, r *http writeJSON(w, http.StatusOK, bucketMetricsResponse(days, rows)) } +// forYouPickFamilies defines the For You breakdown rows (#1249), keyed +// by play_events.pick_kind. "" (NULL pick_kind) = plays recorded before +// attribution shipped, or whose track had already rotated out of the +// snapshot at ingestion — kept visible so the parent row's sums stay +// transparent instead of silently shrinking. +var forYouPickFamilies = map[string]recFamily{ + "taste": {"for_you_taste", "Taste picks", intentGoTo}, + "fresh": {"for_you_fresh", "Fresh picks", intentGoTo}, + "": {"for_you_unattributed", "Earlier plays", intentGoTo}, +} + +// forYouBreakdown folds pick-kind accums into the parent metric's +// Breakdown. Attached only when at least one attributed (taste/fresh) +// play exists — an all-unattributed breakdown would just repeat the +// parent row. +func forYouBreakdown(picks map[string]*familyAccum) []surfaceMetric { + attributed := int64(0) + for kind, acc := range picks { + if kind != "" { + attributed += acc.plays + } + } + if attributed == 0 { + return nil + } + out := make([]surfaceMetric, 0, len(picks)) + for _, kind := range []string{"taste", "fresh", ""} { + if acc, ok := picks[kind]; ok && acc.plays > 0 { + out = append(out, acc.metric()) + } + } + return out +} + // bucketMetricsResponse folds the raw per-source rows into the grouped, // baseline-anchored response shape. Split from the handler for pure-unit // testability. @@ -171,6 +210,7 @@ func bucketMetricsResponse( ) recommendationMetricsResp { baseline := &familyAccum{fam: recFamily{"manual", "Manual library plays", ""}} families := map[string]*familyAccum{} + forYouPicks := map[string]*familyAccum{} for _, row := range rows { if row.Source == nil || *row.Source == "" { baseline.add(row) @@ -183,6 +223,18 @@ func bucketMetricsResponse( families[fam.key] = acc } acc.add(row) + if fam.key == "for_you" { + kind := "" + if row.PickKind != nil { + kind = *row.PickKind + } + pick, ok := forYouPicks[kind] + if !ok { + pick = &familyAccum{fam: forYouPickFamilies[kind]} + forYouPicks[kind] = pick + } + pick.add(row) + } } resp := recommendationMetricsResp{WindowDays: days, Groups: []surfaceGroup{}} @@ -198,7 +250,11 @@ func bucketMetricsResponse( group := surfaceGroup{Intent: g.intent, Label: g.label} for _, acc := range families { if acc.fam.intent == g.intent { - group.Surfaces = append(group.Surfaces, acc.metric()) + m := acc.metric() + if acc.fam.key == "for_you" { + m.Breakdown = forYouBreakdown(forYouPicks) + } + group.Surfaces = append(group.Surfaces, m) } } if len(group.Surfaces) == 0 { diff --git a/internal/api/me_recommendation_metrics_test.go b/internal/api/me_recommendation_metrics_test.go index 1c1f49dc..c17b8761 100644 --- a/internal/api/me_recommendation_metrics_test.go +++ b/internal/api/me_recommendation_metrics_test.go @@ -23,16 +23,18 @@ func newMetricsRouter(h *handlers) chi.Router { // seedSourcedPlay inserts a play_event with an explicit source + completion + // skip flag. A nil source inserts NULL (manual library play → baseline). +// pickKind is the For You taste/fresh attribution (#1249); nil everywhere +// except attributed for_you plays. func seedSourcedPlay( t *testing.T, h *handlers, userID, trackID, sessionID pgtype.UUID, - source *string, completion float64, skipped bool, + source, pickKind *string, completion float64, skipped bool, ) { t.Helper() if _, err := h.pool.Exec(context.Background(), `INSERT INTO play_events - (user_id, track_id, session_id, started_at, source, completion_ratio, was_skipped) - VALUES ($1, $2, $3, now(), $4, $5, $6)`, - userID, trackID, sessionID, source, completion, skipped); err != nil { + (user_id, track_id, session_id, started_at, source, pick_kind, completion_ratio, was_skipped) + VALUES ($1, $2, $3, now(), $4, $5, $6, $7)`, + userID, trackID, sessionID, source, pickKind, completion, skipped); err != nil { t.Fatalf("seed sourced play: %v", err) } } @@ -105,6 +107,12 @@ func TestBucketMetricsResponse_FamiliesGroupsBaseline(t *testing.T) { t.Error("NULL source must not appear as a surface family") } + // No attributed (taste/fresh) plays in this fixture → no breakdown; + // an all-unattributed breakdown would just repeat the parent row. + if fy := findSurface(resp, "for_you"); fy == nil || fy.Breakdown != nil { + t.Errorf("for_you breakdown = %+v, want nil without attributed plays", fy) + } + // Group ordering is intent-banded: go_to before discovery before direct. wantOrder := []string{intentGoTo, intentDiscovery, intentDirect} if len(resp.Groups) != len(wantOrder) { @@ -117,6 +125,49 @@ func TestBucketMetricsResponse_FamiliesGroupsBaseline(t *testing.T) { } } +func TestBucketMetricsResponse_ForYouBreakdown(t *testing.T) { + src := func(s string) *string { return &s } + kind := func(s string) *string { return &s } + rows := []dbq.RecommendationSourceMetricsForUserRow{ + {Source: src("for_you"), PickKind: kind("taste"), Plays: 30, Skips: 3, + CompletionN: 30, AvgCompletion: 0.9}, + {Source: src("for_you"), PickKind: kind("fresh"), Plays: 10, Skips: 4, + CompletionN: 10, AvgCompletion: 0.5}, + // NULL pick_kind = plays that predate attribution. + {Source: src("for_you"), Plays: 5, Skips: 1, CompletionN: 5, AvgCompletion: 0.7}, + } + resp := bucketMetricsResponse(recMetricsDefaultDays, rows) + + fy := findSurface(resp, "for_you") + if fy == nil { + t.Fatal("for_you family missing") + } + // The parent row stays the sum of its breakdown. + if fy.Plays != 45 || fy.Skips != 8 { + t.Errorf("for_you plays/skips = %d/%d, want 45/8", fy.Plays, fy.Skips) + } + if len(fy.Breakdown) != 3 { + t.Fatalf("breakdown rows = %d, want 3 (taste, fresh, earlier)", len(fy.Breakdown)) + } + wantKeys := []string{"for_you_taste", "for_you_fresh", "for_you_unattributed"} + for i, k := range wantKeys { + if fy.Breakdown[i].Key != k { + t.Errorf("breakdown[%d].key = %s, want %s", i, fy.Breakdown[i].Key, k) + } + } + taste, fresh := fy.Breakdown[0], fy.Breakdown[1] + if taste.Plays != 30 || taste.SkipRate != 0.1 || taste.LowConfidence { + t.Errorf("taste = %+v, want plays=30 skip_rate=0.1 confident", taste) + } + if fresh.Plays != 10 || fresh.SkipRate != 0.4 || !fresh.LowConfidence { + t.Errorf("fresh = %+v, want plays=10 skip_rate=0.4 low-confidence", fresh) + } + // Breakdown rows never appear as their own surface families. + if s := findSurface(resp, "for_you_taste"); s != nil { + t.Error("for_you_taste must not be a top-level surface") + } +} + func TestRecommendationMetrics_BucketsWithBaseline(t *testing.T) { if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" { t.Skip("MINSTREL_TEST_DATABASE_URL not set") @@ -130,14 +181,16 @@ func TestRecommendationMetrics_BucketsWithBaseline(t *testing.T) { forYou := "for_you" radioA := "radio:11111111-1111-1111-1111-111111111111" + taste, fresh := "taste", "fresh" // for_you: 3 plays, 1 skipped; completions 1.0, 0.95, 0.05 → mean 0.6667. - seedSourcedPlay(t, h, user.ID, tk.ID, session, &forYou, 1.0, false) - seedSourcedPlay(t, h, user.ID, tk.ID, session, &forYou, 0.95, false) - seedSourcedPlay(t, h, user.ID, tk.ID, session, &forYou, 0.05, true) + // Two attributed as taste picks, the skipped one as a fresh pick (#1249). + seedSourcedPlay(t, h, user.ID, tk.ID, session, &forYou, &taste, 1.0, false) + seedSourcedPlay(t, h, user.ID, tk.ID, session, &forYou, &taste, 0.95, false) + seedSourcedPlay(t, h, user.ID, tk.ID, session, &forYou, &fresh, 0.05, true) // A radio session play collapses into the "radio" family. - seedSourcedPlay(t, h, user.ID, tk.ID, session, &radioA, 0.8, false) + seedSourcedPlay(t, h, user.ID, tk.ID, session, &radioA, nil, 0.8, false) // Manual play (NULL source) — the baseline row. - seedSourcedPlay(t, h, user.ID, tk.ID, session, nil, 1.0, false) + seedSourcedPlay(t, h, user.ID, tk.ID, session, nil, nil, 1.0, false) req := httptest.NewRequest(http.MethodGet, "/api/me/recommendation-metrics", nil) req = withUser(req, user) @@ -170,6 +223,16 @@ func TestRecommendationMetrics_BucketsWithBaseline(t *testing.T) { if fy.AvgCompletion < 0.66 || fy.AvgCompletion > 0.67 { t.Errorf("for_you avg_completion = %.4f, want ~0.6667", fy.AvgCompletion) } + // Pick-kind attribution surfaces as the For You breakdown (#1249). + if len(fy.Breakdown) != 2 { + t.Fatalf("for_you breakdown rows = %d, want 2 (taste, fresh)", len(fy.Breakdown)) + } + if b := fy.Breakdown[0]; b.Key != "for_you_taste" || b.Plays != 2 || b.Skips != 0 { + t.Errorf("breakdown[0] = %+v, want for_you_taste plays=2 skips=0", b) + } + if b := fy.Breakdown[1]; b.Key != "for_you_fresh" || b.Plays != 1 || b.Skips != 1 { + t.Errorf("breakdown[1] = %+v, want for_you_fresh plays=1 skips=1", b) + } if radio := findSurface(resp, "radio"); radio == nil || radio.Plays != 1 { t.Errorf("radio family = %+v, want plays=1 (collapsed from radio:)", radio) } diff --git a/internal/db/dbq/events.sql.go b/internal/db/dbq/events.sql.go index 59f08d04..27b94317 100644 --- a/internal/db/dbq/events.sql.go +++ b/internal/db/dbq/events.sql.go @@ -31,6 +31,33 @@ func (q *Queries) GetCurrentSessionVectorForUser(ctx context.Context, userID pgt return session_vector_at_play, err } +const getForYouPickKindForTrack = `-- name: GetForYouPickKindForTrack :one +SELECT pt.pick_kind +FROM playlist_tracks pt +JOIN playlists p ON p.id = pt.playlist_id +WHERE p.user_id = $1 + AND p.system_variant = 'for_you' + AND pt.track_id = $2 +LIMIT 1 +` + +type GetForYouPickKindForTrackParams struct { + UserID pgtype.UUID + TrackID pgtype.UUID +} + +// Looks a track up in the user's CURRENT For You snapshot and returns +// its pick_kind ('taste'/'fresh'). Used at play-ingestion time to +// freeze exploration attribution onto the play_event — the snapshot is +// rebuilt daily, so attribution can't be reconstructed at read time +// (#1249). No row = track not in today's snapshot (caller stores NULL). +func (q *Queries) GetForYouPickKindForTrack(ctx context.Context, arg GetForYouPickKindForTrackParams) (*string, error) { + row := q.db.QueryRow(ctx, getForYouPickKindForTrack, arg.UserID, arg.TrackID) + var pick_kind *string + err := row.Scan(&pick_kind) + return pick_kind, err +} + const getMostRecentPlaySessionForUser = `-- name: GetMostRecentPlaySessionForUser :one SELECT id, user_id, started_at, ended_at, last_event_at, track_count, client_id FROM play_sessions WHERE user_id = $1 @@ -54,7 +81,7 @@ func (q *Queries) GetMostRecentPlaySessionForUser(ctx context.Context, userID pg } const getOpenPlayEventForUser = `-- name: GetOpenPlayEventForUser :one -SELECT id, user_id, track_id, session_id, started_at, ended_at, duration_played_ms, completion_ratio, was_skipped, client_id, session_vector_at_play, scrobbled_at, source FROM play_events +SELECT id, user_id, track_id, session_id, started_at, ended_at, duration_played_ms, completion_ratio, was_skipped, client_id, session_vector_at_play, scrobbled_at, source, pick_kind FROM play_events WHERE user_id = $1 AND ended_at IS NULL ORDER BY started_at DESC LIMIT 1 @@ -79,12 +106,13 @@ func (q *Queries) GetOpenPlayEventForUser(ctx context.Context, userID pgtype.UUI &i.SessionVectorAtPlay, &i.ScrobbledAt, &i.Source, + &i.PickKind, ) return i, err } const getPlayEventByID = `-- name: GetPlayEventByID :one -SELECT id, user_id, track_id, session_id, started_at, ended_at, duration_played_ms, completion_ratio, was_skipped, client_id, session_vector_at_play, scrobbled_at, source FROM play_events WHERE id = $1 +SELECT id, user_id, track_id, session_id, started_at, ended_at, duration_played_ms, completion_ratio, was_skipped, client_id, session_vector_at_play, scrobbled_at, source, pick_kind FROM play_events WHERE id = $1 ` func (q *Queries) GetPlayEventByID(ctx context.Context, id pgtype.UUID) (PlayEvent, error) { @@ -104,15 +132,16 @@ func (q *Queries) GetPlayEventByID(ctx context.Context, id pgtype.UUID) (PlayEve &i.SessionVectorAtPlay, &i.ScrobbledAt, &i.Source, + &i.PickKind, ) return i, err } const insertPlayEvent = `-- name: InsertPlayEvent :one INSERT INTO play_events ( - user_id, track_id, session_id, started_at, client_id, source -) VALUES ($1, $2, $3, $4, $5, $6) -RETURNING id, user_id, track_id, session_id, started_at, ended_at, duration_played_ms, completion_ratio, was_skipped, client_id, session_vector_at_play, scrobbled_at, source + user_id, track_id, session_id, started_at, client_id, source, pick_kind +) VALUES ($1, $2, $3, $4, $5, $6, $7::text) +RETURNING id, user_id, track_id, session_id, started_at, ended_at, duration_played_ms, completion_ratio, was_skipped, client_id, session_vector_at_play, scrobbled_at, source, pick_kind ` type InsertPlayEventParams struct { @@ -122,8 +151,11 @@ type InsertPlayEventParams struct { StartedAt pgtype.Timestamptz ClientID *string Source *string + PickKind *string } +// pick_kind is non-NULL only for source='for_you' plays whose track was +// found in the user's live For You snapshot at ingestion time (#1249). func (q *Queries) InsertPlayEvent(ctx context.Context, arg InsertPlayEventParams) (PlayEvent, error) { row := q.db.QueryRow(ctx, insertPlayEvent, arg.UserID, @@ -132,6 +164,7 @@ func (q *Queries) InsertPlayEvent(ctx context.Context, arg InsertPlayEventParams arg.StartedAt, arg.ClientID, arg.Source, + arg.PickKind, ) var i PlayEvent err := row.Scan( @@ -148,6 +181,7 @@ func (q *Queries) InsertPlayEvent(ctx context.Context, arg InsertPlayEventParams &i.SessionVectorAtPlay, &i.ScrobbledAt, &i.Source, + &i.PickKind, ) return i, err } @@ -291,7 +325,7 @@ SET ended_at = $2, completion_ratio = $4, was_skipped = $5 WHERE id = $1 -RETURNING id, user_id, track_id, session_id, started_at, ended_at, duration_played_ms, completion_ratio, was_skipped, client_id, session_vector_at_play, scrobbled_at, source +RETURNING id, user_id, track_id, session_id, started_at, ended_at, duration_played_ms, completion_ratio, was_skipped, client_id, session_vector_at_play, scrobbled_at, source, pick_kind ` type UpdatePlayEventEndedParams struct { @@ -328,6 +362,7 @@ func (q *Queries) UpdatePlayEventEnded(ctx context.Context, arg UpdatePlayEventE &i.SessionVectorAtPlay, &i.ScrobbledAt, &i.Source, + &i.PickKind, ) return i, err } diff --git a/internal/db/dbq/models.go b/internal/db/dbq/models.go index bdf64e72..12f5ae8b 100644 --- a/internal/db/dbq/models.go +++ b/internal/db/dbq/models.go @@ -383,6 +383,7 @@ type PlayEvent struct { SessionVectorAtPlay []byte ScrobbledAt pgtype.Timestamptz Source *string + PickKind *string } type PlaySession struct { @@ -433,6 +434,7 @@ type PlaylistTrack struct { AlbumTitle string DurationSec int32 AddedAt pgtype.Timestamptz + PickKind *string } type RegistrationSetting struct { diff --git a/internal/db/dbq/playlists.sql.go b/internal/db/dbq/playlists.sql.go index 71825f80..e4a9855e 100644 --- a/internal/db/dbq/playlists.sql.go +++ b/internal/db/dbq/playlists.sql.go @@ -12,7 +12,7 @@ import ( ) const appendPlaylistTrack = `-- name: AppendPlaylistTrack :one -INSERT INTO playlist_tracks (playlist_id, position, track_id, title, artist_name, album_title, duration_sec) +INSERT INTO playlist_tracks (playlist_id, position, track_id, title, artist_name, album_title, duration_sec, pick_kind) SELECT $1::uuid, COALESCE((SELECT MAX(position) + 1 FROM playlist_tracks WHERE playlist_id = $1::uuid), 0), @@ -20,24 +20,27 @@ SELECT t.title, artists.name, albums.title, - (t.duration_ms / 1000)::integer + (t.duration_ms / 1000)::integer, + $2::text FROM tracks t JOIN albums ON albums.id = t.album_id JOIN artists ON artists.id = t.artist_id -WHERE t.id = $2::uuid -RETURNING playlist_id, position, track_id, title, artist_name, album_title, duration_sec, added_at +WHERE t.id = $3::uuid +RETURNING playlist_id, position, track_id, title, artist_name, album_title, duration_sec, added_at, pick_kind ` type AppendPlaylistTrackParams struct { PlaylistID pgtype.UUID + PickKind *string TrackID pgtype.UUID } // Inserts at the next available position. Snapshot fields are copied // from the tracks/albums/artists join at insert time. tracks.duration_ms -// is converted to seconds for the snapshot. +// is converted to seconds for the snapshot. pick_kind is NULL for manual +// playlists; the For You builder stamps 'taste'/'fresh' (#1249). func (q *Queries) AppendPlaylistTrack(ctx context.Context, arg AppendPlaylistTrackParams) (PlaylistTrack, error) { - row := q.db.QueryRow(ctx, appendPlaylistTrack, arg.PlaylistID, arg.TrackID) + row := q.db.QueryRow(ctx, appendPlaylistTrack, arg.PlaylistID, arg.PickKind, arg.TrackID) var i PlaylistTrack err := row.Scan( &i.PlaylistID, @@ -48,6 +51,7 @@ func (q *Queries) AppendPlaylistTrack(ctx context.Context, arg AppendPlaylistTra &i.AlbumTitle, &i.DurationSec, &i.AddedAt, + &i.PickKind, ) return i, err } @@ -257,7 +261,7 @@ func (q *Queries) ListAllPlaylistTracksForCollage(ctx context.Context, arg ListA } const listPlaylistTracks = `-- name: ListPlaylistTracks :many -SELECT pt.playlist_id, pt.position, pt.track_id, pt.title, pt.artist_name, pt.album_title, pt.duration_sec, pt.added_at, +SELECT pt.playlist_id, pt.position, pt.track_id, pt.title, pt.artist_name, pt.album_title, pt.duration_sec, pt.added_at, pt.pick_kind, t.id AS live_track_id, albums.id AS album_id, artists.id AS artist_id @@ -278,6 +282,7 @@ type ListPlaylistTracksRow struct { AlbumTitle string DurationSec int32 AddedAt pgtype.Timestamptz + PickKind *string LiveTrackID pgtype.UUID AlbumID pgtype.UUID ArtistID pgtype.UUID @@ -305,6 +310,7 @@ func (q *Queries) ListPlaylistTracks(ctx context.Context, playlistID pgtype.UUID &i.AlbumTitle, &i.DurationSec, &i.AddedAt, + &i.PickKind, &i.LiveTrackID, &i.AlbumID, &i.ArtistID, diff --git a/internal/db/dbq/recommendation_metrics.sql.go b/internal/db/dbq/recommendation_metrics.sql.go index acace5c9..0fbdc37c 100644 --- a/internal/db/dbq/recommendation_metrics.sql.go +++ b/internal/db/dbq/recommendation_metrics.sql.go @@ -15,6 +15,7 @@ const recommendationSourceMetricsForUser = `-- name: RecommendationSourceMetrics SELECT pe.source, + pe.pick_kind, count(*)::bigint AS plays, count(*) FILTER (WHERE pe.was_skipped)::bigint AS skips, count(pe.completion_ratio)::bigint AS completion_n, @@ -22,7 +23,7 @@ SELECT FROM play_events pe WHERE pe.user_id = $1 AND pe.started_at > now() - ($2::float8 * INTERVAL '1 day') -GROUP BY pe.source +GROUP BY pe.source, pe.pick_kind ORDER BY plays DESC ` @@ -33,6 +34,7 @@ type RecommendationSourceMetricsForUserParams struct { type RecommendationSourceMetricsForUserRow struct { Source *string + PickKind *string Plays int64 Skips int64 CompletionN int64 @@ -50,6 +52,8 @@ type RecommendationSourceMetricsForUserRow struct { // avg_completion correctly. // $1 user_id, $2 window_days. plays/skips are counts; avg_completion is the // mean completion ratio over the completion_n plays that recorded one. +// pick_kind splits For You plays into taste/fresh/unattributed (#1249); +// it is NULL for every other source, so those still group to one row. func (q *Queries) RecommendationSourceMetricsForUser(ctx context.Context, arg RecommendationSourceMetricsForUserParams) ([]RecommendationSourceMetricsForUserRow, error) { rows, err := q.db.Query(ctx, recommendationSourceMetricsForUser, arg.UserID, arg.Column2) if err != nil { @@ -61,6 +65,7 @@ func (q *Queries) RecommendationSourceMetricsForUser(ctx context.Context, arg Re var i RecommendationSourceMetricsForUserRow if err := rows.Scan( &i.Source, + &i.PickKind, &i.Plays, &i.Skips, &i.CompletionN, diff --git a/internal/db/migrations/0038_for_you_pick_kind.down.sql b/internal/db/migrations/0038_for_you_pick_kind.down.sql new file mode 100644 index 00000000..7158bd2f --- /dev/null +++ b/internal/db/migrations/0038_for_you_pick_kind.down.sql @@ -0,0 +1,2 @@ +ALTER TABLE play_events DROP COLUMN pick_kind; +ALTER TABLE playlist_tracks DROP COLUMN pick_kind; diff --git a/internal/db/migrations/0038_for_you_pick_kind.up.sql b/internal/db/migrations/0038_for_you_pick_kind.up.sql new file mode 100644 index 00000000..fdf86a39 --- /dev/null +++ b/internal/db/migrations/0038_for_you_pick_kind.up.sql @@ -0,0 +1,34 @@ +-- For You exploration attribution (milestone #127 step 2, Scribe #1249). +-- +-- For You is deliberately composed of two populations: a head of +-- top-scored similarity matches ("taste" picks) and a tail sampled from +-- deeper in the candidate ranking ("fresh" picks — the freshness +-- injection). Until now the metrics could only judge For You as one +-- blob, so a high skip rate couldn't be read as "the taste engine is +-- missing" vs "the freshness tax is too high". +-- +-- Two denormalized columns, both nullable: +-- +-- 1. playlist_tracks.pick_kind — stamped by the system-playlist builder +-- at snapshot build time. NULL for manual playlists and for system +-- variants that don't split (Discover, Songs like X, discovery +-- mixes). System snapshots are atomically replaced daily, so the +-- column never goes stale within a snapshot. +-- +-- 2. play_events.pick_kind — stamped at play-ingestion time by looking +-- the track up in the user's CURRENT For You snapshot. Attribution +-- must be frozen at play time: For You rebuilds daily, so a 30-day +-- metrics window spans ~30 snapshots and a join-at-read against the +-- live snapshot would misattribute nearly everything. Plays that +-- predate this feature (or whose track already rotated out, e.g. a +-- late offline replay) stay NULL → reported as unattributed. + +ALTER TABLE playlist_tracks + ADD COLUMN pick_kind text + CONSTRAINT playlist_tracks_pick_kind_check + CHECK (pick_kind IN ('taste', 'fresh')); + +ALTER TABLE play_events + ADD COLUMN pick_kind text + CONSTRAINT play_events_pick_kind_check + CHECK (pick_kind IN ('taste', 'fresh')); diff --git a/internal/db/queries/events.sql b/internal/db/queries/events.sql index db0d4829..c0c0a059 100644 --- a/internal/db/queries/events.sql +++ b/internal/db/queries/events.sql @@ -24,11 +24,27 @@ ORDER BY started_at DESC LIMIT 1; -- name: InsertPlayEvent :one +-- pick_kind is non-NULL only for source='for_you' plays whose track was +-- found in the user's live For You snapshot at ingestion time (#1249). INSERT INTO play_events ( - user_id, track_id, session_id, started_at, client_id, source -) VALUES ($1, $2, $3, $4, $5, $6) + user_id, track_id, session_id, started_at, client_id, source, pick_kind +) VALUES ($1, $2, $3, $4, $5, $6, sqlc.narg(pick_kind)::text) RETURNING *; +-- name: GetForYouPickKindForTrack :one +-- Looks a track up in the user's CURRENT For You snapshot and returns +-- its pick_kind ('taste'/'fresh'). Used at play-ingestion time to +-- freeze exploration attribution onto the play_event — the snapshot is +-- rebuilt daily, so attribution can't be reconstructed at read time +-- (#1249). No row = track not in today's snapshot (caller stores NULL). +SELECT pt.pick_kind +FROM playlist_tracks pt +JOIN playlists p ON p.id = pt.playlist_id +WHERE p.user_id = $1 + AND p.system_variant = 'for_you' + AND pt.track_id = $2 +LIMIT 1; + -- name: UpdatePlayEventEnded :one -- Closes a play_event by id with the given ended_at, duration, and skip flag. -- completion_ratio is computed from duration_played_ms and the track duration diff --git a/internal/db/queries/playlists.sql b/internal/db/queries/playlists.sql index 349e1940..058a1ffa 100644 --- a/internal/db/queries/playlists.sql +++ b/internal/db/queries/playlists.sql @@ -68,8 +68,9 @@ ORDER BY pt.position; -- name: AppendPlaylistTrack :one -- Inserts at the next available position. Snapshot fields are copied -- from the tracks/albums/artists join at insert time. tracks.duration_ms --- is converted to seconds for the snapshot. -INSERT INTO playlist_tracks (playlist_id, position, track_id, title, artist_name, album_title, duration_sec) +-- is converted to seconds for the snapshot. pick_kind is NULL for manual +-- playlists; the For You builder stamps 'taste'/'fresh' (#1249). +INSERT INTO playlist_tracks (playlist_id, position, track_id, title, artist_name, album_title, duration_sec, pick_kind) SELECT sqlc.arg(playlist_id)::uuid, COALESCE((SELECT MAX(position) + 1 FROM playlist_tracks WHERE playlist_id = sqlc.arg(playlist_id)::uuid), 0), @@ -77,7 +78,8 @@ SELECT t.title, artists.name, albums.title, - (t.duration_ms / 1000)::integer + (t.duration_ms / 1000)::integer, + sqlc.narg(pick_kind)::text FROM tracks t JOIN albums ON albums.id = t.album_id JOIN artists ON artists.id = t.artist_id diff --git a/internal/db/queries/recommendation_metrics.sql b/internal/db/queries/recommendation_metrics.sql index 2d2f4ebd..90b89fb9 100644 --- a/internal/db/queries/recommendation_metrics.sql +++ b/internal/db/queries/recommendation_metrics.sql @@ -11,8 +11,11 @@ -- name: RecommendationSourceMetricsForUser :many -- $1 user_id, $2 window_days. plays/skips are counts; avg_completion is the -- mean completion ratio over the completion_n plays that recorded one. +-- pick_kind splits For You plays into taste/fresh/unattributed (#1249); +-- it is NULL for every other source, so those still group to one row. SELECT pe.source, + pe.pick_kind, count(*)::bigint AS plays, count(*) FILTER (WHERE pe.was_skipped)::bigint AS skips, count(pe.completion_ratio)::bigint AS completion_n, @@ -20,5 +23,5 @@ SELECT FROM play_events pe WHERE pe.user_id = $1 AND pe.started_at > now() - ($2::float8 * INTERVAL '1 day') -GROUP BY pe.source +GROUP BY pe.source, pe.pick_kind ORDER BY plays DESC; diff --git a/internal/playevents/writer.go b/internal/playevents/writer.go index a65ba169..48a2d82d 100644 --- a/internal/playevents/writer.go +++ b/internal/playevents/writer.go @@ -97,6 +97,35 @@ var systemPlaylistSources = map[string]bool{ "songs_like_artist": true, } +// sourceForYou is the one source whose plays carry exploration +// attribution (taste vs fresh pick, #1249). +const sourceForYou = "for_you" + +// lookupForYouPickKind resolves a For-You play's pick_kind ('taste' or +// 'fresh') from the user's live snapshot at ingestion time. Attribution +// must be frozen now — the snapshot rebuilds daily, so it can't be +// reconstructed at metrics-read time. Returns nil (unattributed) when +// the track isn't in the current snapshot: plays predating the feature, +// or an offline replay arriving after the track rotated out. Real DB +// errors propagate and fail the caller's transaction. +func lookupForYouPickKind( + ctx context.Context, + q *dbq.Queries, + userID, trackID pgtype.UUID, +) (*string, error) { + kind, err := q.GetForYouPickKindForTrack(ctx, dbq.GetForYouPickKindForTrackParams{ + UserID: userID, + TrackID: trackID, + }) + if errors.Is(err, pgx.ErrNoRows) { + return nil, nil + } + if err != nil { + return nil, err + } + return kind, nil +} + // RecordPlayStartedWithSource is RecordPlayStarted plus a `source` // tag identifying which surface the play came from. When source is a // known system-playlist kind the track is appended to that user's @@ -127,6 +156,13 @@ func (w *Writer) RecordPlayStartedWithSource( if source != "" { sourcePtr = &source } + var pickKind *string + if source == sourceForYou { + pickKind, err = lookupForYouPickKind(ctx, q, userID, trackID) + if err != nil { + return err + } + } ev, err := q.InsertPlayEvent(ctx, dbq.InsertPlayEventParams{ UserID: userID, TrackID: trackID, @@ -134,6 +170,7 @@ func (w *Writer) RecordPlayStartedWithSource( StartedAt: pgtype.Timestamptz{Time: at, Valid: true}, ClientID: clientIDPtr, Source: sourcePtr, + PickKind: pickKind, }) if err != nil { return err @@ -395,6 +432,13 @@ func (w *Writer) RecordOfflinePlay( if source != "" { sourcePtr = &source } + var pickKind *string + if source == sourceForYou { + pickKind, err = lookupForYouPickKind(ctx, q, userID, trackID) + if err != nil { + return err + } + } ev, err := q.InsertPlayEvent(ctx, dbq.InsertPlayEventParams{ UserID: userID, TrackID: trackID, @@ -402,6 +446,7 @@ func (w *Writer) RecordOfflinePlay( StartedAt: pgtype.Timestamptz{Time: at, Valid: true}, ClientID: clientIDPtr, Source: sourcePtr, + PickKind: pickKind, }) if err != nil { return err diff --git a/internal/playevents/writer_test.go b/internal/playevents/writer_test.go index a0158c11..d707f7dc 100644 --- a/internal/playevents/writer_test.go +++ b/internal/playevents/writer_test.go @@ -488,3 +488,102 @@ func TestRecordPlayStarted_NoRotationWithoutSource(t *testing.T) { t.Errorf("expected no rotation row for a source-less play") } } + +// seedForYouSnapshot creates a system For You playlist containing the +// fixture track with the given pick_kind, mimicking what the builder +// persists (#1249). +func seedForYouSnapshot(t *testing.T, f fixture, pickKind string) { + t.Helper() + variant := "for_you" + pl, err := f.q.CreateSystemPlaylist(context.Background(), dbq.CreateSystemPlaylistParams{ + UserID: f.user, Name: "For You", SystemVariant: &variant, + }) + if err != nil { + t.Fatalf("CreateSystemPlaylist: %v", err) + } + if _, err := f.q.AppendPlaylistTrack(context.Background(), dbq.AppendPlaylistTrackParams{ + PlaylistID: pl.ID, TrackID: f.track, PickKind: &pickKind, + }); err != nil { + t.Fatalf("AppendPlaylistTrack: %v", err) + } +} + +func TestRecordPlayStartedWithSource_StampsForYouPickKind(t *testing.T) { + f := newFixture(t, 200_000) + seedForYouSnapshot(t, f, "fresh") + res, err := f.w.RecordPlayStartedWithSource( + context.Background(), f.user, f.track, "c", "for_you", time.Now().UTC(), + ) + if err != nil { + t.Fatalf("RecordPlayStartedWithSource: %v", err) + } + got, err := f.q.GetPlayEventByID(context.Background(), res.PlayEventID) + if err != nil { + t.Fatalf("get: %v", err) + } + if got.PickKind == nil || *got.PickKind != "fresh" { + t.Errorf("pick_kind = %v, want fresh (frozen from live snapshot)", got.PickKind) + } +} + +func TestRecordPlayStartedWithSource_NoSnapshotMatch_PickKindNull(t *testing.T) { + // A for_you play whose track isn't in the current snapshot (rotated + // out, or no snapshot exists) stays unattributed rather than guessing. + f := newFixture(t, 200_000) + res, err := f.w.RecordPlayStartedWithSource( + context.Background(), f.user, f.track, "c", "for_you", time.Now().UTC(), + ) + if err != nil { + t.Fatalf("RecordPlayStartedWithSource: %v", err) + } + got, err := f.q.GetPlayEventByID(context.Background(), res.PlayEventID) + if err != nil { + t.Fatalf("get: %v", err) + } + if got.PickKind != nil { + t.Errorf("pick_kind = %q, want NULL (track not in snapshot)", *got.PickKind) + } +} + +func TestRecordPlayStartedWithSource_NonForYouSource_PickKindNull(t *testing.T) { + // Even with a For You snapshot containing the track, a play from a + // different surface must not pick up For You attribution. + f := newFixture(t, 200_000) + seedForYouSnapshot(t, f, "taste") + res, err := f.w.RecordPlayStartedWithSource( + context.Background(), f.user, f.track, "c", "discover", time.Now().UTC(), + ) + if err != nil { + t.Fatalf("RecordPlayStartedWithSource: %v", err) + } + got, err := f.q.GetPlayEventByID(context.Background(), res.PlayEventID) + if err != nil { + t.Fatalf("get: %v", err) + } + if got.PickKind != nil { + t.Errorf("pick_kind = %q, want NULL for non-for_you source", *got.PickKind) + } +} + +func TestRecordOfflinePlay_StampsForYouPickKind(t *testing.T) { + // The offline replay path threads source the same way; a replayed + // for_you play whose track is still in the snapshot gets attributed. + f := newFixture(t, 200_000) + seedForYouSnapshot(t, f, "taste") + at := time.Now().UTC().Add(-time.Hour) + if err := f.w.RecordOfflinePlay( + context.Background(), f.user, f.track, "c", "for_you", at, 180_000, + ); err != nil { + t.Fatalf("RecordOfflinePlay: %v", err) + } + var pickKind *string + if err := f.pool.QueryRow(context.Background(), + `SELECT pick_kind FROM play_events WHERE user_id = $1 AND track_id = $2`, + f.user, f.track, + ).Scan(&pickKind); err != nil { + t.Fatalf("select: %v", err) + } + if pickKind == nil || *pickKind != "taste" { + t.Errorf("pick_kind = %v, want taste", pickKind) + } +} diff --git a/internal/playlists/foryou_test.go b/internal/playlists/foryou_test.go index 0d69a7ae..ab389136 100644 --- a/internal/playlists/foryou_test.go +++ b/internal/playlists/foryou_test.go @@ -243,3 +243,53 @@ func TestPickTopN_DiversityCap(t *testing.T) { t.Errorf("len = %d, want 3 (artist 100 capped at 3)", len(got)) } } + +func TestPickHeadAndTail_MarksPickKinds(t *testing.T) { + // 100-deep pool with headN=20, tailN=5: the 20 head entries are the + // taste picks, the 5 tail entries the freshness injection (#1249). + in := make([]recommendation.Candidate, 0, 100) + for i := 0; i < 100; i++ { + in = append(in, makeCand(i+1, i+1, i+1, float64(100-i))) + } + got := pickHeadAndTail(in, testUserID, "2026-05-07", time.Now(), 20, 5) + if len(got) != 25 { + t.Fatalf("len = %d, want 25", len(got)) + } + for i := 0; i < 20; i++ { + if got[i].PickKind != pickKindTaste { + t.Errorf("head[%d].PickKind = %q, want %q", i, got[i].PickKind, pickKindTaste) + } + } + for i := 20; i < 25; i++ { + if got[i].PickKind != pickKindFresh { + t.Errorf("tail[%d].PickKind = %q, want %q", i-20, got[i].PickKind, pickKindFresh) + } + } +} + +func TestPickHeadAndTail_SmallPoolAllTaste(t *testing.T) { + // The small-pool fallback is pure top-N-by-score — that IS the taste + // mechanism, so nothing on this path is an exploration pick. + in := []recommendation.Candidate{ + makeCand(1, 10, 100, 1.0), + makeCand(2, 11, 101, 0.9), + makeCand(3, 12, 102, 0.8), + } + got := pickHeadAndTail(in, testUserID, "2026-05-07", time.Now(), 5, 2) + for i, rc := range got { + if rc.PickKind != pickKindTaste { + t.Errorf("got[%d].PickKind = %q, want %q (fallback is all taste)", + i, rc.PickKind, pickKindTaste) + } + } +} + +func TestPickTopN_NoPickKind(t *testing.T) { + // Songs-like-X and the discovery mixes don't split; their rows must + // persist pick_kind NULL (empty string here). + in := []recommendation.Candidate{makeCand(1, 10, 100, 1.0)} + got := pickTopN(in, testUserID, "2026-05-07", time.Now(), 25) + if len(got) != 1 || got[0].PickKind != "" { + t.Errorf("pickTopN PickKind = %q, want empty (persists as NULL)", got[0].PickKind) + } +} diff --git a/internal/playlists/system.go b/internal/playlists/system.go index b618d9d7..e1527873 100644 --- a/internal/playlists/system.go +++ b/internal/playlists/system.go @@ -127,12 +127,22 @@ func pickSeedArtistsForDay(pool []pgtype.UUID, userID pgtype.UUID, dateStr strin // rankedCandidate is a (track_id, score) pair used during in-memory // sorting before insert into playlist_tracks. T5 fills these from -// recommendation.Candidate scores. +// recommendation.Candidate scores. PickKind is set only by For-You's +// head/tail composition (#1249); empty persists as NULL. type rankedCandidate struct { - TrackID pgtype.UUID - Score float64 + TrackID pgtype.UUID + Score float64 + PickKind string } +// For-You pick kinds, persisted on playlist_tracks.pick_kind so plays +// can attribute skips to "the taste engine missing" vs "the freshness +// tax" (#1249). Values mirror the CHECK in migration 0038. +const ( + pickKindTaste = "taste" + pickKindFresh = "fresh" +) + const systemMixLength = 25 // systemMixWeights are the fixed scoring weights used by the cron worker. @@ -632,14 +642,17 @@ func pickHeadAndTail(cands []recommendation.Candidate, userID pgtype.UUID, dateS total := headN + tailN if len(capped) <= total { // Pool too small for a head/tail split — return up to total entries. + // All marked taste: top-N-by-score IS the taste mechanism; no + // exploration sampling happens on this path. if len(capped) < total { total = len(capped) } out := make([]rankedCandidate, total) for i := 0; i < total; i++ { out[i] = rankedCandidate{ - TrackID: capped[i].Track.ID, - Score: recommendation.Score(capped[i].Inputs, systemMixWeights, now, rng.Float64), + TrackID: capped[i].Track.ID, + Score: recommendation.Score(capped[i].Inputs, systemMixWeights, now, rng.Float64), + PickKind: pickKindTaste, } } return out @@ -667,15 +680,22 @@ func pickHeadAndTail(cands []recommendation.Candidate, userID pgtype.UUID, dateS // Combine: head order preserved (score-sorted), tail in tieBreakHash // order. "First similar, then surprise" reads naturally in playback. + // Head entries are the taste picks; tail entries are the freshness + // injection (#1249) — the split the metrics page attributes skips to. combined := make([]recommendation.Candidate, 0, len(head)+len(tail)) combined = append(combined, head...) combined = append(combined, tail...) out := make([]rankedCandidate, len(combined)) for i, c := range combined { + kind := pickKindTaste + if i >= len(head) { + kind = pickKindFresh + } out[i] = rankedCandidate{ - TrackID: c.Track.ID, - Score: recommendation.Score(c.Inputs, systemMixWeights, now, rng.Float64), + TrackID: c.Track.ID, + Score: recommendation.Score(c.Inputs, systemMixWeights, now, rng.Float64), + PickKind: kind, } } return out @@ -709,9 +729,15 @@ func insertSystemPlaylist(ctx context.Context, qtx *dbq.Queries, userID pgtype.U } for _, t := range tracks { + var pickKind *string + if t.PickKind != "" { + k := t.PickKind + pickKind = &k + } if _, err := qtx.AppendPlaylistTrack(ctx, dbq.AppendPlaylistTrackParams{ PlaylistID: p.ID, TrackID: t.TrackID, + PickKind: pickKind, }); err != nil { // Track may have been deleted between candidate-load and insert; // skip silently rather than failing the whole build. diff --git a/web/src/lib/api/metrics.ts b/web/src/lib/api/metrics.ts index 9a21df3b..01175ffd 100644 --- a/web/src/lib/api/metrics.ts +++ b/web/src/lib/api/metrics.ts @@ -12,6 +12,9 @@ export type SurfaceMetric = { skip_rate: number; avg_completion: number; low_confidence: boolean; + // Only For You carries a breakdown (#1249): taste picks vs fresh picks + // (the deliberate freshness injection), plus pre-attribution plays. + breakdown?: SurfaceMetric[]; }; export type SurfaceIntent = 'go_to' | 'discovery' | 'direct'; diff --git a/web/src/routes/settings/+page.svelte b/web/src/routes/settings/+page.svelte index b32970fa..711ed1f9 100644 --- a/web/src/routes/settings/+page.svelte +++ b/web/src/routes/settings/+page.svelte @@ -372,6 +372,40 @@ {/if} + + {#each m.breakdown ?? [] as b (b.key)} + + + ↳ {b.label}{#if b.low_confidence}· low data{/if} + + {b.plays} + + {pct(b.skip_rate)} + {#if baseline} + + {deltaPts(b.skip_rate, baseline.skip_rate)} + + {/if} + + + {pct(b.avg_completion)} + {#if baseline} + + {deltaPts(b.avg_completion, baseline.avg_completion)} + + {/if} + + + {/each} {/each} diff --git a/web/src/routes/settings/settings.test.ts b/web/src/routes/settings/settings.test.ts index 1347c0d2..0ea50d1d 100644 --- a/web/src/routes/settings/settings.test.ts +++ b/web/src/routes/settings/settings.test.ts @@ -20,10 +20,16 @@ vi.mock('$lib/api/me', () => ({ regenerateAPIToken: vi.fn() })); +// Mutable holder so individual tests can inject populated metrics; +// vi.mock is hoisted, hence vi.hoisted for the shared reference. +const metricsMock = vi.hoisted(() => ({ + data: { window_days: 30, baseline: null, groups: [] } as unknown +})); + vi.mock('$lib/api/metrics', () => ({ createRecommendationMetricsQuery: () => ({ subscribe: (run: (v: unknown) => void) => { - run({ isPending: false, isError: false, data: { window_days: 30, baseline: null, groups: [] } }); + run({ isPending: false, isError: false, data: metricsMock.data }); return () => {}; } }) @@ -52,7 +58,10 @@ function mockMutationStore(mutateFn: (vars: unknown) => void = vi.fn()) { return readable({ isPending: false, mutate: mutateFn, mutateAsync: vi.fn() }); } -afterEach(() => vi.clearAllMocks()); +afterEach(() => { + vi.clearAllMocks(); + metricsMock.data = { window_days: 30, baseline: null, groups: [] }; +}); describe('Settings page — ListenBrainz', () => { test('token-not-set state renders input + Save button', async () => { @@ -177,6 +186,70 @@ describe('Settings page — Password card', () => { }); }); +describe('Settings page — Recommendation metrics card', () => { + const metric = (key: string, label: string, over: Record = {}) => ({ + key, + label, + plays: 30, + skips: 3, + skip_rate: 0.1, + avg_completion: 0.9, + low_confidence: false, + ...over + }); + + test('renders the For You taste/fresh breakdown rows (#1249)', async () => { + setupPage(); + metricsMock.data = { + window_days: 30, + baseline: metric('manual', 'Manual library plays', { plays: 100 }), + groups: [ + { + intent: 'go_to', + label: 'Go-to surfaces', + surfaces: [ + metric('for_you', 'For You', { + plays: 45, + breakdown: [ + metric('for_you_taste', 'Taste picks'), + metric('for_you_fresh', 'Fresh picks', { + plays: 10, + skip_rate: 0.4, + low_confidence: true + }), + metric('for_you_unattributed', 'Earlier plays', { plays: 5 }) + ] + }) + ] + } + ] + }; + render(SettingsPage); + await waitFor(() => expect(screen.getByText('For You')).toBeInTheDocument()); + expect(screen.getByText(/Taste picks/)).toBeInTheDocument(); + expect(screen.getByText(/Fresh picks/)).toBeInTheDocument(); + expect(screen.getByText(/Earlier plays/)).toBeInTheDocument(); + }); + + test('surfaces without a breakdown render no sub-rows', async () => { + setupPage(); + metricsMock.data = { + window_days: 30, + baseline: null, + groups: [ + { + intent: 'go_to', + label: 'Go-to surfaces', + surfaces: [metric('radio', 'Radio')] + } + ] + }; + render(SettingsPage); + await waitFor(() => expect(screen.getByText('Radio')).toBeInTheDocument()); + expect(screen.queryByText(/Taste picks/)).not.toBeInTheDocument(); + }); +}); + describe('Settings page — API Token card', () => { test('first click on Regenerate shows "Click again to confirm"', async () => { setupPage();