From efa3bf54cec0b5cd2ade28c4b5ac0ba63ddd1a20 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 7 Oct 2026 23:10:17 -0400 Subject: [PATCH] fix(playlists): a missing track never seeds For You or Songs-like (#2701) PickTopPlayedTracksForUser's liked tier read general_likes without joining tracks. With no plays to seed from, For You could pick a liked track whose file is gone. The play tiers were already safe: their play_events join filters missing_since. The same gap was in PickTopPlayedTrackForArtistByUser's fallback, which seeds Songs-like from the artist's newest album when there are no recent plays. It could pick a missing track, and since #5296 a missing track is never fetched for similarity, so that seed has no edges either. The caller already skips an empty seed, so an artist whose tracks are all missing gets no Songs-like mix instead of one aimed at nothing. Integration tests cover both cases: a liked-but-missing track is not a seed, and the Songs-like fallback moves to the next album once the newest one's track goes missing. Co-Authored-By: Claude Opus 5.5 --- internal/db/dbq/system_playlists.sql.go | 6 ++ internal/db/queries/system_playlists.sql | 6 ++ internal/playlists/seed_missing_db_test.go | 85 ++++++++++++++++++++++ 3 files changed, 97 insertions(+) create mode 100644 internal/playlists/seed_missing_db_test.go diff --git a/internal/db/dbq/system_playlists.sql.go b/internal/db/dbq/system_playlists.sql.go index f76d10ff..d8fa1ae4 100644 --- a/internal/db/dbq/system_playlists.sql.go +++ b/internal/db/dbq/system_playlists.sql.go @@ -452,6 +452,7 @@ SELECT COALESCE( FROM tracks t JOIN albums a ON a.id = t.album_id WHERE t.artist_id = $2 + AND t.missing_since IS NULL -- #2701: the play branch filters; this one must too ORDER BY a.release_date DESC NULLS LAST, t.disc_number NULLS LAST, t.track_number NULLS LAST, @@ -497,6 +498,7 @@ alltime AS ( liked AS ( SELECT gl.track_id AS id, 0::bigint AS c, 2 AS tier FROM general_likes gl + JOIN tracks t ON t.id = gl.track_id AND t.missing_since IS NULL WHERE gl.user_id = $1 ), chosen AS ( @@ -526,6 +528,10 @@ SELECT id // Widened from a hard 7-day window, which made For-You disappear // after a week of not listening and never recover on a self-hosted // library with sparse history. +// A likes-only read still joins tracks: a like outlives its file, and a +// seed whose file is gone points For You at something the user can't hear +// (#2701). The other two tiers get the same filter from their play_events +// join. func (q *Queries) PickTopPlayedTracksForUser(ctx context.Context, userID pgtype.UUID) ([]pgtype.UUID, error) { rows, err := q.db.Query(ctx, pickTopPlayedTracksForUser, userID) if err != nil { diff --git a/internal/db/queries/system_playlists.sql b/internal/db/queries/system_playlists.sql index 41c1cff5..7c97a7a1 100644 --- a/internal/db/queries/system_playlists.sql +++ b/internal/db/queries/system_playlists.sql @@ -161,9 +161,14 @@ alltime AS ( AND pe.was_skipped = false GROUP BY t.id ), +-- A likes-only read still joins tracks: a like outlives its file, and a +-- seed whose file is gone points For You at something the user can't hear +-- (#2701). The other two tiers get the same filter from their play_events +-- join. liked AS ( SELECT gl.track_id AS id, 0::bigint AS c, 2 AS tier FROM general_likes gl + JOIN tracks t ON t.id = gl.track_id AND t.missing_since IS NULL WHERE gl.user_id = $1 ), chosen AS ( @@ -201,6 +206,7 @@ SELECT COALESCE( FROM tracks t JOIN albums a ON a.id = t.album_id WHERE t.artist_id = $2 + AND t.missing_since IS NULL -- #2701: the play branch filters; this one must too ORDER BY a.release_date DESC NULLS LAST, t.disc_number NULLS LAST, t.track_number NULLS LAST, diff --git a/internal/playlists/seed_missing_db_test.go b/internal/playlists/seed_missing_db_test.go new file mode 100644 index 00000000..25826b09 --- /dev/null +++ b/internal/playlists/seed_missing_db_test.go @@ -0,0 +1,85 @@ +package playlists_test + +import ( + "context" + "testing" + + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +func likeTrack(t *testing.T, pool *pgxpool.Pool, userID, trackID pgtype.UUID) { + t.Helper() + if _, err := pool.Exec(context.Background(), + `INSERT INTO general_likes (user_id, track_id) VALUES ($1, $2)`, + userID, trackID); err != nil { + t.Fatalf("like track: %v", err) + } +} + +func markMissing(t *testing.T, pool *pgxpool.Pool, trackID pgtype.UUID) { + t.Helper() + if _, err := pool.Exec(context.Background(), + `UPDATE tracks SET missing_since = now() WHERE id = $1`, trackID); err != nil { + t.Fatalf("mark missing: %v", err) + } +} + +// TestPickTopPlayedTracksForUser_LikedTierSkipsMissing: with no plays, For +// You seeds from likes. A liked track whose file is gone is not a seed (#2701). +func TestPickTopPlayedTracksForUser_LikedTierSkipsMissing(t *testing.T) { + pool := newPool(t) + u := seedUser(t, pool, "seedmissing") + present := seedTrack(t, pool, "Present", "Seed Artist A") + gone := seedTrack(t, pool, "Gone", "Seed Artist B") + likeTrack(t, pool, u.ID, present.ID) + likeTrack(t, pool, u.ID, gone.ID) + markMissing(t, pool, gone.ID) + + seeds, err := dbq.New(pool).PickTopPlayedTracksForUser(context.Background(), u.ID) + if err != nil { + t.Fatalf("pick seeds: %v", err) + } + if len(seeds) != 1 || seeds[0] != present.ID { + t.Fatalf("seeds = %v, want only the present liked track %v", seeds, present.ID) + } +} + +// TestPickTopPlayedTrackForArtistByUser_FallbackSkipsMissing: with no plays, +// Songs-like seeds from the artist's newest album. When that album's track is +// missing, the seed comes from the next album rather than the missing file. +func TestPickTopPlayedTrackForArtistByUser_FallbackSkipsMissing(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + u := seedUser(t, pool, "songslikemissing") + older := seedTrack(t, pool, "Older", "Songs Like Artist") + newerAlbum := seedAlbumForArtist(t, pool, "Newer Album", older.ArtistID) + newer := seedTrackForArtist(t, pool, "Newer", newerAlbum, older.ArtistID) + if _, err := pool.Exec(ctx, `UPDATE albums SET release_date = '2010-01-01' WHERE id = $1`, older.AlbumID); err != nil { + t.Fatalf("date older album: %v", err) + } + if _, err := pool.Exec(ctx, `UPDATE albums SET release_date = '2020-01-01' WHERE id = $1`, newerAlbum); err != nil { + t.Fatalf("date newer album: %v", err) + } + + q := dbq.New(pool) + args := dbq.PickTopPlayedTrackForArtistByUserParams{UserID: u.ID, ArtistID: older.ArtistID} + got, err := q.PickTopPlayedTrackForArtistByUser(ctx, args) + if err != nil { + t.Fatalf("pick seed: %v", err) + } + if got != newer.ID { + t.Fatalf("before marking: seed = %v, want the newest album's track %v", got, newer.ID) + } + + markMissing(t, pool, newer.ID) + got, err = q.PickTopPlayedTrackForArtistByUser(ctx, args) + if err != nil { + t.Fatalf("pick seed: %v", err) + } + if got != older.ID { + t.Fatalf("after marking: seed = %v, want the present older track %v", got, older.ID) + } +}