Files
minstrel/internal/playlists/system_test.go
T
bvandeusenandClaude Opus 5 31190657d8
test-go / test (push) Successful in 1m16s
test-go / integration (push) Failing after 3m55s
release / Build signed APK (releases and dev) (push) Successful in 5m11s
release / Build + push container image (push) Successful in 16s
release / Verify release artifacts (tag releases only) (push) Skipped
feat(recommendation): Songs-like can include the seed artist's own music
Operator, 2026-09-10: "it should also be able to include music from the same
artist." Completes #3881 — the weights and pool landed in f367eeaa; this is
the eligibility half.

produceSeedMixes filtered the seed artist out entirely:

    // "Songs like X" excludes X's own songs.
    if !pgtypeUUIDEqual(c.Track.ArtistID, artistID) { ... }

That reads as obviously right and is not. The seed is a TRACK — the artist's
top-played one — and the tracks most likely to sound like it are usually the
rest of that artist's catalogue. The filter threw away the seed's nearest
neighbours, then reached FURTHER OUT to replace them. On the one surface
whose job is staying in a neighbourhood, that is backwards, and it worked
against the coherence tuning rather than with it.

Domination is bounded by the cap instead of by exclusion, which is the
distinction that makes this safe rather than a new problem:
capCandidatesByAlbumAndArtist already allows at most 3 tracks per artist in
a 25-track mix, so the seed artist gets 12% at most — a presence, not a
takeover. Without that bound this would just be the radio failure (#3882)
arriving on a different surface. The seed track itself still cannot appear;
it is passed to LoadCandidatesFromSimilarity as an exclusion.

Guarded end-to-end rather than by reading the source, for two reasons: the
check has to survive the filter returning in a different shape, and an
absence check would now match the comment that explains why the filter is
gone — rule 167's prose trap exactly. The test asserts both directions, that
at least one mix contains its seed artist and that none exceeds the cap.

Its falsification is by construction rather than by execution: under the
previous code every mix's own-artist count was necessarily zero, so the
assertion could not have passed. Running it needs Postgres, which is the
integration lane's job.

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

533 lines
17 KiB
Go

package playlists_test
import (
"context"
"io"
"log/slog"
"sync"
"testing"
"time"
"github.com/jackc/pgx/v5/pgtype"
"github.com/jackc/pgx/v5/pgxpool"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
"git.fabledsword.com/bvandeusen/minstrel/internal/playlists"
)
func discardLogger() *slog.Logger {
return slog.New(slog.NewTextHandler(io.Discard, nil))
}
// seedPlayEvent inserts a play_events row with the given was_skipped flag,
// at startedAt, for the given user/track. Uses raw SQL because the sqlc
// `InsertPlayEvent` doesn't accept was_skipped (that's set by an UPDATE).
func seedPlayEvent(t *testing.T, pool *pgxpool.Pool, userID, trackID pgtype.UUID, startedAt time.Time, wasSkipped bool) {
t.Helper()
// play_events.session_id has a FK to play_sessions; create a parent
// session in the same statement (a random UUID violates the FK).
_, err := pool.Exec(context.Background(), `
WITH s AS (
INSERT INTO play_sessions (user_id, started_at, last_event_at)
VALUES ($1, $3, $3)
RETURNING id
)
INSERT INTO play_events (user_id, track_id, session_id, started_at, was_skipped)
SELECT $1, $2, s.id, $3, $4 FROM s
`, userID, trackID, startedAt, wasSkipped)
if err != nil {
t.Fatalf("seed play_event: %v", err)
}
}
// seedQuarantine inserts a lidarr_quarantine row for (user, track).
func seedQuarantine(t *testing.T, pool *pgxpool.Pool, userID, trackID pgtype.UUID) {
t.Helper()
_, err := pool.Exec(context.Background(), `
INSERT INTO lidarr_quarantine (user_id, track_id, reason)
VALUES ($1, $2, 'other')
`, userID, trackID)
if err != nil {
t.Fatalf("seed quarantine: %v", err)
}
}
// seedActiveLibrary creates a user with N played tracks across M artists,
// all within the last 7 days. Returns the user and the slice of tracks.
// Each track gets ≥3 unskipped plays so PickSeedArtists has a real signal.
func seedActiveLibrary(t *testing.T, pool *pgxpool.Pool, name string, numArtists, tracksPerArtist int) (dbq.User, []dbq.Track) {
t.Helper()
u := seedUser(t, pool, name)
now := time.Now().UTC()
var allTracks []dbq.Track
for a := 0; a < numArtists; a++ {
artistName := name + "-artist-" + string(rune('A'+a))
for k := 0; k < tracksPerArtist; k++ {
tk := seedTrack(t, pool, name+"-track-"+string(rune('A'+a))+string(rune('0'+k)), artistName)
allTracks = append(allTracks, tk)
// 3 unskipped plays per track to populate engagement stats.
for p := 0; p < 3; p++ {
seedPlayEvent(t, pool, u.ID, tk.ID, now.Add(-time.Duration(a*10+k+p)*time.Hour), false)
}
}
}
return u, allTracks
}
func TestBuildSystemPlaylists_SufficientActivity(t *testing.T) {
pool := newPool(t)
logger := discardLogger()
u, _ := seedActiveLibrary(t, pool, "act1", 4, 3)
if err := playlists.BuildSystemPlaylists(context.Background(), pool, logger, u.ID, time.Now().UTC(), t.TempDir()); err != nil {
t.Fatalf("build: %v", err)
}
rows, err := dbq.New(pool).ListPlaylistsByUserAndKind(context.Background(), dbq.ListPlaylistsByUserAndKindParams{
UserID: u.ID, Column2: "system",
})
if err != nil {
t.Fatalf("list: %v", err)
}
if len(rows) == 0 {
t.Fatalf("expected at least one system playlist; got 0")
}
var hasForYou, hasSongsLike bool
for _, r := range rows {
if r.Kind != "system" {
t.Errorf("row %s: kind=%q want 'system'", uuidString(r.ID), r.Kind)
}
if r.SystemVariant == nil {
t.Errorf("row %s: nil system_variant", uuidString(r.ID))
continue
}
switch *r.SystemVariant {
case "for_you":
hasForYou = true
if r.SeedArtistID.Valid {
t.Errorf("for_you row should have NULL seed_artist_id; got valid")
}
case "songs_like_artist":
hasSongsLike = true
if !r.SeedArtistID.Valid {
t.Errorf("songs_like_artist row should have non-NULL seed_artist_id")
}
case "discover", "deep_cuts", "rediscover", "new_for_you", "on_this_day", "first_listens":
// Discover + the #411 discovery mixes are all seedless —
// no seed_artist_id required.
default:
t.Errorf("unknown system_variant=%q", *r.SystemVariant)
}
if r.TrackCount == 0 {
t.Errorf("row %s: empty track_count", uuidString(r.ID))
}
// For-You / Discover / the discovery mixes are sized to ~100
// (#352/#411); songs_like_artist stays at systemMixLength (25).
if r.TrackCount > 100 {
t.Errorf("row %s: track_count=%d exceeds 100", uuidString(r.ID), r.TrackCount)
}
}
if !hasForYou {
t.Error("expected a for_you playlist")
}
if !hasSongsLike {
t.Error("expected at least one songs_like_artist playlist")
}
}
func TestBuildSystemPlaylists_StaleActivity_SongsLikeSurvives(t *testing.T) {
// #1255: the seed-artist query's old hard 7-day window emptied the
// pool after a quiet week, and the atomic-replace build then deleted
// every "Songs like X" mix. With the tiered fallback, plays that are
// ~20 days old (outside 7d, inside 30d) must still seed the mixes —
// and the built tracks carry the tier2 stamp so metrics can compare
// stale-seeded mixes against fresh ones.
pool := newPool(t)
logger := discardLogger()
u := seedUser(t, pool, "stale1")
old := time.Now().UTC().Add(-20 * 24 * time.Hour)
for a := 0; a < 4; a++ {
for k := 0; k < 3; k++ {
tk := seedTrack(t, pool,
"stale1-track-"+string(rune('A'+a))+string(rune('0'+k)),
"stale1-artist-"+string(rune('A'+a)))
for p := 0; p < 3; p++ {
seedPlayEvent(t, pool, u.ID, tk.ID,
old.Add(-time.Duration(a*10+k+p)*time.Hour), false)
}
}
}
if err := playlists.BuildSystemPlaylists(context.Background(), pool, logger, u.ID, time.Now().UTC(), t.TempDir()); err != nil {
t.Fatalf("build: %v", err)
}
rows, err := pool.Query(context.Background(), `
SELECT DISTINCT COALESCE(pt.pick_kind, '<null>')
FROM playlist_tracks pt
JOIN playlists p ON p.id = pt.playlist_id
WHERE p.user_id = $1 AND p.system_variant = 'songs_like_artist'
`, u.ID)
if err != nil {
t.Fatalf("query pick_kinds: %v", err)
}
defer rows.Close()
var kinds []string
for rows.Next() {
var k string
if err := rows.Scan(&k); err != nil {
t.Fatalf("scan: %v", err)
}
kinds = append(kinds, k)
}
if len(kinds) == 0 {
t.Fatal("expected songs_like_artist tracks from the 30-day fallback tier; got none (the vanish bug)")
}
if len(kinds) != 1 || kinds[0] != "tier2" {
t.Errorf("pick_kinds = %v, want exactly [tier2] (30-day fallback seeds)", kinds)
}
}
func TestBuildSystemPlaylists_QuarantineExcluded(t *testing.T) {
pool := newPool(t)
logger := discardLogger()
u, tracks := seedActiveLibrary(t, pool, "qact", 3, 3)
quarantined := tracks[0]
seedQuarantine(t, pool, u.ID, quarantined.ID)
if err := playlists.BuildSystemPlaylists(context.Background(), pool, logger, u.ID, time.Now().UTC(), t.TempDir()); err != nil {
t.Fatalf("build: %v", err)
}
// Inspect playlist_tracks for the user's system playlists; quarantined
// track must not appear.
rows, err := pool.Query(context.Background(), `
SELECT pt.track_id
FROM playlist_tracks pt
JOIN playlists p ON p.id = pt.playlist_id
WHERE p.user_id = $1 AND p.kind = 'system'
`, u.ID)
if err != nil {
t.Fatalf("query playlist_tracks: %v", err)
}
defer rows.Close()
for rows.Next() {
var tid pgtype.UUID
if err := rows.Scan(&tid); err != nil {
t.Fatalf("scan: %v", err)
}
if tid.Bytes == quarantined.ID.Bytes {
t.Fatalf("quarantined track %s appeared in a system playlist", uuidString(quarantined.ID))
}
}
}
func TestBuildSystemPlaylists_AtomicReplace(t *testing.T) {
pool := newPool(t)
logger := discardLogger()
u, _ := seedActiveLibrary(t, pool, "atomic", 3, 3)
ctx := context.Background()
// First build.
if err := playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, time.Now().UTC(), t.TempDir()); err != nil {
t.Fatalf("first build: %v", err)
}
var ids1 []pgtype.UUID
rows1, _ := dbq.New(pool).ListPlaylistsByUserAndKind(ctx, dbq.ListPlaylistsByUserAndKindParams{
UserID: u.ID, Column2: "system",
})
for _, r := range rows1 {
ids1 = append(ids1, r.ID)
}
if len(ids1) == 0 {
t.Skip("no system playlists from first build; can't verify replace")
}
// Second build — old rows must be gone, replaced with fresh ones.
if err := playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, time.Now().UTC(), t.TempDir()); err != nil {
t.Fatalf("second build: %v", err)
}
rows2, _ := dbq.New(pool).ListPlaylistsByUserAndKind(ctx, dbq.ListPlaylistsByUserAndKindParams{
UserID: u.ID, Column2: "system",
})
for _, r := range rows2 {
for _, oldID := range ids1 {
if r.ID.Bytes == oldID.Bytes {
t.Errorf("playlist id %s persisted across rebuilds — atomic replace failed", uuidString(oldID))
}
}
}
}
func TestBuildSystemPlaylists_Concurrency(t *testing.T) {
pool := newPool(t)
logger := discardLogger()
u, _ := seedActiveLibrary(t, pool, "conc", 3, 3)
ctx := context.Background()
now := time.Now().UTC()
var wg sync.WaitGroup
wg.Add(2)
dataDir := t.TempDir()
go func() { defer wg.Done(); _ = playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, now, dataDir) }()
go func() { defer wg.Done(); _ = playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, now, dataDir) }()
wg.Wait()
// Exactly one run must have completed; in_flight must be false either way.
run, err := dbq.New(pool).GetSystemPlaylistRun(ctx, u.ID)
if err != nil {
t.Fatalf("get run: %v", err)
}
if run.InFlight {
t.Errorf("in_flight should be false after both calls return")
}
if !run.LastRunAt.Valid {
t.Errorf("last_run_at should be set after at least one successful build")
}
}
// The seed artist's own tracks are ELIGIBLE for its "Songs like" mix, and are
// bounded by the diversity cap rather than excluded outright (#3881).
//
// produceSeedMixes used to filter them out — "Songs like X excludes X's own
// songs" — which threw away the nearest neighbours of the seed TRACK and then
// reached further out to replace them. Operator, 2026-09-10: "it should also
// be able to include music from the same artist."
//
// Asserted end-to-end rather than by reading the source, because the guard has
// to survive the filter coming back in a different shape — and because an
// absence check would now match the comment explaining why the filter is gone.
func TestBuildSystemPlaylists_SongsLikeIncludesItsSeedArtist(t *testing.T) {
pool := newPool(t)
logger := discardLogger()
u, _ := seedActiveLibrary(t, pool, "seedartist", 4, 5)
ctx := context.Background()
now := time.Date(2026, 5, 4, 12, 0, 0, 0, time.UTC)
if err := playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, now, t.TempDir()); err != nil {
t.Fatalf("build: %v", err)
}
rows, err := pool.Query(ctx, `
SELECT count(*) FILTER (WHERE t.artist_id = p.seed_artist_id) AS own,
count(*) AS total
FROM playlists p
JOIN playlist_tracks pt ON pt.playlist_id = p.id
JOIN tracks t ON t.id = pt.track_id
WHERE p.user_id = $1 AND p.system_variant = 'songs_like_artist'
GROUP BY p.id
`, u.ID)
if err != nil {
t.Fatalf("query: %v", err)
}
defer rows.Close()
// discoverMaxTracksPerArtist, which this package_test cannot reference.
// Duplicated deliberately: if the cap moves, this failing is the point.
const maxPerArtist = 3
mixes, withOwn := 0, 0
for rows.Next() {
var own, total int
if err := rows.Scan(&own, &total); err != nil {
t.Fatalf("scan: %v", err)
}
mixes++
if own > 0 {
withOwn++
}
// The bound is what makes inclusion safe. Without it, "include the
// seed artist" becomes "the mix is mostly the seed artist", which is
// the radio failure (#3882) arriving on a different surface.
if own > maxPerArtist {
t.Errorf("a songs_like mix carries %d tracks by its own seed artist "+
"out of %d; the per-artist cap (%d) is not being applied",
own, total, maxPerArtist)
}
}
if err := rows.Err(); err != nil {
t.Fatalf("rows: %v", err)
}
if mixes == 0 {
t.Fatal("no songs_like_artist mixes were built, so this test asserts nothing")
}
if withOwn == 0 {
t.Errorf("none of the %d songs_like mixes contains a single track by its own "+
"seed artist — the tracks most likely to sound like the seed are being "+
"excluded from the surface whose job is sounding like the seed", mixes)
}
}
func TestBuildSystemPlaylists_DailyNonceDeterminism(t *testing.T) {
pool := newPool(t)
logger := discardLogger()
u, _ := seedActiveLibrary(t, pool, "nonce", 4, 5)
ctx := context.Background()
day1 := time.Date(2026, 5, 4, 12, 0, 0, 0, time.UTC)
dataDir := t.TempDir()
if err := playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, day1, dataDir); err != nil {
t.Fatalf("build day1 first: %v", err)
}
snap1 := snapshotSystemTracks(t, pool, u.ID)
if err := playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, day1, dataDir); err != nil {
t.Fatalf("build day1 second: %v", err)
}
snap2 := snapshotSystemTracks(t, pool, u.ID)
if !equalSnapshots(snap1, snap2) {
t.Errorf("same-day rebuild produced different track lists")
}
}
// snapshotSystemTracks returns a per-playlist-name → []track_id mapping
// for the user's system playlists, in playlist_tracks position order.
func snapshotSystemTracks(t *testing.T, pool *pgxpool.Pool, userID pgtype.UUID) map[string][]string {
t.Helper()
rows, err := pool.Query(context.Background(), `
SELECT p.name, pt.track_id
FROM playlists p
JOIN playlist_tracks pt ON pt.playlist_id = p.id
WHERE p.user_id = $1 AND p.kind = 'system'
ORDER BY p.name, pt.position
`, userID)
if err != nil {
t.Fatalf("snapshot query: %v", err)
}
defer rows.Close()
out := map[string][]string{}
for rows.Next() {
var name string
var tid pgtype.UUID
if err := rows.Scan(&name, &tid); err != nil {
t.Fatalf("snapshot scan: %v", err)
}
out[name] = append(out[name], uuidString(tid))
}
return out
}
func equalSnapshots(a, b map[string][]string) bool {
if len(a) != len(b) {
return false
}
for k, va := range a {
vb, ok := b[k]
if !ok || len(va) != len(vb) {
return false
}
for i := range va {
if va[i] != vb[i] {
return false
}
}
}
return true
}
func TestListActiveUsersForSystemPlaylists(t *testing.T) {
pool := newPool(t)
ctx := context.Background()
activeU, tracks := seedActiveLibrary(t, pool, "active", 1, 1)
inactiveU := seedUser(t, pool, "inactive")
// inactive user has plays older than 7 days.
seedPlayEvent(t, pool, inactiveU.ID, tracks[0].ID, time.Now().UTC().AddDate(0, 0, -30), false)
ids, err := dbq.New(pool).ListActiveUsersForSystemPlaylists(ctx)
if err != nil {
t.Fatalf("list: %v", err)
}
var sawActive, sawInactive bool
for _, id := range ids {
if id.Bytes == activeU.ID.Bytes {
sawActive = true
}
if id.Bytes == inactiveU.ID.Bytes {
sawInactive = true
}
}
if !sawActive {
t.Error("active user not returned")
}
if sawInactive {
t.Error("inactive user (plays > 7d old) was returned")
}
}
func TestBuildSystemPlaylists_DiscoverColdStart(t *testing.T) {
// User has no plays / likes → Discover's dormant + cross-user
// buckets are empty (or minimal), all slots come from random unheard.
pool := newPool(t)
logger := discardLogger()
u, _ := seedActiveLibrary(t, pool, "discover_cold", 4, 5)
// seedActiveLibrary seeds 3 plays per track; delete them so dormant
// bucket (< 10 plays per artist) and random bucket both operate on
// a true cold-start user with no prior play history.
if _, err := pool.Exec(context.Background(),
"DELETE FROM play_events WHERE user_id = $1", u.ID); err != nil {
t.Fatalf("clear plays: %v", err)
}
if err := playlists.BuildSystemPlaylists(context.Background(), pool, logger, u.ID, time.Now().UTC(), t.TempDir()); err != nil {
t.Fatalf("build: %v", err)
}
rows, err := dbq.New(pool).ListPlaylistsByUserAndKind(context.Background(), dbq.ListPlaylistsByUserAndKindParams{
UserID: u.ID, Column2: "system",
})
if err != nil {
t.Fatalf("list: %v", err)
}
var foundDiscover bool
for _, r := range rows {
if r.SystemVariant != nil && *r.SystemVariant == "discover" {
foundDiscover = true
if r.TrackCount == 0 {
t.Errorf("Discover playlist has zero tracks on cold start (random bucket should fill)")
}
if r.TrackCount > 100 {
t.Errorf("Discover track_count = %d, want <= 100", r.TrackCount)
}
}
}
if !foundDiscover {
t.Errorf("expected a 'discover' system playlist; got variants: %v", systemVariantsOf(rows))
}
}
func systemVariantsOf(rows []dbq.ListPlaylistsByUserAndKindRow) []string {
out := make([]string, 0, len(rows))
for _, r := range rows {
if r.SystemVariant != nil {
out = append(out, *r.SystemVariant)
}
}
return out
}
func TestStartupRecoveryClearsStaleInFlight(t *testing.T) {
pool := newPool(t)
ctx := context.Background()
u := seedUser(t, pool, "stale")
// Seed a stale in_flight=true row.
if _, err := pool.Exec(ctx, `INSERT INTO system_playlist_runs (user_id, in_flight) VALUES ($1, true)`, u.ID); err != nil {
t.Fatalf("seed runs: %v", err)
}
if err := dbq.New(pool).ClearStaleSystemPlaylistInFlight(ctx); err != nil {
t.Fatalf("clear: %v", err)
}
run, err := dbq.New(pool).GetSystemPlaylistRun(ctx, u.ID)
if err != nil {
t.Fatalf("get: %v", err)
}
if run.InFlight {
t.Errorf("in_flight should be false after ClearStale; got true")
}
}