c29d25d1cb
Discover playlists could surface the same track twice with the duplicates landing back-to-back — a "first song plays, then plays again, skip works" symptom user reported on v2026.05.13.0. Root cause: interleaveBuckets rotates one track per pass per bucket but never tracks which IDs it has already emitted, so a track that's both a dormant-artist pick AND a random-unheard pick comes out once from each bucket. On a single-user server the crossUser bucket is empty, so the redistribute step rolls its slots into dormant + random. Their output then interleaves d0, r0, d1, r1, … — and when d0 == r0 (common: a dormant-artist track is also valid for random-unheard) the result is [X, X, …] with adjacent duplicates. Fix: track seen track IDs across all buckets while interleaving; skip already-taken IDs and advance to the next index in that bucket. Dedup priority is bucket order, so a track in both dormant and random comes from dormant. Regression test covers the single-user case directly. The existing round-robin test still passes — no shared IDs in that fixture. Note: stale duplicates already written to drift / served as cached playlists will clear naturally on the next playlist rebuild (the 03:00-local refresh, or any manual /api/me/playlists/refresh).
302 lines
9.5 KiB
Go
302 lines
9.5 KiB
Go
package playlists
|
|
|
|
import (
|
|
"context"
|
|
"log/slog"
|
|
|
|
"github.com/jackc/pgx/v5/pgtype"
|
|
|
|
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
|
)
|
|
|
|
const (
|
|
discoverTotalSlots = 100
|
|
discoverDormantSlots = 40
|
|
discoverCrossUserSlots = 30
|
|
discoverRandomSlots = 30
|
|
discoverMaxTracksPerAlbum = 2
|
|
discoverMaxTracksPerArtist = 3
|
|
)
|
|
|
|
// discoverTrack is the common shape used by the bucket allocator. The
|
|
// three sqlc-generated row types collapse into this internal struct so
|
|
// downstream functions don't need to be generic over them.
|
|
type discoverTrack struct {
|
|
ID pgtype.UUID
|
|
AlbumID pgtype.UUID
|
|
ArtistID pgtype.UUID
|
|
}
|
|
|
|
// buildDiscoverCandidates assembles the Discover playlist track list.
|
|
// Pulls from three buckets, applies per-album/per-artist caps, then
|
|
// redistributes any deficit equally across the remaining buckets.
|
|
//
|
|
// Returns up to discoverTotalSlots track IDs in the order they should
|
|
// appear in the playlist (round-robin interleaved across buckets).
|
|
// Empty slice when the library has no eligible tracks at all.
|
|
//
|
|
// Individual bucket query failures are logged and treated as empty —
|
|
// the redistribution algorithm rolls the deficit into the surviving
|
|
// buckets so one broken bucket can't silently kill the whole playlist.
|
|
func buildDiscoverCandidates(ctx context.Context, q *dbq.Queries, logger *slog.Logger, userID pgtype.UUID, dateStr string) ([]rankedCandidate, error) {
|
|
dormantRows, err := q.ListDormantArtistTracksForDiscover(ctx, dbq.ListDormantArtistTracksForDiscoverParams{
|
|
UserID: userID, Column2: dateStr,
|
|
})
|
|
if err != nil {
|
|
logger.Warn("discover: dormant bucket failed; continuing with empty pool",
|
|
"user_id", uuidStringPL(userID), "err", err)
|
|
dormantRows = nil
|
|
}
|
|
crossUserRows, err := q.ListCrossUserLikedTracksForDiscover(ctx, dbq.ListCrossUserLikedTracksForDiscoverParams{
|
|
UserID: userID, Column2: dateStr,
|
|
})
|
|
if err != nil {
|
|
logger.Warn("discover: cross-user bucket failed; continuing with empty pool",
|
|
"user_id", uuidStringPL(userID), "err", err)
|
|
crossUserRows = nil
|
|
}
|
|
randomRows, err := q.ListRandomUnheardTracksForDiscover(ctx, dbq.ListRandomUnheardTracksForDiscoverParams{
|
|
UserID: userID, Column2: dateStr,
|
|
})
|
|
if err != nil {
|
|
logger.Warn("discover: random bucket failed; continuing with empty pool",
|
|
"user_id", uuidStringPL(userID), "err", err)
|
|
randomRows = nil
|
|
}
|
|
|
|
// Adapt sqlc-generated rows into the internal struct, then apply
|
|
// per-album / per-artist caps.
|
|
dormantPool := capByAlbumAndArtist(dormantRowsToTracks(dormantRows))
|
|
crossUserPool := capByAlbumAndArtist(crossUserRowsToTracks(crossUserRows))
|
|
randomPool := capByAlbumAndArtist(randomRowsToTracks(randomRows))
|
|
|
|
// Allocate slots with redistribution.
|
|
allocations := redistributeSlots([]bucketRequest{
|
|
{want: discoverDormantSlots, available: len(dormantPool)},
|
|
{want: discoverCrossUserSlots, available: len(crossUserPool)},
|
|
{want: discoverRandomSlots, available: len(randomPool)},
|
|
})
|
|
|
|
// Take the head of each bucket's capped pool.
|
|
dormantTake := dormantPool[:allocations[0]]
|
|
crossUserTake := crossUserPool[:allocations[1]]
|
|
randomTake := randomPool[:allocations[2]]
|
|
|
|
// Round-robin interleave so the playlist doesn't front-load one
|
|
// flavour.
|
|
out := interleaveBuckets(dormantTake, crossUserTake, randomTake)
|
|
|
|
// Convert to rankedCandidate (the type insertSystemPlaylist accepts).
|
|
// Score is unused for Discover — the daily-deterministic md5 ordering
|
|
// already gave us the ranking.
|
|
ranked := make([]rankedCandidate, 0, len(out))
|
|
for _, t := range out {
|
|
ranked = append(ranked, rankedCandidate{TrackID: t.ID})
|
|
}
|
|
return ranked, nil
|
|
}
|
|
|
|
func dormantRowsToTracks(rows []dbq.ListDormantArtistTracksForDiscoverRow) []discoverTrack {
|
|
out := make([]discoverTrack, len(rows))
|
|
for i, r := range rows {
|
|
out[i] = discoverTrack{ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID}
|
|
}
|
|
return out
|
|
}
|
|
|
|
func crossUserRowsToTracks(rows []dbq.ListCrossUserLikedTracksForDiscoverRow) []discoverTrack {
|
|
out := make([]discoverTrack, len(rows))
|
|
for i, r := range rows {
|
|
out[i] = discoverTrack{ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID}
|
|
}
|
|
return out
|
|
}
|
|
|
|
func randomRowsToTracks(rows []dbq.ListRandomUnheardTracksForDiscoverRow) []discoverTrack {
|
|
out := make([]discoverTrack, len(rows))
|
|
for i, r := range rows {
|
|
out[i] = discoverTrack{ID: r.ID, AlbumID: r.AlbumID, ArtistID: r.ArtistID}
|
|
}
|
|
return out
|
|
}
|
|
|
|
// capByAlbumAndArtist trims a bucket's pool so no single album appears
|
|
// more than discoverMaxTracksPerAlbum times and no single artist
|
|
// appears more than discoverMaxTracksPerArtist times. Preserves input
|
|
// order (which is the daily-deterministic md5 sort).
|
|
//
|
|
// pgtype.UUID's Bytes field is [16]byte, which IS comparable as a map
|
|
// key.
|
|
func capByAlbumAndArtist(rows []discoverTrack) []discoverTrack {
|
|
albumCount := map[pgtype.UUID]int{}
|
|
artistCount := map[pgtype.UUID]int{}
|
|
out := make([]discoverTrack, 0, len(rows))
|
|
for _, r := range rows {
|
|
if albumCount[r.AlbumID] >= discoverMaxTracksPerAlbum {
|
|
continue
|
|
}
|
|
if artistCount[r.ArtistID] >= discoverMaxTracksPerArtist {
|
|
continue
|
|
}
|
|
albumCount[r.AlbumID]++
|
|
artistCount[r.ArtistID]++
|
|
out = append(out, r)
|
|
}
|
|
return out
|
|
}
|
|
|
|
// bucketRequest captures one bucket's desired and available count.
|
|
type bucketRequest struct {
|
|
want int
|
|
available int
|
|
}
|
|
|
|
// redistributeSlots returns the final per-bucket allocation given each
|
|
// bucket's want and available count. When a bucket can't fill its want,
|
|
// the deficit splits equally between the OTHER buckets that still have
|
|
// supply. Sum of returned allocations <= sum of `want`.
|
|
//
|
|
// Algorithm:
|
|
// 1. Initial pass: allocate min(want, available) to each bucket;
|
|
// compute supply = available - allocated.
|
|
// 2. For each deficit bucket (allocated < want), split the deficit
|
|
// equally across peers that still have supply > 0. Track how
|
|
// much of that deficit each pass actually placed (some peers may
|
|
// not have enough supply for their full share).
|
|
// 3. Repeat until no movement happens — handles the case where a
|
|
// peer's supply ran out mid-distribution and the residual needs
|
|
// to roll to other peers in a later pass.
|
|
//
|
|
// Bug history (May 2026): an earlier version recomputed
|
|
// `deficit = b.want - final[i]` every pass without subtracting
|
|
// previously-redistributed amounts, so a bucket with permanently
|
|
// 0 final and want=30 kept handing out 30 to peers each pass. The
|
|
// `redistributed` array below tracks per-source absorbed deficit so
|
|
// subsequent passes only move the residual.
|
|
func redistributeSlots(buckets []bucketRequest) []int {
|
|
n := len(buckets)
|
|
final := make([]int, n)
|
|
supply := make([]int, n)
|
|
redistributed := make([]int, n)
|
|
for i, b := range buckets {
|
|
final[i] = b.want
|
|
if final[i] > b.available {
|
|
final[i] = b.available
|
|
}
|
|
supply[i] = b.available - final[i]
|
|
}
|
|
|
|
for pass := 0; pass < 4; pass++ {
|
|
anyMoved := false
|
|
for i, b := range buckets {
|
|
// Remaining deficit = original gap minus what we've already
|
|
// pushed out to peers in earlier passes.
|
|
deficit := b.want - final[i] - redistributed[i]
|
|
if deficit <= 0 {
|
|
continue
|
|
}
|
|
peers := make([]int, 0, n-1)
|
|
for j := 0; j < n; j++ {
|
|
if j == i {
|
|
continue
|
|
}
|
|
if supply[j] > 0 {
|
|
peers = append(peers, j)
|
|
}
|
|
}
|
|
if len(peers) == 0 {
|
|
continue
|
|
}
|
|
share := deficit / len(peers)
|
|
rem := deficit % len(peers)
|
|
for k, j := range peers {
|
|
take := share
|
|
if k < rem {
|
|
take++
|
|
}
|
|
if take > supply[j] {
|
|
take = supply[j]
|
|
}
|
|
if take > 0 {
|
|
final[j] += take
|
|
supply[j] -= take
|
|
redistributed[i] += take
|
|
anyMoved = true
|
|
}
|
|
}
|
|
}
|
|
if !anyMoved {
|
|
break
|
|
}
|
|
}
|
|
|
|
// Final clamp: total <= discoverTotalSlots.
|
|
sum := 0
|
|
for _, f := range final {
|
|
sum += f
|
|
}
|
|
for sum > discoverTotalSlots {
|
|
// Trim the largest bucket. Should be rare.
|
|
maxVal, idx := -1, -1
|
|
for i, f := range final {
|
|
if f > maxVal {
|
|
maxVal, idx = f, i
|
|
}
|
|
}
|
|
if idx < 0 {
|
|
break
|
|
}
|
|
final[idx]--
|
|
sum--
|
|
}
|
|
return final
|
|
}
|
|
|
|
// interleaveBuckets round-robins items from the buckets in order. The
|
|
// first item of each bucket appears before the second of any bucket.
|
|
// interleaveBuckets walks each bucket round-robin, emitting one track
|
|
// per bucket per pass. Tracks already seen in an earlier bucket (or an
|
|
// earlier pass) are skipped — single-user servers hit this often
|
|
// because the empty crossUser bucket redistributes slots to dormant +
|
|
// random, and a dormant-artist track is also a valid random-unheard
|
|
// pick. Without dedup the same track lands at two interleaved
|
|
// positions, which on a 2-bucket round-robin (dormant+random)
|
|
// produces ADJACENT duplicates in the playlist — exactly the
|
|
// "every song has a duplicate right after it" report from v2026.05.13.0.
|
|
//
|
|
// Dedup priority is bucket order (caller-supplied), so a track in both
|
|
// dormant and random is taken from dormant.
|
|
func interleaveBuckets(buckets ...[]discoverTrack) []discoverTrack {
|
|
total := 0
|
|
for _, b := range buckets {
|
|
total += len(b)
|
|
}
|
|
out := make([]discoverTrack, 0, total)
|
|
seen := make(map[pgtype.UUID]struct{}, total)
|
|
indices := make([]int, len(buckets))
|
|
for {
|
|
anyAppended := false
|
|
for bi, b := range buckets {
|
|
// Advance past tracks already taken from an earlier bucket
|
|
// or an earlier pass.
|
|
for indices[bi] < len(b) {
|
|
if _, dup := seen[b[indices[bi]].ID]; !dup {
|
|
break
|
|
}
|
|
indices[bi]++
|
|
}
|
|
if indices[bi] < len(b) {
|
|
t := b[indices[bi]]
|
|
out = append(out, t)
|
|
seen[t.ID] = struct{}{}
|
|
indices[bi]++
|
|
anyAppended = true
|
|
}
|
|
}
|
|
if !anyAppended {
|
|
break
|
|
}
|
|
}
|
|
return out
|
|
}
|