From ca57133c8a7511a086ea9cbdca459ce638f94a67 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 10 Oct 2026 18:39:35 -0400 Subject: [PATCH] fix: a release group whose title names a live, remix or demo version is wanted only when the profile allows that version (M502 #5573) MusicBrainz types My Chemical Romance's "Common People (Recorded For BBC Radio 1)" as a plain Single, so a studio-only follow wanted it. Passes now reads the title's qualifier (brackets, or after a dash) for the version it names and treats it as a secondary type; a version no type names, an acoustic or instrumental one, no profile passes. The follow check takes off the list an open follow want the rule now refuses. Judge counts bbc, sessions and unplugged as live. Co-Authored-By: Claude Opus 5.5 --- internal/release/match.go | 2 +- internal/release/release_test.go | 2 + internal/release/version.go | 79 ++++++++++++++++++++++++++++++++ internal/release/version_test.go | 32 +++++++++++++ internal/wanted/profiles.go | 11 ++++- internal/wanted/wanted.go | 35 +++++++++++++- internal/wanted/wanted_test.go | 44 ++++++++++++------ 7 files changed, 188 insertions(+), 17 deletions(-) create mode 100644 internal/release/version.go create mode 100644 internal/release/version_test.go diff --git a/internal/release/match.go b/internal/release/match.go index 77ae1575..3fb25006 100644 --- a/internal/release/match.go +++ b/internal/release/match.go @@ -55,7 +55,7 @@ var versions = []struct { {[]string{"karaoke"}, "a karaoke version", ""}, {[]string{"acapella", "acappella", "accapella", "acapellas"}, "an a cappella version", ""}, {[]string{"remix", "remixes", "remixed"}, "a remix release", "Remix"}, - {[]string{"live", "bootleg"}, "a live recording", "Live"}, + {[]string{"live", "bootleg", "bbc", "sessions", "unplugged"}, "a live recording", "Live"}, {[]string{"demo", "demos"}, "demo recordings", "Demo"}, // Not "covers": in a release name that is usually the artwork scans. {[]string{"tribute", "lullaby", "lullabies"}, "a tribute release", ""}, diff --git a/internal/release/release_test.go b/internal/release/release_test.go index a5c07015..0bde6e41 100644 --- a/internal/release/release_test.go +++ b/internal/release/release_test.go @@ -62,6 +62,8 @@ func TestJudge_TheAlbumAndWhatIsNot(t *testing.T) { "Daft Punk - Random Access Memories (Instrumentals) [MP3]": VetoVersion, "Daft Punk - Random Access Memories Remixes": VetoVersion, "Daft Punk - Random Access Memories (Live) FLAC": VetoVersion, + "Daft Punk - Random Access Memories (BBC Sessions) FLAC": VetoVersion, + "Daft Punk - Random Access Memories Unplugged [MP3]": VetoVersion, "Daft Punk - Discovery (2001) [FLAC]": VetoTitle, "Random Access Memories (2013) [FLAC]": VetoArtist, "Daft Punk - Discography 1997-2013 FLAC": VetoTitle, diff --git a/internal/release/version.go b/internal/release/version.go new file mode 100644 index 00000000..c90dbf50 --- /dev/null +++ b/internal/release/version.go @@ -0,0 +1,79 @@ +package release + +import "strings" + +// TitleVersion is the version of a recording a release group's title names +// in its qualifier: the words in brackets, or after a dash. MusicBrainz +// does not always type such a group: My Chemical Romance's "Common People +// (Recorded For BBC Radio 1)" is a plain Single (#5573). Typ is the +// MusicBrainz secondary type the version is, or "" for one no type names +// (an acoustic or instrumental version); Word is the word that said so. +type TitleVersion struct { + Word string + What string + Typ string +} + +// titleVersions are the qualifier words that name a version, in the order +// they are looked for: "rough mix" is a demo before "mix" is a remix. +var titleVersions = []struct { + words []string + what string + typ string +}{ + {[]string{"demo", "demos", "outtake", "outtakes", "rehearsal", "rough mix"}, "demo recordings", "Demo"}, + {[]string{"live", "bootleg", "bbc", "session", "sessions", "unplugged", "recorded for", "in concert"}, "a live recording", "Live"}, + {[]string{"remix", "remixes", "remixed", "rmx", "mix", "mixes"}, "a remix", "Remix"}, + {[]string{"instrumental", "instrumentals", "acoustic", "karaoke", "acapella", "acappella", "a cappella", + "drumless", "nightcore", "slowed", "sped up"}, "another version", ""}, + {[]string{"commentary", "interview", "interviews"}, "commentary or interviews", "Interview"}, +} + +// VersionInTitle reads a release group's title for the version it names, +// if any. Only the qualifier counts, so "Live Through This" and "Live +// Forever" name no version. +func VersionInTitle(title string) (TitleVersion, bool) { + for _, q := range qualifiers(title) { + tokens := Tokens(q) + for _, v := range titleVersions { + for _, w := range v.words { + if contains(tokens, strings.Fields(w)) { + return TitleVersion{Word: w, What: v.what, Typ: v.typ}, true + } + } + } + } + return TitleVersion{}, false +} + +// qualifiers are the parts of a title in brackets, and what follows a +// spaced dash. +func qualifiers(title string) []string { + var out []string + depth, start := 0, 0 + for i, c := range title { + switch c { + case '(', '[', '{': + if depth == 0 { + start = i + 1 + } + depth++ + case ')', ']', '}': + if depth > 0 { + depth-- + if depth == 0 { + out = append(out, title[start:i]) + } + } + } + } + if depth > 0 { + out = append(out, title[start:]) + } + for _, dash := range []string{" - ", " – ", " — "} { + if i := strings.Index(title, dash); i >= 0 { + out = append(out, title[i+len(dash):]) + } + } + return out +} diff --git a/internal/release/version_test.go b/internal/release/version_test.go new file mode 100644 index 00000000..c6a877c9 --- /dev/null +++ b/internal/release/version_test.go @@ -0,0 +1,32 @@ +package release + +import "testing" + +func TestVersionInTitle(t *testing.T) { + for title, want := range map[string]string{ + // My Chemical Romance's groups MusicBrainz types as plain Singles (#5573). + "Common People (Recorded For BBC Radio 1)": "Live", + "Welcome to the Black Parade (Steve Aoki remix)": "Remix", + "The Five of Us Are Dying (Rough mix)": "Demo", + "I'm Not Okay (I Promise) [outtake version 2]": "Demo", + "Na Na Na (live from Sessions@AOL)": "Live", + "Song - Live at Wembley": "Live", + "Song – Club Mix": "Remix", + "Helena (Acoustic)": "", + "Teenagers (a cappella)": "", + "Hybrid Theory (20th Anniversary Edition)": "none", + "Live Through This": "none", + "Live Forever": "none", + "The Black Parade": "none", + "Warped Tour ’05: Bootleg Series (live at Warped Tour": "Live", // unclosed + } { + v, ok := VersionInTitle(title) + got := v.Typ + if !ok { + got = "none" + } + if got != want { + t.Errorf("VersionInTitle(%q) = %q (%q), want %q", title, got, v.Word, want) + } + } +} diff --git a/internal/wanted/profiles.go b/internal/wanted/profiles.go index d0a466b1..3cd86f3b 100644 --- a/internal/wanted/profiles.go +++ b/internal/wanted/profiles.go @@ -12,6 +12,7 @@ import ( "github.com/jackc/pgx/v5/pgtype" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/release" ) // PrimaryTypes and SecondaryTypes are MusicBrainz's release-group types, in @@ -37,7 +38,12 @@ var ( // on profile p wants: its primary type is listed, and every secondary type // it carries is allowed. A group with no secondary type is a studio release, // which every profile passes on its primary type alone. -func Passes(p dbq.ReleaseTypeProfile, primary string, secondary []string) bool { +// +// A version its title names counts as a secondary type MusicBrainz left +// off (#5573): "Common People (Recorded For BBC Radio 1)" is live whatever +// its types say, and a version no type names, an acoustic one, no profile +// passes. +func Passes(p dbq.ReleaseTypeProfile, title, primary string, secondary []string) bool { if !containsFold(p.PrimaryTypes, primary) { return false } @@ -46,6 +52,9 @@ func Passes(p dbq.ReleaseTypeProfile, primary string, secondary []string) bool { return false } } + if v, ok := release.VersionInTitle(title); ok { + return v.Typ != "" && containsFold(p.SecondaryTypes, v.Typ) + } return true } diff --git a/internal/wanted/wanted.go b/internal/wanted/wanted.go index 6f21ec0c..3efbf930 100644 --- a/internal/wanted/wanted.go +++ b/internal/wanted/wanted.go @@ -21,6 +21,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/catalogue" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" "git.fabledsword.com/bvandeusen/minstrel/internal/musicbrainz" + "git.fabledsword.com/bvandeusen/minstrel/internal/release" "git.fabledsword.com/bvandeusen/minstrel/internal/tags" ) @@ -275,7 +276,13 @@ func (s *Service) applyFollow(ctx context.Context, q *dbq.Queries, f dbq.Follow) wanted := 0 for _, g := range groups { judge := (baseline && f.WantExisting) || (!baseline && !g.Seen && s.recent(g.FirstReleaseDate, f.BaselineAt.Time)) - if judge && Passes(profile, g.PrimaryType, g.SecondaryTypes) { + passes := Passes(profile, g.Title, g.PrimaryType, g.SecondaryTypes) + if !passes { + if err := s.dropRefusedVersion(ctx, q, g.Mbid, g.Title); err != nil { + return wanted, err + } + } + if judge && passes { held, err := q.LibraryHoldsReleaseGroup(ctx, dbq.LibraryHoldsReleaseGroupParams{ GroupMbid: g.Mbid, Title: g.Title, ArtistMbid: f.ArtistMbid, }) @@ -305,6 +312,32 @@ func (s *Service) applyFollow(ctx context.Context, q *dbq.Queries, f dbq.Follow) return wanted, q.MarkFollowChecked(ctx, f.ArtistMbid) } +// dropRefusedVersion takes off the list a group the follow wanted before +// Passes read the version its title names (#5573), while it is still only +// wanted: one the operator holds, or wanted for another reason, stays. +func (s *Service) dropRefusedVersion(ctx context.Context, q *dbq.Queries, group, title string) error { + v, ok := release.VersionInTitle(title) + if !ok { + return nil + } + w, err := q.GetWantByReleaseGroup(ctx, group) + if errors.Is(err, pgx.ErrNoRows) { + return nil + } + if err != nil { + return fmt.Errorf("read want %s: %w", title, err) + } + if w.Reason != ReasonFollow || w.FulfilledAt.Valid { + return nil + } + if _, err := q.DeleteWant(ctx, w.ID); err != nil { + return fmt.Errorf("drop want %s: %w", title, err) + } + s.logger.Info("wanted: a followed artist's release is another version, taken off the list", + "want", title, "version", v.What, "word", v.Word) + return nil +} + // recent reports whether a release dated date (MusicBrainz's partial date: // "2026", "2026-10", "2026-10-09", or empty when not yet known) came out no // earlier than NewReleaseGrace before the follow's baseline. A partial date diff --git a/internal/wanted/wanted_test.go b/internal/wanted/wanted_test.go index e0f5270e..a25e9cdf 100644 --- a/internal/wanted/wanted_test.go +++ b/internal/wanted/wanted_test.go @@ -30,21 +30,29 @@ func TestPasses_PrimaryListedAndEverySecondaryAllowed(t *testing.T) { withLive := dbq.ReleaseTypeProfile{PrimaryTypes: []string{"Album"}, SecondaryTypes: []string{"Live"}} for _, c := range []struct { profile dbq.ReleaseTypeProfile + title string primary string secondary []string want bool }{ - {standard, "Album", nil, true}, - {standard, "single", nil, true}, // MusicBrainz's case is not relied on - {standard, "Album", []string{"Live"}, false}, - {standard, "Broadcast", nil, false}, - {withLive, "Album", []string{"Live"}, true}, - {withLive, "Album", []string{"Live", "Compilation"}, false}, - {withLive, "EP", nil, false}, + {standard, "Revenge", "Album", nil, true}, + {standard, "Helena", "single", nil, true}, // MusicBrainz's case is not relied on + {standard, "Live at Koko", "Album", []string{"Live"}, false}, + {standard, "Radio Show", "Broadcast", nil, false}, + {withLive, "Live at Koko", "Album", []string{"Live"}, true}, + {withLive, "Live & Rare", "Album", []string{"Live", "Compilation"}, false}, + {withLive, "Like Phantoms, Forever", "EP", nil, false}, + // MusicBrainz types these plain Singles; their titles say otherwise (#5573). + {standard, "Common People (Recorded For BBC Radio 1)", "Single", nil, false}, + {standard, "Welcome to the Black Parade (Steve Aoki remix)", "Single", nil, false}, + {standard, "The Five of Us Are Dying (Rough mix)", "Single", nil, false}, + {withLive, "Desolation Row (live at BBC Maida Vale)", "Album", nil, true}, + {withLive, "Helena (acoustic)", "Album", nil, false}, + {standard, "Live Through This", "Album", nil, true}, } { - if got := Passes(c.profile, c.primary, c.secondary); got != c.want { - t.Errorf("Passes(%v/%v, %s %v) = %v, want %v", c.profile.PrimaryTypes, c.profile.SecondaryTypes, - c.primary, c.secondary, got, c.want) + if got := Passes(c.profile, c.title, c.primary, c.secondary); got != c.want { + t.Errorf("Passes(%v/%v, %q %s %v) = %v, want %v", c.profile.PrimaryTypes, c.profile.SecondaryTypes, + c.title, c.primary, c.secondary, got, c.want) } } } @@ -177,6 +185,7 @@ const ( theBallad = "11111111-0000-4000-8000-000000000003" // 2023 album, not held newSingle = "11111111-0000-4000-8000-000000000004" // appears after the baseline oldSingle = "11111111-0000-4000-8000-000000000005" // 1997 single, catalogued late + bbcSingle = "11111111-0000-4000-8000-000000000006" // live, typed a plain Single (#5573) parkRelease = "22222222-0000-4000-8000-000000000001" ) @@ -206,12 +215,13 @@ func newFakeMB(t *testing.T) *fakeMB { case r.URL.Path == "/release-group" && r.URL.Query().Get("artist") == blur: groups := group(parklife, "Parklife", "Album", "", "1994-04-25") + "," + group(liveAtWemb, "Live at Wembley Stadium", "Album", `"Live"`, "2009-07-02") + "," + - group(theBallad, "The Ballad of Darren", "Album", "", "2023-07-21") - n := 3 + group(theBallad, "The Ballad of Darren", "Album", "", "2023-07-21") + "," + + group(bbcSingle, "Song 2 (Recorded For BBC Radio 1)", "Single", "", "2023-08-01") + n := 4 if f.added.Load() { groups += "," + group(newSingle, "New Single", "Single", "", "") + "," + group(oldSingle, "Old Single", "Single", "", "1997-02-03") - n = 5 + n = 6 } _, _ = fmt.Fprintf(w, `{"release-group-count": %d, "release-groups": [%s]}`, n, groups) case r.URL.Path == "/release-group/"+theBallad: @@ -328,6 +338,11 @@ func TestFollow_WantExistingWantsWhatPassesAndIsNotHeld_Integration(t *testing.T f := newFakeMB(t) s := newService(t, pool, f) _ = seedHeld(t, q, "Parklife", parkRelease, parklife, 2) + // Wanted before the follow read the version a title names (#5573). + if _, err := WantWith(ctx, q, WantParams{GroupMBID: bbcSingle, ArtistMBID: blur, ArtistName: "Blur", + Title: "Song 2 (Recorded For BBC Radio 1)", PrimaryType: "Single", Reason: ReasonFollow}); err != nil { + t.Fatal(err) + } if _, err := s.Follow(ctx, FollowParams{ArtistMBID: blur, ArtistName: "Blur", WantExisting: true}); err != nil { t.Fatal(err) @@ -338,7 +353,8 @@ func TestFollow_WantExistingWantsWhatPassesAndIsNotHeld_Integration(t *testing.T if _, err := q.GetWantByReleaseGroup(ctx, theBallad); err != nil { t.Errorf("the studio album not held should be wanted: %v", err) } - for group, why := range map[string]string{parklife: "held", liveAtWemb: "live, and Standard is studio only"} { + for group, why := range map[string]string{parklife: "held", liveAtWemb: "live, and Standard is studio only", + bbcSingle: "recorded for the BBC, which its title says, and Standard is studio only"} { if _, err := q.GetWantByReleaseGroup(ctx, group); err == nil { t.Errorf("%s wanted, but it is %s", group, why) }