feat: duplicates settle themselves: retitled copies across releases link, alike versions on one album are dismissed, and a file in another song's slot is removed so the album is fetched again (M498 #6159-#6162)
release / govulncheck (push) Successful in 14s
release / web (push) Failing after 1m26s
release / go (push) Successful in 1m35s
release / integration (push) Successful in 5m8s
release / android (push) Successful in 5m12s
release / Build signed APK (releases and dev) (push) Successful in 5m26s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Skipped
release / Verify release artifacts (tag releases only) (push) Skipped

The operator's library had 536 groups waiting for review, 2026-10-19. A sample
showed most were one recording under two titles on two releases.

- The headline and the Duplicates count leave out cross-release groups, which
  the resolver keeps and links: the headline read 3,479 when 536 needed review.
- Any group spanning albums is cross_release and is linked as one song,
  whatever its titles say. Nothing is deleted.
- One album, two versions named as such ("Lovesick (Instrumental)") and not
  byte-identical: a new class, distinct, dismissed by the resolver.
- One album, one recording under different song titles (mismatch): the
  album's cached MusicBrainz track list decides. When exactly one slot's song
  is as long as the file and every other slot's song is clearly not, the other
  copies are removed and their tracks marked missing, so the reacquisition
  sweeper wants the album back. Otherwise the group stays for review.
- The duplicates_found notice moves from the sweep to the resolver: it names
  only groups a pass first left for review.
- Resolved automatically lists dismissals and wrong-slot removals beside
  merges.

Migration 0094 adds 'distinct' to the class CHECK (rule 36).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-19 23:22:38 -04:00
co-authored by Claude Opus 5.5
parent 8538f0dc68
commit 0f58ebcd6c
22 changed files with 775 additions and 236 deletions
+43 -26
View File
@@ -6,21 +6,26 @@ import (
"unicode"
)
// Duplicate classification (M498 #5435).
// Duplicate classification (M498 #5435; settled further 2026-10-19).
//
// A duplicate group is one recording found more than once. What should happen
// to it depends on where the copies sit, and the measured library (milestone
// 498's body) splits into four shapes:
// to it depends on where the copies sit:
//
// - same_release: every copy is on one album. An album holds a song once, so
// the extra copies are the ones to fold away.
// - cross_release: the copies are on different albums under one title. A
// single and the album it came from, a deluxe edition and the standard one:
// each copy belongs to its own release, so none is deleted (unless it is a
// video rip; the resolver decides).
// - mismatch: byte-identical audio on different albums under different
// titles. One file carries another song's tags — a wrong-file import.
// - review: anything else. The operator decides.
// - same_release: every copy is on one album under one title. An album holds
// a song once, so the extra copies are the ones to fold away.
// - cross_release: the copies are on different albums. A single and the
// album it came from, a deluxe edition and the standard one, or one
// recording a later release retitled ("The Ragtime Cat", "F.O.D. (live)",
// a romanized title): each copy belongs to its own release, so none is
// deleted (unless it is a video rip; the resolver decides). The measured
// library (2026-10-19) had 451 such groups waiting on the operator under
// different titles, and they were retitles, not different songs.
// - distinct: one album, two versions of a song that sound alike, named as
// such ("Lovesick", "Lovesick (Instrumental)"). Not duplicates.
// - mismatch: one album holds the same audio under different song titles:
// one file filling several track slots, a broken download. Which slot is
// right is the resolver's to decide from MusicBrainz's track lengths.
// - review: what none of these fits. The operator decides.
// DuplicateClass is a group's shape. The values are duplicate_groups.class.
type DuplicateClass string
@@ -28,6 +33,7 @@ type DuplicateClass string
const (
ClassSameRelease DuplicateClass = "same_release"
ClassCrossRelease DuplicateClass = "cross_release"
ClassDistinct DuplicateClass = "distinct"
ClassMismatch DuplicateClass = "mismatch"
ClassReview DuplicateClass = "review"
)
@@ -44,17 +50,17 @@ type ClassifyMember struct {
//
// Titles are compared by MatchTitleKey, which drops what a release or an upload
// adds to a title (featuring credits, remaster notes, video markers) and keeps
// what names a different recording (live, demo, remix, instrumental). So one
// album holding "WWW" and "WWW (instrumental)" as an acoustic match is review,
// not a copy to fold away: the #3885 case, where they were in fact different
// recordings mislabelled. Byte-identical copies on one album are the same file
// twice whatever their tags say.
// what names a different recording (live, demo, remix, instrumental). On one
// album, titles that differ only by such a word are two versions (distinct)
// unless the bytes are identical, which is one file twice: the #3885 case of
// "WWW" and "WWW (instrumental)" holding the same file is a wrong slot.
func ClassifyDuplicateGroup(tier string, members []ClassifyMember) DuplicateClass {
if len(members) < 2 {
return ClassReview
}
oneAlbum, oneTitle := true, true
oneAlbum, oneTitle, oneSong := true, true, true
key := MatchTitleKey(members[0].Title, members[0].ArtistName)
song := songKey(members[0].Title, members[0].ArtistName)
for _, m := range members[1:] {
if m.AlbumID != members[0].AlbumID {
oneAlbum = false
@@ -62,22 +68,33 @@ func ClassifyDuplicateGroup(tier string, members []ClassifyMember) DuplicateClas
if MatchTitleKey(m.Title, m.ArtistName) != key {
oneTitle = false
}
if songKey(m.Title, m.ArtistName) != song {
oneSong = false
}
}
exact := tier == string(tierExact)
switch {
case oneAlbum && (oneTitle || exact):
return ClassSameRelease
case oneAlbum:
return ClassReview
case oneTitle:
case !oneAlbum:
return ClassCrossRelease
case exact:
return ClassMismatch
case oneTitle:
return ClassSameRelease
case oneSong && !exact:
return ClassDistinct
default:
return ClassReview
return ClassMismatch
}
}
// songKey is MatchTitleKey with every bracketed segment dropped: the song a
// title names, whatever version of it. "Lovesick (Instrumental)" and
// "Lovesick" share one.
func songKey(title, artist string) string {
if key := MatchTitleKey(bracketed.ReplaceAllString(title, " "), artist); key != "" {
return key
}
return MatchTitleKey(title, artist)
}
var (
// A bracketed credit: "(feat. Panda Bear)", "[ft. X]", "(featuring Y)".
featBracket = regexp.MustCompile(`(?i)[(\[][^)\]]*\b(feat\.?|ft\.?|featuring)\s[^)\]]*[)\]]`)
+53 -9
View File
@@ -1,6 +1,11 @@
package library
import "testing"
import (
"fmt"
"testing"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
)
// The shapes measured on the operator's library (milestone 498's body).
func TestClassifyDuplicateGroup(t *testing.T) {
@@ -17,18 +22,22 @@ func TestClassifyDuplicateGroup(t *testing.T) {
[]ClassifyMember{m("humanz", "Saturnz Barz"), m("humanz", "Saturnz Barz (Official Video)")}, ClassSameRelease},
{"one album, video upload title with artist prefix", "acoustic",
[]ClassifyMember{m("humanz", "Andromeda"), m("humanz", "Gorillaz - Andromeda (Official Audio)")}, ClassSameRelease},
{"one album, identical bytes under different titles", "exact",
[]ClassifyMember{m("www", "WWW"), m("www", "WWW (instrumental)")}, ClassSameRelease},
{"one album, acoustic match, different recordings named", "acoustic",
[]ClassifyMember{m("www", "WWW"), m("www", "WWW (instrumental)")}, ClassReview},
{"one album, identical bytes under a version title: one file in two slots", "exact",
[]ClassifyMember{m("www", "WWW"), m("www", "WWW (instrumental)")}, ClassMismatch},
{"one album, acoustic match, two versions named", "acoustic",
[]ClassifyMember{m("moe", "Lovesick"), m("moe", "Lovesick (Instrumental)")}, ClassDistinct},
{"Void: one album, one audio under three song titles", "acoustic",
[]ClassifyMember{m("void", "Slimewar (into the void)"), m("void", "Void"), m("void", "Reach")}, ClassMismatch},
{"Slasher and Slasher II are not versions of one song", "acoustic",
[]ClassifyMember{m("slasher", "Slasher"), m("slasher", "Slasher II")}, ClassMismatch},
{"Big Me: the single and the album", "exact",
[]ClassifyMember{m("single", "Big Me"), m("album", "Big Me")}, ClassCrossRelease},
{"remaster and featuring credits are one song", "acoustic",
[]ClassifyMember{m("a", "Feel Good Inc. (2017 Remaster)"), m("b", "Feel Good Inc feat. De La Soul")}, ClassCrossRelease},
{"Nervosa / Anorexia: identical bytes, different songs", "exact",
[]ClassifyMember{m("a", "Nervosa"), m("b", "Anorexia")}, ClassMismatch},
{"studio and live across albums", "acoustic",
[]ClassifyMember{m("studio", "Charger"), m("live", "Charger (live)")}, ClassReview},
{"a later release retitles the recording", "acoustic",
[]ClassifyMember{m("coco", "Ragtime Cat"), m("single", "The Ragtime Cat")}, ClassCrossRelease},
{"identical bytes on two albums under two titles", "exact",
[]ClassifyMember{m("dookie", "F.O.D."), m("live", "F.O.D. (live)")}, ClassCrossRelease},
{"a lone copy", "exact", []ClassifyMember{m("a", "Song")}, ClassReview},
}
for _, tc := range cases {
@@ -59,3 +68,38 @@ func TestMatchTitleKey(t *testing.T) {
}
}
}
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 {
out := make([]slotCopy, len(titles))
for i, title := range titles {
out[i] = slotCopy{Title: title, Artist: "Savant", DurationMs: 220000}
}
return out
}
void := []dbq.MbReleaseTrack{track("Slimewar (Into the Void)", 300000), track("Reach", 281000), track("Void", 220500)}
cases := []struct {
name string
copies []slotCopy
tracks []dbq.MbReleaseTrack
keep int
losers []int
ok bool
}{
{"Void: the file is Void's; the other two slots are other songs", copies("Slimewar (into the void)", "Void", "Reach"), void, 1, []int{0, 2}, true},
{"no track list", copies("Void", "Reach"), nil, -1, nil, false},
{"a title the track list does not hold", copies("Void", "Bonus"), void, -1, nil, false},
{"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},
{"an unknown length counts for nothing", copies("Void", "Reach"), []dbq.MbReleaseTrack{track("Void", 220000), track("Reach", 0)}, -1, nil, false},
}
for _, tc := range cases {
keep, losers, ok := wrongSlots(tc.copies, tc.tracks)
if keep != tc.keep || ok != tc.ok || fmt.Sprint(losers) != fmt.Sprint(tc.losers) {
t.Errorf("%s: got keep=%d losers=%v ok=%v, want keep=%d losers=%v ok=%v",
tc.name, keep, losers, ok, tc.keep, tc.losers, tc.ok)
}
}
}
+21 -5
View File
@@ -31,6 +31,11 @@ import (
// - 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.
// - Two versions of a song on one album that sound alike are not duplicates:
// the group is dismissed (duplicate_settle.go).
// - One file filling several of an album's track slots: MusicBrainz's track
// lengths say which slot it belongs to, and the copies in the other slots
// are removed so the album is fetched again (duplicate_settle.go).
// - Anything else waits for the operator.
const (
@@ -50,6 +55,9 @@ type DuplicateResolveResult struct {
MergeFailed int
// SongsLinked counts cross-release groups newly linked as one song.
SongsLinked int
// Dismissed counts groups of distinct versions dismissed; WrongFiles the
// copies removed from track slots that were not theirs.
Dismissed, WrongFiles int
// HeldBack counts tracks newly held back from radio and the mixes as video
// rips; Released counts held-back tracks whose name no longer says so.
HeldBack, Released int64
@@ -96,7 +104,7 @@ func mergePlan(g *resolveGroup) (ok bool, survivor int) {
switch g.class {
case ClassSameRelease:
return true, -1
case ClassCrossRelease, ClassMismatch:
case ClassCrossRelease:
if i := g.lone(); i >= 0 {
return true, i
}
@@ -136,6 +144,7 @@ func ResolveDuplicates(
}
if !act {
notifyNewForReview(ctx, groups, nil)
return res, nil
}
@@ -172,17 +181,23 @@ func ResolveDuplicates(
}
}
res.Dismissed = dismissDistinct(ctx, pool, logger, groups, merged)
wrong, names := settleWrongSlots(ctx, pool, logger, groups, merged)
res.WrongFiles = wrong
mergedNames = append(mergedNames, names...)
linked, err := linkCrossRelease(ctx, pool, logger, groups, merged)
res.SongsLinked = linked
if err != nil {
return res, err
}
if res.Merged > 0 {
if settledCopies := res.Merged + res.WrongFiles; settledCopies > 0 {
notifyAdmins(ctx, notifications.KindDuplicatesResolved, notifications.Payload{
Count: int64(res.Merged), Items: mergedNames,
Count: int64(settledCopies), Items: mergedNames[:min(len(mergedNames), notifyItems)],
})
}
notifyNewForReview(ctx, groups, merged)
return res, nil
}
@@ -276,7 +291,7 @@ func resolveNote(g *resolveGroup) string {
return "Each copy belongs to its own release, so none is removed; they count as one song."
}
case ClassMismatch:
return "Identical audio filed under different titles: one file carries another song's tags."
return "One recording fills several of this album's track slots, and MusicBrainz's track lengths could not say which is its own."
}
return ""
}
@@ -409,5 +424,6 @@ func (w *DuplicateResolveWorker) tickOnce(ctx context.Context) {
}
w.logger.Info("duplicate resolve complete",
"groups", res.Groups, "merged", res.Merged, "merge_failed", res.MergeFailed,
"songs_linked", res.SongsLinked, "held_back", res.HeldBack, "released", res.Released)
"songs_linked", res.SongsLinked, "dismissed", res.Dismissed, "wrong_files", res.WrongFiles,
"held_back", res.HeldBack, "released", res.Released)
}
+163 -7
View File
@@ -242,9 +242,10 @@ func TestResolveDuplicates_KeepsCleanCopiesOnTwoReleases_Integration(t *testing.
}
}
// Identical audio under two titles on two albums, neither a rip: which tags
// are wrong is the operator's call, so nothing is merged or linked.
func TestResolveDuplicates_LeavesAMismatchForTheOperator_Integration(t *testing.T) {
// Identical audio under two titles on two albums, neither a rip: a later
// release retitled the recording ("F.O.D." and "F.O.D. (live)"). Each copy
// belongs to its release, so nothing is removed and they count as one song.
func TestResolveDuplicates_LinksOneRecordingUnderTwoTitles_Integration(t *testing.T) {
f := newCrossReleaseRipFixture(t)
ctx := context.Background()
if _, err := f.pool.Exec(ctx, `UPDATE duplicate_groups SET tier = 'exact' WHERE id = $1`, f.groupID); err != nil {
@@ -258,11 +259,166 @@ func TestResolveDuplicates_LeavesAMismatchForTheOperator_Integration(t *testing.
if err != nil {
t.Fatal(err)
}
if res.Classes[ClassMismatch] != 1 || res.Merged != 0 || res.SongsLinked != 0 {
t.Fatalf("classes %v, merged %d, linked %d; want a mismatch left alone", res.Classes, res.Merged, res.SongsLinked)
if res.Classes[ClassCrossRelease] != 1 || res.Merged != 0 || res.SongsLinked != 1 {
t.Fatalf("classes %v, merged %d, linked %d; want the pair linked and kept", res.Classes, res.Merged, res.SongsLinked)
}
if status, _, note, _ := (resolveFixture{pool: f.pool, groupID: f.groupID}).group(t); status != "pending" || note == nil || *note == "" {
t.Errorf("group = (%s, note %v), want pending with a note", status, note)
}
// oneAlbumFixture is one album holding two tracks in one pending group, each
// with a file on disk.
type oneAlbumFixture struct {
pool *pgxpool.Pool
album dbq.Album
a, b dbq.Track
aPath string
bPath string
groupID pgtype.UUID
}
func newOneAlbumFixture(t *testing.T, tier, titleA, titleB string) oneAlbumFixture {
t.Helper()
pool := newPool(t)
ctx := context.Background()
q := dbq.New(pool)
dir := filepath.Join(t.TempDir(), "Savant", "Void")
if err := os.MkdirAll(dir, 0o755); err != nil {
t.Fatal(err)
}
f := oneAlbumFixture{pool: pool, aPath: filepath.Join(dir, "15 - a.m4a"), bPath: filepath.Join(dir, "17 - b.m4a")}
for _, p := range []string{f.aPath, f.bPath} {
if err := os.WriteFile(p, []byte("audio"), 0o644); err != nil {
t.Fatal(err)
}
}
var artist dbq.Artist
f.a, f.album, artist = seedTrack(t, pool, f.aPath)
b, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{
Title: titleB, AlbumID: f.album.ID, ArtistID: artist.ID,
DurationMs: 220000, FilePath: f.bPath, FileSize: 100, FileFormat: "m4a",
})
if err != nil {
t.Fatal(err)
}
f.b = b
if _, err := pool.Exec(ctx, `UPDATE tracks SET title = $2, duration_ms = 220000 WHERE id = $1`, f.a.ID, titleA); err != nil {
t.Fatal(err)
}
if err := pool.QueryRow(ctx,
`INSERT INTO duplicate_groups (member_key, tier) VALUES ($1, $2) RETURNING id`, t.Name(), tier,
).Scan(&f.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)`,
f.groupID, f.a.ID, f.b.ID); err != nil {
t.Fatal(err)
}
return f
}
func auditCount(t *testing.T, pool *pgxpool.Pool, action string, groupID pgtype.UUID) int {
t.Helper()
var n int
if err := pool.QueryRow(context.Background(),
`SELECT count(*) FROM audit_log WHERE action = $1 AND metadata->>'group_id' = $2`,
action, syncpkg.FormatUUID(groupID)).Scan(&n); err != nil {
t.Fatal(err)
}
return n
}
// Two versions of a song on one album that sound alike are not duplicates:
// the group is dismissed and both files stay.
func TestResolveDuplicates_DismissesTwoVersionsOfASong_Integration(t *testing.T) {
f := newOneAlbumFixture(t, "acoustic", "Lovesick", "Lovesick (Instrumental)")
res, err := ResolveDuplicates(context.Background(), f.pool, nil, "", true)
if err != nil {
t.Fatal(err)
}
if res.Dismissed != 1 || res.Merged != 0 || !exists(f.aPath) || !exists(f.bPath) {
t.Fatalf("result %+v; want the group dismissed and both files kept", res)
}
if status, _, _, auto := (resolveFixture{pool: f.pool, groupID: f.groupID}).group(t); status != "dismissed" || !auto {
t.Errorf("group = (%s, auto=%v), want dismissed by the resolver", status, auto)
}
if n := auditCount(t, f.pool, "duplicate_dismiss", f.groupID); n != 1 {
t.Errorf("dismiss audit rows = %d, want 1", n)
}
}
// seedTrackList holds a release group whose one release lists the songs with
// their lengths, and points the album at it.
func seedTrackList(t *testing.T, pool *pgxpool.Pool, album dbq.Album, songs map[string]int) {
t.Helper()
ctx := context.Background()
rg := "rg-" + syncpkg.FormatUUID(album.ID)
rel := "rel-" + syncpkg.FormatUUID(album.ID)
for _, stmt := range []struct {
sql string
args []any
}{
{`INSERT INTO mb_release_groups (mbid, title) VALUES ($1, 'Void')`, []any{rg}},
{`INSERT INTO mb_releases (mbid, release_group_mbid, title) VALUES ($1, $2, 'Void')`, []any{rel, rg}},
{`UPDATE albums SET release_group_mbid = $2 WHERE id = $1`, []any{album.ID, rg}},
} {
if _, err := pool.Exec(ctx, stmt.sql, stmt.args...); err != nil {
t.Fatalf("exec %q: %v", stmt.sql, err)
}
}
pos := 1
for title, ms := range songs {
if _, err := pool.Exec(ctx,
`INSERT INTO mb_release_tracks (release_mbid, medium_position, position, title, length_ms) VALUES ($1, 1, $2, $3, $4)`,
rel, pos, title, ms); err != nil {
t.Fatal(err)
}
pos++
}
}
// One recording in two slots of an album: MusicBrainz says Void is as long as
// the file and Reach is a minute longer, so the copy in Reach's slot goes. Its
// track stays, marked missing, for the album to be fetched again.
func TestResolveDuplicates_RemovesACopyFromAnotherSongsSlot_Integration(t *testing.T) {
f := newOneAlbumFixture(t, "acoustic", "Void", "Reach")
seedTrackList(t, f.pool, f.album, map[string]int{"Void": 220500, "Reach": 281000})
res, err := ResolveDuplicates(context.Background(), f.pool, nil, "", true)
if err != nil {
t.Fatal(err)
}
if res.WrongFiles != 1 || !exists(f.aPath) || exists(f.bPath) {
t.Fatalf("result %+v, void exists %v, reach exists %v; want only Reach's copy removed",
res, exists(f.aPath), exists(f.bPath))
}
var missing bool
if err := f.pool.QueryRow(context.Background(),
`SELECT missing_since IS NOT NULL FROM tracks WHERE id = $1`, f.b.ID).Scan(&missing); err != nil {
t.Fatalf("Reach's track is gone: %v", err)
}
if !missing {
t.Error("Reach's track is not marked missing, so nothing will fetch it again")
}
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)
}
}
// Without a track list that decides, nothing is removed.
func TestResolveDuplicates_LeavesAnUndecidedSlotForTheOperator_Integration(t *testing.T) {
f := newOneAlbumFixture(t, "acoustic", "Void", "Reach")
seedTrackList(t, f.pool, f.album, map[string]int{"Void": 220500, "Reach": 221000})
res, err := ResolveDuplicates(context.Background(), f.pool, nil, "", true)
if err != nil {
t.Fatal(err)
}
if res.WrongFiles != 0 || !exists(f.aPath) || !exists(f.bPath) {
t.Fatalf("result %+v; want both copies kept when the lengths cannot tell", res)
}
if status, class, _, _ := (resolveFixture{pool: f.pool, groupID: f.groupID}).group(t); status != "pending" ||
class == nil || *class != string(ClassMismatch) {
t.Errorf("group = (%s, %v), want a pending mismatch", status, class)
}
}
+262
View File
@@ -0,0 +1,262 @@
package library
import (
"context"
"errors"
"fmt"
"log/slog"
"os"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgtype"
"github.com/jackc/pgx/v5/pgxpool"
"git.fabledsword.com/bvandeusen/minstrel/internal/audit"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
"git.fabledsword.com/bvandeusen/minstrel/internal/notifications"
syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync"
)
// Duplicates that settle without a merge (M498, operator 2026-10-19: "this
// needs to be automated away"). The measured library had 536 groups waiting
// on the operator; most were retitles across releases, now linked like any
// cross-release group. What is left on one album is two shapes:
//
// - distinct: two versions of a song that sound alike ("Lovesick" and
// "Lovesick (Instrumental)"). Not duplicates, so the group is dismissed.
// - mismatch: one file filling several track slots (Savant's Void held one
// recording as tracks 15, 17 and 20). The album's MusicBrainz track list
// says how long each slot's song is. When exactly one slot's song is as
// 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.
// Anything less certain stays with the operator.
const (
// slotFit is how close a file must be to a slot's MusicBrainz length to be
// that slot's song, and slotOther how far away every length of a slot's
// title must be for the file to be another song's.
slotFit = 3000
slotOther = 10000
)
// slotCopy is what deciding a wrong slot needs to know about one copy.
type slotCopy struct {
Title, Artist string
DurationMs int32
}
// 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.
func wrongSlots(copies []slotCopy, tracks []dbq.MbReleaseTrack) (keep int, losers []int, ok bool) {
keep = -1
for i, c := range copies {
key := MatchTitleKey(c.Title, c.Artist)
fits, other, known := false, true, false
for _, t := range tracks {
if t.LengthMs <= 0 || MatchTitleKey(t.Title, c.Artist) != key {
continue
}
known = true
d := absMs(t.LengthMs - c.DurationMs)
if d <= slotFit {
fits = true
}
if d <= slotOther {
other = false
}
}
switch {
case fits:
if keep >= 0 {
return -1, nil, false
}
keep = i
case known && other:
losers = append(losers, i)
default:
return -1, nil, false
}
}
if keep < 0 || len(losers) == 0 {
return -1, nil, false
}
return keep, losers, true
}
func absMs(d int32) int32 {
if d < 0 {
return -d
}
return d
}
// dismissDistinct dismisses every group of distinct versions and records it
// for the Resolved automatically tab. It adds what it settled to settled.
func dismissDistinct(
ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, groups []*resolveGroup, settled map[pgtype.UUID]bool,
) int {
q := dbq.New(pool)
n := 0
for _, g := range groups {
if g.class != ClassDistinct || settled[g.id] || ctx.Err() != nil {
continue
}
rows, err := q.SettleDuplicateGroupAutomatically(ctx, dbq.SettleDuplicateGroupAutomaticallyParams{
ID: g.id, Status: "dismissed",
})
if err != nil {
logger.Warn("duplicate resolve: dismiss failed", "group_id", syncpkg.FormatUUID(g.id), "err", err)
continue
}
if rows == 0 {
continue
}
settled[g.id] = true
n++
titles := make([]string, len(g.members))
for i, m := range g.members {
titles[i] = m.Title
}
audit.WriteOrLog(ctx, pool, logger, pgtype.UUID{}, pgtype.UUID{}, audit.ActionDuplicateDismiss, map[string]any{
"automatic": true,
"group_id": syncpkg.FormatUUID(g.id),
"artist": g.members[0].ArtistName,
"titles": titles,
})
}
return n
}
// settleWrongSlots removes the copies that sit in another song's slot, for
// every one-album mismatch the track list decides. It returns how many copies
// went and the songs whose slots were emptied, and adds the groups it settled
// to settled.
func settleWrongSlots(
ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, groups []*resolveGroup, settled map[pgtype.UUID]bool,
) (int, []string) {
q := dbq.New(pool)
tracksByGroup := map[string][]dbq.MbReleaseTrack{}
removed := 0
var names []string
for _, g := range groups {
if g.class != ClassMismatch || settled[g.id] || ctx.Err() != nil {
continue
}
rg := g.members[0].ReleaseGroupMbid
if rg == "" {
continue
}
tracks, seen := tracksByGroup[rg]
if !seen {
var err error
if tracks, err = q.ListMBReleaseTracksForGroup(ctx, rg); err != nil {
logger.Warn("duplicate resolve: reading the album's track list failed", "release_group", rg, "err", err)
continue
}
tracksByGroup[rg] = tracks
}
copies := make([]slotCopy, len(g.members))
for i, m := range g.members {
copies[i] = slotCopy{Title: m.Title, Artist: m.ArtistName, DurationMs: m.DurationMs}
}
keep, losers, ok := wrongSlots(copies, tracks)
if !ok {
continue
}
n, err := removeWrongSlots(ctx, pool, logger, g, keep, losers)
removed += n
if err != nil {
logger.Warn("duplicate resolve: removing wrong-slot copies failed", "group_id", syncpkg.FormatUUID(g.id), "err", err)
continue
}
settled[g.id] = true
for _, i := range losers {
names = append(names, TrackLabel(g.members[i].ArtistName, g.members[i].Title))
}
}
return removed, names
}
// removeWrongSlots deletes each loser's file, then marks its track missing in
// the same transaction that closes the group. The row is kept, with its likes
// and plays: it is the slot, and a good copy fetched again takes it over. A
// file that cannot be removed stops the group; the ones already gone are still
// marked, and the group waits for the next pass.
func removeWrongSlots(
ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, g *resolveGroup, keep int, losers []int,
) (int, error) {
var gone []pgtype.UUID
var removedFiles []map[string]string
var removeErr error
for _, i := range losers {
m := g.members[i]
if err := removeFile(m.FilePath); err != nil && !errors.Is(err, os.ErrNotExist) {
removeErr = fmt.Errorf("remove %s: %w", m.FilePath, err)
break
}
gone = append(gone, m.TrackID)
removedFiles = append(removedFiles, map[string]string{
"track_id": syncpkg.FormatUUID(m.TrackID), "file_path": m.FilePath, "title": m.Title,
})
}
if len(gone) == 0 {
return 0, removeErr
}
err := pgx.BeginFunc(ctx, pool, func(tx pgx.Tx) error {
tq := dbq.New(tx)
if _, err := tq.MarkTracksMissing(ctx, gone); err != nil {
return fmt.Errorf("mark missing: %w", err)
}
if err := logTrackChanges(ctx, tq, gone); err != nil {
return fmt.Errorf("log changes: %w", err)
}
if removeErr != nil {
return nil
}
_, err := tq.SettleDuplicateGroupAutomatically(ctx, dbq.SettleDuplicateGroupAutomaticallyParams{
ID: g.id, Status: "merged",
})
return err
})
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,
})
logger.Info("duplicate resolve: removed copies from slots that were not theirs",
"group_id", syncpkg.FormatUUID(g.id), "kept", kept.FilePath, "removed", len(gone))
return len(gone), removeErr
}
// notifyNewForReview tells admins about groups this pass classified for the
// first time and left for them. A group the resolver settled, linked or will
// link is not news: telling admins about those was most of what the notice
// used to say.
func notifyNewForReview(ctx context.Context, groups []*resolveGroup, settled map[pgtype.UUID]bool) {
var count int64
var items []string
for _, g := range groups {
if g.members[0].PriorClass != nil || settled[g.id] {
continue
}
if g.class != ClassReview && g.class != ClassMismatch {
continue
}
count++
if len(items) < notifyItems {
items = append(items, TrackLabel(g.members[0].ArtistName, g.members[0].Title))
}
}
if count > 0 {
notifyAdmins(ctx, notifications.KindDuplicatesFound, notifications.Payload{Count: count, Items: items})
}
}
-27
View File
@@ -14,7 +14,6 @@ import (
"github.com/jackc/pgx/v5/pgxpool"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
"git.fabledsword.com/bvandeusen/minstrel/internal/notifications"
syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync"
)
@@ -102,38 +101,12 @@ func runDuplicateSweep(
}
}
if runErr == nil {
notifyNewDuplicates(finishCtx, q, sweep.StartedAt, logger)
}
logger.Info("duplicate sweep complete",
"candidates", res.Candidates, "groups", res.Groups, "proposed", res.Proposed,
"suppressed", res.Suppressed, "retired", res.Retired, "oversize", res.Oversize, "err", runErr)
return res, runErr
}
// notifyNewDuplicates tells admins when a sweep proposed groups it had not
// proposed before (M489), counting and naming those (M542). A sweep that only
// re-finds known groups says nothing, so reading the notice once is enough
// until something new turns up.
func notifyNewDuplicates(ctx context.Context, q *dbq.Queries, sweepStarted pgtype.Timestamptz, logger *slog.Logger) {
fresh, err := q.ListDuplicateGroupsDetectedSince(ctx, dbq.ListDuplicateGroupsDetectedSinceParams{
Since: sweepStarted, N: notifyItems,
})
if err != nil {
logger.Warn("duplicate sweep: listing new proposals failed", "err", err)
return
}
if len(fresh) == 0 {
return
}
items := make([]string, 0, len(fresh))
for _, g := range fresh {
items = append(items, TrackLabel(g.ArtistName, g.Title))
}
notifyAdmins(ctx, notifications.KindDuplicatesFound, notifications.Payload{Count: fresh[0].Total, Items: items})
}
func sweepDuplicates(
ctx context.Context, q *dbq.Queries, sweepID pgtype.UUID, cfg FingerprintSettings, pageSize int32,
) (DuplicateSweepResult, error) {
+35 -50
View File
@@ -1,7 +1,6 @@
package library
import (
"bytes"
"context"
"encoding/json"
"fmt"
@@ -155,68 +154,54 @@ func TestNotifyStillMissing_NamesOnlyTheTracksStillGone(t *testing.T) {
}
}
func TestDuplicateSweep_NotifiesOnlyWhenSomethingNewIsProposed(t *testing.T) {
// Admins hear about a duplicate group once, when a resolver pass first leaves
// it for them. A group the resolver settles or links is never news, and a
// group already seen by an earlier pass is not news again.
func TestResolveDuplicates_TellsAdminsOnlyOfNewGroupsLeftForThem(t *testing.T) {
pool := newPool(t)
admin := notifyingAdmin(t, pool)
ctx := context.Background()
q := dbq.New(pool)
dir := t.TempDir()
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
_, album, artist := seedTrack(t, pool, filepath.Join(dir, "seed.mp3"))
// Same audio-stream hash: an exact pair, whatever the prints say.
pair := func(name string, b byte, seed uint64) {
t.Helper()
for i := 1; i <= 2; i++ {
tr, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{
Title: name, AlbumID: album.ID, ArtistID: artist.ID, DurationMs: 200000,
FilePath: filepath.Join(dir, fmt.Sprintf("%s-%d.mp3", name, i)), FileSize: 100, FileFormat: "mp3",
})
if err != nil {
t.Fatalf("track %s: %v", name, err)
}
if err := q.UpsertTrackFingerprint(ctx, dbq.UpsertTrackFingerprintParams{
TrackID: tr.ID, AudioStreamSha256: bytes.Repeat([]byte{b}, 32),
Chromaprint: randomPrint(seed+uint64(i), printLen), FingerprintVersion: fingerprintVersion,
ChromaprintLengthSec: defaultChromaprintLengthSec,
}); err != nil {
t.Fatalf("fingerprint %s: %v", name, err)
}
}
first, album, artist := seedTrack(t, pool, filepath.Join(dir, "a.mp3"))
if _, err := pool.Exec(ctx, `UPDATE tracks SET title = 'Alpha' WHERE id = $1`, first.ID); err != nil {
t.Fatal(err)
}
sweep := func() {
t.Helper()
if _, err := runDuplicateSweep(ctx, pool, logger, DefaultFingerprintSettings, duplicateCandidatePage); err != nil {
t.Fatalf("sweep: %v", err)
}
second, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{
Title: "Beta", AlbumID: album.ID, ArtistID: artist.ID,
DurationMs: 1000, FilePath: filepath.Join(dir, "b.mp3"), FileSize: 100, FileFormat: "mp3",
})
if err != nil {
t.Fatal(err)
}
markAllRead := func() {
t.Helper()
if _, err := q.MarkAllNotificationsRead(ctx, dbq.MarkAllNotificationsReadParams{UserID: admin.ID}); err != nil {
t.Fatalf("mark read: %v", err)
}
// One album, one recording under two song titles, no track list to decide.
var groupID pgtype.UUID
if err := pool.QueryRow(ctx,
`INSERT INTO duplicate_groups (member_key, tier) VALUES ('notify-review', '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)`,
groupID, first.ID, second.ID); err != nil {
t.Fatal(err)
}
pair("first", 1, 100)
sweep()
if _, err := ResolveDuplicates(ctx, pool, nil, "", true); err != nil {
t.Fatal(err)
}
got := unreadNotices(t, pool, admin)
if len(got) != 1 || got[0].kind != string(notifications.KindDuplicatesFound) || got[0].count != 1 ||
len(got[0].items) != 1 || got[0].items[0] != "Delete Test Artist – first" {
t.Fatalf("after the first sweep notices = %+v, want one duplicates_found naming the first pair", got)
len(got[0].items) != 1 || got[0].items[0] == "" {
t.Fatalf("after the first pass notices = %+v, want one duplicates_found naming the group", got)
}
// Read, then swept again with nothing new: no reminder.
markAllRead()
sweep()
if _, err := q.MarkAllNotificationsRead(ctx, dbq.MarkAllNotificationsReadParams{UserID: admin.ID}); err != nil {
t.Fatal(err)
}
if _, err := ResolveDuplicates(ctx, pool, nil, "", true); err != nil {
t.Fatal(err)
}
if got := unreadNotices(t, pool, admin); len(got) != 0 {
t.Fatalf("a sweep that found nothing new notified: %+v", got)
}
// A new pair is news, and the notice is about it alone: the first was read.
pair("second", 2, 200)
sweep()
got = unreadNotices(t, pool, admin)
if len(got) != 1 || got[0].count != 1 || len(got[0].items) != 1 || got[0].items[0] != "Delete Test Artist – second" {
t.Fatalf("after a new pair notices = %+v, want one naming only the new pair", got)
t.Fatalf("a group an earlier pass saw was told again: %+v", got)
}
}