From 16886c92cf194c17d98e3024d914639c83169ae1 Mon Sep 17 00:00:00 2001 From: Caleb Allen Date: Fri, 24 Jul 2026 14:31:33 -0400 Subject: [PATCH] perf(smartplaylist): batch-load cover art + MBIDs instead of per-row subquery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The autotag overhaul added cover-art and MusicBrainz-ID columns to leanTrackQuery to support the new track-row styling, reintroducing the per-row correlated subquery anti-pattern (artist_mbid) plus a cover_art join inside the whole-library derived table. Both ran for every track before WHERE/LIMIT, so smart-playlist evaluation cost scaled with library size rather than result size — several seconds for a 500-track playlist that was previously sub-second. Move these presentation-only fields into a batched fetchArtwork pass keyed by the matched recording_ids, mirroring the existing fetchGenres batch. Cost is now proportional to results. Add TestEvaluate_ArtworkEnrichment (no prior coverage of these fields) and an artwork_ms debug metric. Co-Authored-By: Claude Opus 4.8 --- backend/smartplaylist/smartplaylist.go | 224 +++++++++++++++----- backend/smartplaylist/smartplaylist_test.go | 80 +++++++ 2 files changed, 254 insertions(+), 50 deletions(-) diff --git a/backend/smartplaylist/smartplaylist.go b/backend/smartplaylist/smartplaylist.go index 577732b..d1044a4 100644 --- a/backend/smartplaylist/smartplaylist.go +++ b/backend/smartplaylist/smartplaylist.go @@ -555,11 +555,7 @@ const leanTrackQuery = `SELECT af.bitrate, af.file_size, af.play_count, - COALESCE(af.last_played, '') AS last_played, - af.cover_art_path, - af.artist_mbid, - af.release_group_mbid, - af.recording_mbid + COALESCE(af.last_played, '') AS last_played FROM ( SELECT af.id, @@ -591,15 +587,7 @@ FROM ( af.file_size, af.library_id, af.play_count, - af.last_played, - COALESCE(ca.file_path, '') AS cover_art_path, - COALESCE((SELECT a.mbid - FROM artist_credit_artist aca - JOIN artists a ON a.id = aca.artist_id - WHERE aca.credit_id = ac.id - LIMIT 1), '') AS artist_mbid, - COALESCE(rg.mbid, '') AS release_group_mbid, - COALESCE(r.mbid, '') AS recording_mbid + af.last_played FROM audio_files af LEFT JOIN recordings r ON af.recording_id = r.id LEFT JOIN artist_credit ac ON r.artist_credit_id = ac.id @@ -610,7 +598,6 @@ FROM ( GROUP BY recording_id ) rgr ON r.id = rgr.recording_id LEFT JOIN release_groups rg ON rgr.release_group_id = rg.id - LEFT JOIN cover_art ca ON rg.cover_art_id = ca.id LEFT JOIN file_types ft ON af.file_type_id = ft.id ) af` @@ -719,6 +706,38 @@ func Evaluate( genreDuration := time.Since(genreStart) + // Batch-load cover art + MusicBrainz IDs for the matched rows only. + // These fields are presentation-only (track-row styling); keeping + // them out of the lean query avoids a per-row correlated subquery + // and cover-art join over the whole library before WHERE/LIMIT. + artStart := time.Now() + + artworkByRecording, err := fetchArtwork(db, recordingIDs) + if err != nil { + return nil, err + } + + for i, rid := range recordingIDs { + art, ok := artworkByRecording[rid] + if !ok { + continue + } + + tracks[i].ArtistMBID = art.artistMBID + tracks[i].ReleaseGroupMBID = art.releaseGroupMBID + tracks[i].RecordingMBID = art.recordingMBID + + if art.coverArtPath != "" { + urls := coverart.ResolveURLs(art.coverArtPath) + tracks[i].CoverArtPath = urls.Original + tracks[i].CoverArtSmall = urls.Small + tracks[i].CoverArtMedium = urls.Medium + tracks[i].CoverArtLarge = urls.Large + } + } + + artDuration := time.Since(artStart) + // Apply genre-sort and deferred LIMIT in Go if needed. if sortByGenre { dir := 1 @@ -743,6 +762,7 @@ func Evaluate( "tracks", len(tracks), "main_ms", mainDuration.Milliseconds(), "genres_ms", genreDuration.Milliseconds(), + "artwork_ms", artDuration.Milliseconds(), "total_ms", time.Since(start).Milliseconds(), ) @@ -778,11 +798,6 @@ func scanTracks(rows *sql.Rows) ([]library.Track, []int64, error) { fileSize int64 playCount int64 lastPlayed string - - coverArtPath string - artistMBID string - releaseGroupMBID string - recordingMBID string ) if err := rows.Scan( @@ -792,8 +807,6 @@ func scanTracks(rows *sql.Rows) ([]library.Track, []int64, error) { &sampleRate, &bitDepth, &channels, &bitrate, &fileSize, &playCount, &lastPlayed, - &coverArtPath, &artistMBID, - &releaseGroupMBID, &recordingMBID, ); err != nil { return nil, nil, fmt.Errorf( "could not scan smart playlist row: %w", err, @@ -801,34 +814,23 @@ func scanTracks(rows *sql.Rows) ([]library.Track, []int64, error) { } track := library.Track{ - TrackName: title, - ArtistName: artistName, - TrackLength: strconv.FormatInt(lengthMs, 10), - FilePath: filePath, - TrackNumber: trackNumber.Int64, - DiscNumber: discNumber.Int64, - Album: album, - Year: year, - Composer: composer, - FileType: fileType, - SampleRate: sampleRate, - BitDepth: bitDepth, - Channels: channels, - Bitrate: bitrate, - FileSize: fileSize, - PlayCount: playCount, - LastPlayed: lastPlayed, - ArtistMBID: artistMBID, - ReleaseGroupMBID: releaseGroupMBID, - RecordingMBID: recordingMBID, - } - - if coverArtPath != "" { - urls := coverart.ResolveURLs(coverArtPath) - track.CoverArtPath = urls.Original - track.CoverArtSmall = urls.Small - track.CoverArtMedium = urls.Medium - track.CoverArtLarge = urls.Large + TrackName: title, + ArtistName: artistName, + TrackLength: strconv.FormatInt(lengthMs, 10), + FilePath: filePath, + TrackNumber: trackNumber.Int64, + DiscNumber: discNumber.Int64, + Album: album, + Year: year, + Composer: composer, + FileType: fileType, + SampleRate: sampleRate, + BitDepth: bitDepth, + Channels: channels, + Bitrate: bitrate, + FileSize: fileSize, + PlayCount: playCount, + LastPlayed: lastPlayed, } tracks = append(tracks, track) @@ -929,6 +931,128 @@ func fetchGenres( return result, nil } +// trackArtwork holds the presentation-only cover-art path and +// MusicBrainz identifiers attached to a matched track after the main +// filter query, keyed by recording_id. +type trackArtwork struct { + coverArtPath string + artistMBID string + releaseGroupMBID string + recordingMBID string +} + +// fetchArtwork batch-loads cover-art paths and MusicBrainz IDs for the +// given recording_ids in a single IN-list query. These fields drive +// track-row styling only, so scoping them to the matched result set +// keeps the cost proportional to results rather than library size. +func fetchArtwork( + db *database.DB, ids []int64, +) (map[int64]trackArtwork, error) { + if len(ids) == 0 { + return nil, nil + } + + // Deduplicate to keep the IN list minimal. + seen := make(map[int64]struct{}, len(ids)) + unique := make([]int64, 0, len(ids)) + + for _, id := range ids { + if id == 0 { + continue + } + + if _, ok := seen[id]; ok { + continue + } + + seen[id] = struct{}{} + + unique = append(unique, id) + } + + if len(unique) == 0 { + return nil, nil + } + + placeholders := make([]string, len(unique)) + + for i := range unique { + placeholders[i] = "?" + } + + inList := strings.Join(placeholders, ", ") + + // A recording's artist credit can name several artists; the old + // correlated subquery picked one via LIMIT 1. GROUP BY r.id with + // MIN() reproduces a single stable value without multiplying rows. + // SAFETY: placeholders are static "?" tokens; every value is + // parameterized. The IN list is bound twice (subquery + outer). + query := `SELECT r.id, + COALESCE(MIN(ca.file_path), '') AS cover_art_path, + COALESCE(MIN(a.mbid), '') AS artist_mbid, + COALESCE(MIN(rg.mbid), '') AS release_group_mbid, + COALESCE(r.mbid, '') AS recording_mbid + FROM recordings r + LEFT JOIN artist_credit ac ON r.artist_credit_id = ac.id + LEFT JOIN artist_credit_artist aca ON aca.credit_id = ac.id + LEFT JOIN artists a ON a.id = aca.artist_id + LEFT JOIN ( + SELECT recording_id, + MIN(release_group_id) AS release_group_id + FROM release_group_recordings + WHERE recording_id IN (` + inList + `) + GROUP BY recording_id + ) rgr ON r.id = rgr.recording_id + LEFT JOIN release_groups rg ON rgr.release_group_id = rg.id + LEFT JOIN cover_art ca ON rg.cover_art_id = ca.id + WHERE r.id IN (` + inList + `) + GROUP BY r.id` + + args := make([]any, 0, len(unique)*2) + for range 2 { + for _, id := range unique { + args = append(args, id) + } + } + + rows, err := db.QueryContext(query, args...) + if err != nil { + return nil, fmt.Errorf( + "smart playlist artwork fetch failed: %w", err, + ) + } + + defer func() { _ = rows.Close() }() + + result := make(map[int64]trackArtwork, len(unique)) + + for rows.Next() { + var ( + rid int64 + art trackArtwork + ) + + if err := rows.Scan( + &rid, &art.coverArtPath, &art.artistMBID, + &art.releaseGroupMBID, &art.recordingMBID, + ); err != nil { + return nil, fmt.Errorf( + "could not scan smart playlist artwork row: %w", err, + ) + } + + result[rid] = art + } + + if err := rows.Err(); err != nil { + return nil, fmt.Errorf( + "smart playlist artwork iteration error: %w", err, + ) + } + + return result, nil +} + // ParseRuleSet parses a JSON string into a validated RuleSet. func ParseRuleSet(jsonStr string) (RuleSet, error) { var rs RuleSet diff --git a/backend/smartplaylist/smartplaylist_test.go b/backend/smartplaylist/smartplaylist_test.go index d515f45..6b550ca 100644 --- a/backend/smartplaylist/smartplaylist_test.go +++ b/backend/smartplaylist/smartplaylist_test.go @@ -4,6 +4,7 @@ import ( "strings" "testing" + "yellowjacket/backend/coverart" "yellowjacket/backend/database" ) @@ -812,6 +813,85 @@ func TestEvaluate_TextIs(t *testing.T) { } } +// TestEvaluate_ArtworkEnrichment verifies the presentation-only +// cover-art and MusicBrainz-ID fields are attached to matched tracks +// by the batched fetchArtwork pass (they are no longer part of the +// lean filter query). +func TestEvaluate_ArtworkEnrichment(t *testing.T) { + t.Parallel() + + db := database.NewTestDB(t) + + // Minimal FK chain: cover_art → release_group(mbid) → + // release_group_recordings → recording(mbid) → audio_file, plus + // artist_credit → artist_credit_artist → artist(mbid). + exec := func(query string, args ...any) { + t.Helper() + + if _, err := db.ExecContext(query, args...); err != nil { + t.Fatalf("seed %q: %v", query, err) + } + } + + // file_types are pre-seeded by the schema (id 0 = .mp3). + exec("INSERT INTO cover_art (id, file_path, mime_type) " + + "VALUES (1, '/covers/abc123.jpg', 'image/jpeg')") + exec("INSERT INTO artists (id, name, mbid) " + + "VALUES (1, 'Queen', 'artist-mbid-1')") + exec("INSERT INTO artist_credit (id, text) VALUES (1, 'Queen')") + exec("INSERT INTO artist_credit_artist (credit_id, artist_id) " + + "VALUES (1, 1)") + exec("INSERT INTO release_groups (id, name, cover_art_id, mbid) " + + "VALUES (1, 'A Night at the Opera', 1, 'rg-mbid-1')") + exec("INSERT INTO recordings (id, name, artist_credit_id, mbid) " + + "VALUES (1, 'Bohemian Rhapsody', 1, 'rec-mbid-1')") + exec("INSERT INTO release_group_recordings " + + "(release_group_id, recording_id) VALUES (1, 1)") + exec("INSERT INTO audio_files (id, file_path, " + + "length_milliseconds, recording_id, file_type_id) " + + "VALUES (1, '/music/bohemian.mp3', 354000, 1, 0)") + + tracks, err := Evaluate(db, RuleSet{ + Rules: []Rule{ + {Field: "artist", Operator: "is", Value: "Queen"}, + }, + }) + if err != nil { + t.Fatalf("Evaluate: %v", err) + } + + if len(tracks) != 1 { + t.Fatalf("got %d tracks, want 1", len(tracks)) + } + + tr := tracks[0] + + if tr.ArtistMBID != "artist-mbid-1" { + t.Errorf("ArtistMBID = %q, want artist-mbid-1", tr.ArtistMBID) + } + + if tr.ReleaseGroupMBID != "rg-mbid-1" { + t.Errorf("ReleaseGroupMBID = %q, want rg-mbid-1", + tr.ReleaseGroupMBID) + } + + if tr.RecordingMBID != "rec-mbid-1" { + t.Errorf("RecordingMBID = %q, want rec-mbid-1", + tr.RecordingMBID) + } + + wantURLs := coverart.ResolveURLs("/covers/abc123.jpg") + if tr.CoverArtPath != wantURLs.Original { + t.Errorf("CoverArtPath = %q, want %q", + tr.CoverArtPath, wantURLs.Original) + } + + if tr.CoverArtSmall != wantURLs.Small { + t.Errorf("CoverArtSmall = %q, want %q", + tr.CoverArtSmall, wantURLs.Small) + } +} + func TestEvaluate_TextContains(t *testing.T) { t.Parallel()