From 09e74a57a2d0fc7bab22065b67a55fd26e409aab Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 20 Oct 2026 11:55:23 -0400 Subject: [PATCH] feat: an album holding a song twice beside its single keeps one copy (#5401) A cross-release group was only ever linked, never merged, so an album that held the song twice among the releases kept both: Humanz has Saturnz Barz, Andromeda, We Got the Power and Garage Palace as its FLAC and a YouTube mp3, each beside the single. The resolver now merges an album's extra copies into its best (the one that is not a video rip, else ProposeSurvivor: lossless first) with ReplaceTrack, history and all, then links what is left as one song. Co-Authored-By: Claude Opus 5.5 --- internal/library/duplicate_resolve.go | 126 +++++++++++++++++++++++++- internal/library/song_link_test.go | 67 ++++++++++++++ 2 files changed, 192 insertions(+), 1 deletion(-) diff --git a/internal/library/duplicate_resolve.go b/internal/library/duplicate_resolve.go index a4951d59..2fffcbdd 100644 --- a/internal/library/duplicate_resolve.go +++ b/internal/library/duplicate_resolve.go @@ -27,7 +27,10 @@ import ( // - On one album, a song is held once: the extra copies are merged into the // best by ProposeSurvivor. // - Across releases, each copy belongs to its own release (a single and its -// album): nothing is removed, and the copies are linked as one song. +// album): nothing is removed, and the copies are linked as one song. An +// album holding the song twice among them still holds it once: its extra +// copies are merged into its best (Humanz held Saturnz Barz twice beside +// the single, #5401). // - Except that a video rip is no release's copy. When every copy but one is // a rip, across releases or under another title, the rips are merged into // the one that is not. @@ -215,6 +218,10 @@ func ResolveDuplicatesWith( } } + folded, foldedNames := foldAlbumCopies(ctx, pool, logger, dataDir, groups, merged, resolveMergeCap-res.Merged) + res.Merged += folded + mergedNames = append(mergedNames, foldedNames...) + res.Dismissed = dismissDistinct(ctx, pool, logger, groups, merged) wrong, names := settleWrongSlots(ctx, pool, logger, groups, merged, src) res.WrongFiles = wrong @@ -235,6 +242,123 @@ func ResolveDuplicatesWith( return res, nil } +// 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 +// 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. +func foldAlbumCopies( + ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string, + groups []*resolveGroup, merged map[pgtype.UUID]bool, budget int, +) (int, []string) { + n := 0 + var names []string + for _, g := range groups { + if g.class != ClassCrossRelease || merged[g.id] || ctx.Err() != nil || n >= budget { + continue + } + onAlbum := map[string][]int{} + for i, m := range g.members { + k := syncpkg.FormatUUID(m.AlbumID) + "\x00" + MatchTitleKey(m.Title, m.ArtistName) + onAlbum[k] = append(onAlbum[k], i) + } + gone := map[int]bool{} + for _, copies := range onAlbum { + if len(copies) < 2 { + continue + } + keep, reason := albumCopySurvivor(g, copies) + for _, i := range copies { + if i == keep || n >= budget { + continue + } + if err := foldAlbumCopy(ctx, pool, logger, dataDir, g, i, keep, reason); err != nil { + logger.Warn("duplicate resolve: merging an album's extra copy failed", + "group_id", syncpkg.FormatUUID(g.id), "path", g.members[i].FilePath, "err", err) + continue + } + gone[i] = true + n++ + if len(names) < notifyItems { + names = append(names, TrackLabel(g.members[i].ArtistName, g.members[i].Title)) + } + } + } + if len(gone) > 0 { + kept := g.members[:0:0] + for i, m := range g.members { + if !gone[i] { + kept = append(kept, m) + } + } + g.members = kept + } + } + return n, names +} + +// albumCopySurvivor picks which of one album's copies (indexes into the +// group's members) to keep, and why. +func albumCopySurvivor(g *resolveGroup, copies []int) (int, string) { + notRip := -1 + for _, i := range copies { + if isRip(g.members[i]) { + continue + } + if notRip >= 0 { + notRip = -1 + break + } + notRip = i + } + if notRip >= 0 { + return notRip, "the copy that is not a video rip" + } + all := survivorCandidates(g) + cands := make([]SurvivorCandidate, len(copies)) + for j, i := range copies { + cands[j] = all[i] + } + id, reason := ProposeSurvivor(cands) + for _, i := range copies { + if all[i].TrackID == id { + return i, reason + } + } + return copies[0], reason +} + +// foldAlbumCopy replaces one of an album's copies with the copy kept, history +// and all, and records it as the resolver's merge. +func foldAlbumCopy( + ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string, + g *resolveGroup, gone, keep int, reason string, +) error { + from, into := g.members[gone], g.members[keep] + moved, err := ReplaceTrack(ctx, pool, logger, dataDir, from.TrackID, into.TrackID) + if err != nil { + return err + } + audit.WriteOrLog(ctx, pool, logger, pgtype.UUID{}, pgtype.UUID{}, audit.ActionDuplicateMerge, map[string]any{ + "automatic": true, + "group_id": syncpkg.FormatUUID(g.id), + "tier": g.tier, + "class": string(g.class), + "survivor_track_id": syncpkg.FormatUUID(into.TrackID), + "survivor_path": into.FilePath, + "survivor_reason": reason + "; the album held the song twice", + "removed": []map[string]string{ + {"track_id": syncpkg.FormatUUID(from.TrackID), "file_path": from.FilePath}, + }, + "moved": map[string]any{ + "play_events": moved.PlayEvents, "skip_events": moved.SkipEvents, + "likes": moved.Likes, "playlist_entries": moved.PlaylistEntries, + }, + }) + return nil +} + // linkCrossRelease links each cross-release group's copies as one song // (#5438): a like on one is a like on all, and the Liked list, shuffle and the // mixes count the song once. A group the pass merged has nothing left to link. Linking an already-linked group changes nothing, diff --git a/internal/library/song_link_test.go b/internal/library/song_link_test.go index 3ced2aa1..263afee5 100644 --- a/internal/library/song_link_test.go +++ b/internal/library/song_link_test.go @@ -2,6 +2,8 @@ package library import ( "context" + "os" + "path/filepath" "testing" "github.com/jackc/pgx/v5/pgtype" @@ -218,3 +220,68 @@ func songKeys(t *testing.T, q *dbq.Queries, ids ...pgtype.UUID) []pgtype.UUID { } return out } + +// A cross-release group whose album holds the song twice (Humanz: Saturnz +// Barz as a FLAC and a YouTube mp3, beside the single, #5401): the album's +// extra copy is merged into its FLAC, the single stays, and the two copies +// left are linked as one song. +func TestResolveDuplicates_FoldsAnAlbumsExtraCopyInACrossReleaseGroup_Integration(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + dir := t.TempDir() + artist, err := q.UpsertArtist(ctx, dbq.UpsertArtistParams{Name: "Gorillaz", SortName: "Gorillaz"}) + if err != nil { + t.Fatal(err) + } + track := func(album, name, format string) dbq.Track { + t.Helper() + a, err := q.UpsertAlbum(ctx, dbq.UpsertAlbumParams{Title: album, SortTitle: album, ArtistID: artist.ID}) + if err != nil { + t.Fatal(err) + } + path := filepath.Join(dir, album, name) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte("audio"), 0o644); err != nil { + t.Fatal(err) + } + tr, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{ + Title: "Saturnz Barz", AlbumID: a.ID, ArtistID: artist.ID, + DurationMs: 181000, FilePath: path, FileSize: 100, FileFormat: format, + }) + if err != nil { + t.Fatal(err) + } + return tr + } + flac := track("Humanz", "04 Saturnz Barz.flac", "flac") + mp3 := track("Humanz", "06 Saturnz Barz (feat. Popcaan).mp3", "mp3") + single := track("Saturnz Barz", "01 Saturnz Barz.mp3", "mp3") + var groupID pgtype.UUID + if err := pool.QueryRow(ctx, + `INSERT INTO duplicate_groups (member_key, tier) VALUES ('humanz-saturnz', 'exact') RETURNING id`, + ).Scan(&groupID); err != nil { + t.Fatal(err) + } + if _, err := pool.Exec(ctx, `INSERT INTO duplicate_group_members (group_id, track_id) VALUES ($1, $2), ($1, $3), ($1, $4)`, + groupID, flac.ID, mp3.ID, single.ID); err != nil { + t.Fatal(err) + } + + res, err := ResolveDuplicates(ctx, pool, nil, "", true) + if err != nil { + t.Fatal(err) + } + if res.Merged != 1 || exists(mp3.FilePath) || !exists(flac.FilePath) || !exists(single.FilePath) { + t.Fatalf("result %+v, mp3 %v flac %v single %v; want only the album's mp3 merged away", + res, exists(mp3.FilePath), exists(flac.FilePath), exists(single.FilePath)) + } + if _, err := q.GetTrackByID(ctx, mp3.ID); err == nil { + t.Error("the album's extra copy is still a track") + } + if res.SongsLinked != 1 { + t.Errorf("songs linked = %d, want the FLAC and the single linked as one song", res.SongsLinked) + } +}