Compare commits
16
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
f70df9f827 | ||
|
|
439c8625d5 | ||
|
|
1d67c160b2 | ||
|
|
237380b122 | ||
|
|
4f077736b6 | ||
|
|
727f68950e | ||
|
|
aa9f534f3c | ||
|
|
011b4d9a9c | ||
|
|
d5aa081157 | ||
|
|
a99f855e98 | ||
|
|
7e4727fc49 | ||
|
|
1b7fa635d8 | ||
|
|
57d2299180 | ||
|
|
fa7ea41ccf | ||
|
|
324059b2bd | ||
|
|
1138d75a45 |
@@ -101,10 +101,6 @@ func (h *handlers) handleRadio(w http.ResponseWriter, r *http.Request) {
|
||||
candidates, err := recommendation.LoadCandidatesFromSimilarity(
|
||||
r.Context(), q, user.ID, seedID,
|
||||
h.recCfg.RecentlyPlayedHours, currentVec, exclude, limits,
|
||||
// A fresh seed per request (#3889): radio is a new session each time
|
||||
// and SHOULD draw differently. The system mixes are the surfaces that
|
||||
// promise repeatability; this is not one of them.
|
||||
strconv.FormatInt(time.Now().UnixNano(), 36),
|
||||
)
|
||||
if err != nil {
|
||||
h.logger.Warn("api: radio: similarity-pool failed; falling back to whole-library", "err", err)
|
||||
|
||||
@@ -829,7 +829,7 @@ similar_artists AS (
|
||||
JOIN seed_artist sa ON asim.artist_a_id = sa.artist_id
|
||||
WHERE asim.source = 'listenbrainz'
|
||||
AND t.id NOT IN (SELECT id FROM excluded_ids)
|
||||
ORDER BY asim.score DESC, md5(t.id::text || $12::text)
|
||||
ORDER BY asim.score DESC, random()
|
||||
LIMIT $6
|
||||
),
|
||||
tag_overlap AS (
|
||||
@@ -857,7 +857,7 @@ likes_overlap AS (
|
||||
WHERE t.id = gl.track_id
|
||||
AND trim(g_overlap.g) IN (SELECT tag FROM seed_tags)
|
||||
)
|
||||
ORDER BY md5(gl.track_id::text || $12::text)
|
||||
ORDER BY random()
|
||||
LIMIT $8
|
||||
),
|
||||
taste_overlap AS (
|
||||
@@ -884,7 +884,7 @@ coplay_artists AS (
|
||||
WHERE asim.source = 'user_cooccurrence'
|
||||
AND t.id NOT IN (SELECT id FROM excluded_ids)
|
||||
AND t.id <> $2
|
||||
ORDER BY asim.score DESC, md5(t.id::text || $12::text)
|
||||
ORDER BY asim.score DESC, random()
|
||||
LIMIT $11
|
||||
),
|
||||
random_fill AS (
|
||||
@@ -900,7 +900,7 @@ random_fill AS (
|
||||
UNION SELECT track_id FROM taste_overlap
|
||||
UNION SELECT track_id FROM coplay_artists
|
||||
)
|
||||
ORDER BY md5(t.id::text || $12::text)
|
||||
ORDER BY random()
|
||||
LIMIT $9
|
||||
)
|
||||
SELECT
|
||||
@@ -949,7 +949,6 @@ type LoadRadioCandidatesV2Params struct {
|
||||
Limit_5 int32
|
||||
Limit_6 int32
|
||||
Limit_7 int32
|
||||
Column12 string
|
||||
}
|
||||
|
||||
type LoadRadioCandidatesV2Row struct {
|
||||
@@ -972,22 +971,8 @@ type LoadRadioCandidatesV2Row struct {
|
||||
// enter the pool even when the similarity/random arms miss them; scored
|
||||
// in Go via TasteMatch, so sim_score here is 0 pool-inclusion),
|
||||
// $11 coplay_artists K (#1533 — tracks by artists co-played across the
|
||||
// instance with the seed's artist; source='user_cooccurrence'),
|
||||
// $12 order_seed (text) — see below.
|
||||
// instance with the seed's artist; source='user_cooccurrence').
|
||||
//
|
||||
// $12 REPLACES `ORDER BY random()` IN FOUR ARMS (#3889). Those arms returned
|
||||
// a stable set only while their LIMIT exceeded the rows eligible for them: at
|
||||
// that point they returned all of them and the order stopped mattering,
|
||||
// because the caller sorts by track id before scoring. Below that threshold
|
||||
// they returned a random SUBSET, and two builds on the same day drew
|
||||
// different ones — so "daily determinism" held by accident, and only for
|
||||
// libraries smaller than the limits.
|
||||
//
|
||||
// md5(id || seed) keeps the intent — an arbitrary spread that changes when
|
||||
// the seed does — while making it reproducible for a given seed. The CALLER
|
||||
// decides what that means: system mixes pass a per-(user, day) string and get
|
||||
// the determinism they promise; radio passes a fresh value per request and
|
||||
// keeps varying, which is what a radio should do.
|
||||
// Returns same shape as LoadRadioCandidates plus similarity_score column.
|
||||
func (q *Queries) LoadRadioCandidatesV2(ctx context.Context, arg LoadRadioCandidatesV2Params) ([]LoadRadioCandidatesV2Row, error) {
|
||||
rows, err := q.db.Query(ctx, loadRadioCandidatesV2,
|
||||
@@ -1002,7 +987,6 @@ func (q *Queries) LoadRadioCandidatesV2(ctx context.Context, arg LoadRadioCandid
|
||||
arg.Limit_5,
|
||||
arg.Limit_6,
|
||||
arg.Limit_7,
|
||||
arg.Column12,
|
||||
)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
|
||||
@@ -45,22 +45,7 @@ WHERE t.id <> $2
|
||||
-- enter the pool even when the similarity/random arms miss them; scored
|
||||
-- in Go via TasteMatch, so sim_score here is 0 pool-inclusion),
|
||||
-- $11 coplay_artists K (#1533 — tracks by artists co-played across the
|
||||
-- instance with the seed's artist; source='user_cooccurrence'),
|
||||
-- $12 order_seed (text) — see below.
|
||||
--
|
||||
-- $12 REPLACES `ORDER BY random()` IN FOUR ARMS (#3889). Those arms returned
|
||||
-- a stable set only while their LIMIT exceeded the rows eligible for them: at
|
||||
-- that point they returned all of them and the order stopped mattering,
|
||||
-- because the caller sorts by track id before scoring. Below that threshold
|
||||
-- they returned a random SUBSET, and two builds on the same day drew
|
||||
-- different ones — so "daily determinism" held by accident, and only for
|
||||
-- libraries smaller than the limits.
|
||||
--
|
||||
-- md5(id || seed) keeps the intent — an arbitrary spread that changes when
|
||||
-- the seed does — while making it reproducible for a given seed. The CALLER
|
||||
-- decides what that means: system mixes pass a per-(user, day) string and get
|
||||
-- the determinism they promise; radio passes a fresh value per request and
|
||||
-- keeps varying, which is what a radio should do.
|
||||
-- instance with the seed's artist; source='user_cooccurrence').
|
||||
-- Returns same shape as LoadRadioCandidates plus similarity_score column.
|
||||
|
||||
WITH
|
||||
@@ -102,7 +87,7 @@ similar_artists AS (
|
||||
JOIN seed_artist sa ON asim.artist_a_id = sa.artist_id
|
||||
WHERE asim.source = 'listenbrainz'
|
||||
AND t.id NOT IN (SELECT id FROM excluded_ids)
|
||||
ORDER BY asim.score DESC, md5(t.id::text || $12::text)
|
||||
ORDER BY asim.score DESC, random()
|
||||
LIMIT $6
|
||||
),
|
||||
tag_overlap AS (
|
||||
@@ -130,7 +115,7 @@ likes_overlap AS (
|
||||
WHERE t.id = gl.track_id
|
||||
AND trim(g_overlap.g) IN (SELECT tag FROM seed_tags)
|
||||
)
|
||||
ORDER BY md5(gl.track_id::text || $12::text)
|
||||
ORDER BY random()
|
||||
LIMIT $8
|
||||
),
|
||||
taste_overlap AS (
|
||||
@@ -157,7 +142,7 @@ coplay_artists AS (
|
||||
WHERE asim.source = 'user_cooccurrence'
|
||||
AND t.id NOT IN (SELECT id FROM excluded_ids)
|
||||
AND t.id <> $2
|
||||
ORDER BY asim.score DESC, md5(t.id::text || $12::text)
|
||||
ORDER BY asim.score DESC, random()
|
||||
LIMIT $11
|
||||
),
|
||||
random_fill AS (
|
||||
@@ -173,7 +158,7 @@ random_fill AS (
|
||||
UNION SELECT track_id FROM taste_overlap
|
||||
UNION SELECT track_id FROM coplay_artists
|
||||
)
|
||||
ORDER BY md5(t.id::text || $12::text)
|
||||
ORDER BY random()
|
||||
LIMIT $9
|
||||
)
|
||||
SELECT
|
||||
|
||||
@@ -276,17 +276,6 @@ func SetTasteConfig(c taste.Config) {
|
||||
systemTasteConfig = c
|
||||
}
|
||||
|
||||
// dailyOrderSeed is the value the randomised candidate arms order by (#3889).
|
||||
//
|
||||
// Per (user, day) so a same-day rebuild draws the SAME set — which is what
|
||||
// TestBuildSystemPlaylists_DailyNonceDeterminism asserts and what those arms
|
||||
// only ever achieved by accident before, when their limits happened to exceed
|
||||
// the eligible rows. It changes on the day boundary, so the mixes still move
|
||||
// daily.
|
||||
func dailyOrderSeed(userID pgtype.UUID, dateStr string) string {
|
||||
return uuidStringPL(userID) + ":" + dateStr
|
||||
}
|
||||
|
||||
func currentSongsLikeWeights() recommendation.ScoringWeights {
|
||||
systemTuningMu.RLock()
|
||||
defer systemTuningMu.RUnlock()
|
||||
@@ -640,7 +629,6 @@ func produceForYou(
|
||||
zeroVec,
|
||||
seeds,
|
||||
systemForYouSourceLimits(),
|
||||
dailyOrderSeed(userID, dateStr),
|
||||
)
|
||||
if cerr != nil {
|
||||
logger.Warn("system playlist: for-you candidates load failed for seed; continuing",
|
||||
@@ -728,7 +716,6 @@ func produceSeedMixes(
|
||||
recommendation.ScaleForLibrary(
|
||||
recommendation.SongsLikeCandidateSourceLimits(), librarySize,
|
||||
),
|
||||
dailyOrderSeed(userID, dateStr),
|
||||
)
|
||||
if cerr != nil {
|
||||
logger.Warn("system playlist: seed candidates load failed; skipping",
|
||||
|
||||
@@ -102,7 +102,6 @@ func buildYouMightLike(
|
||||
cands, err := recommendation.LoadCandidatesFromSimilarity(
|
||||
ctx, q, userID, seed, 1, zeroVec,
|
||||
[]pgtype.UUID{seed}, ymlLimits,
|
||||
dailyOrderSeed(userID, dateStr),
|
||||
)
|
||||
if err != nil {
|
||||
logger.Warn("you-might-like: candidate load failed; skipping",
|
||||
|
||||
@@ -139,41 +139,46 @@ func DefaultCandidateSourceLimits() CandidateSourceLimits {
|
||||
// would produce a short mix or none at all, and "no playlist" is a worse
|
||||
// answer than "a few tracks further from the seed than we would like".
|
||||
//
|
||||
// The seed-independent arms are trimmed hardest, because on this surface they
|
||||
// are noise: `taste_overlap` (tracks by the user's top taste artists) and
|
||||
// `random_fill` (any track not already in the pool) both carry
|
||||
// `0.0::float8 AS sim_score`, so nearly a third of the default pool had no
|
||||
// relationship to the seed at all.
|
||||
// DO NOT SHRINK AN ARM ORDERED BY UNSEEDED random(). This is the constraint
|
||||
// that shapes the numbers below, and it is not obvious from reading them.
|
||||
//
|
||||
// THESE TRIMS WERE BLOCKED UNTIL #3889. `likes_overlap` and `random_fill`
|
||||
// used to end in a bare `ORDER BY random()`, which made their output a stable
|
||||
// SET only while the limit exceeded the eligible rows — so SHRINKING them
|
||||
// changed pool membership between same-day rebuilds and broke daily
|
||||
// determinism. Those arms now order by md5(id || seed), so a smaller limit
|
||||
// takes a smaller but REPRODUCIBLE slice, and the trim is safe.
|
||||
// `likes_overlap` and `random_fill` both end in a bare `ORDER BY random()`
|
||||
// (recommendation.sql:118, :161) with no daily seed. Such an arm returns a
|
||||
// STABLE set only while its LIMIT exceeds the rows eligible for it — at that
|
||||
// point it returns all of them and the random order is irrelevant, because
|
||||
// the caller sorts by id before scoring. Drop the limit below the eligible
|
||||
// count and the arm starts returning a random SUBSET, which differs between
|
||||
// two builds on the same day.
|
||||
//
|
||||
// Reduced, never removed. Rule 131: the two seed-independent arms are the
|
||||
// tier-3 floor, and zeroing them would leave a seed with thin ListenBrainz
|
||||
// coverage producing a short mix or none at all. The weights (SimilarityWeight
|
||||
// 4.0, everything seed-independent demoted) keep them ranked last, so they
|
||||
// surface only when the closer tiers cannot fill the mix.
|
||||
// That is a real defect (#3889) rather than a quirk of this function, and it
|
||||
// bit here: cutting RandomFill to 10 broke
|
||||
// TestBuildSystemPlaylists_DailyNonceDeterminism, whose library is smaller
|
||||
// than the default limit and whose determinism was therefore accidental.
|
||||
// Growing an arm is always safe; only shrinking one is.
|
||||
//
|
||||
// likes_overlap is cut hardest of the tier-2 arms for a specific reason: its
|
||||
// SQL assigns a FLAT 0.6 sim_score (recommendation.sql) rather than measuring
|
||||
// anything. It is a collaborative signal wearing similarity's clothes, and a
|
||||
// raised SimilarityWeight amplifies it — if real ListenBrainz scores commonly
|
||||
// land below 0.6 it would outrank genuine matches. Halved pending the
|
||||
// fill-rate measurement in #3879; the honest fix is to stop it claiming a
|
||||
// similarity score it never computed.
|
||||
// So the seed-independent arms are trimmed only where the ordering is
|
||||
// deterministic: `taste_overlap` sorts by `tpa.weight DESC, t.id` and can be
|
||||
// cut, `random_fill` cannot. The reduction is consequently modest — and it
|
||||
// matters less than it looks, because the WEIGHTS are what demote sim_score-0
|
||||
// candidates now. The pool change biases the draw; the songs_like profile is
|
||||
// what actually keeps unrelated tracks out of the result.
|
||||
//
|
||||
// One arm is left alone that arguably should not be: `likes_overlap` assigns
|
||||
// a FLAT 0.6 sim_score (recommendation.sql:108) rather than measuring
|
||||
// anything — a collaborative signal wearing similarity's clothes, which a
|
||||
// raised SimilarityWeight amplifies. If real ListenBrainz scores commonly
|
||||
// land below 0.6 it will outrank genuine matches. It cannot be trimmed here
|
||||
// without the determinism fix landing first; the honest repair is to stop it
|
||||
// claiming a similarity score it never computed (#3879).
|
||||
func SongsLikeCandidateSourceLimits() CandidateSourceLimits {
|
||||
return CandidateSourceLimits{
|
||||
LBSimilar: 60, // tier 1 — doubled; the only arm that measures the seed
|
||||
SimilarArtist: 40, // tier 2 — raised; growing is always safe
|
||||
TagOverlap: 20, // tier 2
|
||||
UserCoplay: 20, // tier 2
|
||||
LikesOverlap: 10, // tier 2, halved — flat 0.6 sim_score, see above
|
||||
TasteOverlap: 10, // tier 3 floor — halved, not removed
|
||||
RandomFill: 10, // tier 3 floor — cut hard, never to zero
|
||||
LikesOverlap: 20, // tier 2 — NOT trimmed: unseeded random(), see above
|
||||
TasteOverlap: 10, // tier 3 floor — halved; deterministic ordering, safe
|
||||
RandomFill: 30, // tier 3 floor — NOT trimmed: unseeded random(), see above
|
||||
}
|
||||
}
|
||||
|
||||
@@ -182,10 +187,6 @@ func SongsLikeCandidateSourceLimits() CandidateSourceLimits {
|
||||
// likes-overlap / random fill) + dedup-by-max sim_score. Returns
|
||||
// []Candidate (same shape as LoadCandidates) so Shuffle is unchanged.
|
||||
//
|
||||
// orderSeed decides whether the randomised arms repeat their draw — see
|
||||
// Column12 below and #3889. Pass a stable per-(user, day) value where the
|
||||
// selection must be reproducible, and a varying one where it should not be.
|
||||
//
|
||||
// Caller (radio handler) falls back to LoadCandidates on error.
|
||||
func LoadCandidatesFromSimilarity(
|
||||
ctx context.Context,
|
||||
@@ -195,7 +196,6 @@ func LoadCandidatesFromSimilarity(
|
||||
currentVector SessionVector,
|
||||
exclude []pgtype.UUID,
|
||||
limits CandidateSourceLimits,
|
||||
orderSeed string,
|
||||
) ([]Candidate, error) {
|
||||
if exclude == nil {
|
||||
exclude = []pgtype.UUID{}
|
||||
@@ -212,12 +212,6 @@ func LoadCandidatesFromSimilarity(
|
||||
Limit_5: int32(limits.RandomFill),
|
||||
Limit_6: int32(limits.TasteOverlap),
|
||||
Limit_7: int32(limits.UserCoplay),
|
||||
// #3889. Four arms used to end in a bare ORDER BY random(), which made
|
||||
// their output a stable SET only while the limit exceeded the eligible
|
||||
// rows. They now order by md5(id || this), so the caller decides
|
||||
// whether the draw repeats: a per-(user, day) seed for the system
|
||||
// mixes that promise daily determinism, a fresh one per radio request.
|
||||
Column12: orderSeed,
|
||||
})
|
||||
if err != nil {
|
||||
return nil, err
|
||||
|
||||
@@ -2,10 +2,6 @@ package recommendation
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"reflect"
|
||||
"sort"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
@@ -54,7 +50,7 @@ func TestLoadCandidatesFromSimilarity_LBSimilarSourceContributes(t *testing.T) {
|
||||
target := f.tracks[1]
|
||||
helperLBSimilarity(t, f, seed.ID, target.ID, 0.85)
|
||||
got, err := LoadCandidatesFromSimilarity(
|
||||
context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(), "test-seed",
|
||||
context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("load: %v", err)
|
||||
@@ -86,7 +82,7 @@ func TestLoadCandidatesFromSimilarity_SimilarArtistTracksContribute(t *testing.T
|
||||
})
|
||||
helperArtistSimilarity(t, f, seed.ArtistID, otherArtist.ID, 0.8)
|
||||
got, err := LoadCandidatesFromSimilarity(
|
||||
context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(), "test-seed",
|
||||
context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("load: %v", err)
|
||||
@@ -110,7 +106,7 @@ func TestLoadCandidatesFromSimilarity_TagOverlapContributes(t *testing.T) {
|
||||
helperSetTrackGenre(t, f, seed.ID, "Rock; Pop")
|
||||
helperSetTrackGenre(t, f, target.ID, "Rock")
|
||||
got, err := LoadCandidatesFromSimilarity(
|
||||
context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(), "test-seed",
|
||||
context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("load: %v", err)
|
||||
@@ -137,7 +133,7 @@ func TestLoadCandidatesFromSimilarity_LikesOverlapContributes(t *testing.T) {
|
||||
t.Fatalf("like: %v", err)
|
||||
}
|
||||
got, err := LoadCandidatesFromSimilarity(
|
||||
context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(), "test-seed",
|
||||
context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("load: %v", err)
|
||||
@@ -159,7 +155,7 @@ func TestLoadCandidatesFromSimilarity_RandomFillReturnsTracks(t *testing.T) {
|
||||
f := newFixture(t, 10) // 10 tracks; no similarity data
|
||||
seed := f.tracks[0]
|
||||
got, err := LoadCandidatesFromSimilarity(
|
||||
context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(), "test-seed",
|
||||
context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("load: %v", err)
|
||||
@@ -180,7 +176,7 @@ func TestLoadCandidatesFromSimilarity_ExcludeListRespected(t *testing.T) {
|
||||
excluded := f.tracks[1].ID
|
||||
got, err := LoadCandidatesFromSimilarity(
|
||||
context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true},
|
||||
[]pgtype.UUID{excluded}, defaultLimits(), "test-seed",
|
||||
[]pgtype.UUID{excluded}, defaultLimits(),
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("load: %v", err)
|
||||
@@ -196,7 +192,7 @@ func TestLoadCandidatesFromSimilarity_SeedAlwaysExcluded(t *testing.T) {
|
||||
f := newFixture(t, 5)
|
||||
seed := f.tracks[0]
|
||||
got, err := LoadCandidatesFromSimilarity(
|
||||
context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(), "test-seed",
|
||||
context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("load: %v", err)
|
||||
@@ -226,7 +222,7 @@ func TestLoadCandidatesFromSimilarity_RecentlyPlayedExcluded(t *testing.T) {
|
||||
t.Fatalf("play_event: %v", err)
|
||||
}
|
||||
got, err := LoadCandidatesFromSimilarity(
|
||||
context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(), "test-seed",
|
||||
context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("load: %v", err)
|
||||
@@ -246,7 +242,7 @@ func TestLoadCandidatesFromSimilarity_DedupTakesMaxScore(t *testing.T) {
|
||||
helperSetTrackGenre(t, f, target.ID, "Rock") // jaccard 1/1 = 1.0 from tag-overlap
|
||||
helperLBSimilarity(t, f, seed.ID, target.ID, 0.5) // weaker LB signal
|
||||
got, err := LoadCandidatesFromSimilarity(
|
||||
context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(), "test-seed",
|
||||
context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("load: %v", err)
|
||||
@@ -299,7 +295,7 @@ func TestLoadCandidatesFromSimilarity_TasteOverlapArm(t *testing.T) {
|
||||
// Only the taste_overlap arm is enabled.
|
||||
limits := CandidateSourceLimits{TasteOverlap: 10}
|
||||
got, err := LoadCandidatesFromSimilarity(
|
||||
ctx, f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, limits, "test-seed",
|
||||
ctx, f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, limits,
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("load: %v", err)
|
||||
@@ -325,7 +321,7 @@ func TestLoadCandidatesFromSimilarity_EmptyLibrary_NoError(t *testing.T) {
|
||||
f := newFixture(t, 1) // just the seed
|
||||
seed := f.tracks[0]
|
||||
got, err := LoadCandidatesFromSimilarity(
|
||||
context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(), "test-seed",
|
||||
context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, defaultLimits(),
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("load: %v", err)
|
||||
@@ -335,97 +331,3 @@ func TestLoadCandidatesFromSimilarity_EmptyLibrary_NoError(t *testing.T) {
|
||||
t.Errorf("got %d candidates from seed-only library, want 0", len(got))
|
||||
}
|
||||
}
|
||||
|
||||
// The randomised arms must draw REPRODUCIBLY for a given seed (#3889).
|
||||
//
|
||||
// Four arms used to end in a bare `ORDER BY random()`. That returned a stable
|
||||
// set only while the arm's LIMIT exceeded the rows eligible for it — at that
|
||||
// point it returned all of them and the order stopped mattering, because the
|
||||
// caller sorts by track id before scoring. Below that threshold it returned a
|
||||
// random SUBSET, so two calls drew different candidates.
|
||||
//
|
||||
// It therefore held by ACCIDENT, and only for libraries smaller than the
|
||||
// limits. Any real library is larger, so same-day rebuilds had been drawing
|
||||
// different mixes since the arm was written — invisible, because a mix that
|
||||
// changes after a refresh looks like a feature.
|
||||
//
|
||||
// Limits deliberately smaller than the fixture, because that is the only
|
||||
// regime where the bug existed at all: with limits above the eligible count
|
||||
// the old code passes this too.
|
||||
func TestLoadCandidatesFromSimilarity_SameSeedDrawsTheSameSet(t *testing.T) {
|
||||
f := newFixture(t, 12)
|
||||
seed := f.tracks[0]
|
||||
|
||||
tight := CandidateSourceLimits{
|
||||
LBSimilar: 2, SimilarArtist: 2, TagOverlap: 2,
|
||||
LikesOverlap: 2, RandomFill: 3, TasteOverlap: 2, UserCoplay: 2,
|
||||
}
|
||||
ids := func(cs []Candidate) []string {
|
||||
out := make([]string, 0, len(cs))
|
||||
for _, c := range cs {
|
||||
out = append(out, fmt.Sprintf("%x", c.Track.ID.Bytes))
|
||||
}
|
||||
sort.Strings(out) // membership, not order — order is settled downstream
|
||||
return out
|
||||
}
|
||||
|
||||
first, err := LoadCandidatesFromSimilarity(
|
||||
context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, tight, "day-one",
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("load: %v", err)
|
||||
}
|
||||
if len(first) == 0 {
|
||||
t.Fatal("no candidates, so this test asserts nothing")
|
||||
}
|
||||
|
||||
for i := 0; i < 3; i++ {
|
||||
again, err := LoadCandidatesFromSimilarity(
|
||||
context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, tight, "day-one",
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("load %d: %v", i, err)
|
||||
}
|
||||
if !reflect.DeepEqual(ids(first), ids(again)) {
|
||||
t.Fatalf("same seed drew a different set on call %d:\n first %v\n again %v",
|
||||
i, ids(first), ids(again))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ...and a different seed is free to draw differently, or the ordering would
|
||||
// be fixed rather than seeded and every day would serve the same mix.
|
||||
//
|
||||
// Asserted as "not pinned to one answer" rather than "always differs": with a
|
||||
// small fixture two seeds can legitimately collide, so requiring a difference
|
||||
// on any single pair would be flaky. Several seeds producing exactly one
|
||||
// distinct set is the real regression — that is what a constant ORDER BY
|
||||
// looks like.
|
||||
func TestLoadCandidatesFromSimilarity_DifferentSeedsCanDrawDifferently(t *testing.T) {
|
||||
f := newFixture(t, 12)
|
||||
seed := f.tracks[0]
|
||||
tight := CandidateSourceLimits{
|
||||
LBSimilar: 2, SimilarArtist: 2, TagOverlap: 2,
|
||||
LikesOverlap: 2, RandomFill: 3, TasteOverlap: 2, UserCoplay: 2,
|
||||
}
|
||||
|
||||
seen := map[string]bool{}
|
||||
for _, orderSeed := range []string{"a", "b", "c", "d", "e", "f"} {
|
||||
cs, err := LoadCandidatesFromSimilarity(
|
||||
context.Background(), f.q, f.user, seed.ID, 1, SessionVector{Seed: true}, nil, tight, orderSeed,
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("load %q: %v", orderSeed, err)
|
||||
}
|
||||
ids := make([]string, 0, len(cs))
|
||||
for _, c := range cs {
|
||||
ids = append(ids, fmt.Sprintf("%x", c.Track.ID.Bytes))
|
||||
}
|
||||
sort.Strings(ids)
|
||||
seen[strings.Join(ids, ",")] = true
|
||||
}
|
||||
if len(seen) < 2 {
|
||||
t.Errorf("six different seeds produced %d distinct set(s); the ordering is not "+
|
||||
"varying with the seed at all", len(seen))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -59,3 +59,41 @@ func TestSongsLikeLimits_KeepThePoolRoughlyTheSameSize(t *testing.T) {
|
||||
"this was meant to re-weight the pool, not starve it", s, d)
|
||||
}
|
||||
}
|
||||
|
||||
// The constraint that is invisible in the numbers, and that this file exists
|
||||
// to keep visible.
|
||||
//
|
||||
// `likes_overlap` and `random_fill` end in a bare `ORDER BY random()` with no
|
||||
// daily seed (recommendation.sql:118, :161). Such an arm returns a stable set
|
||||
// only while its LIMIT exceeds the eligible rows; below that it returns a
|
||||
// random SUBSET that differs between two builds on the same day, and the
|
||||
// daily-determinism promise quietly stops holding.
|
||||
//
|
||||
// This is not hypothetical — it is how this change first failed CI. Cutting
|
||||
// RandomFill to 10 broke TestBuildSystemPlaylists_DailyNonceDeterminism,
|
||||
// whose library is smaller than the default limit and whose determinism was
|
||||
// therefore an accident of the limit exceeding the library.
|
||||
//
|
||||
// Growing these arms is always safe. Only shrinking is, and the fix that
|
||||
// would make shrinking safe is a seeded ordering (#3889), not a smaller
|
||||
// number here.
|
||||
func TestSongsLikeLimits_DoNotShrinkTheUnseededRandomArms(t *testing.T) {
|
||||
d := DefaultCandidateSourceLimits()
|
||||
s := SongsLikeCandidateSourceLimits()
|
||||
|
||||
for _, tc := range []struct {
|
||||
arm string
|
||||
songsLike, dflt int
|
||||
}{
|
||||
{"RandomFill", s.RandomFill, d.RandomFill},
|
||||
{"LikesOverlap", s.LikesOverlap, d.LikesOverlap},
|
||||
} {
|
||||
if tc.songsLike < tc.dflt {
|
||||
t.Errorf("%s cut from %d to %d. That arm is ordered by unseeded random(), "+
|
||||
"so a smaller limit makes pool membership vary between same-day "+
|
||||
"rebuilds — it breaks daily determinism rather than merely narrowing "+
|
||||
"the mix. Fix the ordering (#3889) before trimming this.",
|
||||
tc.arm, tc.dflt, tc.songsLike)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user