From f32abc974fd19d6934a143cfa11ab47483438746 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 20 Oct 2026 12:36:55 -0400 Subject: [PATCH] fix: a duplicate fold never deletes a lossless copy for a lossy one, nor folds two versions (M498) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first album-copy fold pass on 2026-10-20 removed five FLACs (Måneskin Rush!, Weezer 2016) in favour of mp3s whose only edge was a recording MBID, and an explicit "Suite-Pee" for its clean version. ProposeSurvivor now ranks lossless before tag fit; tag fit still outranks file size, which is what the Humanz rips needed. The fold keys copies by their version words (title and file name), so a clean, live or instrumental copy holding the same title tag is linked, not folded. Co-Authored-By: Claude Opus 5.5 --- internal/api/admin_duplicates_test.go | 5 +-- internal/library/duplicate_classify.go | 31 +++++++++++++++++++ internal/library/duplicate_classify_test.go | 34 +++++++++++++++++++++ internal/library/duplicate_resolve.go | 8 +++-- internal/library/duplicate_survivor.go | 24 ++++++++------- internal/library/duplicate_survivor_test.go | 21 +++++++++---- 6 files changed, 102 insertions(+), 21 deletions(-) diff --git a/internal/api/admin_duplicates_test.go b/internal/api/admin_duplicates_test.go index 26f4e814..74c2c00f 100644 --- a/internal/api/admin_duplicates_test.go +++ b/internal/api/admin_duplicates_test.go @@ -65,14 +65,15 @@ func TestFoldDuplicateGroups(t *testing.T) { } // M498: the proposal shown beside a group follows the resolver's rule. The copy -// whose tags fit the album is proposed even when the other is lossless. +// whose tags fit the album is proposed even when the other is larger (only a +// lossless copy outranks it). func TestFoldDuplicateGroups_ProposesTheCopyWhoseTagsFit(t *testing.T) { at := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC) class := "same_release" three := int32(3) rows := []dbq.ListPendingDuplicateGroupMembersRow{ {GroupID: dupUUID(3), Tier: "acoustic", DetectedAt: dupTS(at), TrackID: dupUUID(30), Class: &class, - Title: "Song", FileFormat: "flac", FileSize: 30_000_000, AddedAt: dupTS(at)}, + Title: "Song", FileFormat: "m4a", FileSize: 30_000_000, AddedAt: dupTS(at)}, {GroupID: dupUUID(3), Tier: "acoustic", DetectedAt: dupTS(at), TrackID: dupUUID(31), Class: &class, Title: "Song", FileFormat: "mp3", FileSize: 9_000_000, AddedAt: dupTS(at), TrackNumber: &three, HasMbid: true}, } diff --git a/internal/library/duplicate_classify.go b/internal/library/duplicate_classify.go index d9c3b70b..88acad97 100644 --- a/internal/library/duplicate_classify.go +++ b/internal/library/duplicate_classify.go @@ -1,7 +1,9 @@ package library import ( + "path/filepath" "regexp" + "sort" "strings" "unicode" ) @@ -90,6 +92,35 @@ func ClassifyDuplicateGroup(tier string, members []ClassifyMember) DuplicateClas var versionSuffix = regexp.MustCompile( `(?i)\s-\s[^-]*\b(version|instrumental|remix|mix|live|acoustic|demo|edit|reprise|rework|vip)\b[^-]*$`) +// versionWord matches a word that names a version of a song. +var versionWord = regexp.MustCompile( + `(?i)\b(clean|explicit|instrumentals?|live|acoustic|unplugged|demo|remix|mix|edit|radio|karaoke|a ?cappella|acapella|reprise)\b`) + +// versionWords lists, sorted and lowercased, the version words in a copy's +// title and file name together. Two copies holding one title tag can still be +// two versions, told apart only by the file name: "Suite-Pee" and +// "Suite-Pee (Clean Version)", "Luv (sic) Grand Finale" and its instrumental. +func versionWords(title, path string) string { + seen := map[string]bool{} + name := strings.TrimSuffix(filepath.Base(path), filepath.Ext(path)) + for _, w := range versionWord.FindAllString(title+" "+name, -1) { + w = strings.ToLower(w) + switch { + case w == "instrumentals": + w = "instrumental" + case strings.Contains(w, "cappella") || w == "acapella": + w = "acappella" + } + seen[w] = true + } + words := make([]string, 0, len(seen)) + for w := range seen { + words = append(words, w) + } + sort.Strings(words) + return strings.Join(words, ",") +} + // songKey is MatchTitleKey with every bracketed segment and a dashed version // suffix dropped: the song a title names, whatever version of it. // "Lovesick (Instrumental)", "Lovesick - Instrumental Version" and "Lovesick" diff --git a/internal/library/duplicate_classify_test.go b/internal/library/duplicate_classify_test.go index f0270c51..b19102f2 100644 --- a/internal/library/duplicate_classify_test.go +++ b/internal/library/duplicate_classify_test.go @@ -73,6 +73,40 @@ func TestMatchTitleKey(t *testing.T) { } } +// Copies the fold of 2026-10-20 merged although their file names said they +// were different versions; each pair must disagree. And copies of one version +// named differently must agree. +func TestVersionWords(t *testing.T) { + differ := [][2][2]string{ + {{"Suite-Pee", "/m/SOAD/Sugar/System of a Down - Sugar - 02 - Suite-Pee.mp3"}, + {"Suite-Pee", "/m/SOAD/Sugar/System of a Down - Sugar - 02 - Suite-Pee (Clean Version).mp3"}}, + {{"Luv (sic) Grand Finale", "/m/Nujabes/Hexalogy/Nujabes - Luv(sic) Hexalogy - 06 - Luv (sic) Grand Finale Instrumentals.mp3"}, + {"Luv (sic) Grand Finale", "/m/Nujabes/Hexalogy/0106 - Luvsic Grand Finale.m4a"}}, + {{"Charger", "/m/Gorillaz/Humanz/Gorillaz - Humanz - 08 - Charger - live in Cologne 2017.mp3"}, + {"Charger", "/m/Gorillaz/Humanz/CD 01/08 Charger.flac"}}, + } + for _, p := range differ { + a, b := versionWords(p[0][0], p[0][1]), versionWords(p[1][0], p[1][1]) + if a == b { + t.Errorf("versionWords agree (%q) on %s and %s", a, p[0][1], p[1][1]) + } + } + agree := [][2][2]string{ + {{"Heat Waves (Shakur Ahmad Remix)", "/m/GA/Glass Animals - Dreamland Deluxe - 02 - Heat Waves (Shakur Ahmad Remix).mp3"}, + {"Heat Waves (Shakur Ahmad Remix)", "/m/GA/02. Heat Waves (Shakur Ahmad Remix).flac"}}, + {{"Luv (sic) Instrumentals", "/m/Nujabes/0201 - Luvsic instrumentals.m4a"}, + {"Luv (sic) Instrumentals", "/m/Nujabes/Nujabes - Luv(sic) Hexalogy - 01 - Luv (sic) Instrumentals.mp3"}}, + {{"Henrietta", "/m/The Fratellis - Costello Music - 01 - Henrietta.mp3"}, + {"Henrietta", "/m/The Fratellis - Costello Music - 01 - Henrietta.flac"}}, + } + for _, p := range agree { + a, b := versionWords(p[0][0], p[0][1]), versionWords(p[1][0], p[1][1]) + if a != b { + t.Errorf("versionWords %q vs %q on copies of one version: %s, %s", a, b, p[0][1], p[1][1]) + } + } +} + func TestWrongSlots(t *testing.T) { track := func(title string, ms int32) dbq.MbReleaseTrack { return dbq.MbReleaseTrack{Title: title, LengthMs: ms} } copies := func(titles ...string) []slotCopy { diff --git a/internal/library/duplicate_resolve.go b/internal/library/duplicate_resolve.go index 2fffcbdd..14f71163 100644 --- a/internal/library/duplicate_resolve.go +++ b/internal/library/duplicate_resolve.go @@ -244,7 +244,10 @@ func ResolveDuplicatesWith( // foldAlbumCopies merges, in each cross-release group, the extra copies one // album holds of the song into the best of that album's copies: the one that -// is not a video rip when only one is not, else ProposeSurvivor's choice. The +// is not a video rip when only one is not, else ProposeSurvivor's choice. +// Copies whose title or file name name different versions (versionWords) are +// not folded: "Suite-Pee" and "Suite-Pee (Clean Version)" carried one title tag +// on 2026-10-20, and the explicit copy went. The // copies on other releases stay and are linked as one song. At most budget // copies go. It returns how many went and the songs they were, and drops them // from their groups. @@ -260,7 +263,8 @@ func foldAlbumCopies( } onAlbum := map[string][]int{} for i, m := range g.members { - k := syncpkg.FormatUUID(m.AlbumID) + "\x00" + MatchTitleKey(m.Title, m.ArtistName) + k := syncpkg.FormatUUID(m.AlbumID) + "\x00" + MatchTitleKey(m.Title, m.ArtistName) + + "\x00" + versionWords(m.Title, m.FilePath) onAlbum[k] = append(onAlbum[k], i) } gone := map[int]bool{} diff --git a/internal/library/duplicate_survivor.go b/internal/library/duplicate_survivor.go index 7383d4f8..ea060f80 100644 --- a/internal/library/duplicate_survivor.go +++ b/internal/library/duplicate_survivor.go @@ -52,16 +52,18 @@ var losslessFormats = map[string]bool{"flac": true, "wav": true} // the report shows it and the merge (#3911) lets the operator choose another. // // In order: -// 0. the copy whose tags fit the album best (TagFitScore). It is zero for +// 1. lossless over lossy — the one difference no later step can recover. It +// comes before tag fit: tags can be fixed, a deleted FLAC cannot, and on +// 2026-10-20 five FLACs went for mp3s that only carried a recording MBID +// 2. the copy whose tags fit the album best (TagFitScore). It is zero for // every copy when the caller does not know it, and the order falls through -// to the quality rules below -// 1. lossless over lossy — the one difference no later step can recover -// 2. the larger file — for one recording at one duration that is the higher +// to the rules below +// 3. the larger file — for one recording at one duration that is the higher // bitrate. The scanner does not record bitrate (tracks.bitrate is never // filled), so file size is the signal that actually exists -// 3. the copy in the library longest — the one most likely to carry the play +// 4. the copy in the library longest — the one most likely to carry the play // history and likes, so the merge moves the least -// 4. the lowest track id, so the choice is stable between page loads +// 5. the lowest track id, so the choice is stable between page loads func ProposeSurvivor(cands []SurvivorCandidate) (trackID, reason string) { if len(cands) == 0 { return "", "" @@ -77,10 +79,10 @@ func ProposeSurvivor(cands []SurvivorCandidate) (trackID, reason string) { // runner-up — the rule that actually decided, not every rule it passed. next := ranked[1] switch { - case best.TagFit != next.TagFit: - return best.TrackID, "tags fit the album" case isLossless(best) != isLossless(next): return best.TrackID, "lossless (" + strings.ToLower(best.FileFormat) + ")" + case best.TagFit != next.TagFit: + return best.TrackID, "tags fit the album" case best.FileSize != next.FileSize: return best.TrackID, "largest file" case !best.AddedAt.Equal(next.AddedAt): @@ -91,12 +93,12 @@ func ProposeSurvivor(cands []SurvivorCandidate) (trackID, reason string) { } func survivorBefore(a, b SurvivorCandidate) bool { - if a.TagFit != b.TagFit { - return a.TagFit > b.TagFit - } if isLossless(a) != isLossless(b) { return isLossless(a) } + if a.TagFit != b.TagFit { + return a.TagFit > b.TagFit + } if a.FileSize != b.FileSize { return a.FileSize > b.FileSize } diff --git a/internal/library/duplicate_survivor_test.go b/internal/library/duplicate_survivor_test.go index 09368977..5d11f56b 100644 --- a/internal/library/duplicate_survivor_test.go +++ b/internal/library/duplicate_survivor_test.go @@ -90,10 +90,11 @@ func TestProposeSurvivor_ReasonIsTheDecidingRule(t *testing.T) { } } -// M498 #5436: the copy whose tags fit the album outranks quality, which decides -// only between copies whose tags fit equally. Each case is built so the quality -// rules would choose the other copy, so a reorder that drops tag fit fails. -func TestProposeSurvivor_TagFitFirst(t *testing.T) { +// M498 #5436: the copy whose tags fit the album outranks every quality rule but +// lossless, which decides first: a deleted FLAC cannot be recovered, and on +// 2026-10-20 five went for mp3s whose only edge was a recording MBID. Each tag +// fit case is built so file size would choose the other copy. +func TestProposeSurvivor_LosslessThenTagFit(t *testing.T) { at := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC) cases := []struct { name string @@ -102,10 +103,18 @@ func TestProposeSurvivor_TagFitFirst(t *testing.T) { wantReason string }{ { - name: "tag fit beats lossless", + name: "lossless beats tag fit", cands: []SurvivorCandidate{ - {TrackID: "flac", FileFormat: "flac", FileSize: 40_000_000, AddedAt: at, TagFit: 1}, {TrackID: "fits", FileFormat: "mp3", FileSize: 5_000_000, AddedAt: at, TagFit: 3}, + {TrackID: "flac", FileFormat: "flac", FileSize: 40_000_000, AddedAt: at, TagFit: 1}, + }, + wantID: "flac", wantReason: "lossless (flac)", + }, + { + name: "tag fit beats a larger file", + cands: []SurvivorCandidate{ + {TrackID: "big", FileFormat: "mp3", FileSize: 9_000_000, AddedAt: at, TagFit: 1}, + {TrackID: "fits", FileFormat: "m4a", FileSize: 5_000_000, AddedAt: at, TagFit: 3}, }, wantID: "fits", wantReason: "tags fit the album", },