From 865a3176c9ba11818e8ec669ae21fc3069e42446 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 7 Oct 2026 17:23:32 -0400 Subject: [PATCH] feat(library): fold a missing track into its on-disk replacement (M485 #5286 #5287) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A track marked missing whose replacement is already on disk under another row — on the operator's library 355 of 451 missing tracks, nearly all Lidarr mp3 -> flac upgrades — is folded into the replacement: likes, plays, playlist entries and tags move across and the missing row goes. Move adoption could not catch these: the replacements were re-encodes (no shared audio hash) with no recording MBID at import. Pairs (ListMissingTrackPairs): the same recording MBID within the album group, or the same album row and title ignoring case. Each side must have exactly one candidate; conflicting MBIDs refuse a pair. No duration or track position gate: on the 142 pairs known to be one recording, 18% differed by over 2s and the poorly tagged set is where numbering is broken (spike #5274). Each pair folds in its own transaction after locking both rows and checking the pair still holds. Runs automatically (operator, 2026-10-07) after a full scan, after a watcher batch that added or updated tracks, and after an AcoustID pass that matched any track. The first scan after deploy repairs the existing rows. Co-Authored-By: Claude Opus 5.5 --- internal/db/dbq/missing_pairs.sql.go | 153 ++++++++++++ internal/db/queries/missing_pairs.sql | 75 ++++++ internal/library/acoustid_lookup.go | 8 + internal/library/missing_pairs.go | 151 ++++++++++++ internal/library/missing_pairs_test.go | 314 +++++++++++++++++++++++++ internal/library/scanner.go | 13 + 6 files changed, 714 insertions(+) create mode 100644 internal/db/dbq/missing_pairs.sql.go create mode 100644 internal/db/queries/missing_pairs.sql create mode 100644 internal/library/missing_pairs.go create mode 100644 internal/library/missing_pairs_test.go diff --git a/internal/db/dbq/missing_pairs.sql.go b/internal/db/dbq/missing_pairs.sql.go new file mode 100644 index 00000000..07042637 --- /dev/null +++ b/internal/db/dbq/missing_pairs.sql.go @@ -0,0 +1,153 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 +// source: missing_pairs.sql + +package dbq + +import ( + "context" + + "github.com/jackc/pgx/v5/pgtype" +) + +const listMissingTrackPairs = `-- name: ListMissingTrackPairs :many + +WITH grp AS ( + SELECT id AS album_id, COALESCE(release_group_mbid, id::text) AS g + FROM albums +), +cand AS ( + SELECT m.id AS missing_id, p.id AS present_id, 'mbid'::text AS rule + FROM tracks m + JOIN grp gm ON gm.album_id = m.album_id + JOIN grp gp ON gp.g = gm.g + JOIN tracks p ON p.album_id = gp.album_id + AND p.missing_since IS NULL + AND p.mbid = m.mbid + WHERE m.missing_since IS NOT NULL + AND m.mbid IS NOT NULL + UNION ALL + SELECT m.id, p.id, 'title'::text + FROM tracks m + JOIN tracks p ON p.album_id = m.album_id + AND p.missing_since IS NULL + AND lower(p.title) = lower(m.title) + WHERE m.missing_since IS NOT NULL + AND (m.mbid IS NULL OR p.mbid IS NULL OR m.mbid = p.mbid) +), +pairs AS ( + -- A pair both rules found is one candidate, credited to the stronger rule. + SELECT missing_id, present_id, min(rule)::text AS rule + FROM cand + GROUP BY missing_id, present_id +) +SELECT pr.missing_id, pr.present_id, pr.rule, + m.file_path AS missing_path, p.file_path AS present_path, + (count(*) OVER (PARTITION BY pr.missing_id))::int AS missing_candidates, + (count(*) OVER (PARTITION BY pr.present_id))::int AS present_candidates + FROM pairs pr + JOIN tracks m ON m.id = pr.missing_id + JOIN tracks p ON p.id = pr.present_id + ORDER BY m.file_path, p.file_path +` + +type ListMissingTrackPairsRow struct { + MissingID pgtype.UUID + PresentID pgtype.UUID + Rule string + MissingPath string + PresentPath string + MissingCandidates int32 + PresentCandidates int32 +} + +// Missing-pair pass (M485). A track marked missing whose replacement is already +// on disk under another row — most often a Lidarr quality upgrade, mp3 → flac at +// a new path — is folded into that replacement, so its history moves across and +// re-acquisition stops chasing music the library already has. +// +// Move adoption (#2528) cannot catch these: it links a new file to a missing row +// at the moment the file is first inserted, and the replacement arrives as a +// re-encode with no recording MBID, so it has nothing to match on. AcoustID may +// give it an MBID later; by then both rows exist and only a fold can join them. +// Every candidate (missing, present) pair, with how many candidates each side +// has. Two rules, measured on the operator's library on 2026-10-07 (#5274): +// +// mbid the same recording MBID, within the album group (the album row, or +// albums sharing a release group). 142 of 359 pairs. +// title the same album row and the same title, ignoring case. 213 more. The +// replacements are often not the original file — another edition, +// re-encoded, untagged — so the metadata is what is left to go on. +// +// Deliberately NOT gated on duration or track position. On the 142 pairs known +// to be one recording, 18% differed by over 2s (old mp3 durations are poor +// estimates), and the poorly tagged set is exactly where numbering is broken. +// +// A pair whose MBIDs are both set and differ is not a title match: the tags say +// they are different recordings. The caller folds a pair only when each side has +// exactly one candidate; anything else is ambiguous and left alone. +func (q *Queries) ListMissingTrackPairs(ctx context.Context) ([]ListMissingTrackPairsRow, error) { + rows, err := q.db.Query(ctx, listMissingTrackPairs) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListMissingTrackPairsRow + for rows.Next() { + var i ListMissingTrackPairsRow + if err := rows.Scan( + &i.MissingID, + &i.PresentID, + &i.Rule, + &i.MissingPath, + &i.PresentPath, + &i.MissingCandidates, + &i.PresentCandidates, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const lockTracksForPairing = `-- name: LockTracksForPairing :many +SELECT id, missing_since + FROM tracks + WHERE id = ANY($1::uuid[]) + ORDER BY id + FOR UPDATE +` + +type LockTracksForPairingRow struct { + ID pgtype.UUID + MissingSince pgtype.Timestamptz +} + +// Locks both rows of a pair for the fold's transaction, in id order so two +// passes cannot deadlock, and returns what the fold re-checks: the missing row +// must still be missing and the present one still present. A scan may have +// restored or adopted the first, or marked the second, since the pairs were listed. +func (q *Queries) LockTracksForPairing(ctx context.Context, ids []pgtype.UUID) ([]LockTracksForPairingRow, error) { + rows, err := q.db.Query(ctx, lockTracksForPairing, ids) + if err != nil { + return nil, err + } + defer rows.Close() + var items []LockTracksForPairingRow + for rows.Next() { + var i LockTracksForPairingRow + if err := rows.Scan(&i.ID, &i.MissingSince); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} diff --git a/internal/db/queries/missing_pairs.sql b/internal/db/queries/missing_pairs.sql new file mode 100644 index 00000000..322a5dfb --- /dev/null +++ b/internal/db/queries/missing_pairs.sql @@ -0,0 +1,75 @@ +-- Missing-pair pass (M485). A track marked missing whose replacement is already +-- on disk under another row — most often a Lidarr quality upgrade, mp3 → flac at +-- a new path — is folded into that replacement, so its history moves across and +-- re-acquisition stops chasing music the library already has. +-- +-- Move adoption (#2528) cannot catch these: it links a new file to a missing row +-- at the moment the file is first inserted, and the replacement arrives as a +-- re-encode with no recording MBID, so it has nothing to match on. AcoustID may +-- give it an MBID later; by then both rows exist and only a fold can join them. + +-- name: ListMissingTrackPairs :many +-- Every candidate (missing, present) pair, with how many candidates each side +-- has. Two rules, measured on the operator's library on 2026-10-07 (#5274): +-- +-- mbid the same recording MBID, within the album group (the album row, or +-- albums sharing a release group). 142 of 359 pairs. +-- title the same album row and the same title, ignoring case. 213 more. The +-- replacements are often not the original file — another edition, +-- re-encoded, untagged — so the metadata is what is left to go on. +-- +-- Deliberately NOT gated on duration or track position. On the 142 pairs known +-- to be one recording, 18% differed by over 2s (old mp3 durations are poor +-- estimates), and the poorly tagged set is exactly where numbering is broken. +-- +-- A pair whose MBIDs are both set and differ is not a title match: the tags say +-- they are different recordings. The caller folds a pair only when each side has +-- exactly one candidate; anything else is ambiguous and left alone. +WITH grp AS ( + SELECT id AS album_id, COALESCE(release_group_mbid, id::text) AS g + FROM albums +), +cand AS ( + SELECT m.id AS missing_id, p.id AS present_id, 'mbid'::text AS rule + FROM tracks m + JOIN grp gm ON gm.album_id = m.album_id + JOIN grp gp ON gp.g = gm.g + JOIN tracks p ON p.album_id = gp.album_id + AND p.missing_since IS NULL + AND p.mbid = m.mbid + WHERE m.missing_since IS NOT NULL + AND m.mbid IS NOT NULL + UNION ALL + SELECT m.id, p.id, 'title'::text + FROM tracks m + JOIN tracks p ON p.album_id = m.album_id + AND p.missing_since IS NULL + AND lower(p.title) = lower(m.title) + WHERE m.missing_since IS NOT NULL + AND (m.mbid IS NULL OR p.mbid IS NULL OR m.mbid = p.mbid) +), +pairs AS ( + -- A pair both rules found is one candidate, credited to the stronger rule. + SELECT missing_id, present_id, min(rule)::text AS rule + FROM cand + GROUP BY missing_id, present_id +) +SELECT pr.missing_id, pr.present_id, pr.rule, + m.file_path AS missing_path, p.file_path AS present_path, + (count(*) OVER (PARTITION BY pr.missing_id))::int AS missing_candidates, + (count(*) OVER (PARTITION BY pr.present_id))::int AS present_candidates + FROM pairs pr + JOIN tracks m ON m.id = pr.missing_id + JOIN tracks p ON p.id = pr.present_id + ORDER BY m.file_path, p.file_path; + +-- name: LockTracksForPairing :many +-- Locks both rows of a pair for the fold's transaction, in id order so two +-- passes cannot deadlock, and returns what the fold re-checks: the missing row +-- must still be missing and the present one still present. A scan may have +-- restored or adopted the first, or marked the second, since the pairs were listed. +SELECT id, missing_since + FROM tracks + WHERE id = ANY(sqlc.arg(ids)::uuid[]) + ORDER BY id + FOR UPDATE; diff --git a/internal/library/acoustid_lookup.go b/internal/library/acoustid_lookup.go index 272cccef..0f4dfa15 100644 --- a/internal/library/acoustid_lookup.go +++ b/internal/library/acoustid_lookup.go @@ -215,6 +215,14 @@ func (w *AcoustIDLookupWorker) runOnce(ctx context.Context) { "processed", res.Processed, "matched", res.Matched, "ambiguous", res.Ambiguous, "no_match", res.NoMatch, "failed", res.Failed, "inconclusive", res.Inconclusive) } + // A recording MBID AcoustID just gave a replacement may be the one its + // missing predecessor carried (M485): most of the operator's stale rows + // became pairable exactly this way. + if res.Matched > 0 && ctx.Err() == nil { + if _, err := PairMissingTracks(ctx, w.pool, w.logger); err != nil && ctx.Err() == nil { + w.logger.Warn("acoustid lookup: missing-pair pass failed", "err", err) + } + } } func (w *AcoustIDLookupWorker) setStatus(f func(*AcoustIDLookupStatus)) { diff --git a/internal/library/missing_pairs.go b/internal/library/missing_pairs.go new file mode 100644 index 00000000..9d91d624 --- /dev/null +++ b/internal/library/missing_pairs.go @@ -0,0 +1,151 @@ +package library + +import ( + "context" + "errors" + "fmt" + "log/slog" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// Missing-pair folding (M485). +// +// A track marked missing whose replacement already sits on disk under another +// row is folded into that replacement: its likes, plays, playlist entries and +// tags move across and the missing row goes. On the operator's library this was +// 355 of 451 missing tracks (#5274), nearly all Lidarr quality upgrades — the +// mp3 removed, a flac imported at a new path. +// +// Move adoption (moved.go) can't catch these. It links a new file to a missing +// row when the file is first inserted, matching on the recording MBID or the +// audio hash. A re-encode never shares the hash, and the replacements arrived +// untagged, so there was nothing to match; AcoustID filled many of the MBIDs +// weeks later, by which time both rows existed. Adoption keeps the old row and +// re-points its path, which only works while the new file has no row of its own. +// Here both do, so this folds instead, with the duplicate merge's row half. +// +// The rules are in ListMissingTrackPairs. Only a pair where each side has exactly +// one candidate is folded: a wrong fold can't be undone, a skipped one is only a +// row left in the missing list. +// +// Unlike the duplicate merge this runs without review (operator, 2026-10-07): +// no file is touched, the missing row has nothing to play, and adoption, its +// closest relative, is automatic too. Each fold is logged with both paths so a +// wrong pair can be read back. + +// PairResult tallies one pass. +type PairResult struct { + ByMbid int // folded on a shared recording MBID + ByTitle int // folded on the same album and title + Ambiguous int // missing rows with more than one candidate, or a candidate claimed twice + Skipped int // the pair changed between listing and folding + Failed int +} + +// Folded is how many missing rows the pass folded. +func (r PairResult) Folded() int { return r.ByMbid + r.ByTitle } + +// errPairChanged means a pair no longer holds: a scan restored or adopted the +// missing row, or marked the present one, after the pairs were listed. +var errPairChanged = errors.New("library: missing pair changed before it was folded") + +// PairMissingTracks folds every unambiguous missing pair, one transaction each, +// so a failed pair is logged and skipped rather than stopping the rest. It is +// idempotent and cheap (one query over the missing rows), so callers run it +// wherever a pair can newly appear: after a scan, a watcher batch, or an +// AcoustID pass that set an MBID. +func PairMissingTracks(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger) (PairResult, error) { + if logger == nil { + logger = slog.Default() + } + var res PairResult + pairs, err := dbq.New(pool).ListMissingTrackPairs(ctx) + if err != nil { + return res, fmt.Errorf("list missing pairs: %w", err) + } + + ambiguous := map[[16]byte]struct{}{} + for _, p := range pairs { + if err := ctx.Err(); err != nil { + return res, err + } + if p.MissingCandidates != 1 || p.PresentCandidates != 1 { + ambiguous[p.MissingID.Bytes] = struct{}{} + continue + } + f, err := foldMissingPair(ctx, pool, p.MissingID, p.PresentID) + switch { + case errors.Is(err, errPairChanged): + res.Skipped++ + continue + case err != nil: + if ctx.Err() != nil { + return res, ctx.Err() + } + res.Failed++ + logger.Warn("missing pair: fold failed", + "missing_path", p.MissingPath, "present_path", p.PresentPath, "err", err) + continue + } + if p.Rule == "mbid" { + res.ByMbid++ + } else { + res.ByTitle++ + } + logger.Info("missing pair: folded into its replacement", + "rule", p.Rule, + "missing_path", p.MissingPath, "present_path", p.PresentPath, + "play_events", f.PlayEvents, "likes", len(f.Likers), "playlist_entries", f.PlaylistEntries) + } + res.Ambiguous = len(ambiguous) + + if res.Folded() > 0 || res.Failed > 0 { + logger.Info("missing pair: pass complete", + "by_mbid", res.ByMbid, "by_title", res.ByTitle, + "ambiguous", res.Ambiguous, "skipped", res.Skipped, "failed", res.Failed) + } + return res, nil +} + +// foldMissingPair folds one pair in its own transaction, after locking both rows +// and checking the pair still holds. +func foldMissingPair(ctx context.Context, pool *pgxpool.Pool, missingID, presentID pgtype.UUID) (foldResult, error) { + var f foldResult + err := pgx.BeginFunc(ctx, pool, func(tx pgx.Tx) error { + tq := dbq.New(tx) + rows, err := tq.LockTracksForPairing(ctx, []pgtype.UUID{missingID, presentID}) + if err != nil { + return fmt.Errorf("lock pair: %w", err) + } + if !pairStillHolds(rows, missingID, presentID) { + return errPairChanged + } + changes := mergeChanges{} + f, err = foldTrackInto(ctx, tq, presentID, missingID, &changes) + if err != nil { + return err + } + return changes.log(ctx, tx) + }) + return f, err +} + +// pairStillHolds reports whether both rows still exist, the first still marked +// missing and the second still present. +func pairStillHolds(rows []dbq.LockTracksForPairingRow, missingID, presentID pgtype.UUID) bool { + var missingOK, presentOK bool + for _, r := range rows { + switch r.ID.Bytes { + case missingID.Bytes: + missingOK = r.MissingSince.Valid + case presentID.Bytes: + presentOK = !r.MissingSince.Valid + } + } + return missingOK && presentOK +} diff --git a/internal/library/missing_pairs_test.go b/internal/library/missing_pairs_test.go new file mode 100644 index 00000000..01126f9f --- /dev/null +++ b/internal/library/missing_pairs_test.go @@ -0,0 +1,314 @@ +package library + +import ( + "context" + "crypto/sha256" + "io" + "log/slog" + "os" + "path/filepath" + "testing" + "time" + + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/dbtest" + syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" +) + +func TestPairStillHolds(t *testing.T) { + missing := pgtype.UUID{Bytes: [16]byte{1}, Valid: true} + present := pgtype.UUID{Bytes: [16]byte{2}, Valid: true} + marked := pgtype.Timestamptz{Time: time.Now(), Valid: true} + row := func(id pgtype.UUID, missingSince pgtype.Timestamptz) dbq.LockTracksForPairingRow { + return dbq.LockTracksForPairingRow{ID: id, MissingSince: missingSince} + } + cases := []struct { + name string + rows []dbq.LockTracksForPairingRow + want bool + }{ + {"still a pair", []dbq.LockTracksForPairingRow{row(missing, marked), row(present, pgtype.Timestamptz{})}, true}, + {"missing row restored", []dbq.LockTracksForPairingRow{row(missing, pgtype.Timestamptz{}), row(present, pgtype.Timestamptz{})}, false}, + {"present row marked", []dbq.LockTracksForPairingRow{row(missing, marked), row(present, marked)}, false}, + {"missing row gone", []dbq.LockTracksForPairingRow{row(present, pgtype.Timestamptz{})}, false}, + {"present row gone", []dbq.LockTracksForPairingRow{row(missing, marked)}, false}, + } + for _, c := range cases { + if got := pairStillHolds(c.rows, missing, present); got != c.want { + t.Errorf("%s: pairStillHolds = %v, want %v", c.name, got, c.want) + } + } +} + +// pairFixture seeds tracks straight into the tables: the pass reads rows, not +// files, so no file needs to exist. +type pairFixture struct { + t *testing.T + pool *pgxpool.Pool + q *dbq.Queries + artist dbq.Artist +} + +func (f pairFixture) exec(sql string, args ...any) { + f.t.Helper() + if _, err := f.pool.Exec(context.Background(), sql, args...); err != nil { + f.t.Fatalf("exec %q: %v", sql, err) + } +} + +func (f pairFixture) album(title, releaseGroup string) dbq.Album { + f.t.Helper() + a, err := f.q.UpsertAlbum(context.Background(), dbq.UpsertAlbumParams{ + Title: title, SortTitle: title, ArtistID: f.artist.ID, + }) + if err != nil { + f.t.Fatalf("album %s: %v", title, err) + } + if releaseGroup != "" { + f.exec(`UPDATE albums SET release_group_mbid = $2 WHERE id = $1`, a.ID, releaseGroup) + } + return a +} + +// track adds a row; mbid "" leaves it without one, and missing marks it missing. +func (f pairFixture) track(album dbq.Album, title, path, mbid string, missing bool) dbq.Track { + f.t.Helper() + tr, err := f.q.UpsertTrack(context.Background(), dbq.UpsertTrackParams{ + Title: title, AlbumID: album.ID, ArtistID: f.artist.ID, + DurationMs: 200000, FilePath: path, FileSize: 100, FileFormat: filepath.Ext(path)[1:], + }) + if err != nil { + f.t.Fatalf("track %s: %v", path, err) + } + if mbid != "" { + f.exec(`UPDATE tracks SET mbid = $2, mbid_source = 'tag' WHERE id = $1`, tr.ID, mbid) + } + if missing { + f.exec(`UPDATE tracks SET missing_since = now() - interval '1 day' WHERE id = $1`, tr.ID) + } + return tr +} + +func (f pairFixture) exists(tr dbq.Track) bool { + f.t.Helper() + var n int + if err := f.pool.QueryRow(context.Background(), `SELECT count(*) FROM tracks WHERE id = $1`, tr.ID).Scan(&n); err != nil { + f.t.Fatalf("count: %v", err) + } + return n == 1 +} + +func (f pairFixture) count(sql string, args ...any) int { + f.t.Helper() + var n int + if err := f.pool.QueryRow(context.Background(), sql, args...).Scan(&n); err != nil { + f.t.Fatalf("count %q: %v", sql, err) + } + return n +} + +// The M485 proof on rows: each rule folds the pair it should, history moves onto +// the replacement, and every shape the pass must leave alone is left alone. +func TestPairMissingTracks_Integration(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + artist, err := q.UpsertArtist(ctx, dbq.UpsertArtistParams{Name: "Pair Artist", SortName: "Pair Artist"}) + if err != nil { + t.Fatalf("artist: %v", err) + } + f := pairFixture{t: t, pool: pool, q: q, artist: artist} + + albumA := f.album("Pair Album", "rg-pair") + edition := f.album("Pair Album (Deluxe)", "rg-pair") // same release group as albumA + other := f.album("Other Album", "rg-other") + + // Rule title: the mp3 went, the flac came, no MBID on either, and the title's + // case differs as it does between old and new tags. + oldMp3 := f.track(albumA, "Song One", "/m/a/01 - Song One.mp3", "", true) + newFlac := f.track(albumA, "song one", "/m/a/01 - song one.flac", "", false) + + // Rule mbid: the same recording, retitled and filed under another edition of + // the same release group. + oldByMbid := f.track(albumA, "Old Name", "/m/a/02 - Old Name.mp3", "rec-2", true) + newByMbid := f.track(edition, "New Name", "/m/d/02 - New Name.flac", "rec-2", false) + + // Ambiguous, missing side: two present copies could be its replacement. + twice := f.track(albumA, "Twice", "/m/a/03 - Twice.mp3", "", true) + f.track(albumA, "Twice", "/m/a/03 - Twice.flac", "", false) + f.track(albumA, "Twice", "/m/a/03 - Twice (live).flac", "", false) + + // Ambiguous, present side: one present copy, two missing rows claiming it. + echo1 := f.track(albumA, "Echo", "/m/a/04 - Echo.mp3", "", true) + echo2 := f.track(albumA, "Echo", "/m/a/04 - Echo (1).mp3", "", true) + f.track(albumA, "Echo", "/m/a/04 - Echo.flac", "", false) + + // Conflicting MBIDs: the tags say these are different recordings. + clash := f.track(albumA, "Clash", "/m/a/05 - Clash.mp3", "rec-x", true) + f.track(albumA, "Clash", "/m/a/05 - Clash.flac", "rec-y", false) + + // Negative controls across albums: a title match must stay inside one album + // row, and an MBID match inside one release group. + lonely := f.track(other, "Lonely", "/m/o/01 - Lonely.mp3", "", true) + f.track(albumA, "Lonely", "/m/a/06 - Lonely.flac", "", false) + cross := f.track(other, "Cross", "/m/o/02 - Cross.mp3", "rec-3", true) + f.track(albumA, "Cross Over", "/m/a/07 - Cross Over.flac", "rec-3", false) + + // History on the row being folded. + user, err := q.CreateUser(ctx, dbq.CreateUserParams{ + Username: dbtest.TestUserPrefix + "pair-alice", PasswordHash: "x", ApiTokenHash: "pair-alice-token", + }) + if err != nil { + t.Fatalf("user: %v", err) + } + f.exec(`INSERT INTO general_likes (user_id, track_id) VALUES ($1, $2)`, user.ID, oldMp3.ID) + now := pgtype.Timestamptz{Time: time.Now(), Valid: true} + session, err := q.InsertPlaySession(ctx, dbq.InsertPlaySessionParams{UserID: user.ID, StartedAt: now}) + if err != nil { + t.Fatalf("session: %v", err) + } + for i := 0; i < 2; i++ { + if _, err := q.InsertPlayEvent(ctx, dbq.InsertPlayEventParams{ + UserID: user.ID, TrackID: oldMp3.ID, SessionID: session.ID, StartedAt: now, + }); err != nil { + t.Fatalf("play event: %v", err) + } + } + pl, err := q.CreatePlaylist(ctx, dbq.CreatePlaylistParams{UserID: user.ID, Name: "pair-mix"}) + if err != nil { + t.Fatalf("playlist: %v", err) + } + if _, err := q.AppendPlaylistTrack(ctx, dbq.AppendPlaylistTrackParams{PlaylistID: pl.ID, TrackID: oldMp3.ID}); err != nil { + t.Fatalf("playlist entry: %v", err) + } + + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + res, err := PairMissingTracks(ctx, pool, logger) + if err != nil { + t.Fatalf("PairMissingTracks: %v", err) + } + want := PairResult{ByMbid: 1, ByTitle: 1, Ambiguous: 3} + if res != want { + t.Errorf("result = %+v, want %+v", res, want) + } + + if f.exists(oldMp3) || f.exists(oldByMbid) { + t.Error("a folded missing row is still in the table") + } + if !f.exists(newFlac) || !f.exists(newByMbid) { + t.Fatal("a replacement row was deleted") + } + for _, tr := range []dbq.Track{twice, echo1, echo2, clash, lonely, cross} { + if !f.exists(tr) { + t.Errorf("%s was folded; it should have been left alone", tr.FilePath) + } + } + + if n := f.count(`SELECT count(*) FROM general_likes WHERE track_id = $1`, newFlac.ID); n != 1 { + t.Errorf("likes on the replacement = %d, want 1", n) + } + if n := f.count(`SELECT count(*) FROM play_events WHERE track_id = $1`, newFlac.ID); n != 2 { + t.Errorf("plays on the replacement = %d, want 2", n) + } + if n := f.count(`SELECT count(*) FROM playlist_tracks WHERE playlist_id = $1 AND track_id = $2`, pl.ID, newFlac.ID); n != 1 { + t.Errorf("playlist entries on the replacement = %d, want 1", n) + } + if n := f.count(`SELECT count(*) FROM library_changes WHERE entity_type = 'track' AND entity_id = $1 AND op = 'delete'`, + syncpkg.FormatUUID(oldMp3.ID)); n != 1 { + t.Errorf("track delete sync changes for the folded row = %d, want 1", n) + } + + // Idempotent: a second pass has nothing left to fold. + again, err := PairMissingTracks(ctx, pool, logger) + if err != nil { + t.Fatalf("second pass: %v", err) + } + if again.Folded() != 0 || again.Failed != 0 { + t.Errorf("second pass = %+v, want nothing folded", again) + } +} + +// The case that made M485: a Lidarr quality upgrade. The mp3 is removed and a +// re-encode of it arrives at a new path with no recording MBID, so move adoption +// has nothing to match. The scan must still end with one row, carrying the old +// row's history. +func TestScanner_FoldsUpgradedFile_Integration(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + + root := t.TempDir() + oldPath := filepath.Join(root, "artistU/albumU/01 - Upgraded.mp3") + tags := map[string]string{"TIT2": "Upgraded", "TPE1": "Artist U", "TALB": "Album U", "TRCK": "1"} + writeTestMP3(t, oldPath, tags) + // Filler so one removal stays under the mark cap. + for i := 1; i <= 7; i++ { + writeTestMP3(t, filepath.Join(root, "artistU/albumU/filler", string(rune('a'+i))+".mp3"), + map[string]string{"TIT2": "Filler " + string(rune('0'+i)), "TPE1": "Artist U", "TALB": "Album U"}) + } + + scanner := New(pool, logger, []string{root}, nil) + // A re-encode never shares the audio hash, so each file gets its own: the + // synthetic files would otherwise hash alike and adopt by hash instead. + scanner.fingerprint = func(_ context.Context, path string, _ fingerprintOptions) fingerprintResult { + sum := sha256.Sum256([]byte(path)) + return fingerprintResult{streamSHA256: sum[:]} + } + if _, err := scanner.Scan(ctx, nil); err != nil { + t.Fatalf("first scan: %v", err) + } + + q := dbq.New(pool) + before, err := q.GetTrackByPath(ctx, oldPath) + if err != nil { + t.Fatalf("track not indexed on first scan: %v", err) + } + user, err := q.CreateUser(ctx, dbq.CreateUserParams{ + Username: dbtest.TestUserPrefix + "upgrade-bob", PasswordHash: "x", ApiTokenHash: "upgrade-bob-token", + }) + if err != nil { + t.Fatalf("user: %v", err) + } + if _, err := pool.Exec(ctx, `INSERT INTO general_likes (user_id, track_id) VALUES ($1, $2)`, user.ID, before.ID); err != nil { + t.Fatalf("like: %v", err) + } + + // The upgrade: the old file goes, the new one lands beside it. + if err := os.Remove(oldPath); err != nil { + t.Fatalf("remove: %v", err) + } + newPath := filepath.Join(root, "artistU/albumU/01 - Upgraded (lossless).mp3") + writeTestMP3(t, newPath, tags) + + if _, err := scanner.Scan(ctx, nil); err != nil { + t.Fatalf("second scan: %v", err) + } + + after, err := q.GetTrackByPath(ctx, newPath) + if err != nil { + t.Fatalf("replacement not indexed: %v", err) + } + if after.ID == before.ID { + t.Fatal("the replacement adopted the old row; this test needs the fold path, so adoption must not match") + } + if _, err := q.GetTrackByPath(ctx, oldPath); err == nil { + t.Error("the old row is still there after the scan; it should have been folded") + } + var likes int + if err := pool.QueryRow(ctx, `SELECT count(*) FROM general_likes WHERE track_id = $1`, after.ID).Scan(&likes); err != nil { + t.Fatalf("likes: %v", err) + } + if likes != 1 { + t.Errorf("likes on the replacement = %d, want 1", likes) + } + var total int + if err := pool.QueryRow(ctx, `SELECT count(*) FROM tracks`).Scan(&total); err != nil { + t.Fatalf("count: %v", err) + } + if total != 8 { + t.Errorf("tracks = %d, want 8 — an upgrade must not leave a second row", total) + } +} diff --git a/internal/library/scanner.go b/internal/library/scanner.go index 32fb0b26..e737aab5 100644 --- a/internal/library/scanner.go +++ b/internal/library/scanner.go @@ -175,6 +175,14 @@ func (s *Scanner) Scan(ctx context.Context, progressCb func(Stats)) (Stats, erro if err := ctx.Err(); err != nil { return stats, err } + + // PHASE 4 — fold missing rows into replacements this scan brought in or + // that were already here (M485). After processing, so both this scan's + // marks and its arrivals are in. Not fatal: an unfolded pair is only a row + // left in the missing list, and the next scan tries again. + if _, err := PairMissingTracks(ctx, s.pool, s.logger); err != nil && ctx.Err() == nil { + s.logger.Warn("library scan: missing-pair pass failed", "err", err) + } return stats, nil } @@ -466,6 +474,11 @@ func (s *Scanner) ScanFiles(ctx context.Context, paths []string) ([]pgtype.UUID, "skipped", stats.Skipped, "errored", stats.Errored, ) + // A file the watcher just brought in may be the replacement for a row + // an earlier scan marked missing (M485). + if _, err := PairMissingTracks(ctx, s.pool, s.logger); err != nil && ctx.Err() == nil { + s.logger.Warn("library watch: missing-pair pass failed", "err", err) + } } return changed, nil }