fix(discover): cap the taste-matched arm per album and artist before its LIMIT (#5356)
release / web (push) Successful in 1m37s
release / govulncheck (push) Successful in 53s
release / go (push) Successful in 2m7s
release / integration (push) Successful in 5m2s
release / android (push) Successful in 6m34s
release / Build signed APK (releases and dev) (push) Successful in 6m7s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 1m36s
release / Verify release artifacts (tag releases only) (push) Skipped
release / web (push) Successful in 1m37s
release / govulncheck (push) Successful in 53s
release / go (push) Successful in 2m7s
release / integration (push) Successful in 5m2s
release / android (push) Successful in 6m34s
release / Build signed APK (releases and dev) (push) Successful in 6m7s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 1m36s
release / Verify release artifacts (tag releases only) (push) Skipped
Summed tag weight rewards a track for carrying many of the user's tags, so on the deploy two artists whose every track carries the whole lo-fi profile took all 120 rows of the taste-unheard query. capByAlbumAndArtist ran after the LIMIT and left 6, and the arm with the lowest skip rate (12% against ~23%) handed its slots to dormant and random. The query now ranks within album, then within artist over what the album cap kept, before the LIMIT: the same walk the Go cap makes, so the bucket fills from as many artists as match. The caps come from the Go constants. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -216,35 +216,55 @@ func (q *Queries) ListRandomUnheardTracksForDiscover(ctx context.Context, arg Li
|
|||||||
}
|
}
|
||||||
|
|
||||||
const listTasteUnheardTracksForDiscover = `-- name: ListTasteUnheardTracksForDiscover :many
|
const listTasteUnheardTracksForDiscover = `-- name: ListTasteUnheardTracksForDiscover :many
|
||||||
SELECT t.id, t.album_id, t.artist_id
|
WITH scored AS (
|
||||||
FROM tracks t
|
SELECT t.id, t.album_id, t.artist_id,
|
||||||
JOIN LATERAL regexp_split_to_table(coalesce(t.genre, ''), '[;,]') AS g_split(g) ON true
|
SUM(nt.weight) AS weight,
|
||||||
JOIN taste_profile_tags nt ON nt.user_id = $1 AND trim(g_split.g) = nt.tag
|
md5(t.id::text || $2::text) AS tiebreak
|
||||||
WHERE t.missing_since IS NULL -- #2523: never offer a file that is gone
|
FROM tracks t
|
||||||
AND nt.weight > 0
|
JOIN LATERAL regexp_split_to_table(coalesce(t.genre, ''), '[;,]') AS g_split(g) ON true
|
||||||
AND trim(g_split.g) <> ''
|
JOIN taste_profile_tags nt ON nt.user_id = $3 AND trim(g_split.g) = nt.tag
|
||||||
AND NOT EXISTS (
|
WHERE t.missing_since IS NULL -- #2523: never offer a file that is gone
|
||||||
SELECT 1 FROM play_events pe
|
AND nt.weight > 0
|
||||||
WHERE pe.user_id = $1
|
AND trim(g_split.g) <> ''
|
||||||
AND pe.track_id = t.id
|
AND NOT EXISTS (
|
||||||
AND pe.was_skipped = false
|
SELECT 1 FROM play_events pe
|
||||||
)
|
WHERE pe.user_id = $3
|
||||||
AND NOT EXISTS (
|
AND pe.track_id = t.id
|
||||||
SELECT 1 FROM general_likes gl
|
AND pe.was_skipped = false
|
||||||
WHERE gl.user_id = $1 AND gl.track_id = t.id
|
)
|
||||||
)
|
AND NOT EXISTS (
|
||||||
AND NOT EXISTS (
|
SELECT 1 FROM general_likes gl
|
||||||
SELECT 1 FROM lidarr_quarantine q
|
WHERE gl.user_id = $3 AND gl.track_id = t.id
|
||||||
WHERE q.user_id = $1 AND q.track_id = t.id
|
)
|
||||||
)
|
AND NOT EXISTS (
|
||||||
GROUP BY t.id, t.album_id, t.artist_id
|
SELECT 1 FROM lidarr_quarantine q
|
||||||
ORDER BY SUM(nt.weight) DESC, md5(t.id::text || $2::text)
|
WHERE q.user_id = $3 AND q.track_id = t.id
|
||||||
|
)
|
||||||
|
GROUP BY t.id, t.album_id, t.artist_id
|
||||||
|
),
|
||||||
|
album_capped AS (
|
||||||
|
SELECT s.id, s.album_id, s.artist_id, s.weight, s.tiebreak,
|
||||||
|
row_number() OVER (PARTITION BY s.album_id ORDER BY s.weight DESC, s.tiebreak) AS album_rank
|
||||||
|
FROM scored s
|
||||||
|
),
|
||||||
|
artist_capped AS (
|
||||||
|
SELECT a.id, a.album_id, a.artist_id, a.weight, a.tiebreak, a.album_rank,
|
||||||
|
row_number() OVER (PARTITION BY a.artist_id ORDER BY a.weight DESC, a.tiebreak) AS artist_rank
|
||||||
|
FROM album_capped a
|
||||||
|
WHERE a.album_rank <= $4::int
|
||||||
|
)
|
||||||
|
SELECT c.id, c.album_id, c.artist_id
|
||||||
|
FROM artist_capped c
|
||||||
|
WHERE c.artist_rank <= $1::int
|
||||||
|
ORDER BY c.weight DESC, c.tiebreak
|
||||||
LIMIT 120
|
LIMIT 120
|
||||||
`
|
`
|
||||||
|
|
||||||
type ListTasteUnheardTracksForDiscoverParams struct {
|
type ListTasteUnheardTracksForDiscoverParams struct {
|
||||||
UserID pgtype.UUID
|
MaxPerArtist int32
|
||||||
Column2 string
|
DateSeed string
|
||||||
|
UserID pgtype.UUID
|
||||||
|
MaxPerAlbum int32
|
||||||
}
|
}
|
||||||
|
|
||||||
type ListTasteUnheardTracksForDiscoverRow struct {
|
type ListTasteUnheardTracksForDiscoverRow struct {
|
||||||
@@ -261,9 +281,21 @@ type ListTasteUnheardTracksForDiscoverRow struct {
|
|||||||
// [;,]). Same exclusion filters as the other buckets. Returns nothing
|
// [;,]). Same exclusion filters as the other buckets. Returns nothing
|
||||||
// when the user has no taste tags yet (cold start), so the caller
|
// when the user has no taste tags yet (cold start), so the caller
|
||||||
// redistributes its slots to the other buckets. Stamped 'taste_unheard'.
|
// redistributes its slots to the other buckets. Stamped 'taste_unheard'.
|
||||||
// $1 = user_id, $2 = date string for md5 tiebreak ordering.
|
//
|
||||||
|
// The per-album and per-artist caps apply BEFORE the LIMIT (#5356). Summed
|
||||||
|
// weight rewards a track for carrying many of the user's tags, so a few
|
||||||
|
// artists whose every track is tagged with the whole profile take every row
|
||||||
|
// of a plain LIMIT; the caller's caps then left 6 of 120 on the deploy, and
|
||||||
|
// the best-performing arm handed its slots to the others. Ranking within
|
||||||
|
// album, then within artist over what the album cap kept, is the same walk
|
||||||
|
// capByAlbumAndArtist makes, so the caller's caps keep everything here.
|
||||||
func (q *Queries) ListTasteUnheardTracksForDiscover(ctx context.Context, arg ListTasteUnheardTracksForDiscoverParams) ([]ListTasteUnheardTracksForDiscoverRow, error) {
|
func (q *Queries) ListTasteUnheardTracksForDiscover(ctx context.Context, arg ListTasteUnheardTracksForDiscoverParams) ([]ListTasteUnheardTracksForDiscoverRow, error) {
|
||||||
rows, err := q.db.Query(ctx, listTasteUnheardTracksForDiscover, arg.UserID, arg.Column2)
|
rows, err := q.db.Query(ctx, listTasteUnheardTracksForDiscover,
|
||||||
|
arg.MaxPerArtist,
|
||||||
|
arg.DateSeed,
|
||||||
|
arg.UserID,
|
||||||
|
arg.MaxPerAlbum,
|
||||||
|
)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -115,28 +115,53 @@ SELECT t.id, t.album_id, t.artist_id
|
|||||||
-- [;,]). Same exclusion filters as the other buckets. Returns nothing
|
-- [;,]). Same exclusion filters as the other buckets. Returns nothing
|
||||||
-- when the user has no taste tags yet (cold start), so the caller
|
-- when the user has no taste tags yet (cold start), so the caller
|
||||||
-- redistributes its slots to the other buckets. Stamped 'taste_unheard'.
|
-- redistributes its slots to the other buckets. Stamped 'taste_unheard'.
|
||||||
-- $1 = user_id, $2 = date string for md5 tiebreak ordering.
|
--
|
||||||
SELECT t.id, t.album_id, t.artist_id
|
-- The per-album and per-artist caps apply BEFORE the LIMIT (#5356). Summed
|
||||||
FROM tracks t
|
-- weight rewards a track for carrying many of the user's tags, so a few
|
||||||
JOIN LATERAL regexp_split_to_table(coalesce(t.genre, ''), '[;,]') AS g_split(g) ON true
|
-- artists whose every track is tagged with the whole profile take every row
|
||||||
JOIN taste_profile_tags nt ON nt.user_id = $1 AND trim(g_split.g) = nt.tag
|
-- of a plain LIMIT; the caller's caps then left 6 of 120 on the deploy, and
|
||||||
WHERE t.missing_since IS NULL -- #2523: never offer a file that is gone
|
-- the best-performing arm handed its slots to the others. Ranking within
|
||||||
AND nt.weight > 0
|
-- album, then within artist over what the album cap kept, is the same walk
|
||||||
AND trim(g_split.g) <> ''
|
-- capByAlbumAndArtist makes, so the caller's caps keep everything here.
|
||||||
AND NOT EXISTS (
|
WITH scored AS (
|
||||||
SELECT 1 FROM play_events pe
|
SELECT t.id, t.album_id, t.artist_id,
|
||||||
WHERE pe.user_id = $1
|
SUM(nt.weight) AS weight,
|
||||||
AND pe.track_id = t.id
|
md5(t.id::text || sqlc.arg(date_seed)::text) AS tiebreak
|
||||||
AND pe.was_skipped = false
|
FROM tracks t
|
||||||
)
|
JOIN LATERAL regexp_split_to_table(coalesce(t.genre, ''), '[;,]') AS g_split(g) ON true
|
||||||
AND NOT EXISTS (
|
JOIN taste_profile_tags nt ON nt.user_id = sqlc.arg(user_id) AND trim(g_split.g) = nt.tag
|
||||||
SELECT 1 FROM general_likes gl
|
WHERE t.missing_since IS NULL -- #2523: never offer a file that is gone
|
||||||
WHERE gl.user_id = $1 AND gl.track_id = t.id
|
AND nt.weight > 0
|
||||||
)
|
AND trim(g_split.g) <> ''
|
||||||
AND NOT EXISTS (
|
AND NOT EXISTS (
|
||||||
SELECT 1 FROM lidarr_quarantine q
|
SELECT 1 FROM play_events pe
|
||||||
WHERE q.user_id = $1 AND q.track_id = t.id
|
WHERE pe.user_id = sqlc.arg(user_id)
|
||||||
)
|
AND pe.track_id = t.id
|
||||||
GROUP BY t.id, t.album_id, t.artist_id
|
AND pe.was_skipped = false
|
||||||
ORDER BY SUM(nt.weight) DESC, md5(t.id::text || $2::text)
|
)
|
||||||
|
AND NOT EXISTS (
|
||||||
|
SELECT 1 FROM general_likes gl
|
||||||
|
WHERE gl.user_id = sqlc.arg(user_id) AND gl.track_id = t.id
|
||||||
|
)
|
||||||
|
AND NOT EXISTS (
|
||||||
|
SELECT 1 FROM lidarr_quarantine q
|
||||||
|
WHERE q.user_id = sqlc.arg(user_id) AND q.track_id = t.id
|
||||||
|
)
|
||||||
|
GROUP BY t.id, t.album_id, t.artist_id
|
||||||
|
),
|
||||||
|
album_capped AS (
|
||||||
|
SELECT s.*,
|
||||||
|
row_number() OVER (PARTITION BY s.album_id ORDER BY s.weight DESC, s.tiebreak) AS album_rank
|
||||||
|
FROM scored s
|
||||||
|
),
|
||||||
|
artist_capped AS (
|
||||||
|
SELECT a.*,
|
||||||
|
row_number() OVER (PARTITION BY a.artist_id ORDER BY a.weight DESC, a.tiebreak) AS artist_rank
|
||||||
|
FROM album_capped a
|
||||||
|
WHERE a.album_rank <= sqlc.arg(max_per_album)::int
|
||||||
|
)
|
||||||
|
SELECT c.id, c.album_id, c.artist_id
|
||||||
|
FROM artist_capped c
|
||||||
|
WHERE c.artist_rank <= sqlc.arg(max_per_artist)::int
|
||||||
|
ORDER BY c.weight DESC, c.tiebreak
|
||||||
LIMIT 120;
|
LIMIT 120;
|
||||||
|
|||||||
@@ -95,8 +95,11 @@ type discoverPools struct {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func loadDiscoverPools(ctx context.Context, q *dbq.Queries, logger *slog.Logger, userID pgtype.UUID, dateStr string) discoverPools {
|
func loadDiscoverPools(ctx context.Context, q *dbq.Queries, logger *slog.Logger, userID pgtype.UUID, dateStr string) discoverPools {
|
||||||
|
// The caps go into the query too (#5356): applied only here, after its
|
||||||
|
// LIMIT, they left a heavily tagged artist or two owning the whole bucket.
|
||||||
tasteRows, err := q.ListTasteUnheardTracksForDiscover(ctx, dbq.ListTasteUnheardTracksForDiscoverParams{
|
tasteRows, err := q.ListTasteUnheardTracksForDiscover(ctx, dbq.ListTasteUnheardTracksForDiscoverParams{
|
||||||
UserID: userID, Column2: dateStr,
|
UserID: userID, DateSeed: dateStr,
|
||||||
|
MaxPerAlbum: discoverMaxTracksPerAlbum, MaxPerArtist: discoverMaxTracksPerArtist,
|
||||||
})
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
logger.Warn("discover: taste-unheard bucket failed; continuing with empty pool",
|
logger.Warn("discover: taste-unheard bucket failed; continuing with empty pool",
|
||||||
|
|||||||
@@ -0,0 +1,100 @@
|
|||||||
|
package playlists_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"fmt"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/jackc/pgx/v5/pgtype"
|
||||||
|
"github.com/jackc/pgx/v5/pgxpool"
|
||||||
|
|
||||||
|
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||||
|
)
|
||||||
|
|
||||||
|
func setGenre(t *testing.T, pool *pgxpool.Pool, trackID pgtype.UUID, genre string) {
|
||||||
|
t.Helper()
|
||||||
|
if _, err := pool.Exec(context.Background(),
|
||||||
|
`UPDATE tracks SET genre = $2 WHERE id = $1`, trackID, genre); err != nil {
|
||||||
|
t.Fatalf("set genre: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func seedTasteTag(t *testing.T, pool *pgxpool.Pool, userID pgtype.UUID, tag string, weight float64) {
|
||||||
|
t.Helper()
|
||||||
|
if _, err := pool.Exec(context.Background(),
|
||||||
|
`INSERT INTO taste_profile_tags (user_id, tag, weight) VALUES ($1, $2, $3)
|
||||||
|
ON CONFLICT (user_id, tag) DO UPDATE SET weight = EXCLUDED.weight`,
|
||||||
|
userID, tag, weight); err != nil {
|
||||||
|
t.Fatalf("seed taste tag: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestListTasteUnheardTracksForDiscover_CapsBeforeLimit is the deploy's shape
|
||||||
|
// (#5356): one artist whose every track carries the whole taste profile
|
||||||
|
// outscores everything on summed weight. Capped only after the LIMIT, that
|
||||||
|
// artist took all 120 rows and the bucket shrank to its 3. Capped before, the
|
||||||
|
// artist keeps 3 and the single-tag artists below it fill the rest.
|
||||||
|
func TestListTasteUnheardTracksForDiscover_CapsBeforeLimit(t *testing.T) {
|
||||||
|
pool := newPool(t)
|
||||||
|
ctx := context.Background()
|
||||||
|
u := seedUser(t, pool, "tastecap")
|
||||||
|
|
||||||
|
profile := []string{"Lo-Fi", "Downtempo", "Hip Hop", "Instrumental", "Chillwave"}
|
||||||
|
for _, tag := range profile {
|
||||||
|
seedTasteTag(t, pool, u.ID, tag, 10)
|
||||||
|
}
|
||||||
|
allTags := "Lo-Fi; Downtempo; Hip Hop; Instrumental; Chillwave"
|
||||||
|
|
||||||
|
// 13 albums x 10 tracks = 130, more than the query's LIMIT of 120.
|
||||||
|
heavy := seedTrack(t, pool, "Heavy 0", "Heavy Artist")
|
||||||
|
setGenre(t, pool, heavy.ID, allTags)
|
||||||
|
album := heavy.AlbumID
|
||||||
|
for i := 1; i < 130; i++ {
|
||||||
|
if i%10 == 0 {
|
||||||
|
album = seedAlbumForArtist(t, pool, fmt.Sprintf("Heavy Album %d", i/10), heavy.ArtistID)
|
||||||
|
}
|
||||||
|
tr := seedTrackForArtist(t, pool, fmt.Sprintf("Heavy %d", i), album, heavy.ArtistID)
|
||||||
|
setGenre(t, pool, tr.ID, allTags)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Twelve artists with one track each, matching a single profile tag.
|
||||||
|
const light = 12
|
||||||
|
for i := 0; i < light; i++ {
|
||||||
|
tr := seedTrack(t, pool, fmt.Sprintf("Light %d", i), fmt.Sprintf("Light Artist %d", i))
|
||||||
|
setGenre(t, pool, tr.ID, profile[i%len(profile)])
|
||||||
|
}
|
||||||
|
|
||||||
|
rows, err := dbq.New(pool).ListTasteUnheardTracksForDiscover(ctx, dbq.ListTasteUnheardTracksForDiscoverParams{
|
||||||
|
UserID: u.ID, DateSeed: "2026-10-08", MaxPerAlbum: 2, MaxPerArtist: 3,
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("list taste unheard: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
perArtist := map[pgtype.UUID]int{}
|
||||||
|
perAlbum := map[pgtype.UUID]int{}
|
||||||
|
for _, r := range rows {
|
||||||
|
perArtist[r.ArtistID]++
|
||||||
|
perAlbum[r.AlbumID]++
|
||||||
|
}
|
||||||
|
if got := perArtist[heavy.ArtistID]; got != 3 {
|
||||||
|
t.Errorf("heavy artist rows = %d, want exactly the artist cap of 3", got)
|
||||||
|
}
|
||||||
|
for id, n := range perAlbum {
|
||||||
|
if n > 2 {
|
||||||
|
t.Errorf("album %v has %d rows, over the album cap of 2", id, n)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if got, want := len(perArtist), 1+light; got != want {
|
||||||
|
t.Errorf("distinct artists = %d, want %d (the heavy artist plus every single-tag artist)", got, want)
|
||||||
|
}
|
||||||
|
if got, want := len(rows), 3+light; got != want {
|
||||||
|
t.Errorf("rows = %d, want %d", got, want)
|
||||||
|
}
|
||||||
|
// Still ranked by weight: the heavy artist's three lead.
|
||||||
|
for i := 0; i < 3 && i < len(rows); i++ {
|
||||||
|
if rows[i].ArtistID != heavy.ArtistID {
|
||||||
|
t.Errorf("row %d is not the heavy artist's; higher summed weight should rank first", i)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user