fix(playlists): a missing track never seeds For You or Songs-like (#2701)
release / govulncheck (push) Successful in 14s
release / web (push) Successful in 1m11s
release / go (push) Successful in 1m29s
release / integration (push) Successful in 4m27s
release / android (push) Successful in 5m2s
release / Build signed APK (releases and dev) (push) Successful in 5m17s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 1m15s
release / Verify release artifacts (tag releases only) (push) Skipped
release / govulncheck (push) Successful in 14s
release / web (push) Successful in 1m11s
release / go (push) Successful in 1m29s
release / integration (push) Successful in 4m27s
release / android (push) Successful in 5m2s
release / Build signed APK (releases and dev) (push) Successful in 5m17s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 1m15s
release / Verify release artifacts (tag releases only) (push) Skipped
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user