From a3d9effca333e5c98d0e0f0359e6ce15e1b45a70 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 20 Oct 2026 09:21:43 -0400 Subject: [PATCH] feat: when no slot an album gives one recording is its own, every copy is removed and the album fetched again (M498) Shnabubula's whole album, one 45-minute file, was filed as both Fading Light and Looking for Answers. Neither slot fits, so wrongSlots had nothing to keep and held the group. Every copy now goes when each is clearly another song's; a copy near its slot still holds the group. Co-Authored-By: Claude Opus 5.5 --- internal/library/duplicate_classify_test.go | 3 +- internal/library/duplicate_resolve_test.go | 21 +++++++++++ internal/library/duplicate_settle.go | 36 ++++++++++++------- .../routes/collection/duplicates/+page.svelte | 5 +++ .../collection/duplicates/duplicates.test.ts | 15 ++++++++ 5 files changed, 66 insertions(+), 14 deletions(-) diff --git a/internal/library/duplicate_classify_test.go b/internal/library/duplicate_classify_test.go index b5dba9f8..55a289bf 100644 --- a/internal/library/duplicate_classify_test.go +++ b/internal/library/duplicate_classify_test.go @@ -101,7 +101,8 @@ func TestWrongSlots(t *testing.T) { }, []dbq.MbReleaseTrack{track("Killjoy", 166500), track("Dreamland", 214000)}, 1, []int{0}, true}, {"two slots fit the file", copies("Void", "Reach"), []dbq.MbReleaseTrack{track("Void", 220000), track("Reach", 221500)}, -1, nil, false}, {"a slot near but not within the fit", copies("Void", "Reach"), []dbq.MbReleaseTrack{track("Void", 220000), track("Reach", 226000)}, -1, nil, false}, - {"no slot fits", copies("Reach", "Slimewar (into the void)"), void, -1, nil, false}, + {"no slot fits, and every slot is clearly another song's", copies("Reach", "Slimewar (into the void)"), void, -1, []int{0, 1}, true}, + {"no slot fits, but one is near", copies("Void", "Reach"), []dbq.MbReleaseTrack{track("Void", 226000), track("Reach", 281000)}, -1, nil, false}, {"an unknown length counts for nothing", copies("Void", "Reach"), []dbq.MbReleaseTrack{track("Void", 220000), track("Reach", 0)}, -1, nil, false}, } for _, tc := range cases { diff --git a/internal/library/duplicate_resolve_test.go b/internal/library/duplicate_resolve_test.go index cf82e664..ea707ca4 100644 --- a/internal/library/duplicate_resolve_test.go +++ b/internal/library/duplicate_resolve_test.go @@ -405,6 +405,27 @@ func TestResolveDuplicates_RemovesACopyFromAnotherSongsSlot_Integration(t *testi } } +// One recording filling two slots whose songs are both clearly something else +// (Shnabubula's whole album in one file): both copies go, nothing is kept. +func TestResolveDuplicates_RemovesEveryCopyWhenNoSlotIsTheirs_Integration(t *testing.T) { + f := newOneAlbumFixture(t, "acoustic", "Void", "Reach") + seedTrackList(t, f.pool, f.album, map[string]int{"Void": 282000, "Reach": 421000}) + res, err := ResolveDuplicates(context.Background(), f.pool, nil, "", true) + if err != nil { + t.Fatal(err) + } + if res.WrongFiles != 2 || exists(f.aPath) || exists(f.bPath) { + t.Fatalf("result %+v, void exists %v, reach exists %v; want both copies removed", + res, exists(f.aPath), exists(f.bPath)) + } + if status, _, _, auto := (resolveFixture{pool: f.pool, groupID: f.groupID}).group(t); status != "merged" || !auto { + t.Errorf("group = (%s, auto=%v), want settled by the resolver", status, auto) + } + if n := auditCount(t, f.pool, "duplicate_wrong_file", f.groupID); n != 1 { + t.Errorf("wrong-file audit rows = %d, want 1", n) + } +} + // An album whose track list the catalogue never read: the resolver asks for // it, once, and decides with what comes back. func TestResolveDuplicates_FetchesAMissingTrackList_Integration(t *testing.T) { diff --git a/internal/library/duplicate_settle.go b/internal/library/duplicate_settle.go index bc651a79..0d4cb594 100644 --- a/internal/library/duplicate_settle.go +++ b/internal/library/duplicate_settle.go @@ -31,6 +31,7 @@ import ( // long as the file and every other slot's song is clearly not, the copies // in the other slots are removed and their tracks marked missing, so the // reacquisition sweeper wants the album back and a good copy fills them. +// When every slot's song is clearly not the file, every copy goes. // Anything less certain stays with the operator. const ( @@ -52,8 +53,11 @@ type slotCopy struct { } // wrongSlots decides which copies of a one-album mismatch sit in a slot that -// is not theirs. keep is the copy whose slot fits the recording; ok is false -// when the track list cannot say, and nothing should be removed. +// is not theirs. keep is the copy whose slot fits the recording, or -1 when +// every copy is clearly another song's (Shnabubula's whole album, 45 minutes +// in one file, filed as both "Fading Light" and "Looking for Answers"): then +// all of them go. ok is false when the track list cannot say, and nothing +// should be removed. // // Several copies may fit when they carry the same title ("Killjoy" twice // beside "Dreamland"): they are one song held twice, not a wrong slot, so they @@ -92,7 +96,9 @@ func wrongSlots(copies []slotCopy, tracks []dbq.MbReleaseTrack) (keep int, loser return -1, nil, false } } - if keep < 0 || len(losers) == 0 { + // A copy that neither fits nor is clearly another song's has returned + // above, so with no keep every copy is a loser. + if len(losers) == 0 { return -1, nil, false } return keep, losers, true @@ -261,17 +267,21 @@ func removeWrongSlots( if err != nil { return 0, err } - kept := g.members[keep] - audit.WriteOrLog(ctx, pool, logger, pgtype.UUID{}, pgtype.UUID{}, audit.ActionDuplicateWrongFile, map[string]any{ - "automatic": true, - "group_id": syncpkg.FormatUUID(g.id), - "artist": kept.ArtistName, - "kept_title": kept.Title, - "kept_path": kept.FilePath, - "removed": removedFiles, - }) + details := map[string]any{ + "automatic": true, + "group_id": syncpkg.FormatUUID(g.id), + "artist": g.members[0].ArtistName, + "removed": removedFiles, + } + keptPath := "" + if keep >= 0 { + kept := g.members[keep] + details["kept_title"], details["kept_path"] = kept.Title, kept.FilePath + keptPath = kept.FilePath + } + audit.WriteOrLog(ctx, pool, logger, pgtype.UUID{}, pgtype.UUID{}, audit.ActionDuplicateWrongFile, details) logger.Info("duplicate resolve: removed copies from slots that were not theirs", - "group_id", syncpkg.FormatUUID(g.id), "kept", kept.FilePath, "removed", len(gone)) + "group_id", syncpkg.FormatUUID(g.id), "kept", keptPath, "removed", len(gone)) return len(gone), removeErr } diff --git a/web/src/routes/collection/duplicates/+page.svelte b/web/src/routes/collection/duplicates/+page.svelte index 6d569e16..6667dbf7 100644 --- a/web/src/routes/collection/duplicates/+page.svelte +++ b/web/src/routes/collection/duplicates/+page.svelte @@ -248,6 +248,11 @@ {item.details.artist}: {(item.details.titles ?? []).join(' and ')} are different versions, not duplicates. Both kept.

+ {:else if item.action === 'duplicate_wrong_file' && !item.details.kept_title} +

+ {item.details.artist}: one recording was filed as {removed.map((r) => r.title).join(' and ')}, + and it is none of them. Removed; the album will be fetched again. +

{:else if item.action === 'duplicate_wrong_file'}

{item.details.artist}: the recording of {item.details.kept_title} was also filed as diff --git a/web/src/routes/collection/duplicates/duplicates.test.ts b/web/src/routes/collection/duplicates/duplicates.test.ts index 60ccfcc9..9007accc 100644 --- a/web/src/routes/collection/duplicates/duplicates.test.ts +++ b/web/src/routes/collection/duplicates/duplicates.test.ts @@ -372,6 +372,18 @@ describe('admin duplicates', () => { kept_title: 'Void', removed: [{ track_id: 't-9', file_path: '/data/Music/Savant/Void/17 - Reach.m4a', title: 'Reach' }] } + }, + { + id: 'a-4', + action: 'duplicate_wrong_file', + created_at: ago(HOUR), + details: { + artist: 'Shnabubula', + removed: [ + { track_id: 't-5', file_path: '/data/Music/Shnabubula/05 - Fading Light.mp3', title: 'Fading Light' }, + { track_id: 't-10', file_path: '/data/Music/Shnabubula/10 - Looking for Answers.mp3', title: 'Looking for Answers' } + ] + } } ]); await fireEvent.click(screen.getByRole('tab', { name: /resolved automatically/i })); @@ -381,6 +393,9 @@ describe('admin duplicates', () => { 'Savant: the recording of Void was also filed as Reach. Removed from there; the album will be fetched again.' ); expect(items[1]).toContain('/data/Music/Savant/Void/17 - Reach.m4a'); + expect(items[2]).toContain( + 'Shnabubula: one recording was filed as Fading Light and Looking for Answers, and it is none of them. Removed; the album will be fetched again.' + ); }); });