diff --git a/internal/db/dbq/recommendation.sql.go b/internal/db/dbq/recommendation.sql.go index 13167583..cf1bd09b 100644 --- a/internal/db/dbq/recommendation.sql.go +++ b/internal/db/dbq/recommendation.sql.go @@ -1022,20 +1022,66 @@ func (q *Queries) LoadRadioCandidatesV2(ctx context.Context, arg LoadRadioCandid } const suggestArtistsForUser = `-- name: SuggestArtistsForUser :many -WITH seeds AS ( +-- Per-user artist suggestions ranked by taste signal x similarity, projected +-- through artist_similarity_unmatched (out-of-library candidates only). +-- +-- Seeds are TIERED (rule #131) so the surface never empties: +-- tier 1 - taste_profile_artists.weight: engagement-graded, time-decayed and +-- SIGNED by internal/taste, so an artist the user has drifted away +-- from stops contributing instead of accumulating forever. +-- tier 2 - likes + completed plays, used ONLY when the profile has no rows +-- (new account, or before the first daily recompute). +-- +-- The signal is log-damped: contribution is signal x similarity, and the old +-- undamped sum let one heavily-played artist's neighbours take every slot -- +-- entrenching harder the MORE the user listened (issue #2367 mechanism 2). +-- +-- Candidates already in the library, or already requested and not terminal, +-- are excluded. $1=user_id, $2=half_life_days (tier 2 decay), $3=limit. +WITH artist_plays AS ( + -- Completed plays only. The previous seed query counted every play_event, + -- so skipping an artist repeatedly INCREASED its signal and pushed more of + -- its neighbours at the user (issue #2367 mechanism 3). + SELECT t.artist_id, count(*)::bigint AS play_count + FROM play_events pe + JOIN tracks t ON t.id = pe.track_id + WHERE pe.user_id = $1 AND pe.was_skipped = false + GROUP BY t.artist_id +), +profile_seeds AS ( + SELECT tpa.artist_id, tpa.weight AS raw_signal + FROM taste_profile_artists tpa + WHERE tpa.user_id = $1 AND tpa.weight > 0 +), +fallback_seeds AS ( SELECT a.id AS artist_id, 5.0 * (CASE WHEN gla.artist_id IS NOT NULL THEN 1 ELSE 0 END) + COALESCE(SUM(EXP(- EXTRACT(epoch FROM now() - pe.started_at) / ($2::float8 * 86400.0))), 0) - AS signal, - (gla.artist_id IS NOT NULL) AS is_liked, - COUNT(pe.id)::bigint AS play_count + AS raw_signal FROM artists a LEFT JOIN general_likes_artists gla ON gla.artist_id = a.id AND gla.user_id = $1 LEFT JOIN tracks t ON t.artist_id = a.id - LEFT JOIN play_events pe ON pe.track_id = t.id AND pe.user_id = $1 - WHERE gla.artist_id IS NOT NULL OR pe.id IS NOT NULL + LEFT JOIN play_events pe + ON pe.track_id = t.id AND pe.user_id = $1 AND pe.was_skipped = false + WHERE (gla.artist_id IS NOT NULL OR pe.id IS NOT NULL) + AND NOT EXISTS (SELECT 1 FROM profile_seeds) GROUP BY a.id, gla.artist_id ), +seeds AS ( + SELECT s.artist_id, + ln(1.0 + s.raw_signal) AS signal, + (gla.artist_id IS NOT NULL) AS is_liked, + COALESCE(ap.play_count, 0)::bigint AS play_count + FROM ( + SELECT artist_id, raw_signal FROM profile_seeds + UNION ALL + SELECT artist_id, raw_signal FROM fallback_seeds + ) s + LEFT JOIN general_likes_artists gla + ON gla.artist_id = s.artist_id AND gla.user_id = $1 + LEFT JOIN artist_plays ap ON ap.artist_id = s.artist_id + WHERE s.raw_signal > 0 +), contributions AS ( SELECT u.candidate_mbid, u.candidate_name, diff --git a/internal/db/queries/recommendation.sql b/internal/db/queries/recommendation.sql index bf9e10f7..d4c8de68 100644 --- a/internal/db/queries/recommendation.sql +++ b/internal/db/queries/recommendation.sql @@ -258,27 +258,66 @@ ORDER BY started_at DESC LIMIT 1; -- name: SuggestArtistsForUser :many --- M5c: per-user artist suggestions ranked by signal x similarity. The --- seeds CTE collects the user's likes (x5) plus recency-decayed plays --- (exp(-age_days / $2)). The contributions CTE joins those seeds against --- artist_similarity_unmatched and filters out candidates already in --- library or already in a non-terminal lidarr_request. The outer SELECT --- aggregates per candidate, returning the top-3 contributing seeds for --- attribution. $1=user_id, $2=half_life_days, $3=limit. -WITH seeds AS ( +-- Per-user artist suggestions ranked by taste signal x similarity, projected +-- through artist_similarity_unmatched (out-of-library candidates only). +-- +-- Seeds are TIERED (rule #131) so the surface never empties: +-- tier 1 - taste_profile_artists.weight: engagement-graded, time-decayed and +-- SIGNED by internal/taste, so an artist the user has drifted away +-- from stops contributing instead of accumulating forever. +-- tier 2 - likes + completed plays, used ONLY when the profile has no rows +-- (new account, or before the first daily recompute). +-- +-- The signal is log-damped: contribution is signal x similarity, and the old +-- undamped sum let one heavily-played artist's neighbours take every slot -- +-- entrenching harder the MORE the user listened (issue #2367 mechanism 2). +-- +-- Candidates already in the library, or already requested and not terminal, +-- are excluded. $1=user_id, $2=half_life_days (tier 2 decay), $3=limit. +WITH artist_plays AS ( + -- Completed plays only. The previous seed query counted every play_event, + -- so skipping an artist repeatedly INCREASED its signal and pushed more of + -- its neighbours at the user (issue #2367 mechanism 3). + SELECT t.artist_id, count(*)::bigint AS play_count + FROM play_events pe + JOIN tracks t ON t.id = pe.track_id + WHERE pe.user_id = $1 AND pe.was_skipped = false + GROUP BY t.artist_id +), +profile_seeds AS ( + SELECT tpa.artist_id, tpa.weight AS raw_signal + FROM taste_profile_artists tpa + WHERE tpa.user_id = $1 AND tpa.weight > 0 +), +fallback_seeds AS ( SELECT a.id AS artist_id, 5.0 * (CASE WHEN gla.artist_id IS NOT NULL THEN 1 ELSE 0 END) + COALESCE(SUM(EXP(- EXTRACT(epoch FROM now() - pe.started_at) / ($2::float8 * 86400.0))), 0) - AS signal, - (gla.artist_id IS NOT NULL) AS is_liked, - COUNT(pe.id)::bigint AS play_count + AS raw_signal FROM artists a LEFT JOIN general_likes_artists gla ON gla.artist_id = a.id AND gla.user_id = $1 LEFT JOIN tracks t ON t.artist_id = a.id - LEFT JOIN play_events pe ON pe.track_id = t.id AND pe.user_id = $1 - WHERE gla.artist_id IS NOT NULL OR pe.id IS NOT NULL + LEFT JOIN play_events pe + ON pe.track_id = t.id AND pe.user_id = $1 AND pe.was_skipped = false + WHERE (gla.artist_id IS NOT NULL OR pe.id IS NOT NULL) + AND NOT EXISTS (SELECT 1 FROM profile_seeds) GROUP BY a.id, gla.artist_id ), +seeds AS ( + SELECT s.artist_id, + ln(1.0 + s.raw_signal) AS signal, + (gla.artist_id IS NOT NULL) AS is_liked, + COALESCE(ap.play_count, 0)::bigint AS play_count + FROM ( + SELECT artist_id, raw_signal FROM profile_seeds + UNION ALL + SELECT artist_id, raw_signal FROM fallback_seeds + ) s + LEFT JOIN general_likes_artists gla + ON gla.artist_id = s.artist_id AND gla.user_id = $1 + LEFT JOIN artist_plays ap ON ap.artist_id = s.artist_id + WHERE s.raw_signal > 0 +), contributions AS ( SELECT u.candidate_mbid, u.candidate_name, diff --git a/internal/recommendation/suggestions.go b/internal/recommendation/suggestions.go index dfe970e3..fbef853f 100644 --- a/internal/recommendation/suggestions.go +++ b/internal/recommendation/suggestions.go @@ -1,7 +1,14 @@ -// suggestions.go is the M5c per-user artist-suggestion service. Reads -// the user's likes + plays, projects them through artist_similarity_unmatched -// via a single CTE, returns top-N candidates with top-3 attribution seeds -// resolved to artist names. +// suggestions.go is the per-user artist-suggestion service behind the +// Discover request surface. Seeds from the taste profile (falling back to +// likes + completed plays for a user who has none yet), projects those seeds +// through artist_similarity_unmatched via a single CTE, and returns top-N +// out-of-library candidates with top-3 attribution seeds resolved to names. +// +// Originally M5c, which seeded from raw likes + plays. That signal grew +// without bound and counted skips as engagement, so a few heavily-played +// artists monopolized every slot and the surface entrenched harder the more +// the user listened. Reworked to seed from the taste profile in issue #2367; +// see internal/db/queries/recommendation.sql for the tiering. package recommendation import ( @@ -32,8 +39,12 @@ type SeedContribution struct { } // SuggestArtists returns top-N artist suggestions for the user. limit is -// capped at 50 (default 12 when out of range); halfLifeDays is the -// recency-decay half-life for plays (default 30, operator-tunable). +// capped at 50 (default 12 when out of range). +// +// halfLifeDays (default 30) is the recency-decay half-life for the TIER-2 +// seed path only — the likes + completed-plays fallback used while the user +// has no taste-profile rows yet. Once the profile is populated it seeds +// instead, carrying its own decay, so this knob stops applying. func SuggestArtists(ctx context.Context, pool *pgxpool.Pool, userID pgtype.UUID, halfLifeDays float64, limit int) ([]ArtistSuggestion, error) { if limit <= 0 || limit > 50 { limit = 12 diff --git a/internal/recommendation/suggestions_integration_test.go b/internal/recommendation/suggestions_integration_test.go index 9b5cf0c4..67a4cefd 100644 --- a/internal/recommendation/suggestions_integration_test.go +++ b/internal/recommendation/suggestions_integration_test.go @@ -330,3 +330,145 @@ func TestSuggestArtists_EmptyForNewUser(t *testing.T) { t.Errorf("len = %d, want 0 (new user has no signal)", len(out)) } } + +// --- Slice 1 (#2372): taste-profile seeding, tiering, and the skip fix --- + +func setTasteWeight(t *testing.T, pool *pgxpool.Pool, userID, artistID pgtype.UUID, weight float64) { + t.Helper() + if _, err := pool.Exec(context.Background(), + `INSERT INTO taste_profile_artists (user_id, artist_id, weight) VALUES ($1, $2, $3) + ON CONFLICT (user_id, artist_id) DO UPDATE SET weight = EXCLUDED.weight`, + userID, artistID, weight, + ); err != nil { + t.Fatalf("set taste weight: %v", err) + } +} + +func insertSkippedPlayEvent(t *testing.T, pool *pgxpool.Pool, userID, trackID pgtype.UUID, startedAt time.Time) { + t.Helper() + ctx := context.Background() + var sessionID pgtype.UUID + if err := pool.QueryRow(ctx, + `INSERT INTO play_sessions (user_id, started_at, last_event_at, client_id) + VALUES ($1, $2, $2, 'skip-test') RETURNING id`, + userID, startedAt, + ).Scan(&sessionID); err != nil { + t.Fatalf("insert play_session: %v", err) + } + if _, err := pool.Exec(ctx, + `INSERT INTO play_events (user_id, track_id, session_id, started_at, was_skipped) + VALUES ($1, $2, $3, $4, true)`, + userID, trackID, sessionID, startedAt, + ); err != nil { + t.Fatalf("insert skipped play_event: %v", err) + } +} + +// Tier 1: a taste-profile weight alone seeds a suggestion, with no like and no +// play on the seed artist. Before #2367 the profile was ignored entirely here. +func TestSuggestArtists_TasteProfileWeightSeedsTier1(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool, "alice") + seed := seedArtist(t, pool, "Profile Seed", "") + + setTasteWeight(t, pool, user.ID, seed.ID, 4.0) + seedUnmatched(t, pool, seed.ID, "out-mbid", "Outsider", 0.9) + + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + if err != nil { + t.Fatalf("SuggestArtists: %v", err) + } + if len(out) != 1 { + t.Fatalf("len = %d, want 1 (taste weight alone should seed)", len(out)) + } + if out[0].MBID != "out-mbid" { + t.Errorf("mbid = %q, want out-mbid", out[0].MBID) + } + if out[0].Score <= 0 { + t.Errorf("score = %v, want > 0", out[0].Score) + } +} + +// Once the profile has any positive row, tier 2 is not consulted — so an artist +// the user played but that the taste engine did not keep does NOT seed. That is +// the point of the rewrite: the taste engine decides what counts as affinity. +func TestSuggestArtists_TasteProfileSupersedesRawPlays(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool, "alice") + kept := seedArtist(t, pool, "Kept By Taste", "") + dropped := seedArtist(t, pool, "Dropped By Taste", "") + + setTasteWeight(t, pool, user.ID, kept.ID, 3.0) + + // `dropped` has real completed plays but no profile row. + album := seedAlbumForArtist(t, pool, dropped.ID, "Album") + track := seedTrackOnAlbum(t, pool, album.ID, dropped.ID, "Track") + insertPlayEvent(t, pool, user.ID, track.ID, time.Now().Add(-1*time.Hour)) + + seedUnmatched(t, pool, kept.ID, "kept-cand", "Kept Candidate", 0.9) + seedUnmatched(t, pool, dropped.ID, "dropped-cand", "Dropped Candidate", 0.9) + + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + if err != nil { + t.Fatalf("SuggestArtists: %v", err) + } + if len(out) != 1 { + t.Fatalf("len = %d, want 1 (tier 2 must not run alongside tier 1)", len(out)) + } + if out[0].MBID != "kept-cand" { + t.Errorf("mbid = %q, want kept-cand", out[0].MBID) + } +} + +// A non-positive taste weight is not affinity. Guarded by a second, positive +// row so tier 1 stays active — otherwise an empty tier 1 would fall through to +// tier 2 and the assertion would pass for the wrong reason. +func TestSuggestArtists_NonPositiveTasteWeightDoesNotSeed(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool, "alice") + positive := seedArtist(t, pool, "Still Liked", "") + abandoned := seedArtist(t, pool, "Abandoned", "") + + setTasteWeight(t, pool, user.ID, positive.ID, 2.0) + setTasteWeight(t, pool, user.ID, abandoned.ID, -1.5) + + seedUnmatched(t, pool, positive.ID, "pos-cand", "Positive Candidate", 0.9) + seedUnmatched(t, pool, abandoned.ID, "neg-cand", "Negative Candidate", 0.9) + + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + if err != nil { + t.Fatalf("SuggestArtists: %v", err) + } + for _, s := range out { + if s.MBID == "neg-cand" { + t.Fatalf("negative-weight artist seeded a suggestion: %+v", s) + } + } + if len(out) != 1 || out[0].MBID != "pos-cand" { + t.Errorf("out = %+v, want only pos-cand", out) + } +} + +// Tier 2 counts COMPLETED plays only. Previously every play_event counted, so +// skipping an artist repeatedly increased its signal and pushed more of its +// neighbours at the user (#2367 mechanism 3). +func TestSuggestArtists_SkippedPlaysDoNotSeedTier2(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool, "alice") + skipped := seedArtist(t, pool, "Only Skipped", "") + + album := seedAlbumForArtist(t, pool, skipped.ID, "Album") + track := seedTrackOnAlbum(t, pool, album.ID, skipped.ID, "Track") + for i := 0; i < 5; i++ { + insertSkippedPlayEvent(t, pool, user.ID, track.ID, time.Now().Add(-time.Duration(i+1)*time.Hour)) + } + seedUnmatched(t, pool, skipped.ID, "skip-cand", "Skip Candidate", 0.9) + + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + if err != nil { + t.Fatalf("SuggestArtists: %v", err) + } + if len(out) != 0 { + t.Errorf("len = %d, want 0 (skips are not affinity): %+v", len(out), out) + } +}