feat: a held album below the quality you settle for, or holding a video rip, stays wanted for a better copy, and the import replaces the old tracks with their likes and plays carried over (M502 #5524)

- acquisition_settings.upgrade_until (default High quality lossy, Lidarr's
  cutoff, note #5527; "" is off), an "Upgrade until" row under Searching.
- The wanted worker judges held albums hourly: tier per format from size per
  second (the scanner records no bitrate), the album's worst. Below the
  cutoff it is wanted for quality; a video rip among its tracks always.
- The schedule searches upgrades weekly, after open wants; a release no
  better than the copy held is refused ("not_better").
- The import matches each file to the track it would replace (recording,
  then disc/track) and replaces it when it is a rip or the file is a better
  tier: over the same path in one rename, or filed beside it and the old
  track folded in (library.ReplaceTrack: file first, then foldTrackInto).
  Nothing better: the release is set aside.
- Collection › Upgrades (Lidarr's Cutoff Unmet); Suspect sources says when a
  clean copy was looked for instead of Lidarr's search.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

internal/importer/importer.go

@@ -66,6 +66,9 @@ type Importer struct {
 	// Roots are the library's folders (the scan paths). New music is filed
 	// under the one the settings name, or the first.
 	Roots []string
+	// DataDir is where cached art lives, for tidying an artist an upgrade
+	// leaves with nothing (#5524). Empty skips it.
+	DataDir string
 	// ClientFor builds the download client a grab went to. Defaults to
 	// acquire.DownloadClientFor.
 	ClientFor func(ctx context.Context, code string) (downloader.Driver, error)
@@ -338,6 +341,9 @@ func (im *Importer) place(ctx context.Context, q *dbq.Queries, g dbq.Grab, d dow
 		e = &editions[v.Edition]
 	}
 	plan := Plan(root, w, e, files, v.Pairs)
+	if w.Upgrade != "" {
+		return im.placeUpgrade(ctx, q, g, d, w, e, files, plan, v, det, record)
+	}
 	heldRec, heldPos, err := im.held(ctx, q, w, plan)
 	if err != nil {
 		return record(StateWaiting, "Couldn't read what the library holds: "+err.Error(), "", det, 0, "")
@@ -504,10 +510,14 @@ func (im *Importer) want(ctx context.Context, q *dbq.Queries, id pgtype.UUID) (W
 	if err != nil {
 		return Want{}, err
 	}
-	return Want{
+	w := Want{
 		GroupMBID: row.ReleaseGroupMbid, ArtistMBID: row.ArtistMbid, Artist: row.ArtistName, Title: row.Title,
 		Year: yearOf(row.FirstReleaseDate), PrimaryType: row.PrimaryType, SecondaryTypes: row.SecondaryTypes,
-	}, nil
+	}
+	if row.FulfilledAt.Valid && row.Upgrade != "" && row.AlbumID.Valid {
+		w.Upgrade, w.AlbumID = row.Upgrade, row.AlbumID
+	}
+	return w, nil
 }
 
 func (im *Importer) editions(ctx context.Context, group string) ([]Edition, error) {

internal/importer/place.go

@@ -92,7 +92,19 @@ var taggable = map[string]bool{".flac": true, ".ogg": true, ".opus": true, ".mp3
 // container will not take the file's streams) the file is copied as it is
 // and tagged reports false. An existing dst is never overwritten.
 func Place(ctx context.Context, src, dst string, t Tags) (tagged bool, err error) {
-	if _, err := os.Lstat(dst); err == nil {
+	return placeFile(ctx, src, dst, t, false)
+}
+
+// Overwrite is Place onto a file an upgrade replaces (#5524): the new copy
+// takes the old one's path in one rename, so the library never sees the
+// path empty, and the track row at that path, history and all, carries on
+// with the new file.
+func Overwrite(ctx context.Context, src, dst string, t Tags) (tagged bool, err error) {
+	return placeFile(ctx, src, dst, t, true)
+}
+
+func placeFile(ctx context.Context, src, dst string, t Tags, overwrite bool) (tagged bool, err error) {
+	if _, err := os.Lstat(dst); err == nil && !overwrite {
 		return false, fmt.Errorf("%w: %s", ErrOccupied, dst)
 	}
 	dir := filepath.Dir(dst)
@@ -114,7 +126,7 @@ func Place(ctx context.Context, src, dst string, t Tags) (tagged bool, err error
 	if err := os.Chmod(tmp, 0o644); err != nil {
 		return false, fmt.Errorf("chmod %s: %w", tmp, err)
 	}
-	if _, err := os.Lstat(dst); err == nil {
+	if _, err := os.Lstat(dst); err == nil && !overwrite {
 		return false, fmt.Errorf("%w: %s", ErrOccupied, dst)
 	}
 	if err := os.Rename(tmp, dst); err != nil {

internal/importer/upgrade.go

@@ -0,0 +1,304 @@
+package importer
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+
+	"github.com/jackc/pgx/v5/pgtype"
+
+	"git.fabledsword.com/bvandeusen/minstrel/internal/acquire"
+	"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
+	"git.fabledsword.com/bvandeusen/minstrel/internal/downloader"
+	"git.fabledsword.com/bvandeusen/minstrel/internal/library"
+	"git.fabledsword.com/bvandeusen/minstrel/internal/release"
+)
+
+// Importing an upgrade (M502 #5524): a better copy of an album the library
+// holds. Each file is matched to the track it would replace, by recording
+// and then by disc and track. It replaces that track when the track is a
+// video rip or the new file is a better tier; otherwise the old track stays.
+// A file with no track to replace fills a gap, as any import does.
+//
+// The new copy is filed before the old one goes. On the old file's own path
+// it takes the path in one rename and the row carries on; at a new path it
+// is scanned in as a track of its own and the old track is folded into it
+// (library.ReplaceTrack), so likes, plays and playlist entries carry over.
+
+// OldTrack is a library track an upgrade may replace.
+type OldTrack struct {
+	ID          pgtype.UUID
+	Path        string
+	Recording   string
+	Disc, Track int
+	Format      string
+	Bytes       int64
+	Ms          int64
+	Rip         bool
+}
+
+// Tier is the old track's tier, estimated as the upgrade check estimates
+// an album's; ok is false for a format the ladder cannot place.
+func (o OldTrack) Tier() (release.Tier, bool) {
+	return release.TierOfFile(o.Format, release.FileKbps(o.Bytes, o.Ms))
+}
+
+// What an upgrade does with one placement.
+const (
+	stepPlace     = "place"     // no track to replace: fills a gap
+	stepReplace   = "replace"   // replaces Old from a new path
+	stepOverwrite = "overwrite" // replaces Old on its own path
+	stepKeep      = "keep"      // Old is as good: left as it is
+)
+
+type upgradeStep struct {
+	Placement int
+	Action    string
+	Old       int // index into olds, or -1
+}
+
+// planUpgrade matches each placement to the track it would replace and
+// decides what to do with it. newTiers is each placement's tier.
+func planUpgrade(plan []Placement, newTiers []release.Tier, olds []OldTrack) []upgradeStep {
+	used := make([]bool, len(olds))
+	match := make([]int, len(plan))
+	for i := range match {
+		match[i] = -1
+	}
+	for i, pl := range plan {
+		if pl.Recording == "" {
+			continue
+		}
+		for j, o := range olds {
+			if !used[j] && o.Recording == pl.Recording {
+				match[i], used[j] = j, true
+				break
+			}
+		}
+	}
+	for i, pl := range plan {
+		if match[i] >= 0 || pl.Track <= 0 {
+			continue
+		}
+		for j, o := range olds {
+			if !used[j] && o.Track == pl.Track && max(o.Disc, 1) == max(pl.Disc, 1) {
+				match[i], used[j] = j, true
+				break
+			}
+		}
+	}
+	steps := make([]upgradeStep, len(plan))
+	for i, pl := range plan {
+		j := match[i]
+		steps[i] = upgradeStep{Placement: i, Old: j, Action: stepPlace}
+		if j < 0 {
+			continue
+		}
+		o := olds[j]
+		oldTier, _ := o.Tier()
+		better := release.Rank(newTiers[i]) > release.Rank(oldTier)
+		switch {
+		case !o.Rip && !better:
+			steps[i].Action = stepKeep
+		case filepath.Clean(pl.Dest) == filepath.Clean(o.Path):
+			steps[i].Action = stepOverwrite
+		default:
+			steps[i].Action = stepReplace
+		}
+	}
+	return steps
+}
+
+// fileTier estimates a downloaded file's tier from its size and length, the
+// way the library's are estimated, so the two compare like for like. A file
+// that cannot be measured takes the release's tier.
+func fileTier(f File, fallback release.Tier) release.Tier {
+	if info, err := os.Stat(f.Path); err == nil && f.LengthMs > 0 {
+		if t, ok := release.TierOfFile(filepath.Ext(f.Path), release.FileKbps(info.Size(), int64(f.LengthMs))); ok {
+			return t
+		}
+	}
+	return fallback
+}
+
+func oldTracks(rows []dbq.ListUpgradeTracksRow) []OldTrack {
+	out := make([]OldTrack, 0, len(rows))
+	for _, r := range rows {
+		o := OldTrack{ID: r.ID, Path: r.FilePath, Disc: int(r.Disc), Format: r.FileFormat,
+			Bytes: r.FileSize, Ms: int64(r.DurationMs), Rip: r.SourceVerdict != nil && *r.SourceVerdict == "suspect"}
+		if r.Mbid != nil {
+			o.Recording = *r.Mbid
+		}
+		if r.TrackNumber != nil {
+			o.Track = int(*r.TrackNumber)
+		}
+		out = append(out, o)
+	}
+	return out
+}
+
+// placeUpgrade files an accepted upgrade, replacing the tracks it betters.
+func (im *Importer) placeUpgrade(ctx context.Context, q *dbq.Queries, g dbq.Grab, d downloader.Driver, w Want,
+	e *Edition, files []File, plan []Placement, v Verdict, det Detail, record recordFunc) string {
+	rows, err := q.ListUpgradeTracks(ctx, w.AlbumID)
+	if err != nil {
+		return record(StateWaiting, "Couldn't read the album's tracks: "+err.Error(), "", det, 0, "")
+	}
+	olds := oldTracks(rows)
+	grabTier := release.Tier(g.Quality)
+	newTiers := make([]release.Tier, len(plan))
+	for i, pl := range plan {
+		newTiers[i] = fileTier(files[pl.File], grabTier)
+	}
+	steps := planUpgrade(plan, newTiers, olds)
+	heldRec, heldPos, err := im.held(ctx, q, w, plan)
+	if err != nil {
+		return record(StateWaiting, "Couldn't read what the library holds: "+err.Error(), "", det, 0, "")
+	}
+
+	type swap struct {
+		old  OldTrack
+		dest string
+	}
+	var (
+		placed  []string // every file written, for the scan
+		swaps   []swap
+		inPlace []OldTrack
+		failed  []string
+	)
+	for _, st := range steps {
+		pl := plan[st.Placement]
+		if st.Action == stepKeep {
+			det.Skipped++
+			continue
+		}
+		if st.Action == stepPlace && ((pl.Recording != "" && heldRec[pl.Recording]) || heldPos[[2]int{pl.Disc, pl.Track}]) {
+			det.Skipped++
+			continue
+		}
+		tags := Tags{Title: pl.Title, Artist: w.Artist, Album: w.Title, AlbumArtist: w.Artist, Year: w.Year,
+			Disc: pl.Disc, Discs: pl.Discs, Track: pl.Track, Tracks: pl.Tracks,
+			GroupMBID: w.GroupMBID, RecordingMBID: pl.Recording, ArtistMBID: w.ArtistMBID}
+		if e != nil {
+			tags.ReleaseMBID = e.MBID
+		}
+		src := files[pl.File].Path
+		var tagged bool
+		if st.Action == stepOverwrite {
+			tagged, err = Overwrite(ctx, src, pl.Dest, tags)
+		} else {
+			tagged, err = Place(ctx, src, pl.Dest, tags)
+		}
+		switch {
+		case errors.Is(err, ErrOccupied):
+			det.Skipped++
+			continue
+		case err != nil:
+			// Each replacement stands alone: the old track stays where a
+			// copy failed, and the rest go ahead.
+			failed = append(failed, pl.Title)
+			im.logger.Warn("import: copy an upgrade into the library", "grab", g.Name, "dest", pl.Dest, "err", err)
+			continue
+		}
+		placed = append(placed, pl.Dest)
+		if !tagged {
+			det.Untagged++
+		}
+		switch st.Action {
+		case stepOverwrite:
+			inPlace = append(inPlace, olds[st.Old])
+		case stepReplace:
+			swaps = append(swaps, swap{old: olds[st.Old], dest: pl.Dest})
+		}
+	}
+	det.Placed = len(placed)
+
+	if len(placed) == 0 {
+		why := fmt.Sprintf("Nothing in it is better than the copy of %s you have, so nothing was imported.", w.Title)
+		if len(failed) > 0 {
+			return record(StateFailed, fmt.Sprintf("Copying into the library failed for %s.", strings.Join(failed, ", ")),
+				"", det, 0, "")
+		}
+		// Set aside, so the weekly search doesn't take it again.
+		if err := acquire.NewMemory(im.pool).Block(ctx, dbq.BlockReleaseParams{
+			InfoHash: g.InfoHash, Name: g.Name, Reason: acquire.BlockImport, Detail: why, WantID: g.WantID,
+		}); err != nil {
+			im.logger.Warn("import: set the release aside", "grab", g.Name, "err", err)
+		}
+		if err := q.MarkWantSearched(ctx, g.WantID); err != nil {
+			im.logger.Warn("import: mark the want searched", "grab", g.Name, "err", err)
+		}
+		im.removeFromClient(ctx, d, g)
+		return record(StateHeld, why, StateHeld, det, 0, "")
+	}
+
+	if _, err := im.Scanner.ScanFiles(ctx, placed); err != nil {
+		im.logger.Warn("import: scan the new files", "grab", g.Name, "err", err)
+	}
+	for _, o := range inPlace {
+		if o.Rip {
+			if err := q.ClearTrackSourceVerdict(ctx, o.ID); err != nil {
+				im.logger.Warn("import: release a replaced rip", "path", o.Path, "err", err)
+			}
+		}
+	}
+	replaced, kept := len(inPlace), 0
+	for _, s := range swaps {
+		nt, err := q.GetTrackByPath(ctx, s.dest)
+		if err != nil {
+			kept++
+			im.logger.Warn("import: find the new track", "path", s.dest, "err", err)
+			continue
+		}
+		moved, err := library.ReplaceTrack(ctx, im.pool, im.logger, im.DataDir, s.old.ID, nt.ID)
+		if err != nil {
+			// The old file stays; the duplicate sweep finds the two copies.
+			kept++
+			im.logger.Warn("import: replace the old track", "old", s.old.Path, "new", s.dest, "err", err)
+			continue
+		}
+		replaced++
+		im.logger.Info("import: replaced a track with a better copy", "old", s.old.Path, "new", s.dest,
+			"play_events", moved.PlayEvents, "likes", moved.Likes, "playlist_entries", moved.PlaylistEntries)
+	}
+
+	// The want holds the album the new tracks are in, once the old album row
+	// has none left.
+	var album pgtype.UUID
+	if left, err := q.ListUpgradeTracks(ctx, w.AlbumID); err == nil && len(left) == 0 {
+		if nt, err := q.GetTrackByPath(ctx, placed[0]); err == nil {
+			album = nt.AlbumID
+		}
+	}
+	if err := q.FinishUpgrade(ctx, dbq.FinishUpgradeParams{ID: g.WantID, AlbumID: album}); err != nil {
+		im.logger.Warn("import: finish the upgrade", "grab", g.Name, "err", err)
+	}
+	if err := q.MarkWantSearched(ctx, g.WantID); err != nil {
+		im.logger.Warn("import: mark the want searched", "grab", g.Name, "err", err)
+	}
+
+	dest := filepath.Dir(placed[0])
+	if e != nil && e.Media() > 1 {
+		dest = filepath.Dir(dest)
+	}
+	reason := fmt.Sprintf("Replaced %s of %s with a better copy.", tracks(replaced), w.Title)
+	if added := len(placed) - replaced - kept; added > 0 {
+		reason += fmt.Sprintf(" Added %s it was missing.", tracks(added))
+	}
+	if det.Skipped > 0 {
+		reason += fmt.Sprintf(" %s already as good left as they were.", tracks(det.Skipped))
+	}
+	if kept > 0 {
+		reason += fmt.Sprintf(" %s couldn't be removed, so both copies are in the library.", tracks(kept))
+	}
+	if len(failed) > 0 {
+		reason += fmt.Sprintf(" Copying failed for %s; those stay as they were.", strings.Join(failed, ", "))
+	}
+	if det.Untagged > 0 {
+		reason += fmt.Sprintf(" %s copied with their own tags: ffmpeg could not rewrite them.", tracks(det.Untagged))
+	}
+	return record(StateImported, reason+" "+v.Reason, StateImported, det, len(placed), dest)
+}

internal/importer/upgrade_test.go

@@ -0,0 +1,53 @@
+package importer
+
+import (
+	"testing"
+
+	"git.fabledsword.com/bvandeusen/minstrel/internal/release"
+)
+
+func TestPlanUpgrade_ReplacesOnlyWhatItBetters(t *testing.T) {
+	plan := []Placement{
+		{File: 0, Dest: "/music/A/B (2001)/A - B - 01 - One.flac", Disc: 1, Track: 1, Recording: "rec-1"},
+		{File: 1, Dest: "/music/A/B (2001)/A - B - 02 - Two.mp3", Disc: 1, Track: 2},
+		{File: 2, Dest: "/music/A/B (2001)/A - B - 03 - Three.flac", Disc: 1, Track: 3},
+		{File: 3, Dest: "/music/A/B (2001)/A - B - 04 - Four.flac", Disc: 1, Track: 4},
+		{File: 4, Dest: "/music/A/B (2001)/A - B - 05 - Five.flac", Disc: 1, Track: 5},
+	}
+	newTiers := []release.Tier{release.TierLossless, release.TierLossyHigh, release.TierLossless,
+		release.TierLossless, release.TierLossyLow}
+	olds := []OldTrack{
+		// Matched by recording though its number is wrong: a 192 MP3.
+		{Path: "/music/A/B/01 One.mp3", Recording: "rec-1", Disc: 1, Track: 9, Format: "mp3", Bytes: 4_800_000, Ms: 200_000},
+		// Same path as the new file, a 320 MP3: the new 320 is no better.
+		{Path: "/music/A/B (2001)/A - B - 02 - Two.mp3", Disc: 1, Track: 2, Format: "mp3", Bytes: 8_000_000, Ms: 200_000},
+		// Already lossless: kept.
+		{Path: "/music/A/B/03 Three.flac", Disc: 1, Track: 3, Format: "flac", Bytes: 22_000_000, Ms: 200_000},
+		// A video rip, even though lossless: replaced.
+		{Path: "/music/A/B/04 Four (Official Video).flac", Disc: 0, Track: 4, Format: "flac", Bytes: 22_000_000, Ms: 200_000, Rip: true},
+	}
+	steps := planUpgrade(plan, newTiers, olds)
+	want := []struct {
+		action string
+		old    int
+	}{
+		{stepReplace, 0}, {stepKeep, 1}, {stepKeep, 2}, {stepReplace, 3}, {stepPlace, -1},
+	}
+	for i, w := range want {
+		if steps[i].Action != w.action || steps[i].Old != w.old {
+			t.Errorf("placement %d: %s old %d, want %s old %d", i, steps[i].Action, steps[i].Old, w.action, w.old)
+		}
+	}
+}
+
+func TestPlanUpgrade_OverwritesAWorseFileOnItsOwnPath(t *testing.T) {
+	dest := "/music/A/B (2001)/A - B - 01 - One.mp3"
+	steps := planUpgrade(
+		[]Placement{{File: 0, Dest: dest, Disc: 1, Track: 1}},
+		[]release.Tier{release.TierLossyHigh},
+		[]OldTrack{{Path: dest, Disc: 1, Track: 1, Format: "mp3", Bytes: 3_200_000, Ms: 200_000}}, // 128
+	)
+	if steps[0].Action != stepOverwrite || steps[0].Old != 0 {
+		t.Errorf("got %+v", steps[0])
+	}
+}

internal/importer/verify.go

@@ -16,6 +16,8 @@ import (
 	"sort"
 	"strings"
 
+	"github.com/jackc/pgx/v5/pgtype"
+
 	"git.fabledsword.com/bvandeusen/minstrel/internal/release"
 )
 
@@ -111,6 +113,11 @@ type Want struct {
 	Year           int
 	PrimaryType    string
 	SecondaryTypes []string
+	// Upgrade: a held album wanted for a better copy (#5524), quality or
+	// rip; AlbumID is the library album it holds, whose tracks the import
+	// replaces.
+	Upgrade string
+	AlbumID pgtype.UUID
 }
 
 // Pair is a file matched to an edition's track, and what agreed.