From 31190657d80096297cd56114b59a83c451d9503b Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 10 Sep 2026 21:13:32 -0400 Subject: [PATCH] feat(recommendation): Songs-like can include the seed artist's own music MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH --- internal/playlists/system.go | 28 ++++++++---- internal/playlists/system_test.go | 73 +++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 8 deletions(-) diff --git a/internal/playlists/system.go b/internal/playlists/system.go index 08ef2186..7bac0a1e 100644 --- a/internal/playlists/system.go +++ b/internal/playlists/system.go @@ -703,14 +703,26 @@ func produceSeedMixes( "artist_id", uuidStringPL(artistID), "err", cerr) continue } - // "Songs like X" excludes X's own songs. - filtered := make([]recommendation.Candidate, 0, len(cands)) - for _, c := range cands { - if !pgtypeUUIDEqual(c.Track.ArtistID, artistID) { - filtered = append(filtered, c) - } - } - tracks := pickTopN(filtered, userID, dateStr, now, systemMixLength) + // The seed artist's own songs are ELIGIBLE here, deliberately. + // + // This used to filter them out — "Songs like X excludes X's own + // songs" — which reads as obviously right and is not. The seed is a + // TRACK, and the tracks most likely to sound like it are usually the + // rest of that artist's catalogue; excluding them threw away the + // nearest neighbours of the very thing the mix is built around, and + // then reached further out to replace them. On a surface whose whole + // job is staying in one neighbourhood, that is backwards. + // + // Operator, 2026-09-10: "it should also be able to include music from + // the same artist." + // + // Domination is bounded by the cap rather than by exclusion, which is + // the distinction that makes this safe: capCandidatesByAlbumAndArtist + // inside pickTopN allows at most discoverMaxTracksPerArtist (3) of a + // 25-track mix — 12%, a presence rather than a takeover. The seed + // track itself still cannot appear; it is passed as an exclusion to + // LoadCandidatesFromSimilarity above. + tracks := pickTopN(cands, userID, dateStr, now, systemMixLength) if len(tracks) == 0 { continue } diff --git a/internal/playlists/system_test.go b/internal/playlists/system_test.go index e0757758..18effa26 100644 --- a/internal/playlists/system_test.go +++ b/internal/playlists/system_test.go @@ -287,6 +287,79 @@ func TestBuildSystemPlaylists_Concurrency(t *testing.T) { } } +// 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()