Files
minstrel/internal/recommendation/shuffle.go
T
bvandeusenandClaude Opus 5 633d4f591f
test-go / test (push) Successful in 1m13s
test-go / integration (push) Successful in 3m53s
release / Build signed APK (releases and dev) (push) Successful in 5m9s
release / Build + push container image (push) Successful in 1m52s
release / Verify release artifacts (tag releases only) (push) Skipped
fix(radio): cap any one artist's share of a radio session
Operator, 2026-09-10: started radio from a song and "literally all of the
songs in the playlist after that were from a single artist which was not
expected."

There was no per-artist cap anywhere in the radio path. radio.go built the
pool and handed it straight to Shuffle, which scores, sorts and takes the
top N — nothing between those steps bounded any artist's share, so a pool
dominated by one artist produced an output dominated by it. The asymmetry
was the tell: discover.go, you_might_like.go and home.go all cap; radio
never got one.

With the fixture that reproduces it — 20 liked tracks by one artist plus 10
by ten others — the old path returns 10 tracks from 1 artist. It now returns
10 from 8.

TWO PASSES, and that is the whole design. A hard cap was the easy mistake:
radio asks for 50 tracks by default and 200 at most, so capping at three per
artist over a concentrated pool would hand back a six-track "radio". Pass
one takes candidates that fit under the caps; pass two fills any remaining
slots from those it skipped, still in score order. The result always holds
min(limit, len(candidates)) — the caps change WHICH tracks are picked, never
HOW MANY. Rule 131's principle past the system mixes it was written for.

The caps SCALE with the requested length rather than being a constant.
Three-per-artist is a sensible 12% of a 25-track mix and an absurd 1.5% of a
200-track radio, where every selection would sit in the relaxation path and
the cap would be decorative. RadioDiversityCaps holds the system mixes'
proportion at any length: 3/2 at 25, 6/4 at 50, 24/16 at 200, with floors so
a very short radio is not capped down to one track per artist.

A BOUND, NOT AN EXCLUSION — the operator asked for the opposite of removal:
"again it should be able to add songs from the same artist." The dominant
artist still appears, just not exclusively. Guarded, because the tempting
wrong fix is the filter songs-like used to carry.

Shuffle grew the parameter rather than gaining a capped twin: radio is its
only production caller, so a second function would have left the original
dead (rule 22).

Falsified against each named regression: uncapped gives 10/10 to one artist;
a hard cap returns 3 of 10 on a single-artist pool; a cap-as-exclusion drops
the artist entirely; a fixed cap stays 3 where the scaled one reaches 24.

Caught while writing the guards: the artist-key constant was hand-written
hex and wrong — the fixture's artist UUID carries 0001 in its fourth group,
so the lookup missed and the assertion measured nothing. Derived from the
same construction the fixture uses now.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH
2026-09-10 22:14:42 -04:00

135 lines
4.3 KiB
Go

package recommendation
import (
"sort"
"time"
"github.com/jackc/pgx/v5/pgtype"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
)
// Candidate pairs a track with the inputs needed to score it.
type Candidate struct {
Track dbq.Track
Inputs ScoringInputs
}
// DiversityCaps bounds how much of one selection a single artist or album
// may occupy. A zero value means "no cap", which is what Shuffle did
// unconditionally before #3882.
type DiversityCaps struct {
MaxPerArtist int
MaxPerAlbum int
}
// radioCapArtistPer25 / radioCapAlbumPer25 hold the caps at the same
// PROPORTION the system mixes already use — 3 artist / 2 album tracks in a
// 25-track mix — so a 20-track radio and a 200-track one feel alike rather
// than one of them being effectively uncapped.
//
// A fixed count cannot do that. Three-per-artist is a reasonable 12% of a
// 25-track mix and an absurd 1.5% of a 200-track radio, where it would put
// the selection permanently in the relaxation path below and quietly undo
// the cap it was meant to enforce.
const (
radioCapArtistPer25 = 3
radioCapAlbumPer25 = 2
)
// RadioDiversityCaps scales the diversity caps to the requested radio
// length. Floors of 2 and 1 keep a very short radio from being capped into
// a single track per artist, which would be its own kind of wrong.
func RadioDiversityCaps(limit int) DiversityCaps {
artist := limit * radioCapArtistPer25 / 25
if artist < 2 {
artist = 2
}
album := limit * radioCapAlbumPer25 / 25
if album < 1 {
album = 1
}
return DiversityCaps{MaxPerArtist: artist, MaxPerAlbum: album}
}
// Shuffle scores each candidate, sorts descending by score, and returns the
// top `limit` candidates, preferring artist/album diversity. limit <= 0
// returns nil; nil input returns nil. Pure — no IO, no global state beyond
// the rng callback.
//
// THE COUNT IS NEVER REDUCED BY THE CAPS. Selection runs in two passes: the
// first takes candidates that fit under the caps, and the second fills any
// remaining slots from those the first pass skipped, still in score order.
// So the result holds min(limit, len(candidates)) either way — the caps
// change WHICH tracks are chosen, never HOW MANY.
//
// That two-pass shape is the whole design, and a hard cap would have been
// the easy mistake. Radio asks for 50 tracks by default and 200 at most; a
// pool concentrated on a few artists would return six tracks and call it a
// radio. Rule 131's principle — degrade, never vanish — applies past the
// system mixes it was written for.
//
// Before #3882 there was no cap here at all, while discover.go,
// you_might_like.go and home.go all had one. That asymmetry is what let a
// radio session come back entirely from a single artist: nothing between
// the pool and the output bounded any artist's share, so a pool dominated
// by one artist produced an output dominated by it too.
func Shuffle(
candidates []Candidate,
weights ScoringWeights,
now time.Time,
rng func() float64,
limit int,
caps DiversityCaps,
) []Candidate {
if len(candidates) == 0 || limit <= 0 {
return nil
}
scored := make([]struct {
c Candidate
score float64
}, len(candidates))
for i, c := range candidates {
scored[i].c = c
scored[i].score = Score(c.Inputs, weights, now, rng)
}
sort.Slice(scored, func(i, j int) bool {
return scored[i].score > scored[j].score
})
if limit > len(scored) {
limit = len(scored)
}
out := make([]Candidate, 0, limit)
deferred := make([]Candidate, 0, len(scored)-limit)
artistCount := map[pgtype.UUID]int{}
albumCount := map[pgtype.UUID]int{}
for _, s := range scored {
if len(out) == limit {
break
}
overArtist := caps.MaxPerArtist > 0 && artistCount[s.c.Track.ArtistID] >= caps.MaxPerArtist
overAlbum := caps.MaxPerAlbum > 0 && albumCount[s.c.Track.AlbumID] >= caps.MaxPerAlbum
if overArtist || overAlbum {
// Held back, not discarded — pass two may still need it.
deferred = append(deferred, s.c)
continue
}
artistCount[s.c.Track.ArtistID]++
albumCount[s.c.Track.AlbumID]++
out = append(out, s.c)
}
// Pass two: the caps could not fill the request, so relax them rather
// than hand back a short radio. Still score order, so the best of the
// held-back candidates go first.
for _, c := range deferred {
if len(out) == limit {
break
}
out = append(out, c)
}
return out
}