Files
minstrel/internal/playlists/system_test.go
T
bvandeusenandClaude Opus 5 f5dd4462de
test-go / test (push) Successful in 1m25s
test-go / integration (push) Successful in 4m46s
release / Build signed APK (releases and dev) (push) Successful in 5m45s
release / Build + push container image (push) Successful in 17s
release / Verify release artifacts (tag releases only) (push) Skipped
test(playlists): the same-artist guard needed a fixture that has same artists
Fixes the integration failure from 31190657. The test was wrong, not the
code: it could not have passed whatever produceSeedMixes did.

seedActiveLibrary builds its tracks through seedTrack, whose own comment
says "artist and album are not deduplicated across calls (mbid-less
upsert)". So every track gets a fresh artist row despite sharing a name —
4 artists x 5 tracks is really 20 artists with one track each. A seed
artist's only track IS the seed, which is excluded from its own mix, so
"does this mix contain a track by its seed artist" was structurally
answerable only as no.

That is the failure mode worth naming: the assertion was measuring the
fixture, not the behaviour, and it reported the behaviour as broken.

seedSharedArtistLibrary upserts each artist ONCE and reuses the id across
its tracks, so a seed artist genuinely owns five others. Albums are still
not deduplicated, which suits this test — the per-album cap never binds, so
the per-artist cap (3) is unambiguously what is under test. Noted in the
fixture, because "tidying" the album titles into something shared would
silently change which cap the assertion measures.

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

597 lines
19 KiB
Go

package playlists_test
import (
"context"
"fmt"
"io"
"log/slog"
"path/filepath"
"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")
}
}
// seedSharedArtistLibrary seeds artists that genuinely OWN several tracks.
//
// seedActiveLibrary cannot be used for this: its helper documents that
// "artist and album are not deduplicated across calls (mbid-less upsert)",
// so every track gets its own artist row and a seed artist always has
// exactly one track — the seed itself, which is excluded. A same-artist
// assertion against that fixture can never pass no matter what the code
// does, which is how the first version of this test failed.
//
// Albums are not deduplicated either, for the same mbid-less reason, so each
// track ends up under its own album row however the titles are written. That
// is convenient here rather than a problem: it means the per-ALBUM cap (2)
// never binds, and the per-ARTIST cap (3) is unambiguously the thing under
// test. Do not "fix" the album titles into something shared without checking
// which cap you are then measuring.
func seedSharedArtistLibrary(
t *testing.T, pool *pgxpool.Pool, name string, numArtists, tracksPerArtist int,
) (dbq.User, []pgtype.UUID) {
t.Helper()
q := dbq.New(pool)
ctx := context.Background()
u := seedUser(t, pool, name)
now := time.Now().UTC()
dir := t.TempDir()
artistIDs := make([]pgtype.UUID, 0, numArtists)
for a := 0; a < numArtists; a++ {
artistName := name + "-shared-" + string(rune('A'+a))
ar, err := q.UpsertArtist(ctx, dbq.UpsertArtistParams{Name: artistName, SortName: artistName})
if err != nil {
t.Fatalf("seed artist: %v", err)
}
artistIDs = append(artistIDs, ar.ID)
for k := 0; k < tracksPerArtist; k++ {
albumTitle := fmt.Sprintf("%s - Album %d", artistName, k/2)
al, err := q.UpsertAlbum(ctx, dbq.UpsertAlbumParams{
Title: albumTitle, SortTitle: albumTitle, ArtistID: ar.ID,
})
if err != nil {
t.Fatalf("seed album: %v", err)
}
tk, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{
Title: fmt.Sprintf("%s-t%d", artistName, k), AlbumID: al.ID, ArtistID: ar.ID,
DurationMs: 1000,
FilePath: filepath.Join(dir, fmt.Sprintf("%s-%d-%d.mp3", name, a, k)),
FileSize: 100, FileFormat: "mp3",
})
if err != nil {
t.Fatalf("seed track: %v", err)
}
// Well clear of the recently-played exclusion window, which uses
// the DATABASE clock rather than the build's `now`.
for pl := 0; pl < 3; pl++ {
seedPlayEvent(t, pool, u.ID, tk.ID,
now.Add(-time.Duration(24+a*10+k+pl)*time.Hour), false)
}
}
}
return u, artistIDs
}
// 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, _ := seedSharedArtistLibrary(t, pool, "seedartist", 4, 6)
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")
}
}