From 210f884b2d42e691b40e7c292e9b7c62a096d056 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 20 Oct 2026 14:44:45 -0400 Subject: [PATCH] fix: a file tagged with no release group joins the same-named album its folder holds Under e45561e2 a file with no group tag took the oldest same-named album row, which left three of Weezer's 1994 bonus tracks on the 2016 album. The lookup now ranks same-named rows by how many tracks they hold from the file's folder; tagReadVersion 8 re-places the existing ones. Co-Authored-By: Claude Opus 5.5 --- internal/db/dbq/albums.sql.go | 26 +++++++++++++++++++------- internal/db/queries/albums.sql | 18 ++++++++++++------ internal/library/scanner.go | 11 +++++++---- internal/library/scanner_test.go | 19 ++++++++++++++----- 4 files changed, 52 insertions(+), 22 deletions(-) diff --git a/internal/db/dbq/albums.sql.go b/internal/db/dbq/albums.sql.go index 5f21694d..fff470c2 100644 --- a/internal/db/dbq/albums.sql.go +++ b/internal/db/dbq/albums.sql.go @@ -72,11 +72,14 @@ func (q *Queries) DeleteAlbumIfEmpty(ctx context.Context, id pgtype.UUID) (Delet } const getAlbumByArtistAndTitle = `-- name: GetAlbumByArtistAndTitle :one -SELECT id, title, sort_title, artist_id, release_date, mbid, cover_art_path, created_at, updated_at, cover_art_source, cover_art_sources_version, release_group_mbid FROM albums - WHERE artist_id = $1 AND title = $2 - AND ($3::text IS NULL OR release_group_mbid IS NULL - OR release_group_mbid = $3) - ORDER BY (release_group_mbid IS NOT DISTINCT FROM $3) DESC, id +SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, albums.release_group_mbid FROM albums + WHERE albums.artist_id = $1 AND albums.title = $2 + AND ($3::text IS NULL OR albums.release_group_mbid IS NULL + OR albums.release_group_mbid = $3) + ORDER BY (albums.release_group_mbid IS NOT DISTINCT FROM $3) DESC, + (SELECT count(*) FROM tracks t + WHERE t.album_id = albums.id AND starts_with(t.file_path, $4::text || '/')) DESC, + albums.id LIMIT 1 ` @@ -84,15 +87,24 @@ type GetAlbumByArtistAndTitleParams struct { ArtistID pgtype.UUID Title string ReleaseGroupMbid *string + Dir string } // Scanner uses this for the no-mbid dedupe path: resolve-or-create. An album // of the same name in another release group is a different album (Weezer's // six self-titled albums shared one row until 2026-10-20), so a row whose // group is known and differs is not returned. A row of the file's own group -// comes first, then one with no group yet. +// comes first, then one with no group yet, then the one holding the most +// tracks from the file's folder: a file tagged with no group (a live bonus +// track beside Weezer's 1994 album) belongs with its folder, not the oldest +// row, and a few strays already misfiled do not outvote the folder. func (q *Queries) GetAlbumByArtistAndTitle(ctx context.Context, arg GetAlbumByArtistAndTitleParams) (Album, error) { - row := q.db.QueryRow(ctx, getAlbumByArtistAndTitle, arg.ArtistID, arg.Title, arg.ReleaseGroupMbid) + row := q.db.QueryRow(ctx, getAlbumByArtistAndTitle, + arg.ArtistID, + arg.Title, + arg.ReleaseGroupMbid, + arg.Dir, + ) var i Album err := row.Scan( &i.ID, diff --git a/internal/db/queries/albums.sql b/internal/db/queries/albums.sql index 1a298bc0..5fceb24e 100644 --- a/internal/db/queries/albums.sql +++ b/internal/db/queries/albums.sql @@ -40,12 +40,18 @@ ORDER BY release_date NULLS LAST, sort_title; -- of the same name in another release group is a different album (Weezer's -- six self-titled albums shared one row until 2026-10-20), so a row whose -- group is known and differs is not returned. A row of the file's own group --- comes first, then one with no group yet. -SELECT * FROM albums - WHERE artist_id = sqlc.arg(artist_id) AND title = sqlc.arg(title) - AND (sqlc.narg(release_group_mbid)::text IS NULL OR release_group_mbid IS NULL - OR release_group_mbid = sqlc.narg(release_group_mbid)) - ORDER BY (release_group_mbid IS NOT DISTINCT FROM sqlc.narg(release_group_mbid)) DESC, id +-- comes first, then one with no group yet, then the one holding the most +-- tracks from the file's folder: a file tagged with no group (a live bonus +-- track beside Weezer's 1994 album) belongs with its folder, not the oldest +-- row, and a few strays already misfiled do not outvote the folder. +SELECT albums.* FROM albums + WHERE albums.artist_id = sqlc.arg(artist_id) AND albums.title = sqlc.arg(title) + AND (sqlc.narg(release_group_mbid)::text IS NULL OR albums.release_group_mbid IS NULL + OR albums.release_group_mbid = sqlc.narg(release_group_mbid)) + ORDER BY (albums.release_group_mbid IS NOT DISTINCT FROM sqlc.narg(release_group_mbid)) DESC, + (SELECT count(*) FROM tracks t + WHERE t.album_id = albums.id AND starts_with(t.file_path, sqlc.arg(dir)::text || '/')) DESC, + albums.id LIMIT 1; -- name: ListAlbumsByArtist :many diff --git a/internal/library/scanner.go b/internal/library/scanner.go index 40cbb51c..170d74b1 100644 --- a/internal/library/scanner.go +++ b/internal/library/scanner.go @@ -78,7 +78,10 @@ var audioExtensions = map[string]bool{ // (resolveAlbum). Weezer's six self-titled albums were one 118-track row, // so their merges, wants and upgrades all counted as one album; a re-read // files each existing track under its own album. -const tagReadVersion int16 = 7 +// 8: a file tagged with no release group joins the same-named album that +// already holds a track from its folder. Under 7 it took the oldest row, +// which left three of Weezer's 1994 bonus tracks on the 2016 album. +const tagReadVersion int16 = 8 type Stats struct { Scanned int `json:"scanned"` @@ -319,7 +322,7 @@ func (s *Scanner) scanFile( if err != nil { return pgtype.UUID{}, false, fmt.Errorf("artist: %w", err) } - album, err := s.resolveAlbum(ctx, q, artist.ID, albumTitle, meta.Year(), albumMBID, releaseGroupMBID) + album, err := s.resolveAlbum(ctx, q, artist.ID, albumTitle, meta.Year(), albumMBID, releaseGroupMBID, filepath.Dir(path)) if err != nil { return pgtype.UUID{}, false, fmt.Errorf("album: %w", err) } @@ -565,8 +568,8 @@ func (s *Scanner) resolveArtist(ctx context.Context, q *dbq.Queries, name, mbid return artist, nil } -func (s *Scanner) resolveAlbum(ctx context.Context, q *dbq.Queries, artistID pgtype.UUID, title string, year int, mbid, releaseGroupMBID string) (dbq.Album, error) { - lookup := dbq.GetAlbumByArtistAndTitleParams{ArtistID: artistID, Title: title} +func (s *Scanner) resolveAlbum(ctx context.Context, q *dbq.Queries, artistID pgtype.UUID, title string, year int, mbid, releaseGroupMBID, dir string) (dbq.Album, error) { + lookup := dbq.GetAlbumByArtistAndTitleParams{ArtistID: artistID, Title: title, Dir: dir} if releaseGroupMBID != "" { rg := releaseGroupMBID lookup.ReleaseGroupMbid = &rg diff --git a/internal/library/scanner_test.go b/internal/library/scanner_test.go index 66554044..18155eb8 100644 --- a/internal/library/scanner_test.go +++ b/internal/library/scanner_test.go @@ -459,20 +459,29 @@ func TestScanner_SameTitledAlbumsOfTwoGroupsAreTwoAlbums_Integration(t *testing. t.Fatalf("albums by group = %v, want blue with 2 tracks and green with 1", got) } + // A bonus track tagged with no release group joins the album its folder + // holds, whichever row is older. + writeTestMP3(t, filepath.Join(root, "Weezer (2001)/02.mp3"), map[string]string{ + "TIT2": "Photograph (live)", "TPE1": "Weezer", "TALB": "Weezer", + }) + if _, err := scanner.Scan(ctx, nil); err != nil { + t.Fatalf("scan the bonus track: %v", err) + } + if got := albumsByGroup(); len(got) != 2 || got[blue] != 2 || got[green] != 2 { + t.Fatalf("albums by group = %v, want the ungrouped 2001 bonus track on green", got) + } + // The green track as the old lookup left it: on the blue album's row, - // under an older tag-read version, its own album row gone. + // under an older tag-read version. if _, err := pool.Exec(ctx, ` WITH b AS (SELECT id FROM albums WHERE release_group_mbid = $1) UPDATE tracks SET album_id = (SELECT id FROM b), tag_read_version = 6 WHERE file_path = $2`, blue, greenPath); err != nil { t.Fatal(err) } - if _, err := pool.Exec(ctx, `DELETE FROM albums WHERE release_group_mbid = $1`, green); err != nil { - t.Fatal(err) - } if _, err := scanner.Scan(ctx, nil); err != nil { t.Fatalf("rescan: %v", err) } - if got := albumsByGroup(); len(got) != 2 || got[blue] != 2 || got[green] != 1 { + if got := albumsByGroup(); len(got) != 2 || got[blue] != 2 || got[green] != 2 { t.Errorf("after the re-read, albums by group = %v, want the green track on its own album", got) } }