From 01e22944713612baab4d27fb3be6fb0140301fdc Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 8 Oct 2026 20:18:49 -0400 Subject: [PATCH 1/5] ci: probe and relax the integration Postgres over TCP, with retries (#5426) On first boot the postgres image runs initdb under a temporary server that listens on the unix socket only, then stops it. The socket readiness probe could catch that server, and the fsync relax then hit 'the database system is shutting down', leaving the suite ~15x slower on internal/api. Only the real server listens on TCP, so probe and relax via -h 127.0.0.1, retry the relax, and print the settings that took effect. Co-Authored-By: Claude Opus 5.5 --- .gitea/workflows/release.yml | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/.gitea/workflows/release.yml b/.gitea/workflows/release.yml index 492fc566..a8c3a1f4 100644 --- a/.gitea/workflows/release.yml +++ b/.gitea/workflows/release.yml @@ -239,9 +239,16 @@ jobs: # container itself: the run: shell is dash (rule 81), where the # old `/dev/tcp` probe never connects and the loop silently # burned its full two minutes on every run. + # + # Over TCP (-h 127.0.0.1), not the unix socket (#5426). On first + # boot the image's entrypoint runs initdb under a temporary server + # that listens on the socket only, then stops it and starts the + # real one. A socket probe can catch the temporary server, and the + # next step then meets "the database system is shutting down". + # Only the real server listens on TCP. ready="" for i in $(seq 1 60); do - if docker exec "$PG_ID" pg_isready -U minstrel -d minstrel_test -q; then ready=1; break; fi + if docker exec "$PG_ID" pg_isready -h 127.0.0.1 -U minstrel -d minstrel_test -q; then ready=1; break; fi sleep 2 done test -n "$ready" || { echo "FATAL: postgres never became ready"; exit 1; } @@ -259,13 +266,25 @@ jobs: # - fsync / full_page_writes are sighup GUCs and # synchronous_commit is user-context, so pg_reload_conf() picks # all three up with no restart. - # Non-fatal: a perms surprise degrades to "slower", never red CI. - docker exec "$PG_ID" psql -U minstrel -d minstrel_test \ - -c "ALTER SYSTEM SET fsync = off" \ - -c "ALTER SYSTEM SET synchronous_commit = off" \ - -c "ALTER SYSTEM SET full_page_writes = off" \ - -c "SELECT pg_reload_conf()" \ - || echo "WARN: durability relax failed; continuing" + # Non-fatal: a failure degrades to "slower", never red CI. But + # slower is ~15x on internal/api (#5426), so retry a few times and + # print what took effect. + relaxed="" + for i in 1 2 3 4 5; do + if docker exec -e PGPASSWORD=minstrel "$PG_ID" psql -h 127.0.0.1 -U minstrel -d minstrel_test \ + -c "ALTER SYSTEM SET fsync = off" \ + -c "ALTER SYSTEM SET synchronous_commit = off" \ + -c "ALTER SYSTEM SET full_page_writes = off" \ + -c "SELECT pg_reload_conf()"; then relaxed=1; break; fi + sleep 2 + done + if [ -n "$relaxed" ]; then + docker exec -e PGPASSWORD=minstrel "$PG_ID" psql -h 127.0.0.1 -U minstrel -d minstrel_test -At \ + -c "SELECT 'durability: ' || string_agg(name || '=' || setting, ' ' ORDER BY name) FROM pg_settings WHERE name IN ('fsync','synchronous_commit','full_page_writes')" \ + || true + else + echo "::warning::durability relax failed after 5 tries; the suite will run several times slower" + fi # Apply embedded migrations to the fresh test DB, then run the # full suite (no -short → integration tests execute). -p 1: From 4ecff52f19b9f0c29f18dd772bdaecbd41091673 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 8 Oct 2026 21:49:37 -0400 Subject: [PATCH 2/5] feat: duplicates resolve themselves where Lidarr says it is safe (M498) The duplicate sweep proposed 4,197 groups and every one waited for the operator. Most are safe to settle, and Lidarr defines what safe means: it maps one file to each track of the release it monitors and downloads any mapped file that disappears. Deleting a mapped copy opens exactly the hole the operator saw Lidarr fill. Classify (#5435) - Migration 0075: duplicate_groups.class (same_release, cross_release, mismatch, review), resolve_note, resolved_automatically; duplicate_group_members.lidarr_state (tracked, unmapped); fingerprint_settings.auto_resolve; notification kind duplicates_resolved with both kind CHECKs swapped (rule 36). - library.ClassifyDuplicateGroup, with MatchTitleKey dropping featuring credits, remaster notes and video-rip markers, and keeping live, demo, remix and instrumental. The rip markers move from api to library. Choose the copy to keep (#5436) - ProposeSurvivor ranks the copy Lidarr maps first, then tag fit (a clash-free track number, no rip marker in the name, an MBID), then the quality rules. File size picked the wrong Humanz copy in 6 of 21 groups. Act (#5437) - An hourly resolver pass reads Lidarr's unmapped files, matched by the last three path components, and records each copy's state. - Same album, with at most one copy mapped: merged into the mapped copy. The merge is guarded, so a mapped copy can never be removed (MergeDuplicateGroupGuarded, ErrCopyTrackedByLidarr). - Same album, every copy mapped: the monitored release lists the song twice (Humanz's 14x12" box set). The pass moves Lidarr to the release that lists each song once and best covers what is on disk. It never picks one covering less, and is capped at 10 albums per pass. - Fixed point (lesson #4183): the chosen release no longer repeats. - The album is left alone for 24h while Lidarr rescans, so "every copy unmapped" mid-rescan is never read as licence to merge. - Both actions are audited with no actor and summarised to admins. The operator can switch them off in the Fingerprinting card (rule 25). - Manual merges use the same guard: 409 copy_tracked_by_lidarr, or 503 lidarr_unavailable when Lidarr cannot say. Web - Duplicates gets tabs: Needs review, Across releases, Resolved automatically. Each loads as you scroll (rule 172), replacing the pager. - Each copy says whether Lidarr uses it. - The resolver's note shows on each group. - The merge confirm blocks, before sending, a merge that would remove the copy Lidarr uses. Co-Authored-By: Claude Opus 5.5 --- .../ui/NotificationSettingsScreen.kt | 1 + cmd/minstrel/main.go | 13 + internal/api/admin_duplicates.go | 149 +++- internal/api/admin_duplicates_test.go | 26 + internal/api/admin_fingerprint_settings.go | 10 + .../api/admin_fingerprint_settings_test.go | 8 +- internal/api/admin_suspect_sources.go | 71 +- internal/api/admin_suspect_sources_test.go | 47 -- internal/api/api.go | 1 + internal/audit/audit.go | 5 + internal/db/dbq/audit_log.sql.go | 69 ++ internal/db/dbq/duplicates.sql.go | 245 ++++++- internal/db/dbq/fingerprint_settings.sql.go | 9 +- internal/db/dbq/models.go | 25 +- .../0075_duplicate_resolve.down.sql | 17 + .../migrations/0075_duplicate_resolve.up.sql | 56 ++ internal/db/queries/audit_log.sql | 16 + internal/db/queries/duplicates.sql | 109 ++- internal/db/queries/fingerprint_settings.sql | 1 + internal/library/duplicate_classify.go | 143 ++++ internal/library/duplicate_classify_test.go | 71 ++ internal/library/duplicate_merge.go | 28 + internal/library/duplicate_resolve.go | 678 ++++++++++++++++++ internal/library/duplicate_resolve_test.go | 349 +++++++++ internal/library/duplicate_survivor.go | 43 ++ internal/library/duplicate_survivor_test.go | 63 ++ internal/library/fingerprint_settings.go | 7 + internal/library/fingerprint_settings_test.go | 2 + internal/library/source_markers.go | 69 ++ internal/library/source_markers_test.go | 53 ++ internal/lidarr/releases.go | 154 ++++ internal/lidarr/releases_test.go | 116 +++ internal/notifications/kinds.go | 19 +- internal/notifications/kinds_test.go | 2 +- internal/notifications/render.go | 13 +- internal/server/server.go | 11 + internal/tracks/service.go | 50 +- web/src/lib/api/admin.duplicates.test.ts | 33 +- web/src/lib/api/admin.fingerprints.test.ts | 3 +- web/src/lib/api/admin.ts | 56 +- web/src/lib/api/notifications.ts | 1 + web/src/lib/api/queries.ts | 5 +- web/src/lib/api/types.ts | 44 ++ .../components/FingerprintSettingsCard.svelte | 12 + .../FingerprintSettingsCard.test.ts | 17 +- web/src/lib/styles/error-copy.json | 2 + web/src/routes/admin/duplicates/+page.svelte | 269 +++++-- .../admin/duplicates/duplicates.test.ts | 131 +++- 48 files changed, 3068 insertions(+), 254 deletions(-) create mode 100644 internal/db/migrations/0075_duplicate_resolve.down.sql create mode 100644 internal/db/migrations/0075_duplicate_resolve.up.sql create mode 100644 internal/library/duplicate_classify.go create mode 100644 internal/library/duplicate_classify_test.go create mode 100644 internal/library/duplicate_resolve.go create mode 100644 internal/library/duplicate_resolve_test.go create mode 100644 internal/library/source_markers.go create mode 100644 internal/library/source_markers_test.go create mode 100644 internal/lidarr/releases.go create mode 100644 internal/lidarr/releases_test.go 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 @@ + +