diff --git a/android/app/src/main/java/com/fabledsword/minstrel/notifications/ui/NotificationSettingsScreen.kt b/android/app/src/main/java/com/fabledsword/minstrel/notifications/ui/NotificationSettingsScreen.kt index 6b770b7f..5fcbfbf6 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/notifications/ui/NotificationSettingsScreen.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/notifications/ui/NotificationSettingsScreen.kt @@ -301,6 +301,7 @@ internal fun kindLabel(kind: String): String = when (kind) { "scan_failed" -> "Library scan failed" "tracks_missing" -> "Tracks gone missing" "duplicates_found" -> "Duplicates to review" + "duplicates_resolved" -> "Duplicates resolved" "playback_errors" -> "Playback errors" else -> kind } diff --git a/cmd/minstrel/main.go b/cmd/minstrel/main.go index cff4d8c5..95df672f 100644 --- a/cmd/minstrel/main.go +++ b/cmd/minstrel/main.go @@ -325,6 +325,19 @@ func run() error { library.SetNotifier(notifier) go lidarrReconciler.Run(ctx) + // Duplicate resolver (M498): classifies the sweep's groups and, with the + // operator's auto-resolve on, merges copies Lidarr does not map and moves a + // Lidarr release that lists songs twice. Started after SetNotifier so its + // summaries reach admins. See internal/library/duplicate_resolve.go. + resolverLidarr := func() library.LidarrLibrary { + if c := lidarrClientFn(); c != nil { + return c + } + return nil // a nil *lidarr.Client must not become a non-nil interface + } + go library.NewDuplicateResolveWorker(pool, logger.With("component", "duplicate_resolve"), + cfg.Storage.DataDir, fpSettings, resolverLidarr).Run(ctx) + // Missing-file re-acquisition (milestone #290). Turns albums whose files // have been gone longer than the grace window into Lidarr requests, on an // exponential per-album backoff. Hourly tick — the shortest meaningful diff --git a/internal/api/admin_duplicates.go b/internal/api/admin_duplicates.go index cbe2809d..be595999 100644 --- a/internal/api/admin_duplicates.go +++ b/internal/api/admin_duplicates.go @@ -11,8 +11,10 @@ import ( "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgtype" + "git.fabledsword.com/bvandeusen/minstrel/internal/audit" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" "git.fabledsword.com/bvandeusen/minstrel/internal/library" + "git.fabledsword.com/bvandeusen/minstrel/internal/tracks" ) // duplicateMemberView is one copy in a proposed duplicate group. LikeCount and @@ -31,6 +33,12 @@ type duplicateMemberView struct { AddedAt string `json:"added_at"` LikeCount int64 `json:"like_count"` PlayCount int64 `json:"play_count"` + // LidarrState is Lidarr's view of the file as of the resolver's last pass + // (M498): "tracked" (mapped to a track, so it cannot be removed without + // Lidarr downloading it again), "unmapped", or null when not asked. + LidarrState *string `json:"lidarr_state"` + DiscNumber *int32 `json:"disc_number"` + TrackNumber *int32 `json:"track_number"` } // duplicateGroupView is one proposal. SurvivorTrackID and SurvivorReason are @@ -38,13 +46,17 @@ type duplicateMemberView struct { // (library.ProposeSurvivor) — a default the merge (#3911) lets the operator // override. type duplicateGroupView struct { - ID string `json:"id"` - Tier string `json:"tier"` - WorstBitErrorRate *float32 `json:"worst_bit_error_rate"` - DetectedAt string `json:"detected_at"` - SurvivorTrackID string `json:"survivor_track_id"` - SurvivorReason string `json:"survivor_reason"` - Members []duplicateMemberView `json:"members"` + ID string `json:"id"` + Tier string `json:"tier"` + WorstBitErrorRate *float32 `json:"worst_bit_error_rate"` + DetectedAt string `json:"detected_at"` + SurvivorTrackID string `json:"survivor_track_id"` + SurvivorReason string `json:"survivor_reason"` + // Class is the resolver's verdict (library.DuplicateClass), null until its + // first pass; ResolveNote is why it left the group for the operator. + Class *string `json:"class"` + ResolveNote *string `json:"resolve_note"` + Members []duplicateMemberView `json:"members"` } // duplicateSweepView is the latest sweep. State is "never" when none has run, @@ -60,10 +72,26 @@ type duplicateSweepView struct { ErrorMessage *string `json:"error_message"` } -// adminDuplicatesResponse is the paged report. Total counts groups. +// duplicateViewCounts sizes each tab of the report. +type duplicateViewCounts struct { + Review int64 `json:"review"` + CrossRelease int64 `json:"cross_release"` + Resolved int64 `json:"resolved"` +} + +// Report views (M498). cross_release groups are the same song on different +// releases, kept on purpose; everything else pending needs the operator. +const ( + duplicateViewReview = "review" + duplicateViewCrossRelease = "cross_release" +) + +// adminDuplicatesResponse is the paged report. Total counts groups in View. type adminDuplicatesResponse struct { Sweep duplicateSweepView `json:"sweep"` Fingerprints fingerprintCoverageResp `json:"fingerprints"` + View string `json:"view"` + Counts duplicateViewCounts `json:"counts"` Total int64 `json:"total"` Limit int `json:"limit"` Offset int `json:"offset"` @@ -71,6 +99,7 @@ type adminDuplicatesResponse struct { } // handleListDuplicates implements GET /api/admin/library/duplicates (#3912). +// ?view=review (the default) or cross_release picks the tab (M498). // // Read-only. The sweep's state and the fingerprint backfill's progress travel // with the groups because an empty report means three different things — still @@ -81,6 +110,14 @@ func (h *handlers) handleListDuplicates(w http.ResponseWriter, r *http.Request) writeAdminJSONErr(w, http.StatusBadRequest, "invalid_paging") return } + view := r.URL.Query().Get("view") + if view == "" { + view = duplicateViewReview + } + if view != duplicateViewReview && view != duplicateViewCrossRelease { + writeAdminJSONErr(w, http.StatusBadRequest, "invalid_view") + return + } ctx := r.Context() q := dbq.New(h.pool) @@ -101,14 +138,18 @@ func (h *handlers) handleListDuplicates(w http.ResponseWriter, r *http.Request) writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") return } - total, err := q.CountPendingDuplicateGroups(ctx) + counts, err := duplicateCounts(ctx, q) if err != nil { h.logger.Error("admin: count duplicate groups", "err", err) writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") return } + total := counts.Review + if view == duplicateViewCrossRelease { + total = counts.CrossRelease + } rows, err := q.ListPendingDuplicateGroupMembers(ctx, dbq.ListPendingDuplicateGroupMembersParams{ - PageLimit: int32(limit), PageOffset: int32(offset), + View: view, PageLimit: int32(limit), PageOffset: int32(offset), }) if err != nil { h.logger.Error("admin: list duplicate groups", "err", err) @@ -119,6 +160,8 @@ func (h *handlers) handleListDuplicates(w http.ResponseWriter, r *http.Request) writeJSON(w, http.StatusOK, adminDuplicatesResponse{ Sweep: sweep, Fingerprints: cov, + View: view, + Counts: counts, Total: total, Limit: limit, Offset: offset, @@ -126,6 +169,76 @@ func (h *handlers) handleListDuplicates(w http.ResponseWriter, r *http.Request) }) } +// resolvedActions are the audit actions the "Resolved automatically" tab lists. +var resolvedActions = []string{string(audit.ActionDuplicateMerge), string(audit.ActionLidarrReleaseChange)} + +func duplicateCounts(ctx context.Context, q *dbq.Queries) (duplicateViewCounts, error) { + var c duplicateViewCounts + var err error + if c.Review, err = q.CountPendingDuplicateGroupsInView(ctx, duplicateViewReview); err != nil { + return c, err + } + if c.CrossRelease, err = q.CountPendingDuplicateGroupsInView(ctx, duplicateViewCrossRelease); err != nil { + return c, err + } + c.Resolved, err = q.CountAuditLogByActions(ctx, dbq.CountAuditLogByActionsParams{Actions: resolvedActions, SystemOnly: true}) + return c, err +} + +// resolvedActivityView is one thing the resolver did on its own: a merge +// (action duplicate_merge) or a Lidarr release change (lidarr_release_change). +// Details is the audit row's metadata, which names the files or releases. +type resolvedActivityView struct { + ID string `json:"id"` + Action string `json:"action"` + CreatedAt string `json:"created_at"` + Details json.RawMessage `json:"details"` +} + +type resolvedActivityResponse struct { + Total int64 `json:"total"` + Limit int `json:"limit"` + Offset int `json:"offset"` + Items []resolvedActivityView `json:"items"` +} + +// handleListResolvedDuplicates implements GET +// /api/admin/library/duplicates/resolved (M498): what the resolver did by +// itself, newest first, read from the audit log so it outlives the groups. +func (h *handlers) handleListResolvedDuplicates(w http.ResponseWriter, r *http.Request) { + limit, offset, err := parsePaging(r.URL.Query()) + if err != nil { + writeAdminJSONErr(w, http.StatusBadRequest, "invalid_paging") + return + } + q := dbq.New(h.pool) + total, err := q.CountAuditLogByActions(r.Context(), dbq.CountAuditLogByActionsParams{Actions: resolvedActions, SystemOnly: true}) + if err != nil { + h.logger.Error("admin: count resolved duplicates", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + return + } + rows, err := q.ListAuditLogByActions(r.Context(), dbq.ListAuditLogByActionsParams{ + Actions: resolvedActions, SystemOnly: true, PageLimit: int32(limit), PageOffset: int32(offset), + }) + if err != nil { + h.logger.Error("admin: list resolved duplicates", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + return + } + items := make([]resolvedActivityView, 0, len(rows)) + for _, row := range rows { + details := json.RawMessage(row.Metadata) + if len(details) == 0 { + details = json.RawMessage("{}") + } + items = append(items, resolvedActivityView{ + ID: uuidToString(row.ID), Action: row.Action, CreatedAt: formatTimestamp(row.CreatedAt), Details: details, + }) + } + writeJSON(w, http.StatusOK, resolvedActivityResponse{Total: total, Limit: limit, Offset: offset, Items: items}) +} + func duplicateSweepViewOf(s dbq.DuplicateSweep) duplicateSweepView { v := duplicateSweepView{ State: "running", @@ -158,6 +271,8 @@ func foldDuplicateGroups(rows []dbq.ListPendingDuplicateGroupMembersRow) []dupli Tier: row.Tier, WorstBitErrorRate: row.WorstBitErrorRate, DetectedAt: formatTimestamp(row.DetectedAt), + Class: row.Class, + ResolveNote: row.ResolveNote, }) candidates = append(candidates, nil) } @@ -176,9 +291,14 @@ func foldDuplicateGroups(rows []dbq.ListPendingDuplicateGroupMembersRow) []dupli AddedAt: formatTimestamp(row.AddedAt), LikeCount: row.LikeCount, PlayCount: row.PlayCount, + LidarrState: row.LidarrState, + DiscNumber: row.DiscNumber, + TrackNumber: row.TrackNumber, }) candidates[n] = append(candidates[n], library.SurvivorCandidate{ TrackID: trackID, FileFormat: row.FileFormat, FileSize: row.FileSize, AddedAt: row.AddedAt.Time, + LidarrTracked: row.LidarrState != nil && *row.LidarrState == "tracked", + TagFit: library.TagFitScore(row.TrackNumber != nil, row.PositionClash, row.FilePath, row.HasMbid), }) } for i := range groups { @@ -257,6 +377,10 @@ const mergeRequestBodyLimit = 1 << 16 // Errors: // - 409 library_not_writable / 500 file_delete_failed when a file could not be // removed — nothing was changed (fileRemoveAPIError) +// - 409 copy_tracked_by_lidarr when a copy to remove is one Lidarr maps to a +// track: removing it would make Lidarr download it again (M498) +// - 503 lidarr_unavailable when Lidarr is on but did not answer, so that +// cannot be checked // - 404 duplicate_group_not_pending when the group was already resolved // - 400 survivor_not_in_group, invalid_id, invalid_body func (h *handlers) handleMergeDuplicateGroup(w http.ResponseWriter, r *http.Request) { @@ -294,6 +418,11 @@ func (h *handlers) handleMergeDuplicateGroup(w http.ResponseWriter, r *http.Requ writeAdminJSONErr(w, http.StatusNotFound, "duplicate_group_not_pending") case errors.Is(err, library.ErrSurvivorNotInGroup): writeAdminJSONErr(w, http.StatusBadRequest, "survivor_not_in_group") + case errors.Is(err, library.ErrCopyTrackedByLidarr): + writeAdminJSONErr(w, http.StatusConflict, "copy_tracked_by_lidarr") + case errors.Is(err, tracks.ErrLidarrUnavailable): + h.logger.Warn("admin: merge duplicate group: lidarr unavailable", "group_id", uuidToString(groupID), "err", err) + writeAdminJSONErr(w, http.StatusServiceUnavailable, "lidarr_unavailable") default: h.logger.Error("admin: merge duplicate group", "group_id", uuidToString(groupID), "err", err) writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") diff --git a/internal/api/admin_duplicates_test.go b/internal/api/admin_duplicates_test.go index a1c8a332..e67cfe56 100644 --- a/internal/api/admin_duplicates_test.go +++ b/internal/api/admin_duplicates_test.go @@ -63,3 +63,29 @@ func TestFoldDuplicateGroups(t *testing.T) { t.Errorf("group 2 survivor = (%s, %q), want the FLAC copy", g2.SurvivorTrackID, g2.SurvivorReason) } } + +// M498: the proposal shown beside a group follows the resolver's rule. The copy +// Lidarr tracks is proposed even when the other is lossless, because removing +// the tracked one would make Lidarr download it again. +func TestFoldDuplicateGroups_ProposesTheCopyLidarrTracks(t *testing.T) { + at := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC) + tracked, unmapped := "tracked", "unmapped" + class := "same_release" + rows := []dbq.ListPendingDuplicateGroupMembersRow{ + {GroupID: dupUUID(3), Tier: "acoustic", DetectedAt: dupTS(at), TrackID: dupUUID(30), Class: &class, + Title: "Song", FileFormat: "flac", FileSize: 30_000_000, AddedAt: dupTS(at), LidarrState: &unmapped}, + {GroupID: dupUUID(3), Tier: "acoustic", DetectedAt: dupTS(at), TrackID: dupUUID(31), Class: &class, + Title: "Song", FileFormat: "mp3", FileSize: 9_000_000, AddedAt: dupTS(at), LidarrState: &tracked}, + } + got := foldDuplicateGroups(rows) + if len(got) != 1 { + t.Fatalf("folded %d groups, want 1", len(got)) + } + g := got[0] + if g.SurvivorTrackID != uuidToString(dupUUID(31)) || g.SurvivorReason != "the copy Lidarr tracks" { + t.Errorf("survivor = (%s, %q), want the tracked mp3", g.SurvivorTrackID, g.SurvivorReason) + } + if g.Class == nil || *g.Class != "same_release" || g.Members[1].LidarrState == nil || *g.Members[1].LidarrState != "tracked" { + t.Errorf("class or lidarr state not carried: %+v", g) + } +} diff --git a/internal/api/admin_fingerprint_settings.go b/internal/api/admin_fingerprint_settings.go index bf049b68..d21bbbc0 100644 --- a/internal/api/admin_fingerprint_settings.go +++ b/internal/api/admin_fingerprint_settings.go @@ -18,6 +18,9 @@ type fingerprintSettingsBody struct { AcousticMaxBitErrorRate float64 `json:"acoustic_max_bit_error_rate"` BackfillConcurrency int32 `json:"backfill_concurrency"` SweepIntervalHours int32 `json:"sweep_interval_hours"` + // AutoResolve is a pointer so a client that predates it (M498) and leaves + // it out keeps the saved value rather than switching the resolver off. + AutoResolve *bool `json:"auto_resolve"` } func fingerprintSettingsBodyOf(s library.FingerprintSettings) fingerprintSettingsBody { @@ -27,6 +30,7 @@ func fingerprintSettingsBodyOf(s library.FingerprintSettings) fingerprintSetting AcousticMaxBitErrorRate: s.AcousticMaxBitErrorRate, BackfillConcurrency: s.BackfillConcurrency, SweepIntervalHours: s.SweepIntervalHours, + AutoResolve: &s.AutoResolve, } } @@ -39,6 +43,7 @@ func (h *handlers) handleGetFingerprintSettings(w http.ResponseWriter, _ *http.R // // A whole-row write. A body that leaves a field out decodes it as zero, which no // field accepts, so a partial save is refused rather than zeroing what it omitted. +// auto_resolve is the exception: left out, it keeps its saved value. // The saved settings reach the scanner and both workers at once: they share the // service instance. func (h *handlers) handleUpdateFingerprintSettings(w http.ResponseWriter, r *http.Request) { @@ -47,12 +52,17 @@ func (h *handlers) handleUpdateFingerprintSettings(w http.ResponseWriter, r *htt writeErr(w, apierror.BadRequest("invalid_body", "malformed JSON")) return } + autoResolve := h.fingerprintSettings.Get().AutoResolve + if req.AutoResolve != nil { + autoResolve = *req.AutoResolve + } saved, err := h.fingerprintSettings.Set(r.Context(), library.FingerprintSettings{ Enabled: req.Enabled, ChromaprintLengthSec: req.ChromaprintLengthSec, AcousticMaxBitErrorRate: req.AcousticMaxBitErrorRate, BackfillConcurrency: req.BackfillConcurrency, SweepIntervalHours: req.SweepIntervalHours, + AutoResolve: autoResolve, }) if err != nil { // Validation mirrors migration 0061's CHECKs and names the field. diff --git a/internal/api/admin_fingerprint_settings_test.go b/internal/api/admin_fingerprint_settings_test.go index 056447ea..0dfaedf3 100644 --- a/internal/api/admin_fingerprint_settings_test.go +++ b/internal/api/admin_fingerprint_settings_test.go @@ -23,7 +23,13 @@ func TestGetFingerprintSettings_ServesDefaultsWithoutAService(t *testing.T) { if err := json.NewDecoder(rec.Body).Decode(&got); err != nil { t.Fatalf("decode: %v", err) } - if want := fingerprintSettingsBodyOf(library.DefaultFingerprintSettings); got != want { + want := fingerprintSettingsBodyOf(library.DefaultFingerprintSettings) + // auto_resolve is a pointer, so it is compared by value and then set aside. + if got.AutoResolve == nil || *got.AutoResolve != *want.AutoResolve { + t.Fatalf("auto_resolve = %v, want %v", got.AutoResolve, *want.AutoResolve) + } + got.AutoResolve, want.AutoResolve = nil, nil + if got != want { t.Fatalf("body = %+v, want the defaults %+v", got, want) } } diff --git a/internal/api/admin_suspect_sources.go b/internal/api/admin_suspect_sources.go index c58d95e8..7e6c36d5 100644 --- a/internal/api/admin_suspect_sources.go +++ b/internal/api/admin_suspect_sources.go @@ -2,74 +2,11 @@ package api import ( "net/http" - "path" - "regexp" - "strings" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/library" ) -// sourceMarker is one sign that a file was ripped from a video rather than -// taken from a release: words a video title carries and an album track does -// not (#5410). -// -// Each pattern is used twice, so it is written in the subset that Postgres's -// regex flavour and Go's RE2 read the same way: no \b (Postgres spells word -// boundaries \y), no lookaround, and brackets written as (\(|\[) rather than -// as a bracket expression. The SQL side matches it case-insensitively with ~*, -// the Go side with (?i); [^a-zA-Z] is spelled out so neither has to fold case -// inside a negated class. -type sourceMarker struct { - label string - pattern string -} - -// suspectSourceMarkers was calibrated against the operator's library on -// 2026-10-08. Two candidates were left out on purpose: -// - "live in"/"live at": 229 files, nearly all from real live albums. -// - a bare "reaction": it caught Beck's "Chain Reaction". The marker below -// wants the phrases a reaction video actually uses. -var suspectSourceMarkers = []sourceMarker{ - {"music video", `official[^a-zA-Z]*(music[^a-zA-Z]*)?video|music[^a-zA-Z]*video`}, - {"official audio", `official[^a-zA-Z]*audio`}, - {"lyric video", `lyrics?[^a-zA-Z]*video`}, - {"visualiser", `visuali[sz]er`}, - {"reaction", `reaction[^a-zA-Z]*(video|mashup)|reacts?[^a-zA-Z]+to[^a-zA-Z]|first[^a-zA-Z]*time[^a-zA-Z]*(hearing|listening)`}, - {"MV", `(^|[^a-zA-Z])(mv|m/v)([^a-zA-Z]|$)`}, - {"[Audio]", `(\(|\[)audio(\)|\])`}, - {"[HD]", `(\(|\[)(hd|hq|4k)(\)|\])`}, -} - -// suspectSourcePattern is every marker as one alternation, for the SQL filter. -var suspectSourcePattern = func() string { - parts := make([]string, len(suspectSourceMarkers)) - for i, m := range suspectSourceMarkers { - parts[i] = "(" + m.pattern + ")" - } - return strings.Join(parts, "|") -}() - -var suspectSourceRegexps = func() []*regexp.Regexp { - out := make([]*regexp.Regexp, len(suspectSourceMarkers)) - for i, m := range suspectSourceMarkers { - out[i] = regexp.MustCompile("(?i)" + m.pattern) - } - return out -}() - -// sourceMarkersFor returns the labels of every marker the file's basename -// carries, in list order. Empty for a file the SQL filter would not return. -func sourceMarkersFor(filePath string) []string { - base := path.Base(filePath) - labels := make([]string, 0, 2) - for i, re := range suspectSourceRegexps { - if re.MatchString(base) { - labels = append(labels, suspectSourceMarkers[i].label) - } - } - return labels -} - // suspectTrackView is one flagged track. FilePath is the evidence: the // markers are read from its basename, and the operator needs to see it to // judge whether the flag is right. @@ -116,14 +53,14 @@ func (h *handlers) handleListSuspectSources(w http.ResponseWriter, r *http.Reque } q := dbq.New(h.pool) - total, err := q.CountSuspectSourceTracks(r.Context(), suspectSourcePattern) + total, err := q.CountSuspectSourceTracks(r.Context(), library.SuspectSourcePattern) if err != nil { h.logger.Error("admin: count suspect-source tracks", "err", err) writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") return } rows, err := q.ListSuspectSourceTracks(r.Context(), dbq.ListSuspectSourceTracksParams{ - Pattern: suspectSourcePattern, + Pattern: library.SuspectSourcePattern, PageLimit: int32(limit), PageOffset: int32(offset), }) @@ -158,7 +95,7 @@ func groupSuspectByDirectory(rows []dbq.ListSuspectSourceTracksRow) []suspectGro DurationSec: row.DurationMs / 1000, DiscNumber: row.DiscNumber, TrackNumber: row.TrackNumber, - Markers: sourceMarkersFor(row.FilePath), + Markers: library.SourceMarkersFor(row.FilePath), } if n := len(groups); n > 0 && groups[n-1].Directory == row.Directory { groups[n-1].Tracks = append(groups[n-1].Tracks, t) diff --git a/internal/api/admin_suspect_sources_test.go b/internal/api/admin_suspect_sources_test.go index 2956838a..bf812a55 100644 --- a/internal/api/admin_suspect_sources_test.go +++ b/internal/api/admin_suspect_sources_test.go @@ -9,53 +9,6 @@ import ( "testing" ) -// Real basenames from the operator's library (2026-10-08), each with the -// labels it should carry. The negatives are the near misses that shaped the -// list: a song called "Chain Reaction", a live album, an ordinary track. -func TestSourceMarkersFor(t *testing.T) { - cases := []struct { - base string - want []string - }{ - {"Daft Punk - Random Access Memories - 06 - Daft Punk - Doin' It Right (Music Video) ft. Panda Bear.mp3", []string{"music video"}}, - {"Gorillaz - Humanz - 03 - Gorillaz - Saturnz Barz (Official Video).mp3", []string{"music video"}}, - {"Gorillaz - Humanz - 05 - Gorillaz - Andromeda (Official Audio).mp3", []string{"official audio"}}, - {"Gorillaz - Gorillaz - 06 - Gorillaz - P45 (Visualizer).mp3", []string{"visualiser"}}, - {"Gorillaz - Demon Days - 16 - Gorillaz - Don Quixote's Christmas Bonanza (Visualiser).mp3", []string{"visualiser"}}, - {"Artist - Album - 01 - Artist - Song (Lyric Video).mp3", []string{"lyric video"}}, - {"Gorillaz - Humanz - 02 - FIRST TIME HEARING Gorillaz - Ascension REACTION.mp3", []string{"reaction"}}, - {"Watsky - INTENTION - 05 - MANIAC Reacts to Watsky - AWW SHiT.mp3", []string{"reaction"}}, - {"米津玄師 - diorama - 09 - 【MV】米津玄師 - 恋と病熱.mp3", []string{"MV"}}, - {"Andora - Ego - 01 - Andora - Ego (feat. Will Stetson) MV.mp3", []string{"MV"}}, - {"Jimmy Eat World - Something(s) Loud - 05 - Jimmy Eat World - Call to Love (Audio).mp3", []string{"[Audio]"}}, - {"Record Heat - World War IV - 03 - Record Heat - Front Seat Feelin' [Audio].mp3", []string{"[Audio]"}}, - {"Aphex Twin - Come To Daddy - 03 - Aphex Twin - Bucephalus Bouncing Ball (HQ).mp3", []string{"[HD]"}}, - {"Artist - Album - 01 - Artist - Song (Official Lyric Video) [4K].mp3", []string{"lyric video", "[HD]"}}, - - {"Beck - Guero - 15 - Beck - Chain Reaction.mp3", nil}, - {"Nirvana - MTV Unplugged in New York - 01 - About a Girl (live in New York).flac", nil}, - {"Boards of Canada - Music Has the Right to Children - 05 - Roygbiv.flac", nil}, - {"Artist - Album - 01 - Mvula.flac", nil}, - } - for _, c := range cases { - got := sourceMarkersFor("/music/x/" + c.base) - if len(got) == 0 && len(c.want) == 0 { - continue // nil and empty both mean "no markers" - } - if !slices.Equal(got, c.want) { - t.Errorf("%s:\n got %v\n want %v", c.base, got, c.want) - } - } -} - -// The folder is not evidence: a marker word in a directory name must not -// flag the files inside it. -func TestSourceMarkersFor_ReadsOnlyTheBasename(t *testing.T) { - if got := sourceMarkersFor("/music/Official Video Collection/01 - Song.flac"); len(got) != 0 { - t.Errorf("directory name flagged the file: %v", got) - } -} - // The integration test runs the same alternation through Postgres's ~*, so the // one pattern is exercised in both regex dialects it has to work in. func TestHandleListSuspectSources_Integration(t *testing.T) { diff --git a/internal/api/api.go b/internal/api/api.go index 8d752505..de6eef0a 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -269,6 +269,7 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev // trigger to sweep now, dismissal, and the merge (#3911), which deletes // the removed copies' files after moving their history onto the kept one. admin.Get("/library/duplicates", h.handleListDuplicates) + admin.Get("/library/duplicates/resolved", h.handleListResolvedDuplicates) admin.Post("/library/duplicates/sweep", h.handleRunDuplicateSweep) admin.Post("/library/duplicates/{id}/dismiss", h.handleDismissDuplicateGroup) admin.Post("/library/duplicates/{id}/merge", h.handleMergeDuplicateGroup) diff --git a/internal/audit/audit.go b/internal/audit/audit.go index b9e92c5d..3097bc23 100644 --- a/internal/audit/audit.go +++ b/internal/audit/audit.go @@ -61,6 +61,11 @@ const ( // log can answer "where did that file go" long after the report is gone. ActionDuplicateMerge Action = "duplicate_merge" + // Lidarr release change (M498 #5437): the duplicate resolver moved an + // album's monitored release in Lidarr to one that lists each song once. + // Written with no actor; the metadata names the album and both releases. + ActionLidarrReleaseChange Action = "lidarr_release_change" + // Subsonic password (#5026): generated in Settings for t/s-only clients. ActionSubsonicPasswordSet Action = "subsonic_password_set" ActionSubsonicPasswordClear Action = "subsonic_password_clear" diff --git a/internal/db/dbq/audit_log.sql.go b/internal/db/dbq/audit_log.sql.go index e02d9a9c..1a69e07b 100644 --- a/internal/db/dbq/audit_log.sql.go +++ b/internal/db/dbq/audit_log.sql.go @@ -11,6 +11,25 @@ import ( "github.com/jackc/pgx/v5/pgtype" ) +const countAuditLogByActions = `-- name: CountAuditLogByActions :one +SELECT count(*)::bigint FROM audit_log + WHERE action = ANY($1::text[]) + AND (NOT $2::boolean OR actor_id IS NULL) +` + +type CountAuditLogByActionsParams struct { + Actions []string + SystemOnly bool +} + +// System actions carry no actor; system_only narrows to them. +func (q *Queries) CountAuditLogByActions(ctx context.Context, arg CountAuditLogByActionsParams) (int64, error) { + row := q.db.QueryRow(ctx, countAuditLogByActions, arg.Actions, arg.SystemOnly) + var column_1 int64 + err := row.Scan(&column_1) + return column_1, err +} + const listAuditLog = `-- name: ListAuditLog :many SELECT id, actor_id, target_id, action, metadata, created_at FROM audit_log ORDER BY created_at DESC @@ -49,6 +68,56 @@ func (q *Queries) ListAuditLog(ctx context.Context, arg ListAuditLogParams) ([]A return items, nil } +const listAuditLogByActions = `-- name: ListAuditLogByActions :many +SELECT id, actor_id, target_id, action, metadata, created_at FROM audit_log + WHERE action = ANY($1::text[]) + AND (NOT $2::boolean OR actor_id IS NULL) + ORDER BY created_at DESC, id + LIMIT $4 OFFSET $3 +` + +type ListAuditLogByActionsParams struct { + Actions []string + SystemOnly bool + PageOffset int32 + PageLimit int32 +} + +// One page of the given actions, newest first: the duplicates report's +// "resolved automatically" activity (M498) reads the resolver's merges and +// Lidarr release changes from here. +func (q *Queries) ListAuditLogByActions(ctx context.Context, arg ListAuditLogByActionsParams) ([]AuditLog, error) { + rows, err := q.db.Query(ctx, listAuditLogByActions, + arg.Actions, + arg.SystemOnly, + arg.PageOffset, + arg.PageLimit, + ) + if err != nil { + return nil, err + } + defer rows.Close() + var items []AuditLog + for rows.Next() { + var i AuditLog + if err := rows.Scan( + &i.ID, + &i.ActorID, + &i.TargetID, + &i.Action, + &i.Metadata, + &i.CreatedAt, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const writeAuditLog = `-- name: WriteAuditLog :exec INSERT INTO audit_log (actor_id, target_id, action, metadata) VALUES ($1, $2, $3, $4) diff --git a/internal/db/dbq/duplicates.sql.go b/internal/db/dbq/duplicates.sql.go index beb0161e..51aaf8be 100644 --- a/internal/db/dbq/duplicates.sql.go +++ b/internal/db/dbq/duplicates.sql.go @@ -62,6 +62,29 @@ func (q *Queries) CountPendingDuplicateGroups(ctx context.Context) (int64, error return column_1, err } +const countPendingDuplicateGroupsInView = `-- name: CountPendingDuplicateGroupsInView :one +SELECT count(*)::bigint + FROM duplicate_groups g + WHERE g.status = 'pending' + AND (SELECT count(*) FROM duplicate_group_members m WHERE m.group_id = g.id) >= 2 + AND CASE $1::text + WHEN 'cross_release' THEN g.class = 'cross_release' + ELSE g.class IS DISTINCT FROM 'cross_release' + END +` + +// CountPendingDuplicateGroups narrowed to one view of the report (M498): +// +// review what the operator still has to decide: every class but +// cross_release, and groups not yet classified +// cross_release the same song on different releases, which is kept, not merged +func (q *Queries) CountPendingDuplicateGroupsInView(ctx context.Context, view string) (int64, error) { + row := q.db.QueryRow(ctx, countPendingDuplicateGroupsInView, view) + var column_1 int64 + err := row.Scan(&column_1) + return column_1, err +} + const deleteStalePendingDuplicateGroups = `-- name: DeleteStalePendingDuplicateGroups :execrows DELETE FROM duplicate_groups g WHERE g.status = 'pending' @@ -187,6 +210,32 @@ func (q *Queries) GetLatestFingerprintComputedAt(ctx context.Context) (pgtype.Ti return latest, err } +const listAlbumPresentTrackTitles = `-- name: ListAlbumPresentTrackTitles :many +SELECT title FROM tracks WHERE album_id = $1 AND missing_since IS NULL +` + +// The titles an album has on disk, for choosing the Lidarr release that covers +// them best (M498 #5437). +func (q *Queries) ListAlbumPresentTrackTitles(ctx context.Context, albumID pgtype.UUID) ([]string, error) { + rows, err := q.db.Query(ctx, listAlbumPresentTrackTitles, albumID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []string + for rows.Next() { + var title string + if err := rows.Scan(&title); err != nil { + return nil, err + } + items = append(items, title) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const listDismissedDuplicateMemberSets = `-- name: ListDismissedDuplicateMemberSets :many SELECT g.id, array_agg(m.track_id ORDER BY m.track_id)::uuid[] AS track_ids FROM duplicate_groups g @@ -288,6 +337,98 @@ func (q *Queries) ListDuplicateCandidates(ctx context.Context, arg ListDuplicate return items, nil } +const listDuplicateGroupsForResolve = `-- name: ListDuplicateGroupsForResolve :many +SELECT g.id AS group_id, + g.tier, + t.id AS track_id, + t.album_id, + t.title, + artists.name AS artist_name, + t.file_path, + t.file_format, + t.file_size, + t.added_at, + t.disc_number, + t.track_number, + (t.mbid IS NOT NULL)::boolean AS has_mbid, + albums.release_group_mbid, + albums.title AS album_title, + EXISTS (SELECT 1 FROM tracks o + WHERE o.album_id = t.album_id AND o.id <> t.id AND o.missing_since IS NULL + AND t.track_number IS NOT NULL + AND o.disc_number IS NOT DISTINCT FROM t.disc_number + AND o.track_number = t.track_number)::boolean AS position_clash + FROM duplicate_groups g + JOIN duplicate_group_members m ON m.group_id = g.id + JOIN tracks t ON t.id = m.track_id + JOIN albums ON albums.id = t.album_id + JOIN artists ON artists.id = t.artist_id + WHERE g.status = 'pending' + AND t.missing_since IS NULL + ORDER BY g.id, t.id +` + +type ListDuplicateGroupsForResolveRow struct { + GroupID pgtype.UUID + Tier string + TrackID pgtype.UUID + AlbumID pgtype.UUID + Title string + ArtistName string + FilePath string + FileFormat string + FileSize int64 + AddedAt pgtype.Timestamptz + DiscNumber *int32 + TrackNumber *int32 + HasMbid bool + ReleaseGroupMbid *string + AlbumTitle string + PositionClash bool +} + +// Every pending group's present copies, for the resolver (M498 #5435): what it +// needs to classify the group and to choose the copy to keep. One row per copy, +// grouped by group. position_clash is true when another present track on the +// same album claims the same disc and track number: tags that do not fit the +// album, which the survivor rule weighs against a copy. +func (q *Queries) ListDuplicateGroupsForResolve(ctx context.Context) ([]ListDuplicateGroupsForResolveRow, error) { + rows, err := q.db.Query(ctx, listDuplicateGroupsForResolve) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListDuplicateGroupsForResolveRow + for rows.Next() { + var i ListDuplicateGroupsForResolveRow + if err := rows.Scan( + &i.GroupID, + &i.Tier, + &i.TrackID, + &i.AlbumID, + &i.Title, + &i.ArtistName, + &i.FilePath, + &i.FileFormat, + &i.FileSize, + &i.AddedAt, + &i.DiscNumber, + &i.TrackNumber, + &i.HasMbid, + &i.ReleaseGroupMbid, + &i.AlbumTitle, + &i.PositionClash, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const listExactDuplicateHashes = `-- name: ListExactDuplicateHashes :many SELECT f.audio_stream_sha256, array_agg(t.id ORDER BY t.id)::uuid[] AS track_ids @@ -329,17 +470,32 @@ func (q *Queries) ListExactDuplicateHashes(ctx context.Context, currentVersion i const listPendingDuplicateGroupMembers = `-- name: ListPendingDuplicateGroupMembers :many WITH page AS ( - SELECT g.id, g.tier, g.worst_bit_error_rate, g.detected_at + SELECT g.id, g.tier, g.worst_bit_error_rate, g.detected_at, g.class, g.resolve_note FROM duplicate_groups g WHERE g.status = 'pending' AND (SELECT count(*) FROM duplicate_group_members m WHERE m.group_id = g.id) >= 2 + AND CASE $1::text + WHEN 'cross_release' THEN g.class = 'cross_release' + ELSE g.class IS DISTINCT FROM 'cross_release' + END ORDER BY g.detected_at DESC, g.id - LIMIT $2 OFFSET $1 + LIMIT $3 OFFSET $2 ) SELECT p.id AS group_id, p.tier, p.worst_bit_error_rate, p.detected_at, + p.class, + p.resolve_note, + m.lidarr_state, + t.disc_number, + t.track_number, + (t.mbid IS NOT NULL)::boolean AS has_mbid, + EXISTS (SELECT 1 FROM tracks o + WHERE o.album_id = t.album_id AND o.id <> t.id AND o.missing_since IS NULL + AND t.track_number IS NOT NULL + AND o.disc_number IS NOT DISTINCT FROM t.disc_number + AND o.track_number = t.track_number)::boolean AS position_clash, t.id AS track_id, t.title, artists.name AS artist_name, @@ -361,6 +517,7 @@ SELECT p.id AS group_id, ` type ListPendingDuplicateGroupMembersParams struct { + View string PageOffset int32 PageLimit int32 } @@ -370,6 +527,13 @@ type ListPendingDuplicateGroupMembersRow struct { Tier string WorstBitErrorRate *float32 DetectedAt pgtype.Timestamptz + Class *string + ResolveNote *string + LidarrState *string + DiscNumber *int32 + TrackNumber *int32 + HasMbid bool + PositionClash bool TrackID pgtype.UUID Title string ArtistName string @@ -384,12 +548,14 @@ type ListPendingDuplicateGroupMembersRow struct { PlayCount int64 } -// One page of proposals, newest first, flattened to one row per member so the -// handler folds them without a query per group. What each copy carries — likes -// and plays from every user — is here because it is what the operator weighs -// when deciding which copy to keep. +// One page of proposals in one view (see CountPendingDuplicateGroupsInView), +// newest first, flattened to one row per member so the handler folds them +// without a query per group. What each copy carries — likes and plays from +// every user — is here because it is what the operator weighs when deciding +// which copy to keep; Lidarr's view of it, because a copy Lidarr tracks cannot +// be removed without Lidarr downloading it again. func (q *Queries) ListPendingDuplicateGroupMembers(ctx context.Context, arg ListPendingDuplicateGroupMembersParams) ([]ListPendingDuplicateGroupMembersRow, error) { - rows, err := q.db.Query(ctx, listPendingDuplicateGroupMembers, arg.PageOffset, arg.PageLimit) + rows, err := q.db.Query(ctx, listPendingDuplicateGroupMembers, arg.View, arg.PageOffset, arg.PageLimit) if err != nil { return nil, err } @@ -402,6 +568,13 @@ func (q *Queries) ListPendingDuplicateGroupMembers(ctx context.Context, arg List &i.Tier, &i.WorstBitErrorRate, &i.DetectedAt, + &i.Class, + &i.ResolveNote, + &i.LidarrState, + &i.DiscNumber, + &i.TrackNumber, + &i.HasMbid, + &i.PositionClash, &i.TrackID, &i.Title, &i.ArtistName, @@ -425,6 +598,64 @@ func (q *Queries) ListPendingDuplicateGroupMembers(ctx context.Context, arg List return items, nil } +const markDuplicateGroupResolvedAutomatically = `-- name: MarkDuplicateGroupResolvedAutomatically :exec +UPDATE duplicate_groups SET resolved_automatically = true WHERE id = $1 +` + +func (q *Queries) MarkDuplicateGroupResolvedAutomatically(ctx context.Context, id pgtype.UUID) error { + _, err := q.db.Exec(ctx, markDuplicateGroupResolvedAutomatically, id) + return err +} + +const setDuplicateGroupClasses = `-- name: SetDuplicateGroupClasses :exec +UPDATE duplicate_groups g + SET class = v.class, resolve_note = NULLIF(v.note, '') + FROM (SELECT unnest($1::uuid[]) AS id, + unnest($2::text[]) AS class, + unnest($3::text[]) AS note) v + WHERE g.id = v.id + AND g.status = 'pending' + AND (g.class IS DISTINCT FROM v.class OR g.resolve_note IS DISTINCT FROM NULLIF(v.note, '')) +` + +type SetDuplicateGroupClassesParams struct { + GroupIds []pgtype.UUID + Classes []string + Notes []string +} + +// The resolver's verdicts, written in one statement. Only rows whose class or +// note actually changed are touched, so an hourly pass over thousands of +// unchanged groups writes nothing. An empty note clears it. +func (q *Queries) SetDuplicateGroupClasses(ctx context.Context, arg SetDuplicateGroupClassesParams) error { + _, err := q.db.Exec(ctx, setDuplicateGroupClasses, arg.GroupIds, arg.Classes, arg.Notes) + return err +} + +const setDuplicateMemberLidarrStates = `-- name: SetDuplicateMemberLidarrStates :exec +UPDATE duplicate_group_members m + SET lidarr_state = NULLIF(v.state, '') + FROM (SELECT unnest($1::uuid[]) AS group_id, + unnest($2::uuid[]) AS track_id, + unnest($3::text[]) AS state) v + WHERE m.group_id = v.group_id + AND m.track_id = v.track_id + AND m.lidarr_state IS DISTINCT FROM NULLIF(v.state, '') +` + +type SetDuplicateMemberLidarrStatesParams struct { + GroupIds []pgtype.UUID + TrackIds []pgtype.UUID + States []string +} + +// Lidarr's view of each copy as of this pass; an empty state clears it (Lidarr +// not consulted). Unchanged rows are left alone. +func (q *Queries) SetDuplicateMemberLidarrStates(ctx context.Context, arg SetDuplicateMemberLidarrStatesParams) error { + _, err := q.db.Exec(ctx, setDuplicateMemberLidarrStates, arg.GroupIds, arg.TrackIds, arg.States) + return err +} + const startDuplicateSweep = `-- name: StartDuplicateSweep :one INSERT INTO duplicate_sweeps DEFAULT VALUES RETURNING id, started_at ` diff --git a/internal/db/dbq/fingerprint_settings.sql.go b/internal/db/dbq/fingerprint_settings.sql.go index ab1ab634..62086971 100644 --- a/internal/db/dbq/fingerprint_settings.sql.go +++ b/internal/db/dbq/fingerprint_settings.sql.go @@ -10,7 +10,7 @@ import ( ) const getFingerprintSettings = `-- name: GetFingerprintSettings :one -SELECT id, enabled, chromaprint_length_sec, acoustic_max_bit_error_rate, backfill_concurrency, sweep_interval_hours, updated_at FROM fingerprint_settings WHERE id = true +SELECT id, enabled, chromaprint_length_sec, acoustic_max_bit_error_rate, backfill_concurrency, sweep_interval_hours, updated_at, auto_resolve FROM fingerprint_settings WHERE id = true ` func (q *Queries) GetFingerprintSettings(ctx context.Context) (FingerprintSetting, error) { @@ -24,6 +24,7 @@ func (q *Queries) GetFingerprintSettings(ctx context.Context) (FingerprintSettin &i.BackfillConcurrency, &i.SweepIntervalHours, &i.UpdatedAt, + &i.AutoResolve, ) return i, err } @@ -35,9 +36,10 @@ UPDATE fingerprint_settings acoustic_max_bit_error_rate = $3, backfill_concurrency = $4, sweep_interval_hours = $5, + auto_resolve = $6, updated_at = now() WHERE id = true -RETURNING id, enabled, chromaprint_length_sec, acoustic_max_bit_error_rate, backfill_concurrency, sweep_interval_hours, updated_at +RETURNING id, enabled, chromaprint_length_sec, acoustic_max_bit_error_rate, backfill_concurrency, sweep_interval_hours, updated_at, auto_resolve ` type UpdateFingerprintSettingsParams struct { @@ -46,6 +48,7 @@ type UpdateFingerprintSettingsParams struct { AcousticMaxBitErrorRate float64 BackfillConcurrency int32 SweepIntervalHours int32 + AutoResolve bool } // Whole-row write from the admin card; migration 0061's CHECKs are the backstop @@ -57,6 +60,7 @@ func (q *Queries) UpdateFingerprintSettings(ctx context.Context, arg UpdateFinge arg.AcousticMaxBitErrorRate, arg.BackfillConcurrency, arg.SweepIntervalHours, + arg.AutoResolve, ) var i FingerprintSetting err := row.Scan( @@ -67,6 +71,7 @@ func (q *Queries) UpdateFingerprintSettings(ctx context.Context, arg UpdateFinge &i.BackfillConcurrency, &i.SweepIntervalHours, &i.UpdatedAt, + &i.AutoResolve, ) return i, err } diff --git a/internal/db/dbq/models.go b/internal/db/dbq/models.go index 03a4aa19..b80a0111 100644 --- a/internal/db/dbq/models.go +++ b/internal/db/dbq/models.go @@ -323,19 +323,23 @@ type DiscoverTuning struct { } type DuplicateGroup struct { - ID pgtype.UUID - MemberKey string - Tier string - WorstBitErrorRate *float32 - Status string - DetectedAt pgtype.Timestamptz - LastSeenSweepID pgtype.UUID - ResolvedAt pgtype.Timestamptz + ID pgtype.UUID + MemberKey string + Tier string + WorstBitErrorRate *float32 + Status string + DetectedAt pgtype.Timestamptz + LastSeenSweepID pgtype.UUID + ResolvedAt pgtype.Timestamptz + Class *string + ResolveNote *string + ResolvedAutomatically bool } type DuplicateGroupMember struct { - GroupID pgtype.UUID - TrackID pgtype.UUID + GroupID pgtype.UUID + TrackID pgtype.UUID + LidarrState *string } type DuplicateSweep struct { @@ -356,6 +360,7 @@ type FingerprintSetting struct { BackfillConcurrency int32 SweepIntervalHours int32 UpdatedAt pgtype.Timestamptz + AutoResolve bool } type GeneralLike struct { diff --git a/internal/db/migrations/0075_duplicate_resolve.down.sql b/internal/db/migrations/0075_duplicate_resolve.down.sql new file mode 100644 index 00000000..60372195 --- /dev/null +++ b/internal/db/migrations/0075_duplicate_resolve.down.sql @@ -0,0 +1,17 @@ +DELETE FROM user_notifications WHERE kind = 'duplicates_resolved'; +DELETE FROM user_notification_prefs WHERE kind = 'duplicates_resolved'; +ALTER TABLE user_notification_prefs DROP CONSTRAINT user_notification_prefs_kind_check; +ALTER TABLE user_notification_prefs ADD CONSTRAINT user_notification_prefs_kind_check + CHECK (kind IN ('request_approved', 'request_rejected', 'request_completed', + 'request_pending', 'quarantine_flagged', 'scan_failed', + 'tracks_missing', 'duplicates_found', 'playback_errors')); +ALTER TABLE user_notifications DROP CONSTRAINT user_notifications_kind_check; +ALTER TABLE user_notifications ADD CONSTRAINT user_notifications_kind_check + CHECK (kind IN ('request_approved', 'request_rejected', 'request_completed', + 'request_pending', 'quarantine_flagged', 'scan_failed', + 'tracks_missing', 'duplicates_found', 'playback_errors')); +ALTER TABLE fingerprint_settings DROP COLUMN IF EXISTS auto_resolve; +ALTER TABLE duplicate_group_members DROP COLUMN IF EXISTS lidarr_state; +ALTER TABLE duplicate_groups DROP COLUMN IF EXISTS resolved_automatically; +ALTER TABLE duplicate_groups DROP COLUMN IF EXISTS resolve_note; +ALTER TABLE duplicate_groups DROP COLUMN IF EXISTS class; diff --git a/internal/db/migrations/0075_duplicate_resolve.up.sql b/internal/db/migrations/0075_duplicate_resolve.up.sql new file mode 100644 index 00000000..29431611 --- /dev/null +++ b/internal/db/migrations/0075_duplicate_resolve.up.sql @@ -0,0 +1,56 @@ +-- 0075_duplicate_resolve.up.sql — duplicates resolve themselves (Scribe +-- milestone #498: #5435, #5436, #5437). +-- +-- The sweep (0059) proposes groups and, until now, every proposal waited for the +-- operator. The resolver classifies each pending group and acts on the classes +-- that are safe. What makes deletion safe is Lidarr's view of the file: Lidarr +-- maps one file to each track of the release it monitors, so deleting a mapped +-- file opens a hole it fills by downloading again. Only a copy Lidarr does not +-- map may go. + +-- What kind of duplicate a group is. NULL until the resolver has classified it. +-- same_release every copy is on one album +-- cross_release the same song on different releases (a single and its album) +-- mismatch identical audio under different titles on different albums: +-- a file imported as the wrong song +-- review anything else: the operator decides +-- Rule 36: a new value swaps this constraint in the same migration. +ALTER TABLE duplicate_groups ADD COLUMN class text; +ALTER TABLE duplicate_groups ADD CONSTRAINT duplicate_groups_class_check + CHECK (class IN ('same_release', 'cross_release', 'mismatch', 'review')); + +-- Why the resolver left a group for the operator, in words the report shows +-- ("Lidarr tracks every copy as its own track"). NULL when it had nothing to say. +ALTER TABLE duplicate_groups ADD COLUMN resolve_note text; + +-- True when the resolver merged the group itself, so the report can tell its +-- merges from the operator's. +ALTER TABLE duplicate_groups ADD COLUMN resolved_automatically boolean NOT NULL DEFAULT false; + +-- Lidarr's view of each copy, as of the resolver's last pass: +-- tracked Lidarr holds the file and it is not among its unmapped files +-- unmapped Lidarr holds the file but maps it to no track: removing it opens +-- no hole +-- NULL when Lidarr was not consulted (disabled, or unreachable that pass). +ALTER TABLE duplicate_group_members ADD COLUMN lidarr_state text; +ALTER TABLE duplicate_group_members ADD CONSTRAINT duplicate_group_members_lidarr_state_check + CHECK (lidarr_state IN ('tracked', 'unmapped')); + +-- Rule 25: the operator can turn the resolver off. On by default: the operator +-- asked for duplicates to resolve themselves (2026-10-09). +ALTER TABLE fingerprint_settings ADD COLUMN auto_resolve boolean NOT NULL DEFAULT true; + +-- The resolver tells admins what it did (rule 36: the kind CHECKs of 0073 are +-- swapped to carry the new value). Postgres named both from their columns. +ALTER TABLE user_notifications DROP CONSTRAINT user_notifications_kind_check; +ALTER TABLE user_notifications ADD CONSTRAINT user_notifications_kind_check + CHECK (kind IN ('request_approved', 'request_rejected', 'request_completed', + 'request_pending', 'quarantine_flagged', 'scan_failed', + 'tracks_missing', 'duplicates_found', 'duplicates_resolved', + 'playback_errors')); +ALTER TABLE user_notification_prefs DROP CONSTRAINT user_notification_prefs_kind_check; +ALTER TABLE user_notification_prefs ADD CONSTRAINT user_notification_prefs_kind_check + CHECK (kind IN ('request_approved', 'request_rejected', 'request_completed', + 'request_pending', 'quarantine_flagged', 'scan_failed', + 'tracks_missing', 'duplicates_found', 'duplicates_resolved', + 'playback_errors')); diff --git a/internal/db/queries/audit_log.sql b/internal/db/queries/audit_log.sql index aa126da6..153a5cb6 100644 --- a/internal/db/queries/audit_log.sql +++ b/internal/db/queries/audit_log.sql @@ -6,3 +6,19 @@ VALUES ($1, $2, $3, $4); SELECT * FROM audit_log ORDER BY created_at DESC LIMIT $1 OFFSET $2; + +-- name: CountAuditLogByActions :one +-- System actions carry no actor; system_only narrows to them. +SELECT count(*)::bigint FROM audit_log + WHERE action = ANY(sqlc.arg(actions)::text[]) + AND (NOT sqlc.arg(system_only)::boolean OR actor_id IS NULL); + +-- name: ListAuditLogByActions :many +-- One page of the given actions, newest first: the duplicates report's +-- "resolved automatically" activity (M498) reads the resolver's merges and +-- Lidarr release changes from here. +SELECT * FROM audit_log + WHERE action = ANY(sqlc.arg(actions)::text[]) + AND (NOT sqlc.arg(system_only)::boolean OR actor_id IS NULL) + ORDER BY created_at DESC, id + LIMIT sqlc.arg(page_limit) OFFSET sqlc.arg(page_offset); diff --git a/internal/db/queries/duplicates.sql b/internal/db/queries/duplicates.sql index 0aada774..7ca041be 100644 --- a/internal/db/queries/duplicates.sql +++ b/internal/db/queries/duplicates.sql @@ -112,16 +112,36 @@ SELECT count(*)::bigint WHERE g.status = 'pending' AND (SELECT count(*) FROM duplicate_group_members m WHERE m.group_id = g.id) >= 2; +-- name: CountPendingDuplicateGroupsInView :one +-- CountPendingDuplicateGroups narrowed to one view of the report (M498): +-- review what the operator still has to decide: every class but +-- cross_release, and groups not yet classified +-- cross_release the same song on different releases, which is kept, not merged +SELECT count(*)::bigint + FROM duplicate_groups g + WHERE g.status = 'pending' + AND (SELECT count(*) FROM duplicate_group_members m WHERE m.group_id = g.id) >= 2 + AND CASE sqlc.arg(view)::text + WHEN 'cross_release' THEN g.class = 'cross_release' + ELSE g.class IS DISTINCT FROM 'cross_release' + END; + -- name: ListPendingDuplicateGroupMembers :many --- One page of proposals, newest first, flattened to one row per member so the --- handler folds them without a query per group. What each copy carries — likes --- and plays from every user — is here because it is what the operator weighs --- when deciding which copy to keep. +-- One page of proposals in one view (see CountPendingDuplicateGroupsInView), +-- newest first, flattened to one row per member so the handler folds them +-- without a query per group. What each copy carries — likes and plays from +-- every user — is here because it is what the operator weighs when deciding +-- which copy to keep; Lidarr's view of it, because a copy Lidarr tracks cannot +-- be removed without Lidarr downloading it again. WITH page AS ( - SELECT g.id, g.tier, g.worst_bit_error_rate, g.detected_at + SELECT g.id, g.tier, g.worst_bit_error_rate, g.detected_at, g.class, g.resolve_note FROM duplicate_groups g WHERE g.status = 'pending' AND (SELECT count(*) FROM duplicate_group_members m WHERE m.group_id = g.id) >= 2 + AND CASE sqlc.arg(view)::text + WHEN 'cross_release' THEN g.class = 'cross_release' + ELSE g.class IS DISTINCT FROM 'cross_release' + END ORDER BY g.detected_at DESC, g.id LIMIT sqlc.arg(page_limit) OFFSET sqlc.arg(page_offset) ) @@ -129,6 +149,17 @@ SELECT p.id AS group_id, p.tier, p.worst_bit_error_rate, p.detected_at, + p.class, + p.resolve_note, + m.lidarr_state, + t.disc_number, + t.track_number, + (t.mbid IS NOT NULL)::boolean AS has_mbid, + EXISTS (SELECT 1 FROM tracks o + WHERE o.album_id = t.album_id AND o.id <> t.id AND o.missing_since IS NULL + AND t.track_number IS NOT NULL + AND o.disc_number IS NOT DISTINCT FROM t.disc_number + AND o.track_number = t.track_number)::boolean AS position_clash, t.id AS track_id, t.title, artists.name AS artist_name, @@ -164,3 +195,71 @@ SELECT count(*)::bigint FROM duplicate_groups WHERE status = 'pending' AND detected_at >= sqlc.arg(since); + +-- name: ListDuplicateGroupsForResolve :many +-- Every pending group's present copies, for the resolver (M498 #5435): what it +-- needs to classify the group and to choose the copy to keep. One row per copy, +-- grouped by group. position_clash is true when another present track on the +-- same album claims the same disc and track number: tags that do not fit the +-- album, which the survivor rule weighs against a copy. +SELECT g.id AS group_id, + g.tier, + t.id AS track_id, + t.album_id, + t.title, + artists.name AS artist_name, + t.file_path, + t.file_format, + t.file_size, + t.added_at, + t.disc_number, + t.track_number, + (t.mbid IS NOT NULL)::boolean AS has_mbid, + albums.release_group_mbid, + albums.title AS album_title, + EXISTS (SELECT 1 FROM tracks o + WHERE o.album_id = t.album_id AND o.id <> t.id AND o.missing_since IS NULL + AND t.track_number IS NOT NULL + AND o.disc_number IS NOT DISTINCT FROM t.disc_number + AND o.track_number = t.track_number)::boolean AS position_clash + FROM duplicate_groups g + JOIN duplicate_group_members m ON m.group_id = g.id + JOIN tracks t ON t.id = m.track_id + JOIN albums ON albums.id = t.album_id + JOIN artists ON artists.id = t.artist_id + WHERE g.status = 'pending' + AND t.missing_since IS NULL + ORDER BY g.id, t.id; + +-- name: SetDuplicateGroupClasses :exec +-- The resolver's verdicts, written in one statement. Only rows whose class or +-- note actually changed are touched, so an hourly pass over thousands of +-- unchanged groups writes nothing. An empty note clears it. +UPDATE duplicate_groups g + SET class = v.class, resolve_note = NULLIF(v.note, '') + FROM (SELECT unnest(sqlc.arg(group_ids)::uuid[]) AS id, + unnest(sqlc.arg(classes)::text[]) AS class, + unnest(sqlc.arg(notes)::text[]) AS note) v + WHERE g.id = v.id + AND g.status = 'pending' + AND (g.class IS DISTINCT FROM v.class OR g.resolve_note IS DISTINCT FROM NULLIF(v.note, '')); + +-- name: SetDuplicateMemberLidarrStates :exec +-- Lidarr's view of each copy as of this pass; an empty state clears it (Lidarr +-- not consulted). Unchanged rows are left alone. +UPDATE duplicate_group_members m + SET lidarr_state = NULLIF(v.state, '') + FROM (SELECT unnest(sqlc.arg(group_ids)::uuid[]) AS group_id, + unnest(sqlc.arg(track_ids)::uuid[]) AS track_id, + unnest(sqlc.arg(states)::text[]) AS state) v + WHERE m.group_id = v.group_id + AND m.track_id = v.track_id + AND m.lidarr_state IS DISTINCT FROM NULLIF(v.state, ''); + +-- name: MarkDuplicateGroupResolvedAutomatically :exec +UPDATE duplicate_groups SET resolved_automatically = true WHERE id = sqlc.arg(id); + +-- name: ListAlbumPresentTrackTitles :many +-- The titles an album has on disk, for choosing the Lidarr release that covers +-- them best (M498 #5437). +SELECT title FROM tracks WHERE album_id = sqlc.arg(album_id) AND missing_since IS NULL; diff --git a/internal/db/queries/fingerprint_settings.sql b/internal/db/queries/fingerprint_settings.sql index f1022bb1..010f5ff6 100644 --- a/internal/db/queries/fingerprint_settings.sql +++ b/internal/db/queries/fingerprint_settings.sql @@ -10,6 +10,7 @@ UPDATE fingerprint_settings acoustic_max_bit_error_rate = sqlc.arg(acoustic_max_bit_error_rate), backfill_concurrency = sqlc.arg(backfill_concurrency), sweep_interval_hours = sqlc.arg(sweep_interval_hours), + auto_resolve = sqlc.arg(auto_resolve), updated_at = now() WHERE id = true RETURNING *; diff --git a/internal/library/duplicate_classify.go b/internal/library/duplicate_classify.go new file mode 100644 index 00000000..70430d73 --- /dev/null +++ b/internal/library/duplicate_classify.go @@ -0,0 +1,143 @@ +package library + +import ( + "regexp" + "strings" + "unicode" +) + +// Duplicate classification (M498 #5435). +// +// 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: +// +// - same_release: every copy is on one album. An album holds a song once, so +// the extra copies are the ones to fold away — when Lidarr does not need +// them (the resolver checks). +// - 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 fulfils its own release in Lidarr, so none is deleted. +// - 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. + +// DuplicateClass is a group's shape. The values are duplicate_groups.class. +type DuplicateClass string + +const ( + ClassSameRelease DuplicateClass = "same_release" + ClassCrossRelease DuplicateClass = "cross_release" + ClassMismatch DuplicateClass = "mismatch" + ClassReview DuplicateClass = "review" +) + +// ClassifyMember is what classifying needs to know about one copy. +type ClassifyMember struct { + AlbumID string + Title string + ArtistName string +} + +// ClassifyDuplicateGroup decides a group's shape from its tier ("exact" or +// "acoustic") and its copies. +// +// 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. +func ClassifyDuplicateGroup(tier string, members []ClassifyMember) DuplicateClass { + if len(members) < 2 { + return ClassReview + } + oneAlbum, oneTitle := true, true + key := MatchTitleKey(members[0].Title, members[0].ArtistName) + for _, m := range members[1:] { + if m.AlbumID != members[0].AlbumID { + oneAlbum = false + } + if MatchTitleKey(m.Title, m.ArtistName) != key { + oneTitle = false + } + } + exact := tier == string(tierExact) + switch { + case oneAlbum && (oneTitle || exact): + return ClassSameRelease + case oneAlbum: + return ClassReview + case oneTitle: + return ClassCrossRelease + case exact: + return ClassMismatch + default: + return ClassReview + } +} + +var ( + // A bracketed credit: "(feat. Panda Bear)", "[ft. X]", "(featuring Y)". + featBracket = regexp.MustCompile(`(?i)[(\[][^)\]]*\b(feat\.?|ft\.?|featuring)\s[^)\]]*[)\]]`) + // A credit trailing the title without brackets: "Doin' It Right ft. Panda Bear". + featTrailing = regexp.MustCompile(`(?i)\s(feat\.?|ft\.?|featuring)\s.*$`) + // "(2011 Remaster)", "[Remastered]", "- Remastered 2009". + remasterBracket = regexp.MustCompile(`(?i)[(\[][^)\]]*remaster[^)\]]*[)\]]`) + remasterSuffix = regexp.MustCompile(`(?i)\s-\s[^-]*remaster.*$`) + // Any bracketed segment, to test each against the video-rip markers. + bracketed = regexp.MustCompile(`[(\[][^)\]]*[)\]]`) +) + +// MatchTitleKey reduces a title to what identifies the song, for deciding +// whether two copies carry the same one. +// +// Dropped: a leading "Artist - " (a video upload's title), bracketed featuring +// credits and remaster notes, and bracketed segments carrying a video-rip marker +// ("(Official Video)", "[HD]"). Kept: every other bracketed word, so "(live)", +// "(demo)", "(acoustic)", "(remix)" and "(instrumental)" still tell recordings +// apart. Then case, "&" against "and", and punctuation and spacing are ignored. +func MatchTitleKey(title, artist string) string { + t := strings.TrimSpace(title) + if a := strings.TrimSpace(artist); a != "" { + if prefix := a + " - "; len(t) > len(prefix) && strings.EqualFold(t[:len(prefix)], prefix) { + t = t[len(prefix):] + } + } + t = featBracket.ReplaceAllString(t, " ") + t = remasterBracket.ReplaceAllString(t, " ") + t = bracketed.ReplaceAllStringFunc(t, func(seg string) string { + for _, re := range suspectSourceRegexps { + if re.MatchString(seg) { + return " " + } + } + return seg + }) + t = remasterSuffix.ReplaceAllString(t, "") + t = featTrailing.ReplaceAllString(t, "") + if key := alnumKey(t); key != "" { + return key + } + return alnumKey(title) +} + +// ReleaseTitleKey is the lighter comparison for asking whether one release +// lists a song twice: case, "&" and punctuation only. Every word stays, so a +// box set's "Song" and "Song (demo)" are two songs, as the release means them. +func ReleaseTitleKey(title string) string { + return alnumKey(title) +} + +// alnumKey lowercases, reads "&" as "and", and keeps only letters and digits. +func alnumKey(s string) string { + s = strings.ReplaceAll(strings.ToLower(s), "&", " and ") + var b strings.Builder + for _, r := range s { + if unicode.IsLetter(r) || unicode.IsDigit(r) { + b.WriteRune(r) + } + } + return b.String() +} diff --git a/internal/library/duplicate_classify_test.go b/internal/library/duplicate_classify_test.go new file mode 100644 index 00000000..81f4e9f6 --- /dev/null +++ b/internal/library/duplicate_classify_test.go @@ -0,0 +1,71 @@ +package library + +import "testing" + +// The shapes measured on the operator's library (milestone 498's body). +func TestClassifyDuplicateGroup(t *testing.T) { + m := func(album, title string) ClassifyMember { + return ClassifyMember{AlbumID: album, Title: title, ArtistName: "Gorillaz"} + } + cases := []struct { + name string + tier string + members []ClassifyMember + want DuplicateClass + }{ + {"Humanz: one album, one song twice", "exact", + []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}, + {"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 lone copy", "exact", []ClassifyMember{m("a", "Song")}, ClassReview}, + } + for _, tc := range cases { + if got := ClassifyDuplicateGroup(tc.tier, tc.members); got != tc.want { + t.Errorf("%s: got %s, want %s", tc.name, got, tc.want) + } + } +} + +func TestMatchTitleKey(t *testing.T) { + cases := []struct{ title, artist, want string }{ + {"Saturnz Barz (Official Video)", "Gorillaz", "saturnzbarz"}, + {"Gorillaz - Saturnz Barz", "Gorillaz", "saturnzbarz"}, + {"Doin' It Right (Music Video) ft. Panda Bear", "Daft Punk", "doinitright"}, + {"Song [feat. X]", "", "song"}, + {"Song - Remastered 2009", "", "song"}, + {"Rock & Roll", "", "rockandroll"}, + // What names a different recording stays. + {"Song (live)", "", "songlive"}, + {"Song (demo)", "", "songdemo"}, + {"Song (Remix)", "", "songremix"}, + // A title that is nothing but a marker keeps its words rather than vanishing. + {"(Official Video)", "", "officialvideo"}, + } + for _, tc := range cases { + if got := MatchTitleKey(tc.title, tc.artist); got != tc.want { + t.Errorf("MatchTitleKey(%q, %q) = %q, want %q", tc.title, tc.artist, got, tc.want) + } + } +} + +// The release check keeps every word: a box set's demo is its own song. +func TestReleaseTitleKey(t *testing.T) { + if ReleaseTitleKey("Song (demo)") == ReleaseTitleKey("Song") { + t.Error("ReleaseTitleKey collapsed a demo into the song") + } + if ReleaseTitleKey("Hallelujah Money") != ReleaseTitleKey("hallelujah money!") { + t.Error("ReleaseTitleKey kept case or punctuation") + } +} diff --git a/internal/library/duplicate_merge.go b/internal/library/duplicate_merge.go index d6ee8a3b..fc2680e4 100644 --- a/internal/library/duplicate_merge.go +++ b/internal/library/duplicate_merge.go @@ -25,6 +25,16 @@ var ErrDuplicateGroupNotPending = errors.New("library: duplicate group is not pe // ErrSurvivorNotInGroup means the copy chosen to keep is not a member of the group. var ErrSurvivorNotInGroup = errors.New("library: survivor is not a member of the group") +// ErrCopyTrackedByLidarr means a copy the merge would remove is one Lidarr maps +// to a track of the release it monitors (M498). Removing it would open a hole +// that Lidarr fills by downloading the song again, so the merge refuses and +// changes nothing. +var ErrCopyTrackedByLidarr = errors.New("library: a copy to remove is tracked by Lidarr") + +// RemovableFunc reports whether the file at filePath may be removed. A merge +// asks it about every copy it would remove, before touching anything. +type RemovableFunc func(filePath string) bool + // MergedCopy is one copy a merge kept or removed. type MergedCopy struct { TrackID pgtype.UUID @@ -75,6 +85,17 @@ type MergeResult struct { func MergeDuplicateGroup( ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string, groupID, survivorID pgtype.UUID, +) (MergeResult, error) { + return MergeDuplicateGroupGuarded(ctx, pool, logger, dataDir, groupID, survivorID, nil) +} + +// MergeDuplicateGroupGuarded is MergeDuplicateGroup with a check on what it may +// remove: when removable says no to any copy that would go, the merge returns +// ErrCopyTrackedByLidarr (wrapped with the path) and changes nothing. A nil +// removable allows every copy. +func MergeDuplicateGroupGuarded( + ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string, + groupID, survivorID pgtype.UUID, removable RemovableFunc, ) (MergeResult, error) { if logger == nil { logger = slog.Default() @@ -109,6 +130,13 @@ func MergeDuplicateGroup( return MergeResult{}, err } + if removable != nil { + for _, l := range losers { + if !removable(l.FilePath) { + return MergeResult{}, fmt.Errorf("%w: %s", ErrCopyTrackedByLidarr, l.FilePath) + } + } + } for _, l := range losers { if err := removeTrackFileOnDisk(l.FilePath); err != nil { return MergeResult{}, err diff --git a/internal/library/duplicate_resolve.go b/internal/library/duplicate_resolve.go new file mode 100644 index 00000000..604be858 --- /dev/null +++ b/internal/library/duplicate_resolve.go @@ -0,0 +1,678 @@ +package library + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "log/slog" + "sort" + "strings" + "time" + + "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/lidarr" + "git.fabledsword.com/bvandeusen/minstrel/internal/notifications" + syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" +) + +// Duplicate resolver (M498 #5435, #5437). +// +// The sweep proposes duplicate groups; the resolver decides what each one is +// and settles the ones that are safe to settle without asking. Safety is +// Lidarr's to define: Lidarr maps one file to each track of the release it +// monitors, and re-downloads any mapped file that disappears. So: +// +// - A copy Lidarr maps is never removed. +// - On one album, copies Lidarr does not map are merged into the copy it does +// (or, when it maps none, into the best by ProposeSurvivor). +// - When Lidarr maps every copy on one album, the release it monitors lists +// the song twice: the Humanz box set, 14 vinyl sides then the digital +// medium. The resolver moves Lidarr to a release of that album that lists +// each song once. Lidarr then rescans the folder, maps one copy and leaves +// the rest unmapped, and the next pass merges them. +// +// The release change has a fixed point (lesson #4183): it happens only while +// the monitored release repeats a title, and the release it chooses repeats +// none, so the next pass finds nothing to change. + +// LidarrLibrary is the part of Lidarr's API the resolver uses. *lidarr.Client +// satisfies it. +type LidarrLibrary interface { + ListUnmappedTrackFiles(ctx context.Context) ([]lidarr.TrackFile, error) + LookupAlbumByMBID(ctx context.Context, mbid string) (lidarr.LidarrAlbum, error) + ListAlbumTracks(ctx context.Context, albumID int) ([]lidarr.ReleaseTrack, error) + GetAlbumReleases(ctx context.Context, albumID int) ([]lidarr.AlbumRelease, error) + ListReleaseTracks(ctx context.Context, releaseID int) ([]lidarr.ReleaseTrack, error) + SetMonitoredRelease(ctx context.Context, albumID, releaseID int) error +} + +const ( + // resolveMergeCap bounds the merges one pass makes. The first pass after + // deploy meets the whole backlog (433 groups measured on 2026-10-09); a + // cap spreads it over a few hours, each pass short. + resolveMergeCap = 200 + // resolveReleaseCap bounds the Lidarr release changes one pass makes. Each + // makes Lidarr rescan an artist folder. + resolveReleaseCap = 10 + // releaseChangeSettle is how long after a release change the resolver + // leaves that album's groups alone. Lidarr unmaps every file of the album + // and rescans; until the rescan lands, every copy reads as unmapped, and a + // merge then could remove the very file Lidarr is about to map. + releaseChangeSettle = 24 * time.Hour + // duplicateResolveTick is how often the worker runs a pass. + duplicateResolveTick = time.Hour +) + +const ( + lidarrStateTracked = "tracked" + lidarrStateUnmapped = "unmapped" +) + +// ReleaseChange is one Lidarr release the resolver switched. +type ReleaseChange struct { + AlbumTitle string + ArtistName string + From, To lidarr.AlbumRelease +} + +// DuplicateResolveResult tallies one pass. +type DuplicateResolveResult struct { + Groups int + Classes map[DuplicateClass]int + LidarrConsulted bool + Merged int + MergeFailed int + ReleaseChanges []ReleaseChange +} + +// LidarrPathKey is the part of a path Minstrel and Lidarr agree on: the last +// three components (artist folder, album folder, file). Both see the library +// under their own mount; on the operator's deploy both happen to say /music, +// and the key keeps the match from depending on that. +func LidarrPathKey(p string) string { + parts := strings.Split(strings.Trim(p, "/"), "/") + if len(parts) > 3 { + parts = parts[len(parts)-3:] + } + return strings.Join(parts, "/") +} + +// resolveGroup is one pending group and its present copies. +type resolveGroup struct { + id pgtype.UUID + tier string + members []dbq.ListDuplicateGroupsForResolveRow + class DuplicateClass + note string + states []string // per member: tracked, unmapped, or "" when Lidarr was not asked +} + +func (g *resolveGroup) tracked() int { + n := 0 + for _, s := range g.states { + if s == lidarrStateTracked { + n++ + } + } + return n +} + +// ResolveDuplicates runs one pass. lid is nil when Lidarr is disabled; act is +// the operator's auto-resolve setting. Without Lidarr, or with act off, the +// pass only classifies: nothing is removed when Lidarr cannot say what is safe. +func ResolveDuplicates( + ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string, lid LidarrLibrary, act bool, +) (DuplicateResolveResult, error) { + if logger == nil { + logger = slog.Default() + } + q := dbq.New(pool) + res := DuplicateResolveResult{Classes: map[DuplicateClass]int{}} + + rows, err := q.ListDuplicateGroupsForResolve(ctx) + if err != nil { + return res, fmt.Errorf("list duplicate groups: %w", err) + } + groups := foldResolveGroups(rows) + res.Groups = len(groups) + + var unmapped map[string]bool + if lid != nil { + files, err := lid.ListUnmappedTrackFiles(ctx) + if err != nil { + logger.Warn("duplicate resolve: Lidarr's unmapped files unavailable; classifying only", "err", err) + } else { + unmapped = make(map[string]bool, len(files)) + for _, f := range files { + unmapped[LidarrPathKey(f.Path)] = true + } + res.LidarrConsulted = true + } + } + + for _, g := range groups { + cm := make([]ClassifyMember, len(g.members)) + g.states = make([]string, len(g.members)) + for i, m := range g.members { + cm[i] = ClassifyMember{AlbumID: syncpkg.FormatUUID(m.AlbumID), Title: m.Title, ArtistName: m.ArtistName} + if res.LidarrConsulted { + if unmapped[LidarrPathKey(m.FilePath)] { + g.states[i] = lidarrStateUnmapped + } else { + g.states[i] = lidarrStateTracked + } + } + } + g.class = ClassifyDuplicateGroup(g.tier, cm) + g.note = resolveNote(g) + res.Classes[g.class]++ + } + if err := writeResolveVerdicts(ctx, q, groups); err != nil { + return res, err + } + + if !act || !res.LidarrConsulted { + return res, nil + } + + settling, err := recentlyChangedAlbums(ctx, q, time.Now()) + if err != nil { + return res, err + } + + // Merges first, on the state Lidarr reported at the start of the pass. + removable := func(p string) bool { return unmapped[LidarrPathKey(p)] } + for _, g := range groups { + if res.Merged >= resolveMergeCap { + break + } + if ctx.Err() != nil { + return res, ctx.Err() + } + if g.class != ClassSameRelease || g.tracked() > 1 || settling[syncpkg.FormatUUID(g.members[0].AlbumID)] { + continue + } + if err := autoMerge(ctx, pool, logger, dataDir, g, removable); err != nil { + res.MergeFailed++ + logger.Warn("duplicate resolve: merge failed", "group_id", syncpkg.FormatUUID(g.id), "err", err) + continue + } + res.Merged++ + } + + // Then release changes, for albums where Lidarr maps every copy. + changes, err := changeRepeatingReleases(ctx, q, pool, logger, lid, groups, settling) + res.ReleaseChanges = changes + if err != nil { + return res, err + } + + if res.Merged > 0 || len(changes) > 0 { + notifyAdmins(ctx, notifications.KindDuplicatesResolved, notifications.Payload{ + Count: int64(res.Merged + len(changes)), + Detail: resolvedDetail(res.Merged, changes), + }) + } + return res, nil +} + +func foldResolveGroups(rows []dbq.ListDuplicateGroupsForResolveRow) []*resolveGroup { + var groups []*resolveGroup + for _, r := range rows { + if n := len(groups); n == 0 || groups[n-1].id != r.GroupID { + groups = append(groups, &resolveGroup{id: r.GroupID, tier: r.Tier}) + } + g := groups[len(groups)-1] + g.members = append(g.members, r) + } + // A group whose other copies went missing has nothing left to resolve. + out := groups[:0] + for _, g := range groups { + if len(g.members) >= 2 { + out = append(out, g) + } + } + return out +} + +// resolveNote is what the report says beside a group the resolver leaves for +// the operator, when there is something to say. +func resolveNote(g *resolveGroup) string { + switch g.class { + case ClassSameRelease: + if g.tracked() > 1 { + return "Lidarr tracks every copy as its own track, so removing one would make Lidarr download it again." + } + case ClassCrossRelease: + return "Each copy belongs to its own release, and Lidarr keeps each one. Nothing is removed." + case ClassMismatch: + return "Identical audio filed under different titles: one file carries another song's tags." + } + return "" +} + +func writeResolveVerdicts(ctx context.Context, q *dbq.Queries, groups []*resolveGroup) error { + classes := dbq.SetDuplicateGroupClassesParams{} + states := dbq.SetDuplicateMemberLidarrStatesParams{} + for _, g := range groups { + classes.GroupIds = append(classes.GroupIds, g.id) + classes.Classes = append(classes.Classes, string(g.class)) + classes.Notes = append(classes.Notes, g.note) + for i, m := range g.members { + states.GroupIds = append(states.GroupIds, g.id) + states.TrackIds = append(states.TrackIds, m.TrackID) + states.States = append(states.States, g.states[i]) + } + } + if len(classes.GroupIds) == 0 { + return nil + } + if err := q.SetDuplicateGroupClasses(ctx, classes); err != nil { + return fmt.Errorf("write duplicate classes: %w", err) + } + if err := q.SetDuplicateMemberLidarrStates(ctx, states); err != nil { + return fmt.Errorf("write lidarr states: %w", err) + } + return nil +} + +// survivorCandidates builds what ProposeSurvivor weighs for a group's copies. +func survivorCandidates(g *resolveGroup) []SurvivorCandidate { + cands := make([]SurvivorCandidate, len(g.members)) + for i, m := range g.members { + cands[i] = SurvivorCandidate{ + TrackID: syncpkg.FormatUUID(m.TrackID), + FileFormat: m.FileFormat, + FileSize: m.FileSize, + AddedAt: m.AddedAt.Time, + LidarrTracked: g.states[i] == lidarrStateTracked, + TagFit: TagFitScore(m.TrackNumber != nil, m.PositionClash, m.FilePath, m.HasMbid), + } + } + return cands +} + +// autoMerge merges one group into the copy ProposeSurvivor picks, which is the +// copy Lidarr tracks when there is one. removable is the guard: a copy Lidarr +// maps can never be among those removed, whatever the survivor rule said. +func autoMerge( + ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string, g *resolveGroup, removable RemovableFunc, +) error { + survivorKey, reason := ProposeSurvivor(survivorCandidates(g)) + var survivorID pgtype.UUID + if err := survivorID.Scan(survivorKey); err != nil { + return fmt.Errorf("parse survivor id: %w", err) + } + res, err := MergeDuplicateGroupGuarded(ctx, pool, logger, dataDir, g.id, survivorID, removable) + if err != nil { + return err + } + if err := dbq.New(pool).MarkDuplicateGroupResolvedAutomatically(ctx, g.id); err != nil { + logger.Warn("duplicate resolve: marking the merge automatic failed", "group_id", syncpkg.FormatUUID(g.id), "err", err) + } + removed := make([]map[string]string, 0, len(res.Removed)) + for _, c := range res.Removed { + removed = append(removed, map[string]string{"track_id": syncpkg.FormatUUID(c.TrackID), "file_path": c.FilePath}) + } + audit.WriteOrLog(ctx, pool, logger, pgtype.UUID{}, pgtype.UUID{}, audit.ActionDuplicateMerge, map[string]any{ + "automatic": true, + "group_id": syncpkg.FormatUUID(g.id), + "tier": res.Tier, + "class": string(g.class), + "survivor_track_id": syncpkg.FormatUUID(res.Survivor.TrackID), + "survivor_path": res.Survivor.FilePath, + "survivor_reason": reason, + "removed": removed, + "moved": map[string]any{ + "play_events": res.PlayEvents, "skip_events": res.SkipEvents, + "likes": res.Likes, "playlist_entries": res.PlaylistEntries, + }, + }) + return nil +} + +// recentlyChangedAlbums is the Minstrel albums whose Lidarr release the +// resolver changed within releaseChangeSettle, read from the audit log so a +// restart does not forget them. +func recentlyChangedAlbums(ctx context.Context, q *dbq.Queries, now time.Time) (map[string]bool, error) { + rows, err := q.ListAuditLogByActions(ctx, dbq.ListAuditLogByActionsParams{ + Actions: []string{string(audit.ActionLidarrReleaseChange)}, SystemOnly: true, + PageLimit: 200, PageOffset: 0, + }) + if err != nil { + return nil, fmt.Errorf("read recent release changes: %w", err) + } + out := map[string]bool{} + for _, r := range rows { + if now.Sub(r.CreatedAt.Time) > releaseChangeSettle { + break // newest first + } + var meta struct { + AlbumID string `json:"album_id"` + } + if json.Unmarshal(r.Metadata, &meta) == nil && meta.AlbumID != "" { + out[meta.AlbumID] = true + } + } + return out, nil +} + +// repeatingAlbum is a Minstrel album where Lidarr maps more than one copy of a +// song, and the groups that showed it. +type repeatingAlbum struct { + albumID pgtype.UUID + releaseGroupMbid string + title, artist string + groups []*resolveGroup +} + +func changeRepeatingReleases( + ctx context.Context, q *dbq.Queries, pool *pgxpool.Pool, logger *slog.Logger, + lid LidarrLibrary, groups []*resolveGroup, settling map[string]bool, +) ([]ReleaseChange, error) { + albums := map[string]*repeatingAlbum{} + for _, g := range groups { + if g.class != ClassSameRelease || g.tracked() < 2 { + continue + } + m := g.members[0] + key := syncpkg.FormatUUID(m.AlbumID) + if settling[key] || m.ReleaseGroupMbid == nil || *m.ReleaseGroupMbid == "" { + continue + } + a := albums[key] + if a == nil { + a = &repeatingAlbum{albumID: m.AlbumID, releaseGroupMbid: *m.ReleaseGroupMbid, title: m.AlbumTitle, artist: m.ArtistName} + albums[key] = a + } + a.groups = append(a.groups, g) + } + keys := make([]string, 0, len(albums)) + for k := range albums { + keys = append(keys, k) + } + sort.Strings(keys) + + var changes []ReleaseChange + for _, k := range keys { + if len(changes) >= resolveReleaseCap { + break + } + if ctx.Err() != nil { + return changes, ctx.Err() + } + a := albums[k] + change, note, err := changeRelease(ctx, q, lid, a) + if err != nil { + logger.Warn("duplicate resolve: Lidarr release check failed", "album", a.title, "err", err) + continue + } + if change != nil { + changes = append(changes, *change) + audit.WriteOrLog(ctx, pool, logger, pgtype.UUID{}, pgtype.UUID{}, audit.ActionLidarrReleaseChange, map[string]any{ + "automatic": true, + "album_id": k, + "album_title": a.title, + "artist_name": a.artist, + "from_release": releaseLabel(change.From), + "to_release": releaseLabel(change.To), + }) + } + if note != "" { + if err := setGroupNotes(ctx, q, a.groups, note); err != nil { + return changes, err + } + } + } + return changes, nil +} + +// changeRelease checks one album's monitored release and, when it lists a +// song twice, moves Lidarr to the release that lists each song once and best +// covers what is on disk. It returns the change made (nil when none) and the +// note for the album's groups. +func changeRelease( + ctx context.Context, q *dbq.Queries, lid LidarrLibrary, a *repeatingAlbum, +) (*ReleaseChange, string, error) { + la, err := lid.LookupAlbumByMBID(ctx, a.releaseGroupMbid) + if errors.Is(err, lidarr.ErrNotFound) { + return nil, "Lidarr does not hold this album, so its copies are left for you.", nil + } + if err != nil { + return nil, "", err + } + current, err := lid.ListAlbumTracks(ctx, la.ID) + if err != nil { + return nil, "", err + } + if !releaseRepeats(current) { + return nil, "Lidarr maps each copy to a different track of this album, so none can go without a download.", nil + } + releases, err := lid.GetAlbumReleases(ctx, la.ID) + if err != nil { + return nil, "", err + } + titles, err := q.ListAlbumPresentTrackTitles(ctx, a.albumID) + if err != nil { + return nil, "", fmt.Errorf("list album titles: %w", err) + } + onDisk := map[string]bool{} + for _, t := range titles { + if k := ReleaseTitleKey(t); k != "" { + onDisk[k] = true + } + } + + var from lidarr.AlbumRelease + var candidates []releaseCandidate + for _, r := range releases { + if r.Monitored { + from = r + continue + } + tracks, err := lid.ListReleaseTracks(ctx, r.ID) + if err != nil { + return nil, "", err + } + if len(tracks) == 0 || releaseRepeats(tracks) { + continue + } + candidates = append(candidates, releaseCandidate{release: r, coverage: coverage(tracks, onDisk)}) + } + best, ok := pickRelease(candidates, coverage(current, onDisk)) + if !ok { + return nil, "Lidarr's release of this album lists songs twice, and no other release lists each once while keeping what is on disk.", nil + } + if err := lid.SetMonitoredRelease(ctx, la.ID, best.ID); err != nil { + return nil, "", err + } + note := fmt.Sprintf("Lidarr now monitors the %s release, which lists each song once. The extra copies are merged once Lidarr has rescanned.", releaseLabel(best)) + return &ReleaseChange{AlbumTitle: a.title, ArtistName: a.artist, From: from, To: best}, note, nil +} + +type releaseCandidate struct { + release lidarr.AlbumRelease + coverage int +} + +// pickRelease chooses among releases that list each song once: the one +// covering most of the titles on disk, then the fewest tracks (the plainest +// edition that holds them), then a digital one, then the lowest id so the +// choice is stable. It refuses a release covering fewer titles on disk than +// the current one does: the change must not drop a song Lidarr now keeps. +func pickRelease(cands []releaseCandidate, currentCoverage int) (lidarr.AlbumRelease, bool) { + if len(cands) == 0 { + return lidarr.AlbumRelease{}, false + } + sort.SliceStable(cands, func(i, j int) bool { + a, b := cands[i], cands[j] + if a.coverage != b.coverage { + return a.coverage > b.coverage + } + if a.release.TrackCount != b.release.TrackCount { + return a.release.TrackCount < b.release.TrackCount + } + if da, db := isDigital(a.release), isDigital(b.release); da != db { + return da + } + return a.release.ID < b.release.ID + }) + if cands[0].coverage < currentCoverage { + return lidarr.AlbumRelease{}, false + } + return cands[0].release, true +} + +func isDigital(r lidarr.AlbumRelease) bool { + return strings.Contains(strings.ToLower(r.Format), "digital") +} + +// releaseRepeats reports whether a release lists one title more than once. +func releaseRepeats(tracks []lidarr.ReleaseTrack) bool { + seen := map[string]bool{} + for _, t := range tracks { + k := ReleaseTitleKey(t.Title) + if k == "" { + continue + } + if seen[k] { + return true + } + seen[k] = true + } + return false +} + +// coverage counts the distinct titles on disk that the release lists. +func coverage(tracks []lidarr.ReleaseTrack, onDisk map[string]bool) int { + hit := map[string]bool{} + for _, t := range tracks { + if k := ReleaseTitleKey(t.Title); onDisk[k] { + hit[k] = true + } + } + return len(hit) +} + +func releaseLabel(r lidarr.AlbumRelease) string { + label := r.Format + if label == "" { + label = r.Title + } + if r.Disambiguation != "" { + label = r.Disambiguation + ", " + label + } + return fmt.Sprintf("%s (%d tracks)", label, r.TrackCount) +} + +func setGroupNotes(ctx context.Context, q *dbq.Queries, groups []*resolveGroup, note string) error { + p := dbq.SetDuplicateGroupClassesParams{} + for _, g := range groups { + g.note = note + p.GroupIds = append(p.GroupIds, g.id) + p.Classes = append(p.Classes, string(g.class)) + p.Notes = append(p.Notes, note) + } + if err := q.SetDuplicateGroupClasses(ctx, p); err != nil { + return fmt.Errorf("write duplicate notes: %w", err) + } + return nil +} + +func resolvedDetail(merged int, changes []ReleaseChange) string { + var parts []string + if merged > 0 { + parts = append(parts, fmt.Sprintf("Merged %d %s Lidarr does not need.", merged, pluralWord(merged, "copy", "copies"))) + } + switch len(changes) { + case 0: + case 1: + c := changes[0] + parts = append(parts, fmt.Sprintf("Lidarr now monitors the %s release of %s by %s, which lists each song once.", + releaseLabel(c.To), c.AlbumTitle, c.ArtistName)) + default: + parts = append(parts, fmt.Sprintf("Lidarr now monitors a release that lists each song once for %d albums.", len(changes))) + } + return strings.Join(parts, " ") +} + +func pluralWord(n int, one, many string) string { + if n == 1 { + return one + } + return many +} + +// DuplicateResolveWorker runs a resolver pass every hour. +type DuplicateResolveWorker struct { + pool *pgxpool.Pool + logger *slog.Logger + dataDir string + settings *FingerprintSettingsService + lidarr func() LidarrLibrary + tick time.Duration +} + +// NewDuplicateResolveWorker builds a worker with the production cadence. +// lidarrFn is asked each pass, so a Lidarr setting saved in admin takes effect +// without a restart; it returns nil while Lidarr is disabled. +func NewDuplicateResolveWorker( + pool *pgxpool.Pool, logger *slog.Logger, dataDir string, + settings *FingerprintSettingsService, lidarrFn func() LidarrLibrary, +) *DuplicateResolveWorker { + return &DuplicateResolveWorker{ + pool: pool, logger: logger, dataDir: dataDir, settings: settings, lidarr: lidarrFn, tick: duplicateResolveTick, + } +} + +// Run blocks until ctx is cancelled, running a pass at start and then each tick. +func (w *DuplicateResolveWorker) Run(ctx context.Context) { + w.tickOnce(ctx) + t := time.NewTicker(w.tick) + defer t.Stop() + for { + select { + case <-ctx.Done(): + return + case <-t.C: + w.tickOnce(ctx) + } + } +} + +// tickOnce contains one pass so nothing it does can stop the next tick (rule 157). +func (w *DuplicateResolveWorker) tickOnce(ctx context.Context) { + defer func() { + if r := recover(); r != nil { + w.logger.Error("duplicate resolve: tick panicked", "panic", r) + } + }() + // A sweep in flight is rewriting the groups; the next tick sees its result. + _, err := dbq.New(w.pool).GetInFlightDuplicateSweep(ctx) + switch { + case err == nil: + return + case !errors.Is(err, pgx.ErrNoRows): + if ctx.Err() == nil { + w.logger.Warn("duplicate resolve: sweep check failed", "err", err) + } + return + } + var lid LidarrLibrary + if w.lidarr != nil { + lid = w.lidarr() + } + res, err := ResolveDuplicates(ctx, w.pool, w.logger, w.dataDir, lid, w.settings.Get().AutoResolve) + if err != nil && ctx.Err() == nil { + w.logger.Warn("duplicate resolve: pass failed", "err", err) + } + w.logger.Info("duplicate resolve complete", + "groups", res.Groups, "lidarr", res.LidarrConsulted, "merged", res.Merged, + "merge_failed", res.MergeFailed, "release_changes", len(res.ReleaseChanges)) +} diff --git a/internal/library/duplicate_resolve_test.go b/internal/library/duplicate_resolve_test.go new file mode 100644 index 00000000..6570a961 --- /dev/null +++ b/internal/library/duplicate_resolve_test.go @@ -0,0 +1,349 @@ +package library + +import ( + "context" + "errors" + "os" + "path/filepath" + "testing" + + "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/lidarr" + syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" +) + +func TestLidarrPathKey(t *testing.T) { + // Minstrel and Lidarr see the library under different mounts; the key is + // what they agree on. + a := LidarrPathKey("/music/Gorillaz/Humanz (2017)/Gorillaz - Humanz - 01 - Ascension.mp3") + b := LidarrPathKey("/data/media/music/Gorillaz/Humanz (2017)/Gorillaz - Humanz - 01 - Ascension.mp3") + if a != b || a != "Gorillaz/Humanz (2017)/Gorillaz - Humanz - 01 - Ascension.mp3" { + t.Errorf("keys differ or are wrong: %q, %q", a, b) + } + if LidarrPathKey("/music/Gorillaz/Humanz (2017)/a.mp3") == LidarrPathKey("/music/Gorillaz/Demon Days (2005)/a.mp3") { + t.Error("two albums' files share a key") + } +} + +func rt(titles ...string) []lidarr.ReleaseTrack { + out := make([]lidarr.ReleaseTrack, len(titles)) + for i, title := range titles { + out[i] = lidarr.ReleaseTrack{ID: i + 1, Title: title} + } + return out +} + +func TestReleaseRepeats(t *testing.T) { + if !releaseRepeats(rt("Ascension", "Strobelite", "Ascension")) { + t.Error("the box set lists Ascension twice, but no repeat was found") + } + if releaseRepeats(rt("Charger", "Charger (demo)")) { + t.Error("a demo was read as a repeat of the song") + } +} + +// The Humanz releases as Lidarr lists them (2026-10-09): the box set is +// monitored, and the 40-track digital release is the one that lists each song +// once and covers what is on disk. +func TestPickRelease(t *testing.T) { + digital40 := lidarr.AlbumRelease{ID: 10149, Format: "Digital Media", TrackCount: 40} + deluxe26 := lidarr.AlbumRelease{ID: 10150, Format: "CD", TrackCount: 26} + standard20 := lidarr.AlbumRelease{ID: 10151, Format: "Digital Media", TrackCount: 20} + + got, ok := pickRelease([]releaseCandidate{ + {release: standard20, coverage: 20}, {release: deluxe26, coverage: 26}, {release: digital40, coverage: 34}, + }, 34) + if !ok || got.ID != digital40.ID { + t.Errorf("picked %+v (ok=%v), want the 40-track digital release", got, ok) + } + + // Equal coverage: the plainer edition, then digital. + cd := lidarr.AlbumRelease{ID: 1, Format: "CD", TrackCount: 26} + web := lidarr.AlbumRelease{ID: 2, Format: "Digital Media", TrackCount: 26} + if got, _ := pickRelease([]releaseCandidate{{release: cd, coverage: 26}, {release: web, coverage: 26}}, 26); got.ID != web.ID { + t.Errorf("picked %+v, want the digital one of two equal releases", got) + } + + // Never a release that keeps fewer of the songs on disk than the current one. + if got, ok := pickRelease([]releaseCandidate{{release: deluxe26, coverage: 26}}, 34); ok { + t.Errorf("picked %+v, which covers fewer songs on disk than the current release", got) + } +} + +// fakeLidarr is one Lidarr album with several releases. Switching the +// monitored release changes what ListAlbumTracks answers, as in Lidarr. +type fakeLidarr struct { + unmapped []string + unmappedErr error + album lidarr.LidarrAlbum + releases []lidarr.AlbumRelease + releaseTracks map[int][]lidarr.ReleaseTrack + setCalls [][2]int +} + +func (f *fakeLidarr) ListUnmappedTrackFiles(context.Context) ([]lidarr.TrackFile, error) { + out := make([]lidarr.TrackFile, len(f.unmapped)) + for i, p := range f.unmapped { + out[i] = lidarr.TrackFile{ID: i + 1, Path: p} + } + return out, f.unmappedErr +} + +func (f *fakeLidarr) LookupAlbumByMBID(_ context.Context, mbid string) (lidarr.LidarrAlbum, error) { + if f.album.ForeignAlbumID != mbid { + return lidarr.LidarrAlbum{}, lidarr.ErrNotFound + } + return f.album, nil +} + +func (f *fakeLidarr) ListAlbumTracks(context.Context, int) ([]lidarr.ReleaseTrack, error) { + for _, r := range f.releases { + if r.Monitored { + return f.releaseTracks[r.ID], nil + } + } + return nil, nil +} + +func (f *fakeLidarr) GetAlbumReleases(context.Context, int) ([]lidarr.AlbumRelease, error) { + return append([]lidarr.AlbumRelease(nil), f.releases...), nil +} + +func (f *fakeLidarr) ListReleaseTracks(_ context.Context, releaseID int) ([]lidarr.ReleaseTrack, error) { + return f.releaseTracks[releaseID], nil +} + +func (f *fakeLidarr) SetMonitoredRelease(_ context.Context, albumID, releaseID int) error { + f.setCalls = append(f.setCalls, [2]int{albumID, releaseID}) + for i := range f.releases { + f.releases[i].Monitored = f.releases[i].ID == releaseID + } + return nil +} + +// resolveFixture is the Humanz shape: one album, one song twice, both copies on +// disk under an artist/album folder, in one pending exact-tier group. +type resolveFixture struct { + pool *pgxpool.Pool + clean, rip dbq.Track + cleanPath, ripPath string + groupID pgtype.UUID +} + +func newResolveFixture(t *testing.T) resolveFixture { + t.Helper() + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + dir := filepath.Join(t.TempDir(), "Gorillaz", "Humanz (2017)") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + f := resolveFixture{pool: pool} + f.cleanPath = filepath.Join(dir, "Gorillaz - Humanz - 03 - Saturnz Barz.mp3") + f.ripPath = filepath.Join(dir, "Gorillaz - Humanz - 03 - Saturnz Barz (Official Video).mp3") + for _, p := range []string{f.cleanPath, f.ripPath} { + if err := os.WriteFile(p, []byte("audio"), 0o644); err != nil { + t.Fatal(err) + } + } + var album dbq.Album + var artist dbq.Artist + f.clean, album, artist = seedTrack(t, pool, f.cleanPath) + rip, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{ + Title: "Saturnz Barz (Official Video)", AlbumID: album.ID, ArtistID: artist.ID, + DurationMs: 1000, FilePath: f.ripPath, FileSize: 100, FileFormat: "mp3", + }) + if err != nil { + t.Fatal(err) + } + f.rip = rip + exec := func(sql string, args ...any) { + t.Helper() + if _, err := pool.Exec(ctx, sql, args...); err != nil { + t.Fatalf("exec %q: %v", sql, err) + } + } + exec(`UPDATE tracks SET title = 'Saturnz Barz' WHERE id = $1`, f.clean.ID) + exec(`UPDATE albums SET release_group_mbid = 'rg-humanz' WHERE id = $1`, album.ID) + if err := pool.QueryRow(ctx, + `INSERT INTO duplicate_groups (member_key, tier) VALUES ('resolve-fixture', 'exact') RETURNING id`, + ).Scan(&f.groupID); err != nil { + t.Fatal(err) + } + exec(`INSERT INTO duplicate_group_members (group_id, track_id) VALUES ($1, $2), ($1, $3)`, f.groupID, f.clean.ID, f.rip.ID) + return f +} + +// lidarrPath is where Lidarr, mounted elsewhere, sees a fixture file. +func lidarrPath(p string) string { return "/music/" + LidarrPathKey(p) } + +func (f resolveFixture) group(t *testing.T) (status string, class *string, note *string, auto bool) { + t.Helper() + if err := f.pool.QueryRow(context.Background(), + `SELECT status, class, resolve_note, resolved_automatically FROM duplicate_groups WHERE id = $1`, f.groupID, + ).Scan(&status, &class, ¬e, &auto); err != nil { + t.Fatal(err) + } + return status, class, note, auto +} + +func exists(p string) bool { _, err := os.Stat(p); return err == nil } + +// Lidarr maps the clean copy and holds the rip unmapped: the rip goes, the +// clean copy stays, and the merge is recorded as the resolver's. +func TestResolveDuplicates_MergesTheUnmappedCopy_Integration(t *testing.T) { + f := newResolveFixture(t) + lid := &fakeLidarr{unmapped: []string{lidarrPath(f.ripPath)}} + + res, err := ResolveDuplicates(context.Background(), f.pool, nil, "", lid, true) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if res.Merged != 1 || !res.LidarrConsulted { + t.Fatalf("result = %+v, want one merge with Lidarr consulted", res) + } + if exists(f.ripPath) || !exists(f.cleanPath) { + t.Errorf("rip exists=%v clean exists=%v; want only the clean copy left", exists(f.ripPath), exists(f.cleanPath)) + } + status, class, _, auto := f.group(t) + if status != "merged" || class == nil || *class != string(ClassSameRelease) || !auto { + t.Errorf("group = (%s, %v, auto=%v), want merged same_release by the resolver", status, class, auto) + } + var audits int + if err := f.pool.QueryRow(context.Background(), + `SELECT count(*) FROM audit_log WHERE action = 'duplicate_merge' AND actor_id IS NULL + AND metadata->>'group_id' = $1 AND (metadata->>'automatic')::boolean`, + syncpkg.FormatUUID(f.groupID)).Scan(&audits); err != nil { + t.Fatal(err) + } + if audits != 1 { + t.Errorf("automatic merge audit rows = %d, want 1", audits) + } +} + +// Lidarr maps both copies, and its release lists the song twice: nothing is +// deleted, the release moves to the one listing each song once, and the next +// pass leaves it there (lesson #4183's fixed point). +func TestResolveDuplicates_ChangesARepeatingRelease_Integration(t *testing.T) { + f := newResolveFixture(t) + lid := &fakeLidarr{ + album: lidarr.LidarrAlbum{ID: 4723, ForeignAlbumID: "rg-humanz", Title: "Humanz"}, + releases: []lidarr.AlbumRelease{ + {ID: 10148, Format: "14x12\" Vinyl, Digital Media", TrackCount: 4, Monitored: true}, + {ID: 10149, Format: "Digital Media", TrackCount: 2}, + }, + releaseTracks: map[int][]lidarr.ReleaseTrack{ + 10148: rt("Saturnz Barz", "Ascension", "Saturnz Barz", "Ascension"), + 10149: rt("Saturnz Barz", "Ascension"), + }, + } + + res, err := ResolveDuplicates(context.Background(), f.pool, nil, "", lid, true) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if res.Merged != 0 || !exists(f.ripPath) || !exists(f.cleanPath) { + t.Fatalf("merged %d; a copy Lidarr maps was touched", res.Merged) + } + if len(lid.setCalls) != 1 || lid.setCalls[0] != [2]int{4723, 10149} { + t.Fatalf("release changes = %v, want album 4723 to release 10149", lid.setCalls) + } + status, _, note, _ := f.group(t) + if status != "pending" || note == nil || *note == "" { + t.Errorf("group = (%s, note %v), want pending with a note saying what changed", status, note) + } + + // The next pass: Lidarr has not rescanned yet, so both copies still read + // as tracked. The album is settling, so nothing changes. + if _, err := ResolveDuplicates(context.Background(), f.pool, nil, "", lid, true); err != nil { + t.Fatalf("second resolve: %v", err) + } + if len(lid.setCalls) != 1 { + t.Errorf("release changed again: %v", lid.setCalls) + } + // And once it has settled, the check itself finds nothing to do: the + // release it chose lists each song once. + change, _, err := changeRelease(context.Background(), dbq.New(f.pool), lid, + &repeatingAlbum{albumID: f.clean.AlbumID, releaseGroupMbid: "rg-humanz"}) + if err != nil || change != nil || len(lid.setCalls) != 1 { + t.Errorf("after the change, changeRelease = (%+v, %v) with %d calls; want no change", change, err, len(lid.setCalls)) + } +} + +// After a release change Lidarr unmaps every file of the album until it has +// rescanned. A pass in that window must not read "every copy unmapped" as +// licence to merge: the copy it would remove may be the one Lidarr maps next. +func TestResolveDuplicates_LeavesAnAlbumAloneWhileLidarrRescans_Integration(t *testing.T) { + f := newResolveFixture(t) + lid := &fakeLidarr{ + album: lidarr.LidarrAlbum{ID: 4723, ForeignAlbumID: "rg-humanz"}, + releases: []lidarr.AlbumRelease{ + {ID: 10148, TrackCount: 2, Monitored: true}, {ID: 10149, TrackCount: 1}, + }, + releaseTracks: map[int][]lidarr.ReleaseTrack{10148: rt("Saturnz Barz", "Saturnz Barz"), 10149: rt("Saturnz Barz")}, + } + if _, err := ResolveDuplicates(context.Background(), f.pool, nil, "", lid, true); err != nil { + t.Fatal(err) + } + lid.unmapped = []string{lidarrPath(f.cleanPath), lidarrPath(f.ripPath)} + res, err := ResolveDuplicates(context.Background(), f.pool, nil, "", lid, true) + if err != nil { + t.Fatal(err) + } + if res.Merged != 0 || !exists(f.ripPath) || !exists(f.cleanPath) { + t.Errorf("merged %d during the rescan window", res.Merged) + } +} + +// Without Lidarr, or with auto-resolve off, the pass classifies and removes +// nothing. +func TestResolveDuplicates_ClassifiesOnlyWithoutLidarrOrPermission_Integration(t *testing.T) { + for name, run := range map[string]func(resolveFixture) (DuplicateResolveResult, error){ + "lidarr disabled": func(f resolveFixture) (DuplicateResolveResult, error) { + return ResolveDuplicates(context.Background(), f.pool, nil, "", nil, true) + }, + "lidarr unreachable": func(f resolveFixture) (DuplicateResolveResult, error) { + return ResolveDuplicates(context.Background(), f.pool, nil, "", &fakeLidarr{unmappedErr: errors.New("timeout")}, true) + }, + "auto-resolve off": func(f resolveFixture) (DuplicateResolveResult, error) { + lid := &fakeLidarr{unmapped: []string{lidarrPath(f.ripPath)}} + return ResolveDuplicates(context.Background(), f.pool, nil, "", lid, false) + }, + } { + t.Run(name, func(t *testing.T) { + f := newResolveFixture(t) + res, err := run(f) + if err != nil { + t.Fatal(err) + } + if res.Merged != 0 || !exists(f.ripPath) { + t.Errorf("merged %d", res.Merged) + } + status, class, _, _ := f.group(t) + if status != "pending" || class == nil || *class != string(ClassSameRelease) { + t.Errorf("group = (%s, %v), want pending and classified", status, class) + } + }) + } +} + +// The guard itself: a merge asked to remove a copy the check refuses changes +// nothing, file or row. +func TestMergeDuplicateGroupGuarded_RefusesATrackedCopy_Integration(t *testing.T) { + f := newResolveFixture(t) + refuse := func(p string) bool { return p != f.ripPath } + _, err := MergeDuplicateGroupGuarded(context.Background(), f.pool, nil, "", f.groupID, f.clean.ID, refuse) + if !errors.Is(err, ErrCopyTrackedByLidarr) { + t.Fatalf("err = %v, want ErrCopyTrackedByLidarr", err) + } + if !exists(f.ripPath) { + t.Error("the refused copy's file was removed") + } + if status, _, _, _ := f.group(t); status != "pending" { + t.Errorf("group status = %s, want pending", status) + } +} diff --git a/internal/library/duplicate_survivor.go b/internal/library/duplicate_survivor.go index 6f90e64f..aaaa0d28 100644 --- a/internal/library/duplicate_survivor.go +++ b/internal/library/duplicate_survivor.go @@ -13,6 +13,36 @@ type SurvivorCandidate struct { FileFormat string FileSize int64 AddedAt time.Time + + // LidarrTracked: Lidarr maps this file to a track of the release it + // monitors. Removing it opens a hole Lidarr downloads again (M498), so a + // tracked copy outranks everything else. False when Lidarr was not asked. + LidarrTracked bool + // TagFit is TagFitScore: how well the copy's tags and name fit the album. + TagFit int +} + +// TagFitScore counts the signs that a copy belongs where it is filed (M498 +// #5436), one point each: +// - it has a track number that no other track on its album also claims +// - its file name carries no video-rip marker (SourceMarkersFor) +// - it has a MusicBrainz recording id +// +// Two copies of one song differ in these far more often than in anything a +// listener hears: the Humanz rips differ by a few bytes, and file size picked +// the wrong one in 6 of 21 groups. +func TagFitScore(hasTrackNumber, positionClash bool, filePath string, hasMBID bool) int { + score := 0 + if hasTrackNumber && !positionClash { + score++ + } + if len(SourceMarkersFor(filePath)) == 0 { + score++ + } + if hasMBID { + score++ + } + return score } // losslessFormats are the scanned extensions that are lossless by definition. @@ -26,6 +56,9 @@ var losslessFormats = map[string]bool{"flac": true, "wav": true} // the report shows it and the merge (#3911) lets the operator choose another. // // In order: +// 0. the copy Lidarr maps, then the copy whose tags fit the album best +// (TagFitScore). Both are zero for every copy when the caller does not +// know them, and the order falls through to the quality rules below // 1. lossless over lossy — the one difference no later step can recover // 2. the larger file — for one recording at one duration that is the higher // bitrate. The scanner does not record bitrate (tracks.bitrate is never @@ -48,6 +81,10 @@ func ProposeSurvivor(cands []SurvivorCandidate) (trackID, reason string) { // runner-up — the rule that actually decided, not every rule it passed. next := ranked[1] switch { + case best.LidarrTracked != next.LidarrTracked: + return best.TrackID, "the copy Lidarr tracks" + case best.TagFit != next.TagFit: + return best.TrackID, "tags fit the album" case isLossless(best) != isLossless(next): return best.TrackID, "lossless (" + strings.ToLower(best.FileFormat) + ")" case best.FileSize != next.FileSize: @@ -60,6 +97,12 @@ func ProposeSurvivor(cands []SurvivorCandidate) (trackID, reason string) { } func survivorBefore(a, b SurvivorCandidate) bool { + if a.LidarrTracked != b.LidarrTracked { + return a.LidarrTracked + } + if a.TagFit != b.TagFit { + return a.TagFit > b.TagFit + } if isLossless(a) != isLossless(b) { return isLossless(a) } diff --git a/internal/library/duplicate_survivor_test.go b/internal/library/duplicate_survivor_test.go index 4e770d7e..4062716f 100644 --- a/internal/library/duplicate_survivor_test.go +++ b/internal/library/duplicate_survivor_test.go @@ -89,3 +89,66 @@ func TestProposeSurvivor_ReasonIsTheDecidingRule(t *testing.T) { t.Fatalf("got (%q, %q), want (flac-big, largest file)", id, reason) } } + +// M498 #5436: the copy Lidarr maps outranks everything, then the copy whose +// tags fit the album; quality decides only between copies equal on both. Each +// case is built so the older rules would choose the other copy, so a reorder +// that drops the new rules fails. +func TestProposeSurvivor_LidarrThenTagFit(t *testing.T) { + at := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC) + cases := []struct { + name string + cands []SurvivorCandidate + wantID string + wantReason string + }{ + { + name: "tracked beats lossless and better tags", + cands: []SurvivorCandidate{ + {TrackID: "flac", FileFormat: "flac", FileSize: 40_000_000, AddedAt: at, TagFit: 3}, + {TrackID: "tracked", FileFormat: "mp3", FileSize: 5_000_000, AddedAt: at, TagFit: 1, LidarrTracked: true}, + }, + wantID: "tracked", wantReason: "the copy Lidarr tracks", + }, + { + // The Humanz shape: two rips a few bytes apart, one carrying a + // video marker and clashing with another track's position. + name: "tag fit beats a few bytes", + cands: []SurvivorCandidate{ + {TrackID: "rip", FileFormat: "mp3", FileSize: 5_104_498, AddedAt: at, TagFit: 1}, + {TrackID: "clean", FileFormat: "mp3", FileSize: 5_104_492, AddedAt: at, TagFit: 3}, + }, + wantID: "clean", wantReason: "tags fit the album", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + id, reason := ProposeSurvivor(tc.cands) + if id != tc.wantID || reason != tc.wantReason { + t.Fatalf("ProposeSurvivor = (%q, %q), want (%q, %q)", id, reason, tc.wantID, tc.wantReason) + } + }) + } +} + +func TestTagFitScore(t *testing.T) { + cases := []struct { + name string + hasTrackNumber bool + clash bool + path string + hasMBID bool + want int + }{ + {"clean, numbered, identified", true, false, "/music/A/B/A - B - 01 - Song.flac", true, 3}, + {"position taken by another track", true, true, "/music/A/B/A - B - 01 - Song.flac", true, 2}, + {"no track number", false, false, "/music/A/B/Song.flac", true, 2}, + {"video rip", true, false, "/music/A/B/A - B - 01 - Song (Official Video).mp3", true, 2}, + {"rip with nothing else", false, false, "/music/A/B/Song (Official Video).mp3", false, 0}, + } + for _, tc := range cases { + if got := TagFitScore(tc.hasTrackNumber, tc.clash, tc.path, tc.hasMBID); got != tc.want { + t.Errorf("%s: TagFitScore = %d, want %d", tc.name, got, tc.want) + } + } +} diff --git a/internal/library/fingerprint_settings.go b/internal/library/fingerprint_settings.go index fa971b80..b451441a 100644 --- a/internal/library/fingerprint_settings.go +++ b/internal/library/fingerprint_settings.go @@ -55,6 +55,10 @@ type FingerprintSettings struct { AcousticMaxBitErrorRate float64 BackfillConcurrency int32 SweepIntervalHours int32 + // AutoResolve lets the duplicate resolver (M498) act on its own: merge + // copies Lidarr does not need and change a Lidarr release that lists songs + // twice. Off, it only classifies, and every group waits for the operator. + AutoResolve bool // UpdatedAt is when the settings were last saved. Set by the database; // ignored by Set. UpdatedAt time.Time @@ -68,6 +72,7 @@ var DefaultFingerprintSettings = FingerprintSettings{ AcousticMaxBitErrorRate: defaultAcousticMaxBitErrorRate, BackfillConcurrency: fingerprintBackfillConcurrency, SweepIntervalHours: defaultSweepIntervalHours, + AutoResolve: true, } // ErrFingerprintSettingOutOfRange is returned by Set for a value migration @@ -121,6 +126,7 @@ func (s *FingerprintSettingsService) Set(ctx context.Context, in FingerprintSett AcousticMaxBitErrorRate: in.AcousticMaxBitErrorRate, BackfillConcurrency: in.BackfillConcurrency, SweepIntervalHours: in.SweepIntervalHours, + AutoResolve: in.AutoResolve, }) if err != nil { return FingerprintSettings{}, fmt.Errorf("fingerprint settings: save: %w", err) @@ -158,6 +164,7 @@ func fingerprintSettingsFromRow(row dbq.FingerprintSetting) FingerprintSettings AcousticMaxBitErrorRate: row.AcousticMaxBitErrorRate, BackfillConcurrency: row.BackfillConcurrency, SweepIntervalHours: row.SweepIntervalHours, + AutoResolve: row.AutoResolve, UpdatedAt: row.UpdatedAt.Time, } } diff --git a/internal/library/fingerprint_settings_test.go b/internal/library/fingerprint_settings_test.go index 95d5837b..65c33d4f 100644 --- a/internal/library/fingerprint_settings_test.go +++ b/internal/library/fingerprint_settings_test.go @@ -102,6 +102,7 @@ func TestFingerprintSettingsService_Integration(t *testing.T) { AcousticMaxBitErrorRate: minAcousticMaxBitErrorRate, BackfillConcurrency: minBackfillConcurrency, SweepIntervalHours: minSweepIntervalHours, + AutoResolve: false, } highest := FingerprintSettings{ Enabled: true, @@ -109,6 +110,7 @@ func TestFingerprintSettingsService_Integration(t *testing.T) { AcousticMaxBitErrorRate: maxAcousticMaxBitErrorRate, BackfillConcurrency: maxBackfillConcurrency, SweepIntervalHours: maxSweepIntervalHours, + AutoResolve: true, } for _, step := range []struct { name string diff --git a/internal/library/source_markers.go b/internal/library/source_markers.go new file mode 100644 index 00000000..0bc5a825 --- /dev/null +++ b/internal/library/source_markers.go @@ -0,0 +1,69 @@ +package library + +import ( + "path" + "regexp" + "strings" +) + +// sourceMarker is one sign that a file was ripped from a video rather than +// taken from a release: words a video title carries and an album track does +// not (#5410). +// +// Each pattern is used twice, so it is written in the subset that Postgres's +// regex flavour and Go's RE2 read the same way: no \b (Postgres spells word +// boundaries \y), no lookaround, and brackets written as (\(|\[) rather than +// as a bracket expression. The SQL side matches it case-insensitively with ~*, +// the Go side with (?i); [^a-zA-Z] is spelled out so neither has to fold case +// inside a negated class. +type sourceMarker struct { + label string + pattern string +} + +// suspectSourceMarkers was calibrated against the operator's library on +// 2026-10-08. Two candidates were left out on purpose: +// - "live in"/"live at": 229 files, nearly all from real live albums. +// - a bare "reaction": it caught Beck's "Chain Reaction". The marker below +// wants the phrases a reaction video actually uses. +var suspectSourceMarkers = []sourceMarker{ + {"music video", `official[^a-zA-Z]*(music[^a-zA-Z]*)?video|music[^a-zA-Z]*video`}, + {"official audio", `official[^a-zA-Z]*audio`}, + {"lyric video", `lyrics?[^a-zA-Z]*video`}, + {"visualiser", `visuali[sz]er`}, + {"reaction", `reaction[^a-zA-Z]*(video|mashup)|reacts?[^a-zA-Z]+to[^a-zA-Z]|first[^a-zA-Z]*time[^a-zA-Z]*(hearing|listening)`}, + {"MV", `(^|[^a-zA-Z])(mv|m/v)([^a-zA-Z]|$)`}, + {"[Audio]", `(\(|\[)audio(\)|\])`}, + {"[HD]", `(\(|\[)(hd|hq|4k)(\)|\])`}, +} + +// SuspectSourcePattern is every marker as one alternation, for the SQL filter +// behind the admin suspect-sources report. +var SuspectSourcePattern = func() string { + parts := make([]string, len(suspectSourceMarkers)) + for i, m := range suspectSourceMarkers { + parts[i] = "(" + m.pattern + ")" + } + return strings.Join(parts, "|") +}() + +var suspectSourceRegexps = func() []*regexp.Regexp { + out := make([]*regexp.Regexp, len(suspectSourceMarkers)) + for i, m := range suspectSourceMarkers { + out[i] = regexp.MustCompile("(?i)" + m.pattern) + } + return out +}() + +// SourceMarkersFor returns the labels of every marker the file's basename +// carries, in list order. Empty for a file the SQL filter would not return. +func SourceMarkersFor(filePath string) []string { + base := path.Base(filePath) + labels := make([]string, 0, 2) + for i, re := range suspectSourceRegexps { + if re.MatchString(base) { + labels = append(labels, suspectSourceMarkers[i].label) + } + } + return labels +} diff --git a/internal/library/source_markers_test.go b/internal/library/source_markers_test.go new file mode 100644 index 00000000..2282a696 --- /dev/null +++ b/internal/library/source_markers_test.go @@ -0,0 +1,53 @@ +package library + +import ( + "slices" + "testing" +) + +// Real basenames from the operator's library (2026-10-08), each with the +// labels it should carry. The negatives are the near misses that shaped the +// list: a song called "Chain Reaction", a live album, an ordinary track. +func TestSourceMarkersFor(t *testing.T) { + cases := []struct { + base string + want []string + }{ + {"Daft Punk - Random Access Memories - 06 - Daft Punk - Doin' It Right (Music Video) ft. Panda Bear.mp3", []string{"music video"}}, + {"Gorillaz - Humanz - 03 - Gorillaz - Saturnz Barz (Official Video).mp3", []string{"music video"}}, + {"Gorillaz - Humanz - 05 - Gorillaz - Andromeda (Official Audio).mp3", []string{"official audio"}}, + {"Gorillaz - Gorillaz - 06 - Gorillaz - P45 (Visualizer).mp3", []string{"visualiser"}}, + {"Gorillaz - Demon Days - 16 - Gorillaz - Don Quixote's Christmas Bonanza (Visualiser).mp3", []string{"visualiser"}}, + {"Artist - Album - 01 - Artist - Song (Lyric Video).mp3", []string{"lyric video"}}, + {"Gorillaz - Humanz - 02 - FIRST TIME HEARING Gorillaz - Ascension REACTION.mp3", []string{"reaction"}}, + {"Watsky - INTENTION - 05 - MANIAC Reacts to Watsky - AWW SHiT.mp3", []string{"reaction"}}, + {"米津玄師 - diorama - 09 - 【MV】米津玄師 - 恋と病熱.mp3", []string{"MV"}}, + {"Andora - Ego - 01 - Andora - Ego (feat. Will Stetson) MV.mp3", []string{"MV"}}, + {"Jimmy Eat World - Something(s) Loud - 05 - Jimmy Eat World - Call to Love (Audio).mp3", []string{"[Audio]"}}, + {"Record Heat - World War IV - 03 - Record Heat - Front Seat Feelin' [Audio].mp3", []string{"[Audio]"}}, + {"Aphex Twin - Come To Daddy - 03 - Aphex Twin - Bucephalus Bouncing Ball (HQ).mp3", []string{"[HD]"}}, + {"Artist - Album - 01 - Artist - Song (Official Lyric Video) [4K].mp3", []string{"lyric video", "[HD]"}}, + + {"Beck - Guero - 15 - Beck - Chain Reaction.mp3", nil}, + {"Nirvana - MTV Unplugged in New York - 01 - About a Girl (live in New York).flac", nil}, + {"Boards of Canada - Music Has the Right to Children - 05 - Roygbiv.flac", nil}, + {"Artist - Album - 01 - Mvula.flac", nil}, + } + for _, c := range cases { + got := SourceMarkersFor("/music/x/" + c.base) + if len(got) == 0 && len(c.want) == 0 { + continue // nil and empty both mean "no markers" + } + if !slices.Equal(got, c.want) { + t.Errorf("%s:\n got %v\n want %v", c.base, got, c.want) + } + } +} + +// The folder is not evidence: a marker word in a directory name must not +// flag the files inside it. +func TestSourceMarkersFor_ReadsOnlyTheBasename(t *testing.T) { + if got := SourceMarkersFor("/music/Official Video Collection/01 - Song.flac"); len(got) != 0 { + t.Errorf("directory name flagged the file: %v", got) + } +} diff --git a/internal/lidarr/releases.go b/internal/lidarr/releases.go new file mode 100644 index 00000000..2513af12 --- /dev/null +++ b/internal/lidarr/releases.go @@ -0,0 +1,154 @@ +package lidarr + +import ( + "context" + "encoding/json" + "fmt" + "net/url" + "strconv" +) + +// What Lidarr holds on disk and how it maps it (Scribe milestone #498). +// +// Lidarr maps one file to each track of the ONE release it monitors per album. +// A file it holds but maps to no track is "unmapped": removing it opens no hole. +// Removing a mapped file does, and Lidarr fills the hole by downloading again — +// which is why Minstrel's duplicate resolver asks before it deletes anything. + +// TrackFile is a file Lidarr holds on disk. +type TrackFile struct { + ID int `json:"id"` + Path string `json:"path"` + AlbumID int `json:"albumId"` +} + +// ListUnmappedTrackFiles hits GET /api/v1/trackfile?unmapped=true: files under +// Lidarr's root folders that it has scanned but matched to no track. +func (c *Client) ListUnmappedTrackFiles(ctx context.Context) ([]TrackFile, error) { + resp, err := c.get(ctx, "/api/v1/trackfile", url.Values{"unmapped": {"true"}}) + if err != nil { + return nil, err + } + defer func() { _ = resp.Body.Close() }() + var out []TrackFile + if err := json.NewDecoder(resp.Body).Decode(&out); err != nil { + return nil, fmt.Errorf("%w: decode unmapped track files: %v", ErrInvalidPayload, err) + } + return out, nil +} + +// AlbumRelease is one edition of an album as Lidarr knows it: the 14×12" vinyl +// box set, the 2×CD deluxe, the digital standard. Exactly one is Monitored. +type AlbumRelease struct { + ID int `json:"id"` + Title string `json:"title"` + Disambiguation string `json:"disambiguation"` + Format string `json:"format"` + TrackCount int `json:"trackCount"` + MediumCount int `json:"mediumCount"` + Monitored bool `json:"monitored"` +} + +// ReleaseTrack is one track of a release. +type ReleaseTrack struct { + ID int `json:"id"` + Title string `json:"title"` + MediumNumber int `json:"mediumNumber"` + TrackFileID int `json:"trackFileId"` +} + +// GetAlbumReleases hits GET /api/v1/album/{id} and returns the album's releases. +func (c *Client) GetAlbumReleases(ctx context.Context, albumID int) ([]AlbumRelease, error) { + resp, err := c.get(ctx, "/api/v1/album/"+strconv.Itoa(albumID), nil) + if err != nil { + return nil, err + } + defer func() { _ = resp.Body.Close() }() + var album struct { + Releases []AlbumRelease `json:"releases"` + } + if err := json.NewDecoder(resp.Body).Decode(&album); err != nil { + return nil, fmt.Errorf("%w: decode album: %v", ErrInvalidPayload, err) + } + return album.Releases, nil +} + +// ListAlbumTracks hits GET /api/v1/track?albumId= : the tracks of the release +// Lidarr monitors for that album. +func (c *Client) ListAlbumTracks(ctx context.Context, albumID int) ([]ReleaseTrack, error) { + return c.listTracks(ctx, url.Values{"albumId": {strconv.Itoa(albumID)}}) +} + +// ListReleaseTracks hits GET /api/v1/track?albumReleaseId= : the tracks of one +// release, monitored or not. +func (c *Client) ListReleaseTracks(ctx context.Context, releaseID int) ([]ReleaseTrack, error) { + return c.listTracks(ctx, url.Values{"albumReleaseId": {strconv.Itoa(releaseID)}}) +} + +func (c *Client) listTracks(ctx context.Context, q url.Values) ([]ReleaseTrack, error) { + resp, err := c.get(ctx, "/api/v1/track", q) + if err != nil { + return nil, err + } + defer func() { _ = resp.Body.Close() }() + var out []ReleaseTrack + if err := json.NewDecoder(resp.Body).Decode(&out); err != nil { + return nil, fmt.Errorf("%w: decode tracks: %v", ErrInvalidPayload, err) + } + return out, nil +} + +// SetMonitoredRelease makes releaseID the album's monitored release. +// +// It is the round trip Lidarr's own "edit album" makes: GET the album resource, +// flip `monitored` on its releases, PUT the whole resource back. Lidarr's +// UpdateAlbum publishes AlbumEditedEvent, and when the monitored release changed, +// AlbumEditedService unlinks the album's track files and queues +// RescanFoldersCommand for the artist's folder, which maps the files on disk to +// the new release's tracks. Files the new release has no track for come out of +// that rescan unmapped. (Lidarr source, develop: Music/Services/ +// AlbumEditedService.cs and AlbumService.UpdateAlbum.) +// +// The resource is kept as raw JSON so every field Lidarr sent goes back +// unchanged; only the releases' monitored flags differ. Returns ErrNotFound when +// the album has no release with that id. +func (c *Client) SetMonitoredRelease(ctx context.Context, albumID, releaseID int) error { + path := "/api/v1/album/" + strconv.Itoa(albumID) + resp, err := c.get(ctx, path, nil) + if err != nil { + return err + } + var album map[string]any + err = json.NewDecoder(resp.Body).Decode(&album) + _ = resp.Body.Close() + if err != nil { + return fmt.Errorf("%w: decode album: %v", ErrInvalidPayload, err) + } + + releases, _ := album["releases"].([]any) + found := false + for _, r := range releases { + rel, ok := r.(map[string]any) + if !ok { + continue + } + id, _ := rel["id"].(float64) + match := int(id) == releaseID + rel["monitored"] = match + found = found || match + } + if !found { + return fmt.Errorf("%w: release %d on album %d", ErrNotFound, releaseID, albumID) + } + + body, err := json.Marshal(album) + if err != nil { + return fmt.Errorf("%w: marshal album: %v", ErrInvalidPayload, err) + } + putResp, err := c.put(ctx, path, body) + if err != nil { + return err + } + _ = putResp.Body.Close() + return nil +} diff --git a/internal/lidarr/releases_test.go b/internal/lidarr/releases_test.go new file mode 100644 index 00000000..5e3c769e --- /dev/null +++ b/internal/lidarr/releases_test.go @@ -0,0 +1,116 @@ +package lidarr + +import ( + "context" + "encoding/json" + "errors" + "io" + "net/http" + "testing" +) + +func TestListUnmappedTrackFiles(t *testing.T) { + c, srv := newTestClient(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/trackfile" || r.URL.Query().Get("unmapped") != "true" { + t.Errorf("request = %s?%s, want /api/v1/trackfile?unmapped=true", r.URL.Path, r.URL.RawQuery) + } + _, _ = w.Write([]byte(`[{"id":7,"path":"/music/A/B (2017)/A - B - 02 - X.mp3","albumId":0}]`)) + }) + defer srv.Close() + + got, err := c.ListUnmappedTrackFiles(context.Background()) + if err != nil { + t.Fatalf("ListUnmappedTrackFiles: %v", err) + } + if len(got) != 1 || got[0].ID != 7 || got[0].Path != "/music/A/B (2017)/A - B - 02 - X.mp3" { + t.Errorf("got %+v", got) + } +} + +func TestReleaseReads(t *testing.T) { + c, srv := newTestClient(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.URL.Path == "/api/v1/album/4723": + _, _ = w.Write([]byte(`{"id":4723,"releases":[ + {"id":10148,"title":"Humanz","format":"14x12\" Vinyl, Digital Media","trackCount":68,"mediumCount":15,"monitored":true}, + {"id":10149,"title":"Humanz","format":"Digital Media","trackCount":40,"mediumCount":1,"monitored":false}]}`)) + case r.URL.Path == "/api/v1/track" && r.URL.Query().Get("albumId") == "4723": + _, _ = w.Write([]byte(`[{"id":1,"title":"Ascension","mediumNumber":1,"trackFileId":9}]`)) + case r.URL.Path == "/api/v1/track" && r.URL.Query().Get("albumReleaseId") == "10149": + _, _ = w.Write([]byte(`[{"id":2,"title":"Ascension","mediumNumber":1,"trackFileId":0}]`)) + default: + t.Errorf("unexpected request %s?%s", r.URL.Path, r.URL.RawQuery) + w.WriteHeader(http.StatusNotFound) + } + }) + defer srv.Close() + ctx := context.Background() + + rels, err := c.GetAlbumReleases(ctx, 4723) + if err != nil { + t.Fatalf("GetAlbumReleases: %v", err) + } + if len(rels) != 2 || !rels[0].Monitored || rels[0].MediumCount != 15 || rels[1].TrackCount != 40 { + t.Errorf("releases = %+v", rels) + } + tr, err := c.ListAlbumTracks(ctx, 4723) + if err != nil || len(tr) != 1 || tr[0].TrackFileID != 9 { + t.Errorf("ListAlbumTracks = %+v, %v", tr, err) + } + tr, err = c.ListReleaseTracks(ctx, 10149) + if err != nil || len(tr) != 1 || tr[0].Title != "Ascension" { + t.Errorf("ListReleaseTracks = %+v, %v", tr, err) + } +} + +// The PUT carries the whole resource back, with only the releases' monitored +// flags changed: exactly the chosen release is monitored, and a field Minstrel +// does not model survives the round trip. +func TestSetMonitoredRelease_RoundTripsTheResource(t *testing.T) { + var put map[string]any + c, srv := newTestClient(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/album/4723" { + t.Errorf("path = %q", r.URL.Path) + } + switch r.Method { + case http.MethodGet: + _, _ = w.Write([]byte(`{"id":4723,"anyReleaseOk":false,"secondaryTypes":["Studio"],"releases":[ + {"id":10148,"monitored":true},{"id":10149,"monitored":false}]}`)) + case http.MethodPut: + b, _ := io.ReadAll(r.Body) + if err := json.Unmarshal(b, &put); err != nil { + t.Errorf("PUT body: %v", err) + } + w.WriteHeader(http.StatusAccepted) + } + }) + defer srv.Close() + + if err := c.SetMonitoredRelease(context.Background(), 4723, 10149); err != nil { + t.Fatalf("SetMonitoredRelease: %v", err) + } + rels := put["releases"].([]any) + if m := rels[0].(map[string]any)["monitored"]; m != false { + t.Errorf("old release monitored = %v, want false", m) + } + if m := rels[1].(map[string]any)["monitored"]; m != true { + t.Errorf("new release monitored = %v, want true", m) + } + if st, _ := put["secondaryTypes"].([]any); len(st) != 1 || st[0] != "Studio" { + t.Errorf("secondaryTypes = %v, want it carried through unchanged", put["secondaryTypes"]) + } +} + +func TestSetMonitoredRelease_UnknownReleaseSendsNothing(t *testing.T) { + c, srv := newTestClient(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPut { + t.Error("PUT sent for a release the album does not have") + } + _, _ = w.Write([]byte(`{"id":4723,"releases":[{"id":10148,"monitored":true}]}`)) + }) + defer srv.Close() + + if err := c.SetMonitoredRelease(context.Background(), 4723, 999); !errors.Is(err, ErrNotFound) { + t.Errorf("err = %v, want ErrNotFound", err) + } +} diff --git a/internal/notifications/kinds.go b/internal/notifications/kinds.go index 31585a54..13a22a6e 100644 --- a/internal/notifications/kinds.go +++ b/internal/notifications/kinds.go @@ -9,8 +9,9 @@ package notifications // Kind names one sort of notification. The set is CHECK-gated in migration -// 0073 (rule 36): a new kind adds its value there, in the same change, and -// TestKindsMatchMigrationCheck fails until it does. +// 0073, swapped since by 0075 (rule 36): a new kind adds its value in a +// migration of the same change, and TestEveryKindPassesTheSchemaChecks +// fails until it does. type Kind string const ( @@ -22,7 +23,11 @@ const ( KindScanFailed Kind = "scan_failed" KindTracksMissing Kind = "tracks_missing" KindDuplicatesFound Kind = "duplicates_found" - KindPlaybackErrors Kind = "playback_errors" + // KindDuplicatesResolved: the duplicate resolver (M498) merged copies or + // changed a Lidarr release on its own. The operator asked for that to + // happen automatically; this is how it stays visible. + KindDuplicatesResolved Kind = "duplicates_resolved" + KindPlaybackErrors Kind = "playback_errors" ) // Audience is who a kind can reach. Admin kinds are never offered to, or @@ -77,6 +82,9 @@ type spec struct { var ( allOn = Channels{Inbox: true, Phone: true, Email: true} healthAlert = Channels{Inbox: true, Phone: true, Email: false} + // inboxOnly: news an admin may want to read but never needs to be + // interrupted for. + inboxOnly = Channels{Inbox: true} ) var specs = map[Kind]spec{ @@ -88,7 +96,9 @@ var specs = map[Kind]spec{ KindScanFailed: {audience: AudienceAdmin, coalesce: true, sumCount: true, group: EmailBatch, defaults: healthAlert}, KindTracksMissing: {audience: AudienceAdmin, coalesce: true, sumCount: true, group: EmailBatch, defaults: healthAlert}, KindDuplicatesFound: {audience: AudienceAdmin, coalesce: true, group: EmailBatch, defaults: healthAlert}, - KindPlaybackErrors: {audience: AudienceAdmin, coalesce: true, group: EmailBatch, defaults: healthAlert}, + // Each pass reports what it did itself, so the counts add up. + KindDuplicatesResolved: {audience: AudienceAdmin, coalesce: true, sumCount: true, group: EmailBatch, defaults: inboxOnly}, + KindPlaybackErrors: {audience: AudienceAdmin, coalesce: true, group: EmailBatch, defaults: healthAlert}, } // order is the display order for settings screens: the requester's own kinds @@ -102,6 +112,7 @@ var order = []Kind{ KindScanFailed, KindTracksMissing, KindDuplicatesFound, + KindDuplicatesResolved, KindPlaybackErrors, } diff --git a/internal/notifications/kinds_test.go b/internal/notifications/kinds_test.go index 0d9d8856..69c0a14b 100644 --- a/internal/notifications/kinds_test.go +++ b/internal/notifications/kinds_test.go @@ -36,7 +36,7 @@ func TestKinds_OrderAndSpecsAgree(t *testing.T) { func TestKinds_AdminKindsDefaultEmailOffExceptRequestQueue(t *testing.T) { // The burst-prone health kinds stay out of email unless an admin opts // in; the request queue and quarantine flags are things an admin acts on. - for _, k := range []Kind{KindScanFailed, KindTracksMissing, KindDuplicatesFound, KindPlaybackErrors} { + for _, k := range []Kind{KindScanFailed, KindTracksMissing, KindDuplicatesFound, KindDuplicatesResolved, KindPlaybackErrors} { require.True(t, k.AdminOnly(), k) require.False(t, k.Defaults().Email, k) require.NotEmpty(t, k.coalesceKey(), "%s should coalesce", k) diff --git a/internal/notifications/render.go b/internal/notifications/render.go index e634f1de..186c8d22 100644 --- a/internal/notifications/render.go +++ b/internal/notifications/render.go @@ -37,7 +37,8 @@ type Payload struct { Reason string `json:"reason,omitempty"` // Count is the coalesced kinds' running total. Count int64 `json:"count,omitempty"` - // Detail is free text for scan_failed (the error message). + // Detail is free text: scan_failed's error message, and what the duplicate + // resolver changed in Lidarr for duplicates_resolved. Detail string `json:"detail,omitempty"` } @@ -115,6 +116,16 @@ func Render(kind Kind, payload []byte) Rendered { Body: "The duplicate sweep found tracks holding the same recording.", Link: "/admin/duplicates", } + case KindDuplicatesResolved: + body := "Copies Lidarr does not need were merged into the one it tracks." + if p.Detail != "" { + body = p.Detail + } + return Rendered{ + Title: plural(p.Count, "duplicate resolved automatically", "duplicates resolved automatically"), + Body: body, + Link: "/admin/duplicates?view=resolved", + } case KindPlaybackErrors: return Rendered{ Title: plural(p.Count, "playback error reported", "playback errors reported"), diff --git a/internal/server/server.go b/internal/server/server.go index bb77f503..978234bb 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -57,6 +57,17 @@ func (a lidarrUnmonitorAdapter) UnmonitorTrack(ctx context.Context, trackMbid, a return c.UnmonitorTrack(ctx, trackMbid, albumMbid) } +// ListUnmappedTrackFiles lets the duplicate merge refuse to remove a copy +// Lidarr maps (M498). tracks.ErrLidarrDisabled tells it there is no Lidarr to +// protect. +func (a lidarrUnmonitorAdapter) ListUnmappedTrackFiles(ctx context.Context) ([]lidarr.TrackFile, error) { + c := a.fn() + if c == nil { + return nil, tracks.ErrLidarrDisabled + } + return c.ListUnmappedTrackFiles(ctx) +} + // ScanTrigger is the subset of the scanner the HTTP handler needs. Kept as an // interface so tests can stub it without touching the DB. The progressCb // parameter (added in m7-scan-progress) lets the orchestrator drive partial- diff --git a/internal/tracks/service.go b/internal/tracks/service.go index 1d576bd1..1d460677 100644 --- a/internal/tracks/service.go +++ b/internal/tracks/service.go @@ -31,6 +31,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/audit" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" "git.fabledsword.com/bvandeusen/minstrel/internal/library" + "git.fabledsword.com/bvandeusen/minstrel/internal/lidarr" syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" ) @@ -40,6 +41,22 @@ import ( // same pointer. var ErrNotFound = apierror.ErrNotFound +// ErrLidarrDisabled is what a LidarrUnmonitorer that also lists Lidarr's +// unmapped files returns when Lidarr is turned off: there is then no Lidarr +// record a removal could break, and a merge goes ahead unguarded. +var ErrLidarrDisabled = errors.New("tracks: lidarr disabled") + +// ErrLidarrUnavailable means Lidarr is on but did not answer, so a merge cannot +// confirm the copies it would remove are ones Lidarr does not need (M498), and +// refuses rather than guess. +var ErrLidarrUnavailable = errors.New("tracks: lidarr did not answer") + +// lidarrFileLister is the optional part of the Lidarr dependency a merge uses to +// learn which files Lidarr holds without mapping to any track. +type lidarrFileLister interface { + ListUnmappedTrackFiles(ctx context.Context) ([]lidarr.TrackFile, error) +} + // LidarrUnmonitorer is the subset of *lidarr.Client RemoveTrack uses. // Defined as an interface so tests can stub without spinning up a Lidarr // httptest server. UnmonitorTrack failures are non-fatal at the service @@ -151,7 +168,9 @@ func (s *Service) RemoveTrack( } // MergeDuplicates merges a duplicate group into the copy to keep -// (library.MergeDuplicateGroup), then — when asked — tells Lidarr to stop +// (library.MergeDuplicateGroupGuarded, refusing to remove a copy Lidarr maps: +// library.ErrCopyTrackedByLidarr, or ErrLidarrUnavailable when Lidarr cannot +// say), then — when asked — tells Lidarr to stop // monitoring the removed copies so it does not download them again, and records // the merge in the audit log. // @@ -161,7 +180,11 @@ func (s *Service) RemoveTrack( func (s *Service) MergeDuplicates( ctx context.Context, groupID, survivorID, actorID pgtype.UUID, unmonitor bool, ) (res library.MergeResult, lidarrUnmonitorFailed bool, err error) { - res, err = library.MergeDuplicateGroup(ctx, s.pool, s.logger, s.dataDir, groupID, survivorID) + removable, err := s.lidarrRemovable(ctx) + if err != nil { + return res, false, err + } + res, err = library.MergeDuplicateGroupGuarded(ctx, s.pool, s.logger, s.dataDir, groupID, survivorID, removable) if err != nil { return res, false, err } @@ -203,6 +226,29 @@ func (s *Service) MergeDuplicates( return res, lidarrUnmonitorFailed, nil } +// lidarrRemovable is the merge's guard (M498): only a copy Lidarr holds without +// mapping it to a track may be removed, because removing a mapped one makes +// Lidarr download it again. Nil, allowing everything, when Lidarr is disabled +// or the dependency cannot list files. +func (s *Service) lidarrRemovable(ctx context.Context) (library.RemovableFunc, error) { + lister, ok := s.lidarr.(lidarrFileLister) + if !ok { + return nil, nil + } + files, err := lister.ListUnmappedTrackFiles(ctx) + if errors.Is(err, ErrLidarrDisabled) { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("%w: %v", ErrLidarrUnavailable, err) + } + unmapped := make(map[string]bool, len(files)) + for _, f := range files { + unmapped[library.LidarrPathKey(f.Path)] = true + } + return func(p string) bool { return unmapped[library.LidarrPathKey(p)] }, nil +} + // sameLidarrTrack reports whether two copies are the same Lidarr track: one // recording on one album. Lidarr monitors per album track, so when the removed // copy is a second file of the kept copy's own album track — the #3885 case — diff --git a/web/src/lib/api/admin.duplicates.test.ts b/web/src/lib/api/admin.duplicates.test.ts index 5be0cac9..3f0c8d15 100644 --- a/web/src/lib/api/admin.duplicates.test.ts +++ b/web/src/lib/api/admin.duplicates.test.ts @@ -1,5 +1,13 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { dismissDuplicateGroup, listDuplicates, mergeDuplicateGroup, runDuplicateSweep } from './admin'; +import { + dismissDuplicateGroup, + duplicatesNextOffset, + listDuplicates, + listResolvedDuplicates, + mergeDuplicateGroup, + runDuplicateSweep +} from './admin'; +import type { AdminDuplicatesResponse } from './types'; vi.mock('./client', () => ({ api: { get: vi.fn(), post: vi.fn() } @@ -10,10 +18,27 @@ import { api } from './client'; describe('admin duplicates API', () => { beforeEach(() => vi.clearAllMocks()); - it('listDuplicates GETs the paged report', async () => { + it('listDuplicates GETs one tab of the paged report', async () => { (api.get as unknown as ReturnType).mockResolvedValueOnce({ groups: [] }); - await listDuplicates(25, 25); - expect(api.get).toHaveBeenCalledWith('/api/admin/library/duplicates?limit=25&offset=25'); + await listDuplicates('cross_release', 25); + expect(api.get).toHaveBeenCalledWith( + '/api/admin/library/duplicates?view=cross_release&limit=25&offset=25' + ); + }); + + it('listResolvedDuplicates GETs what the resolver did', async () => { + (api.get as unknown as ReturnType).mockResolvedValueOnce({ items: [] }); + await listResolvedDuplicates(50); + expect(api.get).toHaveBeenCalledWith('/api/admin/library/duplicates/resolved?limit=25&offset=50'); + }); + + // The next offset counts groups, and stops at the total or on an empty page. + it('duplicatesNextOffset pages by groups', () => { + const page = (offset: number, n: number, total: number) => + ({ offset, total, groups: Array.from({ length: n }, () => ({})) }) as unknown as AdminDuplicatesResponse; + expect(duplicatesNextOffset(page(0, 25, 60))).toBe(25); + expect(duplicatesNextOffset(page(50, 10, 60))).toBeUndefined(); + expect(duplicatesNextOffset(page(25, 0, 60))).toBeUndefined(); }); it('runDuplicateSweep POSTs the trigger', async () => { diff --git a/web/src/lib/api/admin.fingerprints.test.ts b/web/src/lib/api/admin.fingerprints.test.ts index 1ae3669c..9d8aa991 100644 --- a/web/src/lib/api/admin.fingerprints.test.ts +++ b/web/src/lib/api/admin.fingerprints.test.ts @@ -49,7 +49,8 @@ describe('admin fingerprint coverage API', () => { chromaprint_length_sec: 120, acoustic_max_bit_error_rate: 0.15, backfill_concurrency: 2, - sweep_interval_hours: 1 + sweep_interval_hours: 1, + auto_resolve: true }; (api.get as unknown as ReturnType).mockResolvedValueOnce(settings); (api.put as unknown as ReturnType).mockResolvedValueOnce(settings); diff --git a/web/src/lib/api/admin.ts b/web/src/lib/api/admin.ts index 6056a008..06a80e30 100644 --- a/web/src/lib/api/admin.ts +++ b/web/src/lib/api/admin.ts @@ -6,7 +6,9 @@ import type { AdminMissingResponse, AdminSuspectResponse, AdminDuplicatesResponse, + DuplicatesView, MergeDuplicateResult, + ResolvedDuplicatesResponse, AdminPlaybackError, AdminQuarantineRow, LidarrConfig, @@ -353,6 +355,9 @@ export type FingerprintSettings = { acoustic_max_bit_error_rate: number; backfill_concurrency: number; sweep_interval_hours: number; + // Lets the duplicate resolver act on its own (M498): merge copies Lidarr + // doesn't need and change a Lidarr release that lists songs twice. + auto_resolve: boolean; }; export async function getFingerprintSettings(): Promise { @@ -829,24 +834,55 @@ export async function updatePublicUrl(publicUrl: string): Promise { return api.get( - `/api/admin/library/duplicates?limit=${limit}&offset=${offset}` + `/api/admin/library/duplicates?view=${view}&limit=${limit}&offset=${offset}` ); } -// Takes a plain offset, like createMissingFilesQuery, so a $derived caller -// re-creates the query on paging. Polls while a sweep might be running: the -// page is where the operator waits for one to finish. -export function createDuplicatesQuery(offset: number = 0, limit: number = 25) { - return createQuery({ - queryKey: qk.adminDuplicates(offset), - queryFn: () => listDuplicates(offset, limit), +export function duplicatesNextOffset(last: AdminDuplicatesResponse): number | undefined { + const loaded = last.offset + last.groups.length; + return last.groups.length === 0 || loaded >= last.total ? undefined : loaded; +} + +// Loads as the page scrolls (rule 172), one tab at a time. Polls so a sweep +// or resolver pass finishing shows up while the operator is on the page. +export function createDuplicatesQuery(view: DuplicatesView = 'review') { + return createInfiniteQuery({ + queryKey: qk.adminDuplicates(view), + queryFn: ({ pageParam }) => listDuplicates(view, pageParam as number), + initialPageParam: 0, + getNextPageParam: duplicatesNextOffset, staleTime: 30_000, - refetchInterval: 15_000 + refetchInterval: 30_000 + }); +} + +export async function listResolvedDuplicates(offset: number = 0): Promise { + return api.get( + `/api/admin/library/duplicates/resolved?limit=${DUPLICATES_PAGE_SIZE}&offset=${offset}` + ); +} + +export function resolvedDuplicatesNextOffset(last: ResolvedDuplicatesResponse): number | undefined { + const loaded = last.offset + last.items.length; + return last.items.length === 0 || loaded >= last.total ? undefined : loaded; +} + +// What the resolver did by itself (M498), newest first. +export function createResolvedDuplicatesQuery() { + return createInfiniteQuery({ + queryKey: qk.adminResolvedDuplicates(), + queryFn: ({ pageParam }) => listResolvedDuplicates(pageParam as number), + initialPageParam: 0, + getNextPageParam: resolvedDuplicatesNextOffset, + staleTime: 60_000 }); } diff --git a/web/src/lib/api/notifications.ts b/web/src/lib/api/notifications.ts index 9599cab5..75eefd18 100644 --- a/web/src/lib/api/notifications.ts +++ b/web/src/lib/api/notifications.ts @@ -122,5 +122,6 @@ export const NOTIFICATION_KIND_LABELS: Record = { scan_failed: 'Library scan failed', tracks_missing: 'Tracks gone missing', duplicates_found: 'Duplicates to review', + duplicates_resolved: 'Duplicates resolved', playback_errors: 'Playback errors' }; diff --git a/web/src/lib/api/queries.ts b/web/src/lib/api/queries.ts index 7edf5155..8ed04848 100644 --- a/web/src/lib/api/queries.ts +++ b/web/src/lib/api/queries.ts @@ -56,8 +56,9 @@ export const qk = { ['adminDiagnostics', f] as const, adminMissingFiles: (offset?: number) => ['adminMissingFiles', { offset: offset ?? 0 }] as const, - adminDuplicates: (offset?: number) => - ['adminDuplicates', { offset: offset ?? 0 }] as const, + adminDuplicates: (view: string = 'review') => + ['adminDuplicates', { view }] as const, + adminResolvedDuplicates: () => ['adminResolvedDuplicates'] as const, adminSuspectSources: () => ['adminSuspectSources'] as const, adminDiagnosticDevices: (userId?: string) => ['adminDiagnosticDevices', { userId: userId ?? 'all' }] as const, diff --git a/web/src/lib/api/types.ts b/web/src/lib/api/types.ts index 10887d41..e18795eb 100644 --- a/web/src/lib/api/types.ts +++ b/web/src/lib/api/types.ts @@ -471,8 +471,17 @@ export type AdminDuplicateMember = { added_at: string; like_count: number; play_count: number; + // Lidarr's view of the file as of the resolver's last pass (M498): tracked + // means Lidarr maps it to a track, so removing it would make Lidarr download + // it again. null when Lidarr wasn't asked. + lidarr_state: 'tracked' | 'unmapped' | null; + disc_number: number | null; + track_number: number | null; }; +// The resolver's verdict on a group (M498). null until its first pass. +export type DuplicateClass = 'same_release' | 'cross_release' | 'mismatch' | 'review'; + // exact: identical encoded audio. acoustic: the same recording, differently // encoded; worst_bit_error_rate is the weakest link between any two members. export type AdminDuplicateGroup = { @@ -482,6 +491,9 @@ export type AdminDuplicateGroup = { detected_at: string; survivor_track_id: string; survivor_reason: string; + class: DuplicateClass | null; + // Why the resolver left the group for the operator, when it had something to say. + resolve_note: string | null; members: AdminDuplicateMember[]; }; @@ -513,8 +525,40 @@ export type AdminDuplicatesResponse = { pending: number; enabled: boolean; }; + view: DuplicatesView; + // Each tab's size, for its pill. + counts: { review: number; cross_release: number; resolved: number }; total: number; limit: number; offset: number; groups: AdminDuplicateGroup[]; }; + +// The report's group tabs. 'resolved' is the third tab, served separately. +export type DuplicatesView = 'review' | 'cross_release'; + +// One thing the resolver did on its own (M498), from the audit log. +export type ResolvedDuplicateActivity = + | { + id: string; + action: 'duplicate_merge'; + created_at: string; + details: { + survivor_path?: string; + survivor_reason?: string; + removed?: { track_id: string; file_path: string }[]; + }; + } + | { + id: string; + action: 'lidarr_release_change'; + created_at: string; + details: { album_title?: string; artist_name?: string; from_release?: string; to_release?: string }; + }; + +export type ResolvedDuplicatesResponse = { + total: number; + limit: number; + offset: number; + items: ResolvedDuplicateActivity[]; +}; diff --git a/web/src/lib/components/FingerprintSettingsCard.svelte b/web/src/lib/components/FingerprintSettingsCard.svelte index 2ccad5ef..b4f49557 100644 --- a/web/src/lib/components/FingerprintSettingsCard.svelte +++ b/web/src/lib/components/FingerprintSettingsCard.svelte @@ -115,6 +115,18 @@ + +