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
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
135 lines
4.3 KiB
Go
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
|
|
}
|