feat(recommendation): rework Rediscover - track-derived eligibility + daily rotation + caps

User request 2026-06-01: the previous Rediscover only fired on explicit album/artist likes, so users who only liked at the track level got an empty Rediscover row. Also no daily rotation - the section never changed between data updates - and 25 items felt long for the Home carousel. SQL (internal/db/queries/recommendation.sql): - ListRediscoverAlbumsForUser UNIONs the existing explicit album-like signal with a new track-derived path: an album qualifies if it has >=2 liked tracks where the earliest like is >30 days old. - ListRediscoverArtistsForUser does the same with threshold 3 (artists span more releases, so the bar is higher to avoid one-hit-wonder affinities). - Both ordering switched from longest-since-last-play to a daily- stable random hash md5(entity_id || user_id || current_date) so the row randomizes but stays consistent within a day. - Fallback queries also switched to the daily-stable hash so the fallback rows don't reshuffle on every refresh. Go (internal/recommendation/home.go): - HomeRediscoverLimit dropped from 25 to 10 (carousel-sized). - rediscoverInnerLimit (30) is the per-query cap; gives headroom so the Go-layer filters don't undershoot. - applyRediscoverAlbumFilters does cross-section dedup vs Most Played (no point surfacing albums the user is actively spinning) plus a diversity cap of max 2 albums per artist (prevents one liked-but- forgotten artist dominating the row). - applyRediscoverArtistFilters does cross-section dedup vs Most Played's artist set; no diversity cap needed (one row per artist). Tests (internal/recommendation/home_integration_test.go): - TrackDerived path: 2 liked tracks >30d old + no recent plays = album appears in Rediscover. - Threshold guard: 1 liked track = album does NOT appear. - Diversity cap: 4 explicit album-likes from same artist = 2 in output. - Dedup vs MostPlayed: eligible album whose track is in MostPlayed gets filtered out. - Existing FallbackWhenSparse test preserved (semantics unchanged for recent likes that miss the >30d primary filter). Clients unchanged - they render whatever /api/home/index returns. No parity-map row needed (this is a server-internal recommendation change, not a layout divergence).
2026-06-01 00:57:40 -04:00
parent bf61d6cfa3
commit 8b586c2e50
4 changed files with 431 additions and 77 deletions
@@ -18,9 +18,22 @@ import (
 // Section size caps. SPA splits these into rows on the page.
 const (
 	HomeRecentlyAddedLimit = 50
-	HomeRediscoverLimit    = 25
-	HomeMostPlayedLimit    = 75
-	HomeLastPlayedLimit    = 25
+	// HomeRediscoverLimit is the OUTPUT count clients render. The SQL
+	// query asks for rediscoverInnerLimit so the Go layer has headroom
+	// to apply cross-section dedup (vs Most Played) and the diversity
+	// cap (max 2 albums per artist) without undershooting.
+	HomeRediscoverLimit = 10
+	HomeMostPlayedLimit = 75
+	HomeLastPlayedLimit = 25
+
+	// rediscoverInnerLimit is the per-query cap; 3x HomeRediscoverLimit
+	// gives enough slack that filtering rarely undershoots.
+	rediscoverInnerLimit = 30
+
+	// maxAlbumsPerArtistInRediscover prevents one liked-but-forgotten
+	// artist from dominating the row. Two is enough variety; higher
+	// reads as "this artist's discography" instead of "rediscover".
+	maxAlbumsPerArtistInRediscover = 2
 )

 // HomePayload is the composite returned by HomeData. All slices are
@@ -35,10 +48,13 @@ type HomePayload struct {
 }

 // HomeData runs five queries in parallel and assembles the payload.
-// Rediscover sections fall back to a random sample of liked entities
-// when the eligibility query returns fewer than HomeRediscoverLimit rows.
-// Any single query error fails the whole call (predictable empty-or-full
-// UX for the SPA — partial payloads are not a feature).
+// Rediscover sections fan out to inner-limit results (track-derived +
+// explicit-like eligibility, plus a daily-stable random fallback when
+// the eligibility query is sparse), then the Go layer trims to
+// HomeRediscoverLimit after applying cross-section dedup (vs Most
+// Played) and the per-artist diversity cap. Any single query error
+// fails the whole call (predictable empty-or-full UX for the SPA —
+// partial payloads are not a feature).
 func HomeData(ctx context.Context, pool *pgxpool.Pool, userID pgtype.UUID) (*HomePayload, error) {
 	q := dbq.New(pool)

@@ -118,21 +134,106 @@ func HomeData(ctx context.Context, pool *pgxpool.Pool, userID pgtype.UUID) (*Hom
 	if firstErr != nil {
 		return nil, firstErr
 	}
+
+	// Cross-section dedup + diversity cap, applied after the parallel
+	// fan-out so the SQL stays simple and reusable. Both filters trim
+	// the rediscover lists down toward HomeRediscoverLimit.
+	out.RediscoverAlbums = applyRediscoverAlbumFilters(out.RediscoverAlbums, out.MostPlayedTracks)
+	out.RediscoverArtists = applyRediscoverArtistFilters(out.RediscoverArtists, out.MostPlayedTracks)
 	return out, nil
 }

+// mostPlayedAlbumIDs returns the set of album IDs whose tracks appear
+// in the Most Played row. Used to dedup Rediscover so it doesn't list
+// albums the user is actively spinning.
+func mostPlayedAlbumIDs(rows []dbq.ListMostPlayedTracksForUserRow) map[pgtype.UUID]struct{} {
+	out := make(map[pgtype.UUID]struct{}, len(rows))
+	for _, r := range rows {
+		out[r.Track.AlbumID] = struct{}{}
+	}
+	return out
+}
+
+// mostPlayedArtistIDs returns the set of artist IDs whose tracks appear
+// in the Most Played row.
+func mostPlayedArtistIDs(rows []dbq.ListMostPlayedTracksForUserRow) map[pgtype.UUID]struct{} {
+	out := make(map[pgtype.UUID]struct{}, len(rows))
+	for _, r := range rows {
+		out[r.Track.ArtistID] = struct{}{}
+	}
+	return out
+}
+
+// applyRediscoverAlbumFilters trims the inner-limit query result down
+// to HomeRediscoverLimit via:
+//  1. Cross-section dedup vs Most Played (drop albums actively played).
+//  2. Diversity cap: at most maxAlbumsPerArtistInRediscover per artist.
+//  3. Final trim to HomeRediscoverLimit.
+//
+// Input order is preserved (daily-stable hash from the SQL), so the
+// row stays stable for the day even after filtering.
+func applyRediscoverAlbumFilters(
+	rows []dbq.ListRediscoverAlbumsForUserRow,
+	mostPlayed []dbq.ListMostPlayedTracksForUserRow,
+) []dbq.ListRediscoverAlbumsForUserRow {
+	excludedAlbums := mostPlayedAlbumIDs(mostPlayed)
+	perArtist := make(map[pgtype.UUID]int, len(rows))
+	out := make([]dbq.ListRediscoverAlbumsForUserRow, 0, HomeRediscoverLimit)
+	for _, r := range rows {
+		if _, dup := excludedAlbums[r.Album.ID]; dup {
+			continue
+		}
+		if perArtist[r.Album.ArtistID] >= maxAlbumsPerArtistInRediscover {
+			continue
+		}
+		perArtist[r.Album.ArtistID]++
+		out = append(out, r)
+		if len(out) >= HomeRediscoverLimit {
+			break
+		}
+	}
+	return out
+}
+
+// applyRediscoverArtistFilters trims the artist list via cross-section
+// dedup vs Most Played, then truncates to HomeRediscoverLimit.
+// No diversity cap needed (one row per artist by construction).
+func applyRediscoverArtistFilters(
+	rows []dbq.ListRediscoverArtistsForUserRow,
+	mostPlayed []dbq.ListMostPlayedTracksForUserRow,
+) []dbq.ListRediscoverArtistsForUserRow {
+	excludedArtists := mostPlayedArtistIDs(mostPlayed)
+	out := make([]dbq.ListRediscoverArtistsForUserRow, 0, HomeRediscoverLimit)
+	for _, r := range rows {
+		if _, dup := excludedArtists[r.Artist.ID]; dup {
+			continue
+		}
+		out = append(out, r)
+		if len(out) >= HomeRediscoverLimit {
+			break
+		}
+	}
+	return out
+}
+
+// loadRediscoverAlbums runs the primary eligibility query (track-
+// derived + explicit album-likes UNION'd) and tops up with the
+// fallback (random sample of any liked albums) if the primary returns
+// fewer than rediscoverInnerLimit. Both use the same daily-stable
+// hash ordering. The Go layer (applyRediscoverAlbumFilters) trims to
+// HomeRediscoverLimit after cross-section dedup + diversity cap.
 func loadRediscoverAlbums(ctx context.Context, q *dbq.Queries, userID pgtype.UUID) ([]dbq.ListRediscoverAlbumsForUserRow, error) {
 	primary, err := q.ListRediscoverAlbumsForUser(ctx, dbq.ListRediscoverAlbumsForUserParams{
-		UserID: userID, Limit: HomeRediscoverLimit,
+		UserID: userID, Limit: rediscoverInnerLimit,
 	})
 	if err != nil {
 		return nil, err
 	}
-	if len(primary) >= HomeRediscoverLimit {
+	if len(primary) >= rediscoverInnerLimit {
 		return primary, nil
 	}
 	fallback, err := q.ListRediscoverAlbumsFallbackForUser(ctx, dbq.ListRediscoverAlbumsFallbackForUserParams{
-		UserID: userID, Limit: HomeRediscoverLimit,
+		UserID: userID, Limit: rediscoverInnerLimit,
 	})
 	if err != nil {
 		return nil, err
@@ -146,7 +247,7 @@ func loadRediscoverAlbums(ctx context.Context, q *dbq.Queries, userID pgtype.UUI
 			continue
 		}
 		primary = append(primary, dbq.ListRediscoverAlbumsForUserRow(r))
-		if len(primary) >= HomeRediscoverLimit {
+		if len(primary) >= rediscoverInnerLimit {
 			break
 		}
 		seen[r.Album.ID] = struct{}{}
@@ -156,16 +257,16 @@ func loadRediscoverAlbums(ctx context.Context, q *dbq.Queries, userID pgtype.UUI

 func loadRediscoverArtists(ctx context.Context, q *dbq.Queries, userID pgtype.UUID) ([]dbq.ListRediscoverArtistsForUserRow, error) {
 	primary, err := q.ListRediscoverArtistsForUser(ctx, dbq.ListRediscoverArtistsForUserParams{
-		UserID: userID, Limit: HomeRediscoverLimit,
+		UserID: userID, Limit: rediscoverInnerLimit,
 	})
 	if err != nil {
 		return nil, err
 	}
-	if len(primary) >= HomeRediscoverLimit {
+	if len(primary) >= rediscoverInnerLimit {
 		return primary, nil
 	}
 	fallback, err := q.ListRediscoverArtistsFallbackForUser(ctx, dbq.ListRediscoverArtistsFallbackForUserParams{
-		UserID: userID, Limit: HomeRediscoverLimit,
+		UserID: userID, Limit: rediscoverInnerLimit,
 	})
 	if err != nil {
 		return nil, err
@@ -179,7 +280,7 @@ func loadRediscoverArtists(ctx context.Context, q *dbq.Queries, userID pgtype.UU
 			continue
 		}
 		primary = append(primary, dbq.ListRediscoverArtistsForUserRow(r))
-		if len(primary) >= HomeRediscoverLimit {
+		if len(primary) >= rediscoverInnerLimit {
 			break
 		}
 		seen[r.Artist.ID] = struct{}{}
@@ -6,6 +6,8 @@ import (
 	"time"

 	"github.com/jackc/pgx/v5/pgtype"
+
+	"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
 )

 func TestHomeData_NewUser_AllSectionsEmpty(t *testing.T) {
@@ -128,3 +130,162 @@ func TestHomeData_RediscoverAlbums_FallbackWhenSparse(t *testing.T) {
 		t.Errorf("got %s, want %s", got.RediscoverAlbums[0].Album.Title, al.Title)
 	}
 }
+
+// TestHomeData_Rediscover_AlbumFromTrackLikes verifies the track-derived
+// path: 2+ liked tracks from an album (with the earliest like >30 days
+// ago) is enough for the album to appear in Rediscover, even with no
+// explicit general_likes_albums entry.
+func TestHomeData_Rediscover_AlbumFromTrackLikes(t *testing.T) {
+	pool := newPool(t)
+	user := seedUser(t, pool, "home-redis-tracklike")
+	artist := seedArtist(t, pool, "ArtistTL", "")
+	al := seedAlbumForArtist(t, pool, artist.ID, "TrackLiked")
+	t1 := seedTrackOnAlbum(t, pool, al.ID, artist.ID, "T1")
+	t2 := seedTrackOnAlbum(t, pool, al.ID, artist.ID, "T2")
+	// Two track-likes, both >30 days old; no explicit album-like.
+	if _, err := pool.Exec(context.Background(),
+		`INSERT INTO general_likes (user_id, track_id, liked_at) VALUES
+		   ($1, $2, now() - interval '60 days'),
+		   ($1, $3, now() - interval '45 days')`,
+		user.ID, t1.ID, t2.ID,
+	); err != nil {
+		t.Fatalf("insert track likes: %v", err)
+	}
+	got, err := HomeData(context.Background(), pool, user.ID)
+	if err != nil {
+		t.Fatalf("HomeData: %v", err)
+	}
+	if len(got.RediscoverAlbums) != 1 {
+		t.Fatalf("len = %d, want 1 (track-derived)", len(got.RediscoverAlbums))
+	}
+	if got.RediscoverAlbums[0].Album.ID != al.ID {
+		t.Errorf("got %s, want %s", got.RediscoverAlbums[0].Album.Title, al.Title)
+	}
+}
+
+// TestHomeData_Rediscover_AlbumTrackLikeThreshold verifies that ONE
+// liked track from an album is not enough — threshold is 2 for the
+// track-derived path. The fallback should also miss this since no
+// album-like exists.
+func TestHomeData_Rediscover_AlbumTrackLikeThreshold(t *testing.T) {
+	pool := newPool(t)
+	user := seedUser(t, pool, "home-redis-threshold")
+	artist := seedArtist(t, pool, "ArtistTh", "")
+	al := seedAlbumForArtist(t, pool, artist.ID, "OneTrackLiked")
+	t1 := seedTrackOnAlbum(t, pool, al.ID, artist.ID, "Solo")
+	if _, err := pool.Exec(context.Background(),
+		`INSERT INTO general_likes (user_id, track_id, liked_at) VALUES
+		   ($1, $2, now() - interval '60 days')`,
+		user.ID, t1.ID,
+	); err != nil {
+		t.Fatalf("insert track like: %v", err)
+	}
+	got, err := HomeData(context.Background(), pool, user.ID)
+	if err != nil {
+		t.Fatalf("HomeData: %v", err)
+	}
+	if len(got.RediscoverAlbums) != 0 {
+		t.Errorf("len = %d, want 0 (below 2-track threshold)", len(got.RediscoverAlbums))
+	}
+}
+
+// TestHomeData_Rediscover_DiversityCap verifies that no more than 2
+// albums from the same artist appear in Rediscover even when more are
+// eligible.
+func TestHomeData_Rediscover_DiversityCap(t *testing.T) {
+	pool := newPool(t)
+	user := seedUser(t, pool, "home-redis-cap")
+	artist := seedArtist(t, pool, "ArtistDom", "")
+	albums := []dbq.Album{
+		seedAlbumForArtist(t, pool, artist.ID, "A1"),
+		seedAlbumForArtist(t, pool, artist.ID, "A2"),
+		seedAlbumForArtist(t, pool, artist.ID, "A3"),
+		seedAlbumForArtist(t, pool, artist.ID, "A4"),
+	}
+	// Explicit album-likes >30 days old on all four.
+	for _, al := range albums {
+		if _, err := pool.Exec(context.Background(),
+			`INSERT INTO general_likes_albums (user_id, album_id, liked_at)
+			 VALUES ($1, $2, now() - interval '40 days')`,
+			user.ID, al.ID,
+		); err != nil {
+			t.Fatalf("insert like: %v", err)
+		}
+	}
+	got, err := HomeData(context.Background(), pool, user.ID)
+	if err != nil {
+		t.Fatalf("HomeData: %v", err)
+	}
+	if len(got.RediscoverAlbums) != maxAlbumsPerArtistInRediscover {
+		t.Errorf("len = %d, want %d (diversity cap)",
+			len(got.RediscoverAlbums), maxAlbumsPerArtistInRediscover)
+	}
+}
+
+// TestHomeData_Rediscover_DedupVsMostPlayed verifies that an otherwise-
+// eligible album whose tracks appear in the Most Played section is
+// dropped from Rediscover (the "you forgot about this" contract is
+// broken if the user is actively spinning it).
+func TestHomeData_Rediscover_DedupVsMostPlayed(t *testing.T) {
+	pool := newPool(t)
+	user := seedUser(t, pool, "home-redis-dedup")
+	artist := seedArtist(t, pool, "ArtistD", "")
+	// "Forgotten" album: liked >30d ago, no recent plays — would be
+	// eligible on its own.
+	forgotten := seedAlbumForArtist(t, pool, artist.ID, "Forgotten")
+	if _, err := pool.Exec(context.Background(),
+		`INSERT INTO general_likes_albums (user_id, album_id, liked_at)
+		 VALUES ($1, $2, now() - interval '40 days')`,
+		user.ID, forgotten.ID,
+	); err != nil {
+		t.Fatalf("insert forgotten like: %v", err)
+	}
+	// "Active" album: liked >30d ago BUT not played in last 14 days at
+	// the album level (uses one >30-day-old track-like, no recent plays
+	// — qualifies for Rediscover). Its track ALSO appears in MostPlayed
+	// via many old plays >14 days ago... wait, we need the track to be
+	// in MostPlayed, which means it needs play_events that aren't
+	// skipped. MostPlayed has no recency filter; any plays count.
+	active := seedAlbumForArtist(t, pool, artist.ID, "Active")
+	activeT1 := seedTrackOnAlbum(t, pool, active.ID, artist.ID, "AT1")
+	activeT2 := seedTrackOnAlbum(t, pool, active.ID, artist.ID, "AT2")
+	if _, err := pool.Exec(context.Background(),
+		`INSERT INTO general_likes (user_id, track_id, liked_at) VALUES
+		   ($1, $2, now() - interval '60 days'),
+		   ($1, $3, now() - interval '60 days')`,
+		user.ID, activeT1.ID, activeT2.ID,
+	); err != nil {
+		t.Fatalf("insert active track likes: %v", err)
+	}
+	// Many plays >14 days ago (so still Rediscover-eligible by the
+	// "not played in last 14 days" rule) but enough to put it in
+	// MostPlayed.
+	for i := 0; i < 5; i++ {
+		insertPlayEvent(t, pool, user.ID, activeT1.ID,
+			time.Now().Add(-30*24*time.Hour))
+	}
+
+	got, err := HomeData(context.Background(), pool, user.ID)
+	if err != nil {
+		t.Fatalf("HomeData: %v", err)
+	}
+	// MostPlayed must include the active track.
+	foundInMP := false
+	for _, mp := range got.MostPlayedTracks {
+		if mp.Track.ID == activeT1.ID {
+			foundInMP = true
+		}
+	}
+	if !foundInMP {
+		t.Fatalf("active track not in MostPlayed; test fixture broken")
+	}
+	// Rediscover must contain forgotten but NOT active.
+	if len(got.RediscoverAlbums) != 1 {
+		t.Fatalf("RediscoverAlbums len = %d, want 1 (active deduped)",
+			len(got.RediscoverAlbums))
+	}
+	if got.RediscoverAlbums[0].Album.ID != forgotten.ID {
+		t.Errorf("got %s, want %s (active should have been deduped)",
+			got.RediscoverAlbums[0].Album.Title, forgotten.Title)
+	}
+}