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: 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..0bc48c42 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 { @@ -211,15 +331,17 @@ func (h *handlers) handleRunDuplicateSweep(w http.ResponseWriter, _ *http.Reques // handleDismissDuplicateGroup implements POST // /api/admin/library/duplicates/{id}/dismiss: "these are not duplicates". The -// sweep keeps the dismissal and will not propose that set of tracks again. 404 -// duplicate_group_not_pending when the group was already resolved or is gone. +// sweep keeps the dismissal and will not propose that set of tracks again. On a +// cross-release group it also undoes the song link (M498 #5438): the copies +// stop sharing likes from here on. 404 duplicate_group_not_pending when the +// group was already resolved or is gone. func (h *handlers) handleDismissDuplicateGroup(w http.ResponseWriter, r *http.Request) { id, ok := parseUUID(chi.URLParam(r, "id")) if !ok { writeAdminJSONErr(w, http.StatusBadRequest, "invalid_id") return } - n, err := dbq.New(h.pool).DismissDuplicateGroup(r.Context(), id) + n, err := h.dismissDuplicateGroup(r.Context(), id) if err != nil { h.logger.Error("admin: dismiss duplicate group", "err", err) writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") @@ -232,6 +354,25 @@ func (h *handlers) handleDismissDuplicateGroup(w http.ResponseWriter, r *http.Re writeJSON(w, http.StatusOK, map[string]string{"status": "dismissed"}) } +// dismissDuplicateGroup dismisses the group and unlinks its copies as one song, +// together. +func (h *handlers) dismissDuplicateGroup(ctx context.Context, id pgtype.UUID) (int64, error) { + tx, err := h.pool.Begin(ctx) + if err != nil { + return 0, err + } + defer func() { _ = tx.Rollback(ctx) }() + tq := dbq.New(tx) + n, err := tq.DismissDuplicateGroup(ctx, id) + if err != nil || n == 0 { + return n, err + } + if err := tq.UnlinkDuplicateGroupSongs(ctx, id); err != nil { + return 0, err + } + return n, tx.Commit(ctx) +} + // mergeDuplicateRequest chooses the copy to keep. An empty survivor_track_id // keeps the report's proposal. type mergeDuplicateRequest struct { @@ -257,6 +398,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 +439,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..cca97934 100644 --- a/internal/api/admin_suspect_sources.go +++ b/internal/api/admin_suspect_sources.go @@ -1,75 +1,16 @@ package api import ( + "encoding/json" "net/http" - "path" - "regexp" - "strings" + "time" + + "github.com/go-chi/chi/v5" "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. @@ -85,6 +26,11 @@ type suspectTrackView struct { DiscNumber *int32 `json:"disc_number"` TrackNumber *int32 `json:"track_number"` Markers []string `json:"markers"` + // Verdict is "suspect" while the track is held back from radio and the + // mixes, "fine" once the operator let it back, null before the resolver's + // first pass (M498 #5439). VerdictAt is when it was last set. + Verdict *string `json:"verdict"` + VerdictAt *string `json:"verdict_at"` } // suspectGroupView is a folder's worth of flagged tracks. As on the @@ -105,9 +51,10 @@ type adminSuspectSourcesResponse struct { // handleListSuspectSources implements GET /api/admin/library/suspect-sources. // -// Read-only. A marker is a reason to look, not proof: a band can title a song -// "Music Video". What to do about a flagged file — merge it in Duplicates, -// quarantine it, replace it in Lidarr — stays the operator's call. +// A marker is a reason to look, not proof: a band can title a song "Music +// Video". The duplicate resolver holds a flagged track back from radio and the +// mixes and merges it away where a clean copy exists (M498 #5439); this report +// is where the operator sees that and can say a track is fine. func (h *handlers) handleListSuspectSources(w http.ResponseWriter, r *http.Request) { limit, offset, err := parsePaging(r.URL.Query()) if err != nil { @@ -116,14 +63,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 +105,12 @@ 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), + Verdict: row.SourceVerdict, + } + if row.SourceVerdictAt.Valid { + at := row.SourceVerdictAt.Time.UTC().Format(time.RFC3339) + t.VerdictAt = &at } if n := len(groups); n > 0 && groups[n-1].Directory == row.Directory { groups[n-1].Tracks = append(groups[n-1].Tracks, t) @@ -171,3 +123,39 @@ func groupSuspectByDirectory(rows []dbq.ListSuspectSourceTracksRow) []suspectGro } return groups } + +type suspectVerdictRequest struct { + Verdict string `json:"verdict"` +} + +// handleSetSuspectSourceVerdict implements PUT +// /api/admin/library/suspect-sources/{id}: {"verdict": "fine"} lets a flagged +// track back into radio and the mixes for good, {"verdict": "suspect"} holds +// it back again. 404 suspect_track_not_found when the track was never flagged. +func (h *handlers) handleSetSuspectSourceVerdict(w http.ResponseWriter, r *http.Request) { + id, ok := parseUUID(chi.URLParam(r, "id")) + if !ok { + writeAdminJSONErr(w, http.StatusBadRequest, "invalid_id") + return + } + var req suspectVerdictRequest + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + writeAdminJSONErr(w, http.StatusBadRequest, "invalid_body") + return + } + if req.Verdict != "fine" && req.Verdict != "suspect" { + writeAdminJSONErr(w, http.StatusBadRequest, "invalid_verdict") + return + } + n, err := dbq.New(h.pool).SetTrackSourceVerdict(r.Context(), dbq.SetTrackSourceVerdictParams{Verdict: req.Verdict, ID: id}) + if err != nil { + h.logger.Error("admin: set suspect-source verdict", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + return + } + if n == 0 { + writeAdminJSONErr(w, http.StatusNotFound, "suspect_track_not_found") + return + } + writeJSON(w, http.StatusOK, map[string]string{"verdict": req.Verdict}) +} 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..1162ec25 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -253,6 +253,7 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev // not be mistaken for it (#2527). admin.Get("/library/missing", h.handleListMissingTracks) admin.Get("/library/suspect-sources", h.handleListSuspectSources) + admin.Put("/library/suspect-sources/{id}", h.handleSetSuspectSourceVerdict) admin.Get("/library/coverage", h.handleGetLibraryCoverage) admin.Get("/library/fingerprints", h.handleGetFingerprintCoverage) @@ -269,6 +270,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/api/likes.go b/internal/api/likes.go index b36dfcd7..aad2a663 100644 --- a/internal/api/likes.go +++ b/internal/api/likes.go @@ -3,6 +3,7 @@ package api import ( "errors" "net/http" + "slices" "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgtype" @@ -50,20 +51,36 @@ func (h *handlers) handleLikeTrack(w http.ResponseWriter, r *http.Request) { writeErr(w, apierror.InternalMsg("lookup failed", err)) return } - rows, err := q.LikeTrack(r.Context(), dbq.LikeTrackParams{UserID: user.ID, TrackID: id}) + liked, err := q.LikeTrack(r.Context(), dbq.LikeTrackParams{UserID: user.ID, TrackID: id}) if err != nil { h.logger.Error("api: like track insert", "err", err) writeErr(w, apierror.InternalMsg("insert failed", err)) return } - if rows == 1 { + if slices.Contains(liked, id) { _ = playevents.CaptureContextualLikeIfPlaying(r.Context(), q, user.ID, id, h.logger) } - h.logLikeChange(r, syncpkg.EntityLikeTrack, user.ID, id, syncpkg.OpUpsert) - h.publishLikeEvent(user.ID, id, "track", true) + // The like is on the song (M498 #5438): every copy it reached changes on + // the client too. + for _, t := range withTrack(id, liked) { + h.logLikeChange(r, syncpkg.EntityLikeTrack, user.ID, t, syncpkg.OpUpsert) + h.publishLikeEvent(user.ID, t, "track", true) + } w.WriteHeader(http.StatusNoContent) } +// withTrack is the tracks a like or unlike changed, led by the one asked for, +// which is reported even when it was already in that state. +func withTrack(id pgtype.UUID, changed []pgtype.UUID) []pgtype.UUID { + out := []pgtype.UUID{id} + for _, t := range changed { + if t != id { + out = append(out, t) + } + } + return out +} + func (h *handlers) handleUnlikeTrack(w http.ResponseWriter, r *http.Request) { user, ok := requireUser(w, r) if !ok { @@ -74,7 +91,8 @@ func (h *handlers) handleUnlikeTrack(w http.ResponseWriter, r *http.Request) { return } q := dbq.New(h.pool) - if err := q.UnlikeTrack(r.Context(), dbq.UnlikeTrackParams{UserID: user.ID, TrackID: id}); err != nil { + unliked, err := q.UnlikeTrack(r.Context(), dbq.UnlikeTrackParams{UserID: user.ID, TrackID: id}) + if err != nil { h.logger.Error("api: unlike track", "err", err) writeErr(w, apierror.InternalMsg("delete failed", err)) return @@ -83,8 +101,10 @@ func (h *handlers) handleUnlikeTrack(w http.ResponseWriter, r *http.Request) { h.logger.Error("api: soft-delete contextual_likes", "err", err) // Don't fail the response — soft-delete is best-effort. } - h.logLikeChange(r, syncpkg.EntityLikeTrack, user.ID, id, syncpkg.OpDelete) - h.publishLikeEvent(user.ID, id, "track", false) + for _, t := range withTrack(id, unliked) { + h.logLikeChange(r, syncpkg.EntityLikeTrack, user.ID, t, syncpkg.OpDelete) + h.publishLikeEvent(user.ID, t, "track", false) + } w.WriteHeader(http.StatusNoContent) } 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/events.sql.go b/internal/db/dbq/events.sql.go index d626f477..ab656653 100644 --- a/internal/db/dbq/events.sql.go +++ b/internal/db/dbq/events.sql.go @@ -261,7 +261,7 @@ func (q *Queries) InsertSkipEvent(ctx context.Context, arg InsertSkipEventParams } const listRecentSessionTracks = `-- name: ListRecentSessionTracks :many -SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source FROM tracks t +SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key, t.source_verdict, t.source_verdict_at FROM tracks t JOIN play_events pe ON pe.track_id = t.id WHERE pe.session_id = $1 AND pe.started_at < $2 @@ -308,6 +308,10 @@ func (q *Queries) ListRecentSessionTracks(ctx context.Context, arg ListRecentSes &i.TagReadVersion, &i.MissingSince, &i.MbidSource, + &i.SongID, + &i.SongKey, + &i.SourceVerdict, + &i.SourceVerdictAt, ); err != nil { return nil, err } 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/history.sql.go b/internal/db/dbq/history.sql.go index 0d0a4efa..32943d28 100644 --- a/internal/db/dbq/history.sql.go +++ b/internal/db/dbq/history.sql.go @@ -14,7 +14,7 @@ import ( const listUserHistory = `-- name: ListUserHistory :many SELECT pe.id AS event_id, pe.started_at, - t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, + t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key, t.source_verdict, t.source_verdict_at, albums.title AS album_title, artists.name AS artist_name FROM play_events pe @@ -82,6 +82,10 @@ func (q *Queries) ListUserHistory(ctx context.Context, arg ListUserHistoryParams &i.Track.TagReadVersion, &i.Track.MissingSince, &i.Track.MbidSource, + &i.Track.SongID, + &i.Track.SongKey, + &i.Track.SourceVerdict, + &i.Track.SourceVerdictAt, &i.AlbumTitle, &i.ArtistName, ); err != nil { diff --git a/internal/db/dbq/likes.sql.go b/internal/db/dbq/likes.sql.go index b85e1d49..d7480f00 100644 --- a/internal/db/dbq/likes.sql.go +++ b/internal/db/dbq/likes.sql.go @@ -34,9 +34,12 @@ func (q *Queries) CountLikedArtists(ctx context.Context, userID pgtype.UUID) (in } const countLikedTracks = `-- name: CountLikedTracks :one -SELECT count(*) FROM general_likes WHERE user_id = $1 +SELECT count(DISTINCT t.song_key) FROM general_likes l + JOIN tracks t ON t.id = l.track_id + WHERE l.user_id = $1 ` +// Liked songs, each counted once however many releases hold it. func (q *Queries) CountLikedTracks(ctx context.Context, userID pgtype.UUID) (int64, error) { row := q.db.QueryRow(ctx, countLikedTracks, userID) var count int64 @@ -76,10 +79,13 @@ func (q *Queries) LikeArtist(ctx context.Context, arg LikeArtistParams) error { return err } -const likeTrack = `-- name: LikeTrack :execrows +const likeTrack = `-- name: LikeTrack :many INSERT INTO general_likes (user_id, track_id) -VALUES ($1, $2) +SELECT $1::uuid, t.id + FROM tracks t + WHERE t.song_key = (SELECT s.song_key FROM tracks s WHERE s.id = $2::uuid) ON CONFLICT (user_id, track_id) DO NOTHING +RETURNING track_id ` type LikeTrackParams struct { @@ -87,12 +93,27 @@ type LikeTrackParams struct { TrackID pgtype.UUID } -func (q *Queries) LikeTrack(ctx context.Context, arg LikeTrackParams) (int64, error) { - result, err := q.db.Exec(ctx, likeTrack, arg.UserID, arg.TrackID) +// A like is on the song (M498 #5438): every copy of it sharing the track's +// song_key is liked. Returns the tracks newly liked, for the sync log; empty +// when the song was already liked. +func (q *Queries) LikeTrack(ctx context.Context, arg LikeTrackParams) ([]pgtype.UUID, error) { + rows, err := q.db.Query(ctx, likeTrack, arg.UserID, arg.TrackID) if err != nil { - return 0, err + return nil, err } - return result.RowsAffected(), nil + defer rows.Close() + var items []pgtype.UUID + for rows.Next() { + var track_id pgtype.UUID + if err := rows.Scan(&track_id); err != nil { + return nil, err + } + items = append(items, track_id) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil } const listLikedAlbumIDs = `-- name: ListLikedAlbumIDs :many @@ -260,10 +281,15 @@ func (q *Queries) ListLikedTrackIDs(ctx context.Context, userID pgtype.UUID) ([] } const listLikedTrackRows = `-- name: ListLikedTrackRows :many -SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source FROM tracks t -JOIN general_likes l ON l.track_id = t.id -WHERE l.user_id = $1 -ORDER BY l.liked_at DESC +SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key, t.source_verdict, t.source_verdict_at FROM tracks t +JOIN (SELECT DISTINCT ON (lt.song_key) l.track_id, l.liked_at + FROM general_likes l + JOIN tracks lt ON lt.id = l.track_id + WHERE l.user_id = $1 + ORDER BY lt.song_key, (lt.missing_since IS NOT NULL), + (lt.source_verdict IS NOT DISTINCT FROM 'suspect'), l.liked_at, lt.id) l + ON l.track_id = t.id +ORDER BY l.liked_at DESC, t.id LIMIT $2 OFFSET $3 ` @@ -273,6 +299,8 @@ type ListLikedTrackRowsParams struct { Offset int32 } +// One row per liked song: of its copies, the one present on disk, then one +// that is not a held-back video rip, then the one liked first. func (q *Queries) ListLikedTrackRows(ctx context.Context, arg ListLikedTrackRowsParams) ([]Track, error) { rows, err := q.db.Query(ctx, listLikedTrackRows, arg.UserID, arg.Limit, arg.Offset) if err != nil { @@ -303,6 +331,10 @@ func (q *Queries) ListLikedTrackRows(ctx context.Context, arg ListLikedTrackRows &i.TagReadVersion, &i.MissingSince, &i.MbidSource, + &i.SongID, + &i.SongKey, + &i.SourceVerdict, + &i.SourceVerdictAt, ); err != nil { return nil, err } @@ -342,8 +374,13 @@ func (q *Queries) UnlikeArtist(ctx context.Context, arg UnlikeArtistParams) erro return err } -const unlikeTrack = `-- name: UnlikeTrack :exec -DELETE FROM general_likes WHERE user_id = $1 AND track_id = $2 +const unlikeTrack = `-- name: UnlikeTrack :many +DELETE FROM general_likes + WHERE user_id = $1::uuid + AND (track_id = $2::uuid + OR track_id IN (SELECT t.id FROM tracks t + WHERE t.song_key = (SELECT s.song_key FROM tracks s WHERE s.id = $2::uuid))) +RETURNING track_id ` type UnlikeTrackParams struct { @@ -351,7 +388,24 @@ type UnlikeTrackParams struct { TrackID pgtype.UUID } -func (q *Queries) UnlikeTrack(ctx context.Context, arg UnlikeTrackParams) error { - _, err := q.db.Exec(ctx, unlikeTrack, arg.UserID, arg.TrackID) - return err +// Unlikes the song: every copy sharing the track's song_key. Returns the +// tracks unliked, for the sync log. +func (q *Queries) UnlikeTrack(ctx context.Context, arg UnlikeTrackParams) ([]pgtype.UUID, error) { + rows, err := q.db.Query(ctx, unlikeTrack, arg.UserID, arg.TrackID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []pgtype.UUID + for rows.Next() { + var track_id pgtype.UUID + if err := rows.Scan(&track_id); err != nil { + return nil, err + } + items = append(items, track_id) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil } diff --git a/internal/db/dbq/models.go b/internal/db/dbq/models.go index 03a4aa19..556c54c9 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 { @@ -749,6 +754,10 @@ type Track struct { TagReadVersion int16 MissingSince pgtype.Timestamptz MbidSource *string + SongID pgtype.UUID + SongKey pgtype.UUID + SourceVerdict *string + SourceVerdictAt pgtype.Timestamptz } type TrackAcoustidLookup struct { diff --git a/internal/db/dbq/recommendation.sql.go b/internal/db/dbq/recommendation.sql.go index bab29ed0..442fafee 100644 --- a/internal/db/dbq/recommendation.sql.go +++ b/internal/db/dbq/recommendation.sql.go @@ -208,7 +208,7 @@ WITH plays AS ( WHERE user_id = $2 AND was_skipped = false GROUP BY track_id ) -SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, +SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key, t.source_verdict, t.source_verdict_at, albums.title AS album_title, artists.name AS artist_name FROM plays p @@ -271,6 +271,10 @@ func (q *Queries) ListMostPlayedTracksForArtist(ctx context.Context, arg ListMos &i.Track.TagReadVersion, &i.Track.MissingSince, &i.Track.MbidSource, + &i.Track.SongID, + &i.Track.SongKey, + &i.Track.SourceVerdict, + &i.Track.SourceVerdictAt, &i.AlbumTitle, &i.ArtistName, ); err != nil { @@ -291,7 +295,7 @@ WITH plays AS ( WHERE user_id = $1 AND was_skipped = false GROUP BY track_id ) -SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, +SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key, t.source_verdict, t.source_verdict_at, albums.title AS album_title, artists.name AS artist_name FROM plays p @@ -356,6 +360,10 @@ func (q *Queries) ListMostPlayedTracksForUser(ctx context.Context, arg ListMostP &i.Track.TagReadVersion, &i.Track.MissingSince, &i.Track.MbidSource, + &i.Track.SongID, + &i.Track.SongKey, + &i.Track.SourceVerdict, + &i.Track.SourceVerdictAt, &i.AlbumTitle, &i.ArtistName, ); err != nil { @@ -695,7 +703,7 @@ func (q *Queries) ListRediscoverArtistsForUser(ctx context.Context, arg ListRedi const loadRadioCandidates = `-- name: LoadRadioCandidates :many SELECT - t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, + t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key, t.source_verdict, t.source_verdict_at, (l.user_id IS NOT NULL)::bool AS is_liked, pe.last_played_at::timestamptz AS last_played_at, pe.play_count, @@ -777,6 +785,10 @@ func (q *Queries) LoadRadioCandidates(ctx context.Context, arg LoadRadioCandidat &i.Track.TagReadVersion, &i.Track.MissingSince, &i.Track.MbidSource, + &i.Track.SongID, + &i.Track.SongKey, + &i.Track.SourceVerdict, + &i.Track.SourceVerdictAt, &i.IsLiked, &i.LastPlayedAt, &i.PlayCount, @@ -925,7 +937,7 @@ random_fill AS ( LIMIT $9 ) SELECT - t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, + t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key, t.source_verdict, t.source_verdict_at, (l.user_id IS NOT NULL)::bool AS is_liked, pe.last_played_at::timestamptz AS last_played_at, pe.play_count, @@ -1053,6 +1065,10 @@ func (q *Queries) LoadRadioCandidatesV2(ctx context.Context, arg LoadRadioCandid &i.Track.TagReadVersion, &i.Track.MissingSince, &i.Track.MbidSource, + &i.Track.SongID, + &i.Track.SongKey, + &i.Track.SourceVerdict, + &i.Track.SourceVerdictAt, &i.IsLiked, &i.LastPlayedAt, &i.PlayCount, diff --git a/internal/db/dbq/songs.sql.go b/internal/db/dbq/songs.sql.go new file mode 100644 index 00000000..0820427d --- /dev/null +++ b/internal/db/dbq/songs.sql.go @@ -0,0 +1,154 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 +// source: songs.sql + +package dbq + +import ( + "context" + + "github.com/jackc/pgx/v5/pgtype" +) + +const linkTracksAsSong = `-- name: LinkTracksAsSong :one + +WITH keys AS ( + SELECT DISTINCT song_key FROM tracks WHERE id = ANY($1::uuid[]) +), target AS ( + SELECT song_key FROM keys ORDER BY song_key LIMIT 1 +), moved AS ( + UPDATE tracks t + SET song_id = (SELECT song_key FROM target) + WHERE t.song_key IN (SELECT song_key FROM keys) + AND t.song_key <> (SELECT song_key FROM target) + RETURNING t.id +) +SELECT (SELECT song_key FROM target)::uuid AS song_key, + (SELECT count(*) FROM moved)::bigint AS moved +` + +type LinkTracksAsSongRow struct { + SongKey pgtype.UUID + Moved int64 +} + +// Song links (M498 #5438): copies of one song on different releases share a +// song_key (migration 0076). +// Links the tracks into one song, joining any songs they already belong to. +// The song keeps the lowest key among them, so linking the same set again +// changes nothing. Returns that key and how many tracks moved to it. +func (q *Queries) LinkTracksAsSong(ctx context.Context, trackIds []pgtype.UUID) (LinkTracksAsSongRow, error) { + row := q.db.QueryRow(ctx, linkTracksAsSong, trackIds) + var i LinkTracksAsSongRow + err := row.Scan(&i.SongKey, &i.Moved) + return i, err +} + +const listTrackSongKeys = `-- name: ListTrackSongKeys :many +SELECT id, song_key, (source_verdict IS NOT DISTINCT FROM 'suspect')::boolean AS held_back + FROM tracks WHERE id = ANY($1::uuid[]) +` + +type ListTrackSongKeysRow struct { + ID pgtype.UUID + SongKey pgtype.UUID + HeldBack bool +} + +// What the mix writer needs to place a track: its song, and whether it is a +// video rip held back from the mixes (#5439). +func (q *Queries) ListTrackSongKeys(ctx context.Context, ids []pgtype.UUID) ([]ListTrackSongKeysRow, error) { + rows, err := q.db.Query(ctx, listTrackSongKeys, ids) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListTrackSongKeysRow + for rows.Next() { + var i ListTrackSongKeysRow + if err := rows.Scan(&i.ID, &i.SongKey, &i.HeldBack); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const mergeInheritSongLink = `-- name: MergeInheritSongLink :exec +UPDATE tracks s + SET song_id = l.song_key + FROM tracks l + WHERE s.id = $1::uuid + AND l.id = $2::uuid + AND s.song_id IS NULL + AND l.song_key <> l.id +` + +type MergeInheritSongLinkParams struct { + SurvivorID pgtype.UUID + LoserID pgtype.UUID +} + +// A merge keeps the removed copy's song link: when the copy being removed was +// linked to copies on other releases and the survivor was not, the survivor +// joins that song. +func (q *Queries) MergeInheritSongLink(ctx context.Context, arg MergeInheritSongLinkParams) error { + _, err := q.db.Exec(ctx, mergeInheritSongLink, arg.SurvivorID, arg.LoserID) + return err +} + +const shareSongLikes = `-- name: ShareSongLikes :many +INSERT INTO general_likes (user_id, track_id, liked_at) +SELECT l.user_id, t.id, min(l.liked_at) + FROM general_likes l + JOIN tracks lt ON lt.id = l.track_id + JOIN tracks t ON t.song_key = lt.song_key + WHERE lt.song_key = $1::uuid + GROUP BY l.user_id, t.id +ON CONFLICT (user_id, track_id) DO NOTHING +RETURNING user_id, track_id +` + +type ShareSongLikesRow struct { + UserID pgtype.UUID + TrackID pgtype.UUID +} + +// A like on any copy of the song becomes a like on every copy, dated the +// earliest. Returns the likes added, for the sync log. +func (q *Queries) ShareSongLikes(ctx context.Context, songKey pgtype.UUID) ([]ShareSongLikesRow, error) { + rows, err := q.db.Query(ctx, shareSongLikes, songKey) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ShareSongLikesRow + for rows.Next() { + var i ShareSongLikesRow + if err := rows.Scan(&i.UserID, &i.TrackID); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const unlinkDuplicateGroupSongs = `-- name: UnlinkDuplicateGroupSongs :exec +UPDATE tracks SET song_id = NULL + WHERE song_id IS NOT NULL + AND id IN (SELECT track_id FROM duplicate_group_members WHERE group_id = $1) +` + +// "Not the same song": the group's copies stop sharing a song. Likes already +// shared stay where they are. +func (q *Queries) UnlinkDuplicateGroupSongs(ctx context.Context, groupID pgtype.UUID) error { + _, err := q.db.Exec(ctx, unlinkDuplicateGroupSongs, groupID) + return err +} diff --git a/internal/db/dbq/tracks.sql.go b/internal/db/dbq/tracks.sql.go index bbdf7f65..6a9de912 100644 --- a/internal/db/dbq/tracks.sql.go +++ b/internal/db/dbq/tracks.sql.go @@ -39,6 +39,23 @@ func (q *Queries) AdoptTrackPath(ctx context.Context, arg AdoptTrackPathParams) return result.RowsAffected(), nil } +const clearStaleSuspectSourceTracks = `-- name: ClearStaleSuspectSourceTracks :execrows +UPDATE tracks + SET source_verdict = NULL, source_verdict_at = NULL + WHERE source_verdict = 'suspect' + AND NOT regexp_replace(file_path, '^.*/', '') ~* $1::text +` + +// A held-back track whose name no longer carries a marker (renamed, retagged +// by Lidarr) is released again. +func (q *Queries) ClearStaleSuspectSourceTracks(ctx context.Context, pattern string) (int64, error) { + result, err := q.db.Exec(ctx, clearStaleSuspectSourceTracks, pattern) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + const clearTracksMissing = `-- name: ClearTracksMissing :execrows UPDATE tracks SET missing_since = NULL @@ -246,8 +263,27 @@ func (q *Queries) FindMissingTrackByMbid(ctx context.Context, mbid string) ([]Fi return items, nil } +const flagSuspectSourceTracks = `-- name: FlagSuspectSourceTracks :execrows +UPDATE tracks + SET source_verdict = 'suspect', source_verdict_at = now() + WHERE missing_since IS NULL + AND source_verdict IS NULL + AND regexp_replace(file_path, '^.*/', '') ~* $1::text +` + +// Holds back present tracks whose basename carries a video-rip marker (M498 +// #5439), the same pattern the report lists. A track the operator called fine +// keeps that verdict. +func (q *Queries) FlagSuspectSourceTracks(ctx context.Context, pattern string) (int64, error) { + result, err := q.db.Exec(ctx, flagSuspectSourceTracks, pattern) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + const getTrackByID = `-- name: GetTrackByID :one -SELECT id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at, tag_source, tag_sources_version, tag_read_version, missing_since, mbid_source FROM tracks WHERE id = $1 +SELECT id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at, tag_source, tag_sources_version, tag_read_version, missing_since, mbid_source, song_id, song_key, source_verdict, source_verdict_at FROM tracks WHERE id = $1 ` func (q *Queries) GetTrackByID(ctx context.Context, id pgtype.UUID) (Track, error) { @@ -274,12 +310,16 @@ func (q *Queries) GetTrackByID(ctx context.Context, id pgtype.UUID) (Track, erro &i.TagReadVersion, &i.MissingSince, &i.MbidSource, + &i.SongID, + &i.SongKey, + &i.SourceVerdict, + &i.SourceVerdictAt, ) return i, err } const getTrackByPath = `-- name: GetTrackByPath :one -SELECT id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at, tag_source, tag_sources_version, tag_read_version, missing_since, mbid_source FROM tracks WHERE file_path = $1 +SELECT id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at, tag_source, tag_sources_version, tag_read_version, missing_since, mbid_source, song_id, song_key, source_verdict, source_verdict_at FROM tracks WHERE file_path = $1 ` func (q *Queries) GetTrackByPath(ctx context.Context, filePath string) (Track, error) { @@ -306,12 +346,16 @@ func (q *Queries) GetTrackByPath(ctx context.Context, filePath string) (Track, e &i.TagReadVersion, &i.MissingSince, &i.MbidSource, + &i.SongID, + &i.SongKey, + &i.SourceVerdict, + &i.SourceVerdictAt, ) return i, err } const getTracksByIDs = `-- name: GetTracksByIDs :many -SELECT id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at, tag_source, tag_sources_version, tag_read_version, missing_since, mbid_source FROM tracks WHERE id = ANY($1::uuid[]) +SELECT id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at, tag_source, tag_sources_version, tag_read_version, missing_since, mbid_source, song_id, song_key, source_verdict, source_verdict_at FROM tracks WHERE id = ANY($1::uuid[]) ` // Batched lookup used by /api/library/sync to hydrate upsert payloads @@ -346,6 +390,10 @@ func (q *Queries) GetTracksByIDs(ctx context.Context, dollar_1 []pgtype.UUID) ([ &i.TagReadVersion, &i.MissingSince, &i.MbidSource, + &i.SongID, + &i.SongKey, + &i.SourceVerdict, + &i.SourceVerdictAt, ); err != nil { return nil, err } @@ -358,7 +406,7 @@ func (q *Queries) GetTracksByIDs(ctx context.Context, dollar_1 []pgtype.UUID) ([ } const listArtistTracksForUser = `-- name: ListArtistTracksForUser :many -SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, +SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key, t.source_verdict, t.source_verdict_at, albums.title AS album_title, artists.name AS artist_name FROM tracks t @@ -419,6 +467,10 @@ func (q *Queries) ListArtistTracksForUser(ctx context.Context, arg ListArtistTra &i.Track.TagReadVersion, &i.Track.MissingSince, &i.Track.MbidSource, + &i.Track.SongID, + &i.Track.SongKey, + &i.Track.SourceVerdict, + &i.Track.SourceVerdictAt, &i.AlbumTitle, &i.ArtistName, ); err != nil { @@ -519,7 +571,7 @@ func (q *Queries) ListMissingTracks(ctx context.Context, arg ListMissingTracksPa } const listRandomTracksForUser = `-- name: ListRandomTracksForUser :many -SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, +SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key, t.source_verdict, t.source_verdict_at, albums.title AS album_title, artists.name AS artist_name FROM tracks t @@ -577,6 +629,10 @@ func (q *Queries) ListRandomTracksForUser(ctx context.Context, arg ListRandomTra &i.Track.TagReadVersion, &i.Track.MissingSince, &i.Track.MbidSource, + &i.Track.SongID, + &i.Track.SongKey, + &i.Track.SourceVerdict, + &i.Track.SourceVerdictAt, &i.AlbumTitle, &i.ArtistName, ); err != nil { @@ -601,7 +657,9 @@ SELECT t.id, albums.id AS album_id, albums.title AS album_title, artists.id AS artist_id, - artists.name AS artist_name + artists.name AS artist_name, + t.source_verdict, + t.source_verdict_at FROM tracks t JOIN albums ON albums.id = t.album_id JOIN artists ON artists.id = t.artist_id @@ -618,17 +676,19 @@ type ListSuspectSourceTracksParams struct { } type ListSuspectSourceTracksRow struct { - ID pgtype.UUID - Title string - FilePath string - Directory string - DurationMs int32 - DiscNumber *int32 - TrackNumber *int32 - AlbumID pgtype.UUID - AlbumTitle string - ArtistID pgtype.UUID - ArtistName string + ID pgtype.UUID + Title string + FilePath string + Directory string + DurationMs int32 + DiscNumber *int32 + TrackNumber *int32 + AlbumID pgtype.UUID + AlbumTitle string + ArtistID pgtype.UUID + ArtistName string + SourceVerdict *string + SourceVerdictAt pgtype.Timestamptz } // The admin report of present tracks whose filename looks like a video rip @@ -662,6 +722,8 @@ func (q *Queries) ListSuspectSourceTracks(ctx context.Context, arg ListSuspectSo &i.AlbumTitle, &i.ArtistID, &i.ArtistName, + &i.SourceVerdict, + &i.SourceVerdictAt, ); err != nil { return nil, err } @@ -709,7 +771,7 @@ func (q *Queries) ListTrackPathsForReconcile(ctx context.Context) ([]ListTrackPa } const listTracksByAlbum = `-- name: ListTracksByAlbum :many -SELECT id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at, tag_source, tag_sources_version, tag_read_version, missing_since, mbid_source FROM tracks +SELECT id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at, tag_source, tag_sources_version, tag_read_version, missing_since, mbid_source, song_id, song_key, source_verdict, source_verdict_at FROM tracks WHERE album_id = $1 AND NOT EXISTS ( SELECT 1 FROM lidarr_quarantine q @@ -756,6 +818,10 @@ func (q *Queries) ListTracksByAlbum(ctx context.Context, arg ListTracksByAlbumPa &i.TagReadVersion, &i.MissingSince, &i.MbidSource, + &i.SongID, + &i.SongKey, + &i.SourceVerdict, + &i.SourceVerdictAt, ); err != nil { return nil, err } @@ -829,7 +895,7 @@ func (q *Queries) MarkTracksMissing(ctx context.Context, ids []pgtype.UUID) (int } const searchTracks = `-- name: SearchTracks :many -SELECT id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at, tag_source, tag_sources_version, tag_read_version, missing_since, mbid_source FROM tracks +SELECT id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at, tag_source, tag_sources_version, tag_read_version, missing_since, mbid_source, song_id, song_key, source_verdict, source_verdict_at FROM tracks WHERE title ILIKE '%' || $1::text || '%' AND NOT EXISTS ( SELECT 1 FROM lidarr_quarantine q @@ -883,6 +949,10 @@ func (q *Queries) SearchTracks(ctx context.Context, arg SearchTracksParams) ([]T &i.TagReadVersion, &i.MissingSince, &i.MbidSource, + &i.SongID, + &i.SongKey, + &i.SourceVerdict, + &i.SourceVerdictAt, ); err != nil { return nil, err } @@ -912,6 +982,28 @@ func (q *Queries) SetTrackMbidIfNull(ctx context.Context, arg SetTrackMbidIfNull return err } +const setTrackSourceVerdict = `-- name: SetTrackSourceVerdict :execrows +UPDATE tracks + SET source_verdict = $1::text, source_verdict_at = now() + WHERE id = $2 + AND source_verdict IS NOT NULL +` + +type SetTrackSourceVerdictParams struct { + Verdict string + ID pgtype.UUID +} + +// The operator's call on a flagged track: 'fine' lets it back into radio and +// the mixes for good; 'suspect' holds it back again. +func (q *Queries) SetTrackSourceVerdict(ctx context.Context, arg SetTrackSourceVerdictParams) (int64, error) { + result, err := q.db.Exec(ctx, setTrackSourceVerdict, arg.Verdict, arg.ID) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + const upsertTrack = `-- name: UpsertTrack :one INSERT INTO tracks ( title, album_id, artist_id, track_number, disc_number, @@ -939,7 +1031,7 @@ ON CONFLICT (file_path) DO UPDATE SET -- next scan can short-circuit them again (#2499). tag_read_version = EXCLUDED.tag_read_version, updated_at = now() -RETURNING id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at, tag_source, tag_sources_version, tag_read_version, missing_since, mbid_source +RETURNING id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at, tag_source, tag_sources_version, tag_read_version, missing_since, mbid_source, song_id, song_key, source_verdict, source_verdict_at ` type UpsertTrackParams struct { @@ -999,6 +1091,10 @@ func (q *Queries) UpsertTrack(ctx context.Context, arg UpsertTrackParams) (Track &i.TagReadVersion, &i.MissingSince, &i.MbidSource, + &i.SongID, + &i.SongKey, + &i.SourceVerdict, + &i.SourceVerdictAt, ) return i, err } 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/migrations/0076_track_song_link.down.sql b/internal/db/migrations/0076_track_song_link.down.sql new file mode 100644 index 00000000..f81c06f2 --- /dev/null +++ b/internal/db/migrations/0076_track_song_link.down.sql @@ -0,0 +1,3 @@ +DROP INDEX IF EXISTS tracks_song_key_idx; +ALTER TABLE tracks DROP COLUMN IF EXISTS song_key; +ALTER TABLE tracks DROP COLUMN IF EXISTS song_id; diff --git a/internal/db/migrations/0076_track_song_link.up.sql b/internal/db/migrations/0076_track_song_link.up.sql new file mode 100644 index 00000000..b314512c --- /dev/null +++ b/internal/db/migrations/0076_track_song_link.up.sql @@ -0,0 +1,15 @@ +-- 0076_track_song_link.up.sql — the same song on several releases counts as +-- one song (Scribe milestone #498, #5438; operator decision D-b). +-- +-- A single and the album it came from each hold the song, and Lidarr keeps +-- both: each fulfils its own release. Minstrel keeps both files and links +-- them, so a like on one is a like on the song, and a mix or radio picks the +-- song once rather than once per release. +-- +-- song_id is set only on a linked track, to the song_key the link chose. +-- song_key is what everything reads: the track's own id until it is linked. +-- No foreign key: the track whose id names a song may itself be merged away +-- later, and the key still groups the copies left. +ALTER TABLE tracks ADD COLUMN song_id uuid; +ALTER TABLE tracks ADD COLUMN song_key uuid GENERATED ALWAYS AS (COALESCE(song_id, id)) STORED; +CREATE INDEX tracks_song_key_idx ON tracks (song_key); diff --git a/internal/db/migrations/0077_track_source_verdict.down.sql b/internal/db/migrations/0077_track_source_verdict.down.sql new file mode 100644 index 00000000..17b626c6 --- /dev/null +++ b/internal/db/migrations/0077_track_source_verdict.down.sql @@ -0,0 +1,3 @@ +DROP INDEX IF EXISTS tracks_source_suspect_idx; +ALTER TABLE tracks DROP COLUMN IF EXISTS source_verdict_at; +ALTER TABLE tracks DROP COLUMN IF EXISTS source_verdict; diff --git a/internal/db/migrations/0077_track_source_verdict.up.sql b/internal/db/migrations/0077_track_source_verdict.up.sql new file mode 100644 index 00000000..c1361d5f --- /dev/null +++ b/internal/db/migrations/0077_track_source_verdict.up.sql @@ -0,0 +1,15 @@ +-- 0077_track_source_verdict.up.sql — video rips are held back from mixes and +-- radio until the operator says otherwise (Scribe milestone #498, #5439; +-- operator decision D-d). +-- +-- A file whose name carries a video-rip marker ("(Official Video)", "[HD]", +-- #5410) holds the audio of an upload, which can carry intros, skits or a +-- different take. The duplicate resolver sets 'suspect' on such a present +-- track each pass, and clears it when the name no longer matches. 'suspect' +-- keeps the track out of radio and the system mixes; it still plays when +-- chosen. 'fine' is the operator's "this one is fine" and is never +-- overwritten by the resolver. +ALTER TABLE tracks ADD COLUMN source_verdict text + CONSTRAINT tracks_source_verdict_check CHECK (source_verdict IN ('suspect', 'fine')); +ALTER TABLE tracks ADD COLUMN source_verdict_at timestamptz; +CREATE INDEX tracks_source_suspect_idx ON tracks (id) WHERE source_verdict = 'suspect'; 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/db/queries/likes.sql b/internal/db/queries/likes.sql index 5a8b76f5..350b7f59 100644 --- a/internal/db/queries/likes.sql +++ b/internal/db/queries/likes.sql @@ -1,20 +1,43 @@ --- name: LikeTrack :execrows +-- name: LikeTrack :many +-- A like is on the song (M498 #5438): every copy of it sharing the track's +-- song_key is liked. Returns the tracks newly liked, for the sync log; empty +-- when the song was already liked. INSERT INTO general_likes (user_id, track_id) -VALUES ($1, $2) -ON CONFLICT (user_id, track_id) DO NOTHING; +SELECT sqlc.arg(user_id)::uuid, t.id + FROM tracks t + WHERE t.song_key = (SELECT s.song_key FROM tracks s WHERE s.id = sqlc.arg(track_id)::uuid) +ON CONFLICT (user_id, track_id) DO NOTHING +RETURNING track_id; --- name: UnlikeTrack :exec -DELETE FROM general_likes WHERE user_id = $1 AND track_id = $2; +-- name: UnlikeTrack :many +-- Unlikes the song: every copy sharing the track's song_key. Returns the +-- tracks unliked, for the sync log. +DELETE FROM general_likes + WHERE user_id = sqlc.arg(user_id)::uuid + AND (track_id = sqlc.arg(track_id)::uuid + OR track_id IN (SELECT t.id FROM tracks t + WHERE t.song_key = (SELECT s.song_key FROM tracks s WHERE s.id = sqlc.arg(track_id)::uuid))) +RETURNING track_id; -- name: ListLikedTrackRows :many +-- One row per liked song: of its copies, the one present on disk, then one +-- that is not a held-back video rip, then the one liked first. SELECT t.* FROM tracks t -JOIN general_likes l ON l.track_id = t.id -WHERE l.user_id = $1 -ORDER BY l.liked_at DESC +JOIN (SELECT DISTINCT ON (lt.song_key) l.track_id, l.liked_at + FROM general_likes l + JOIN tracks lt ON lt.id = l.track_id + WHERE l.user_id = $1 + ORDER BY lt.song_key, (lt.missing_since IS NOT NULL), + (lt.source_verdict IS NOT DISTINCT FROM 'suspect'), l.liked_at, lt.id) l + ON l.track_id = t.id +ORDER BY l.liked_at DESC, t.id LIMIT $2 OFFSET $3; -- name: CountLikedTracks :one -SELECT count(*) FROM general_likes WHERE user_id = $1; +-- Liked songs, each counted once however many releases hold it. +SELECT count(DISTINCT t.song_key) FROM general_likes l + JOIN tracks t ON t.id = l.track_id + WHERE l.user_id = $1; -- name: ListLikedTrackIDs :many SELECT track_id FROM general_likes WHERE user_id = $1 ORDER BY liked_at DESC; diff --git a/internal/db/queries/songs.sql b/internal/db/queries/songs.sql new file mode 100644 index 00000000..bc3e200a --- /dev/null +++ b/internal/db/queries/songs.sql @@ -0,0 +1,58 @@ +-- Song links (M498 #5438): copies of one song on different releases share a +-- song_key (migration 0076). + +-- name: LinkTracksAsSong :one +-- Links the tracks into one song, joining any songs they already belong to. +-- The song keeps the lowest key among them, so linking the same set again +-- changes nothing. Returns that key and how many tracks moved to it. +WITH keys AS ( + SELECT DISTINCT song_key FROM tracks WHERE id = ANY(sqlc.arg(track_ids)::uuid[]) +), target AS ( + SELECT song_key FROM keys ORDER BY song_key LIMIT 1 +), moved AS ( + UPDATE tracks t + SET song_id = (SELECT song_key FROM target) + WHERE t.song_key IN (SELECT song_key FROM keys) + AND t.song_key <> (SELECT song_key FROM target) + RETURNING t.id +) +SELECT (SELECT song_key FROM target)::uuid AS song_key, + (SELECT count(*) FROM moved)::bigint AS moved; + +-- name: ShareSongLikes :many +-- A like on any copy of the song becomes a like on every copy, dated the +-- earliest. Returns the likes added, for the sync log. +INSERT INTO general_likes (user_id, track_id, liked_at) +SELECT l.user_id, t.id, min(l.liked_at) + FROM general_likes l + JOIN tracks lt ON lt.id = l.track_id + JOIN tracks t ON t.song_key = lt.song_key + WHERE lt.song_key = sqlc.arg(song_key)::uuid + GROUP BY l.user_id, t.id +ON CONFLICT (user_id, track_id) DO NOTHING +RETURNING user_id, track_id; + +-- name: UnlinkDuplicateGroupSongs :exec +-- "Not the same song": the group's copies stop sharing a song. Likes already +-- shared stay where they are. +UPDATE tracks SET song_id = NULL + WHERE song_id IS NOT NULL + AND id IN (SELECT track_id FROM duplicate_group_members WHERE group_id = sqlc.arg(group_id)); + +-- name: ListTrackSongKeys :many +-- What the mix writer needs to place a track: its song, and whether it is a +-- video rip held back from the mixes (#5439). +SELECT id, song_key, (source_verdict IS NOT DISTINCT FROM 'suspect')::boolean AS held_back + FROM tracks WHERE id = ANY(sqlc.arg(ids)::uuid[]); + +-- name: MergeInheritSongLink :exec +-- A merge keeps the removed copy's song link: when the copy being removed was +-- linked to copies on other releases and the survivor was not, the survivor +-- joins that song. +UPDATE tracks s + SET song_id = l.song_key + FROM tracks l + WHERE s.id = sqlc.arg(survivor_id)::uuid + AND l.id = sqlc.arg(loser_id)::uuid + AND s.song_id IS NULL + AND l.song_key <> l.id; diff --git a/internal/db/queries/tracks.sql b/internal/db/queries/tracks.sql index 23e1b603..b8e7d9d3 100644 --- a/internal/db/queries/tracks.sql +++ b/internal/db/queries/tracks.sql @@ -285,7 +285,9 @@ SELECT t.id, albums.id AS album_id, albums.title AS album_title, artists.id AS artist_id, - artists.name AS artist_name + artists.name AS artist_name, + t.source_verdict, + t.source_verdict_at FROM tracks t JOIN albums ON albums.id = t.album_id JOIN artists ON artists.id = t.artist_id @@ -299,3 +301,29 @@ SELECT t.id, SELECT COUNT(*) FROM tracks t WHERE t.missing_since IS NULL AND regexp_replace(t.file_path, '^.*/', '') ~* sqlc.arg(pattern)::text; + +-- name: FlagSuspectSourceTracks :execrows +-- Holds back present tracks whose basename carries a video-rip marker (M498 +-- #5439), the same pattern the report lists. A track the operator called fine +-- keeps that verdict. +UPDATE tracks + SET source_verdict = 'suspect', source_verdict_at = now() + WHERE missing_since IS NULL + AND source_verdict IS NULL + AND regexp_replace(file_path, '^.*/', '') ~* sqlc.arg(pattern)::text; + +-- name: ClearStaleSuspectSourceTracks :execrows +-- A held-back track whose name no longer carries a marker (renamed, retagged +-- by Lidarr) is released again. +UPDATE tracks + SET source_verdict = NULL, source_verdict_at = NULL + WHERE source_verdict = 'suspect' + AND NOT regexp_replace(file_path, '^.*/', '') ~* sqlc.arg(pattern)::text; + +-- name: SetTrackSourceVerdict :execrows +-- The operator's call on a flagged track: 'fine' lets it back into radio and +-- the mixes for good; 'suspect' holds it back again. +UPDATE tracks + SET source_verdict = sqlc.arg(verdict)::text, source_verdict_at = now() + WHERE id = sqlc.arg(id) + AND source_verdict IS NOT NULL; 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..aa5e411b 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 @@ -241,6 +269,9 @@ func foldTrackInto( if err := tq.MergeInheritTrackMbid(ctx, dbq.MergeInheritTrackMbidParams{SurvivorID: survivorID, LoserID: loserID}); err != nil { return f, fmt.Errorf("inherit recording mbid: %w", err) } + if err := tq.MergeInheritSongLink(ctx, dbq.MergeInheritSongLinkParams{SurvivorID: survivorID, LoserID: loserID}); err != nil { + return f, fmt.Errorf("inherit song link: %w", err) + } // Everything the loser carried now sits on the survivor, so the CASCADE // this delete sets off has nothing left to destroy. diff --git a/internal/library/duplicate_resolve.go b/internal/library/duplicate_resolve.go new file mode 100644 index 00000000..5dd4b9ee --- /dev/null +++ b/internal/library/duplicate_resolve.go @@ -0,0 +1,791 @@ +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 + // SongsLinked counts cross-release groups newly linked as one song. + SongsLinked int + // HeldBack counts tracks newly held back from radio and the mixes as video + // rips; Released counts held-back tracks whose name no longer says so. + HeldBack, Released int64 +} + +// 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 +} + +// autoMergeable says whether the resolver may merge the group on its own: when +// every copy it would remove is one Lidarr does not map (D-a rule 1). On one +// album that is any group with at most one mapped copy. Across releases each +// mapped copy fulfils its own release (D-b), so only a group with exactly one +// mapped copy qualifies: the others fulfil nothing — a rip beside the clean +// copy on another release, or a wrong-file import Lidarr holds unmapped +// (#5439) — and the mapped copy is the clear one to keep. +func autoMergeable(g *resolveGroup) bool { + switch g.class { + case ClassSameRelease: + return g.tracked() <= 1 + case ClassCrossRelease, ClassMismatch: + return g.tracked() == 1 + default: + return false + } +} + +// settling says whether any of the group's albums had its Lidarr release +// changed recently, so Lidarr may still be remapping its files. +func (g *resolveGroup) settling(albums map[string]bool) bool { + for _, m := range g.members { + if albums[syncpkg.FormatUUID(m.AlbumID)] { + return true + } + } + return false +} + +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 { + return res, nil + } + + // Holding rips back and linking songs remove nothing and need nothing from + // Lidarr. + if res.HeldBack, err = q.FlagSuspectSourceTracks(ctx, SuspectSourcePattern); err != nil { + return res, fmt.Errorf("hold back video rips: %w", err) + } + if res.Released, err = q.ClearStaleSuspectSourceTracks(ctx, SuspectSourcePattern); err != nil { + return res, fmt.Errorf("release renamed tracks: %w", err) + } + linked, err := linkCrossRelease(ctx, pool, logger, groups) + res.SongsLinked = linked + if err != nil { + return res, err + } + + if !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 !autoMergeable(g) || g.settling(settling) { + 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 +} + +// linkCrossRelease links each cross-release group's copies as one song +// (#5438): a like on one is a like on all, and the Liked list, shuffle and the +// mixes count the song once. Linking an already-linked group changes nothing, +// so this runs every pass; likes are shared only when a link is new, since a +// like made after it reaches every copy as it is made. +func linkCrossRelease(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, groups []*resolveGroup) (int, error) { + linked := 0 + for _, g := range groups { + if g.class != ClassCrossRelease { + continue + } + if ctx.Err() != nil { + return linked, ctx.Err() + } + ids := make([]pgtype.UUID, len(g.members)) + for i, m := range g.members { + ids[i] = m.TrackID + } + moved, err := linkSong(ctx, pool, ids) + if err != nil { + logger.Warn("duplicate resolve: song link failed", "group_id", syncpkg.FormatUUID(g.id), "err", err) + continue + } + if moved { + linked++ + } + } + return linked, nil +} + +// linkSong links the tracks as one song and, when that changed anything, +// shares their likes across the song and logs each added like for sync. +func linkSong(ctx context.Context, pool *pgxpool.Pool, ids []pgtype.UUID) (bool, error) { + tx, err := pool.Begin(ctx) + if err != nil { + return false, err + } + defer func() { _ = tx.Rollback(ctx) }() + tq := dbq.New(tx) + link, err := tq.LinkTracksAsSong(ctx, ids) + if err != nil { + return false, fmt.Errorf("link: %w", err) + } + if link.Moved == 0 { + return false, nil + } + added, err := tq.ShareSongLikes(ctx, link.SongKey) + if err != nil { + return false, fmt.Errorf("share likes: %w", err) + } + likeIDs := make([]string, len(added)) + for i, a := range added { + likeIDs[i] = syncpkg.EncodeLikeID(syncpkg.FormatUUID(a.UserID), syncpkg.FormatUUID(a.TrackID)) + } + if err := syncpkg.LogChanges(ctx, tx, syncpkg.EntityLikeTrack, likeIDs, syncpkg.OpUpsert); err != nil { + return false, fmt.Errorf("log shared likes: %w", err) + } + return true, tx.Commit(ctx) +} + +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; they count as one song." + 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), "songs_linked", res.SongsLinked, + "held_back", res.HeldBack, "released", res.Released) +} diff --git a/internal/library/duplicate_resolve_test.go b/internal/library/duplicate_resolve_test.go new file mode 100644 index 00000000..d1b67a83 --- /dev/null +++ b/internal/library/duplicate_resolve_test.go @@ -0,0 +1,504 @@ +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) + } +} + +// crossReleaseRipFixture is a video rip on one release with the clean copy on +// another: the album cut, and the same song's video filed as a single. +type crossReleaseRipFixture struct { + pool *pgxpool.Pool + clean, rip dbq.Track + cleanPath, ripPath string + groupID pgtype.UUID +} + +func newCrossReleaseRipFixture(t *testing.T) crossReleaseRipFixture { + t.Helper() + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + root := t.TempDir() + f := crossReleaseRipFixture{pool: pool} + f.cleanPath = filepath.Join(root, "Gorillaz", "Demon Days (2005)", "06 - Feel Good Inc.mp3") + f.ripPath = filepath.Join(root, "Gorillaz", "Feel Good Inc (2005)", "01 - Feel Good Inc. (Official Video).mp3") + artist, err := q.UpsertArtist(ctx, dbq.UpsertArtistParams{Name: "Gorillaz", SortName: "Gorillaz"}) + if err != nil { + t.Fatal(err) + } + track := func(albumTitle, title, path string) dbq.Track { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte("audio"), 0o644); err != nil { + t.Fatal(err) + } + album, err := q.UpsertAlbum(ctx, dbq.UpsertAlbumParams{Title: albumTitle, SortTitle: albumTitle, ArtistID: artist.ID}) + if err != nil { + t.Fatal(err) + } + tr, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{ + Title: title, AlbumID: album.ID, ArtistID: artist.ID, + DurationMs: 1000, FilePath: path, FileSize: 100, FileFormat: "mp3", + }) + if err != nil { + t.Fatal(err) + } + return tr + } + f.clean = track("Demon Days", "Feel Good Inc.", f.cleanPath) + f.rip = track("Feel Good Inc.", "Feel Good Inc. (Official Video)", f.ripPath) + if err := pool.QueryRow(ctx, + `INSERT INTO duplicate_groups (member_key, tier) VALUES ('cross-rip', 'acoustic') RETURNING id`, + ).Scan(&f.groupID); err != nil { + t.Fatal(err) + } + if _, err := pool.Exec(ctx, + `INSERT INTO duplicate_group_members (group_id, track_id) VALUES ($1, $2), ($1, $3)`, + f.groupID, f.clean.ID, f.rip.ID); err != nil { + t.Fatal(err) + } + return f +} + +// The rip fulfils nothing in Lidarr while the clean copy on the album does: +// the rip goes and the album copy stays (#5439, D-a rule 1). +func TestResolveDuplicates_MergesAnUnmappedRipAcrossReleases_Integration(t *testing.T) { + f := newCrossReleaseRipFixture(t) + lid := &fakeLidarr{unmapped: []string{lidarrPath(f.ripPath)}} + res, err := ResolveDuplicates(context.Background(), f.pool, nil, "", lid, true) + if err != nil { + t.Fatal(err) + } + if res.Classes[ClassCrossRelease] != 1 || res.Merged != 1 { + t.Fatalf("classes %v, merged %d; want the cross-release group merged", res.Classes, res.Merged) + } + if exists(f.ripPath) || !exists(f.cleanPath) { + t.Errorf("rip exists %v, clean exists %v; want only the clean copy", exists(f.ripPath), exists(f.cleanPath)) + } +} + +// When Lidarr maps both copies, each fulfils its own release: nothing is +// removed, and the copies are linked as one song instead. +func TestResolveDuplicates_KeepsCopiesLidarrMapsOnTwoReleases_Integration(t *testing.T) { + f := newCrossReleaseRipFixture(t) + res, err := ResolveDuplicates(context.Background(), f.pool, nil, "", &fakeLidarr{}, true) + if err != nil { + t.Fatal(err) + } + if res.Merged != 0 || !exists(f.ripPath) { + t.Errorf("merged %d; want both copies kept", res.Merged) + } + if res.SongsLinked != 1 { + t.Errorf("linked %d, want the pair linked as one song", res.SongsLinked) + } +} + +// A rip is held back from radio and the mixes; the operator's "fine" sticks +// through later passes; a renamed file is released. +func TestResolveDuplicates_HoldsBackRips_Integration(t *testing.T) { + f := newCrossReleaseRipFixture(t) + ctx := context.Background() + q := dbq.New(f.pool) + verdict := func(id pgtype.UUID) *string { + t.Helper() + var v *string + if err := f.pool.QueryRow(ctx, `SELECT source_verdict FROM tracks WHERE id = $1`, id).Scan(&v); err != nil { + t.Fatal(err) + } + return v + } + resolve := func() DuplicateResolveResult { + t.Helper() + res, err := ResolveDuplicates(ctx, f.pool, nil, "", &fakeLidarr{}, true) + if err != nil { + t.Fatal(err) + } + return res + } + + if res := resolve(); res.HeldBack != 1 { + t.Errorf("held back %d, want the rip", res.HeldBack) + } + if v := verdict(f.rip.ID); v == nil || *v != "suspect" { + t.Errorf("rip verdict %v, want suspect", v) + } + if v := verdict(f.clean.ID); v != nil { + t.Errorf("clean copy verdict %v, want none", *v) + } + + if n, err := q.SetTrackSourceVerdict(ctx, dbq.SetTrackSourceVerdictParams{Verdict: "fine", ID: f.rip.ID}); err != nil || n != 1 { + t.Fatalf("set fine: %d, %v", n, err) + } + if res := resolve(); res.HeldBack != 0 { + t.Errorf("held back %d after the operator's fine, want 0", res.HeldBack) + } + if v := verdict(f.rip.ID); v == nil || *v != "fine" { + t.Errorf("rip verdict %v, want fine kept", v) + } + + // Held back again, then renamed: the next pass lets it go. + if _, err := q.SetTrackSourceVerdict(ctx, dbq.SetTrackSourceVerdictParams{Verdict: "suspect", ID: f.rip.ID}); err != nil { + t.Fatal(err) + } + if _, err := f.pool.Exec(ctx, `UPDATE tracks SET file_path = $2 WHERE id = $1`, + f.rip.ID, filepath.Join(filepath.Dir(f.ripPath), "01 - Feel Good Inc.mp3")); err != nil { + t.Fatal(err) + } + if res := resolve(); res.Released != 1 { + t.Errorf("released %d, want the renamed track", res.Released) + } + if v := verdict(f.rip.ID); v != nil { + t.Errorf("renamed track verdict %v, want none", *v) + } + + // The clean copy can't be flagged by hand: there is nothing to judge. + if n, err := q.SetTrackSourceVerdict(ctx, dbq.SetTrackSourceVerdictParams{Verdict: "fine", ID: f.clean.ID}); err != nil || n != 0 { + t.Errorf("set on an unflagged track: %d, %v; want 0 rows", n, err) + } +} 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/song_link_test.go b/internal/library/song_link_test.go new file mode 100644 index 00000000..97bf3aab --- /dev/null +++ b/internal/library/song_link_test.go @@ -0,0 +1,208 @@ +package library + +import ( + "context" + "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/dbtest" +) + +// songLinkFixture is the single and the album it came from: one song on two +// releases, in one pending acoustic group, and a user who liked the single. +type songLinkFixture struct { + single, albumCut dbq.Track + groupID pgtype.UUID + user dbq.User +} + +func newSongLinkFixture(t *testing.T, pool *pgxpool.Pool) songLinkFixture { + t.Helper() + ctx := context.Background() + q := dbq.New(pool) + exec := func(sql string, args ...any) { + t.Helper() + if _, err := pool.Exec(ctx, sql, args...); err != nil { + t.Fatalf("exec %q: %v", sql, err) + } + } + artist, err := q.UpsertArtist(ctx, dbq.UpsertArtistParams{Name: "Link Artist", SortName: "Link Artist"}) + if err != nil { + t.Fatal(err) + } + track := func(album, path string) dbq.Track { + t.Helper() + a, err := q.UpsertAlbum(ctx, dbq.UpsertAlbumParams{Title: album, SortTitle: album, ArtistID: artist.ID}) + if err != nil { + t.Fatal(err) + } + tr, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{ + Title: "Feel Good Inc.", AlbumID: a.ID, ArtistID: artist.ID, + DurationMs: 1000, FilePath: path, FileSize: 100, FileFormat: "mp3", + }) + if err != nil { + t.Fatal(err) + } + return tr + } + f := songLinkFixture{ + single: track("Feel Good Inc. (single)", "/music/Link Artist/Feel Good Inc/01 Feel Good Inc.mp3"), + albumCut: track("Demon Days", "/music/Link Artist/Demon Days/06 Feel Good Inc.mp3"), + } + f.user, err = q.CreateUser(ctx, dbq.CreateUserParams{ + Username: dbtest.TestUserPrefix + "song-link", PasswordHash: "x", ApiTokenHash: "song-link-token", + }) + if err != nil { + t.Fatal(err) + } + if err := pool.QueryRow(ctx, + `INSERT INTO duplicate_groups (member_key, tier) VALUES ('song-link', 'acoustic') 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.single.ID, f.albumCut.ID) + exec(`INSERT INTO general_likes (user_id, track_id) VALUES ($1, $2)`, f.user.ID, f.single.ID) + return f +} + +// A cross-release group becomes one song: its copies share a song key, the +// like on the single reaches the album cut (and is logged for sync), the Liked +// list shows the song once, a like or unlike on either copy reaches both, and +// "not the same song" undoes the link. +func TestCrossReleaseCopiesCountAsOneSong_Integration(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + f := newSongLinkFixture(t, pool) + + res, err := ResolveDuplicates(ctx, pool, nil, "", nil, true) + if err != nil { + t.Fatal(err) + } + if res.Classes[ClassCrossRelease] != 1 || res.SongsLinked != 1 { + t.Fatalf("classes %v, linked %d; want one cross-release group linked", res.Classes, res.SongsLinked) + } + keys := songKeys(t, q, f.single.ID, f.albumCut.ID) + if keys[0] != keys[1] { + t.Fatalf("song keys differ after linking: %v", keys) + } + + liked, err := q.ListLikedTrackIDs(ctx, f.user.ID) + if err != nil { + t.Fatal(err) + } + if len(liked) != 2 { + t.Errorf("liked track ids = %d, want both copies", len(liked)) + } + var logged int + if err := pool.QueryRow(ctx, + `SELECT count(*) FROM library_changes WHERE entity_type = 'like_track' AND op = 'upsert'`, + ).Scan(&logged); err != nil { + t.Fatal(err) + } + if logged != 1 { + t.Errorf("logged %d shared likes, want 1 (the album cut)", logged) + } + if n, err := q.CountLikedTracks(ctx, f.user.ID); err != nil || n != 1 { + t.Errorf("CountLikedTracks = %d (%v), want 1 song", n, err) + } + rows, err := q.ListLikedTrackRows(ctx, dbq.ListLikedTrackRowsParams{UserID: f.user.ID, Limit: 10}) + if err != nil { + t.Fatal(err) + } + if len(rows) != 1 { + t.Errorf("Liked list has %d rows, want the song once", len(rows)) + } + + // A second pass changes nothing. + if res, err := ResolveDuplicates(ctx, pool, nil, "", nil, true); err != nil || res.SongsLinked != 0 { + t.Errorf("second pass linked %d (%v), want 0", res.SongsLinked, err) + } + + unliked, err := q.UnlikeTrack(ctx, dbq.UnlikeTrackParams{UserID: f.user.ID, TrackID: f.albumCut.ID}) + if err != nil { + t.Fatal(err) + } + if len(unliked) != 2 { + t.Errorf("unlike removed %d likes, want both copies", len(unliked)) + } + added, err := q.LikeTrack(ctx, dbq.LikeTrackParams{UserID: f.user.ID, TrackID: f.single.ID}) + if err != nil { + t.Fatal(err) + } + if len(added) != 2 { + t.Errorf("like added %d, want both copies", len(added)) + } + if again, err := q.LikeTrack(ctx, dbq.LikeTrackParams{UserID: f.user.ID, TrackID: f.single.ID}); err != nil || len(again) != 0 { + t.Errorf("repeat like added %d (%v), want 0", len(again), err) + } + + if err := q.UnlinkDuplicateGroupSongs(ctx, f.groupID); err != nil { + t.Fatal(err) + } + keys = songKeys(t, q, f.single.ID, f.albumCut.ID) + if keys[0] == keys[1] { + t.Error("song keys still shared after unlinking") + } +} + +// Without the operator's permission the resolver links nothing. +func TestCrossReleaseLinkNeedsAutoResolve_Integration(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + f := newSongLinkFixture(t, pool) + if _, err := ResolveDuplicates(ctx, pool, nil, "", nil, false); err != nil { + t.Fatal(err) + } + keys := songKeys(t, q, f.single.ID, f.albumCut.ID) + if keys[0] == keys[1] { + t.Error("linked with auto-resolve off") + } +} + +// A merge keeps the removed copy's link to the song on another release. +func TestMergeInheritsSongLink_Integration(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + f := newSongLinkFixture(t, pool) + if _, err := q.LinkTracksAsSong(ctx, []pgtype.UUID{f.single.ID, f.albumCut.ID}); err != nil { + t.Fatal(err) + } + // A third, unlinked copy of the album cut stands in for a survivor. + other, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{ + Title: "Feel Good Inc.", AlbumID: f.albumCut.AlbumID, ArtistID: f.albumCut.ArtistID, + DurationMs: 1000, FilePath: "/music/Link Artist/Demon Days/06 Feel Good Inc (1).mp3", FileSize: 90, FileFormat: "mp3", + }) + if err != nil { + t.Fatal(err) + } + if err := q.MergeInheritSongLink(ctx, dbq.MergeInheritSongLinkParams{SurvivorID: other.ID, LoserID: f.albumCut.ID}); err != nil { + t.Fatal(err) + } + keys := songKeys(t, q, other.ID, f.single.ID) + if keys[0] != keys[1] { + t.Error("survivor did not join the song") + } +} + +func songKeys(t *testing.T, q *dbq.Queries, ids ...pgtype.UUID) []pgtype.UUID { + t.Helper() + rows, err := q.ListTrackSongKeys(context.Background(), ids) + if err != nil { + t.Fatal(err) + } + byID := map[pgtype.UUID]pgtype.UUID{} + for _, r := range rows { + byID[r.ID] = r.SongKey + } + out := make([]pgtype.UUID, len(ids)) + for i, id := range ids { + out[i] = byID[id] + } + return out +} 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/playlists/export_test.go b/internal/playlists/export_test.go new file mode 100644 index 00000000..26920459 --- /dev/null +++ b/internal/playlists/export_test.go @@ -0,0 +1,10 @@ +package playlists + +// Bridges for the external playlists_test package, whose integration helpers +// (newPool, seedTrack) live there. + +// RankedCandidate is rankedCandidate, for tests. +type RankedCandidate = rankedCandidate + +// OneCopyPerSong is oneCopyPerSong, for tests. +var OneCopyPerSong = oneCopyPerSong diff --git a/internal/playlists/song_copies_test.go b/internal/playlists/song_copies_test.go new file mode 100644 index 00000000..5488b1b2 --- /dev/null +++ b/internal/playlists/song_copies_test.go @@ -0,0 +1,50 @@ +package playlists_test + +import ( + "context" + "testing" + + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/playlists" +) + +// A mix holds a song once (M498 #5438) and never a held-back video rip +// (#5439): of a single and its album cut, the better-ranked copy stays; a rip +// is left out and the next copy of its song takes the place. +func TestOneCopyPerSong_Integration(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + single := seedTrack(t, pool, "copies-single", "copies-artist") + albumCut := seedTrack(t, pool, "copies-album-cut", "copies-artist") + rip := seedTrack(t, pool, "copies-rip", "copies-artist") + clean := seedTrack(t, pool, "copies-clean", "copies-artist") + other := seedTrack(t, pool, "copies-other", "copies-artist") + for _, pair := range [][]pgtype.UUID{{single.ID, albumCut.ID}, {rip.ID, clean.ID}} { + if _, err := q.LinkTracksAsSong(ctx, pair); err != nil { + t.Fatal(err) + } + } + if _, err := pool.Exec(ctx, `UPDATE tracks SET source_verdict = 'suspect' WHERE id = $1`, rip.ID); err != nil { + t.Fatal(err) + } + + in := []playlists.RankedCandidate{ + {TrackID: single.ID}, {TrackID: rip.ID}, {TrackID: albumCut.ID}, {TrackID: other.ID}, {TrackID: clean.ID}, + } + out, err := playlists.OneCopyPerSong(ctx, q, in) + if err != nil { + t.Fatal(err) + } + want := []pgtype.UUID{single.ID, other.ID, clean.ID} + if len(out) != len(want) { + t.Fatalf("got %d tracks, want %d", len(out), len(want)) + } + for i, w := range want { + if out[i].TrackID != w { + t.Errorf("position %d = %s, want %s", i, uuidString(out[i].TrackID), uuidString(w)) + } + } +} diff --git a/internal/playlists/system.go b/internal/playlists/system.go index 420c0811..807d38f8 100644 --- a/internal/playlists/system.go +++ b/internal/playlists/system.go @@ -1070,6 +1070,10 @@ func insertSystemPlaylist(ctx context.Context, qtx *dbq.Queries, userID pgtype.U return pgtype.UUID{}, fmt.Errorf("insert playlist row: %w", err) } + tracks, err = oneCopyPerSong(ctx, qtx, tracks) + if err != nil { + return pgtype.UUID{}, err + } for _, t := range tracks { var pickKind *string if t.PickKind != "" { @@ -1096,6 +1100,40 @@ func insertSystemPlaylist(ctx context.Context, qtx *dbq.Queries, userID pgtype.U return p.ID, nil } +// oneCopyPerSong keeps the first of a song's copies on different releases +// (M498 #5438): a mix holds the song once, in its best-ranked place. A video +// rip held back from the mixes (#5439) is left out, and a clean copy of the +// same song can take its place. +func oneCopyPerSong(ctx context.Context, qtx *dbq.Queries, tracks []rankedCandidate) ([]rankedCandidate, error) { + ids := make([]pgtype.UUID, len(tracks)) + for i, t := range tracks { + ids[i] = t.TrackID + } + rows, err := qtx.ListTrackSongKeys(ctx, ids) + if err != nil { + return nil, fmt.Errorf("song keys: %w", err) + } + songOf := make(map[pgtype.UUID]dbq.ListTrackSongKeysRow, len(rows)) + for _, r := range rows { + songOf[r.ID] = r + } + seen := make(map[pgtype.UUID]bool, len(tracks)) + out := tracks[:0:0] + for _, t := range tracks { + row, ok := songOf[t.TrackID] + if ok && row.HeldBack { + continue + } + song := row.SongKey + if ok && seen[song] { + continue + } + seen[song] = true + out = append(out, t) + } + return out, nil +} + // uuidStringPL renders a pgtype.UUID as the canonical 8-4-4-4-12 form. // (pgtype.UUID's String method exists on some pgx versions but not others; // also the playlists package needs this independent of test helpers.) diff --git a/internal/recommendation/shuffle.go b/internal/recommendation/shuffle.go index b7033d24..91173c22 100644 --- a/internal/recommendation/shuffle.go +++ b/internal/recommendation/shuffle.go @@ -52,6 +52,12 @@ func RadioDiversityCaps(limit int) DiversityCaps { return DiversityCaps{MaxPerArtist: artist, MaxPerAlbum: album} } +// heldBack says whether the candidate is a video rip held back from radio +// (M498 #5439): it plays when chosen, but radio does not choose it. +func heldBack(c Candidate) bool { + return c.Track.SourceVerdict != nil && *c.Track.SourceVerdict == "suspect" +} + // Shuffle scores each candidate, sorts descending by score, and returns the // top `limit` candidates, preferring artist/album diversity. limit <= 0 // returns nil; nil input returns nil. Pure — no IO, no global state beyond @@ -61,7 +67,9 @@ func RadioDiversityCaps(limit int) DiversityCaps { // first takes candidates that fit under the caps, and the second fills any // remaining slots from those the first pass skipped, still in score order. // So the result holds min(limit, len(candidates)) either way — the caps -// change WHICH tracks are chosen, never HOW MANY. +// change WHICH tracks are chosen, never HOW MANY. (What is not a candidate at +// all is dropped first: a held-back video rip, and a second copy of a song +// already chosen from another release, M498.) // // That two-pass shape is the whole design, and a hard cap would have been // the easy mistake. Radio asks for 50 tracks by default and 200 at most; a @@ -104,11 +112,24 @@ func Shuffle( deferred := make([]Candidate, 0, len(scored)-limit) artistCount := map[pgtype.UUID]int{} albumCount := map[pgtype.UUID]int{} + // One song on several releases is played once (M498 #5438): after its + // best-scoring copy, the others are dropped, not held back. + songs := map[pgtype.UUID]bool{} + repeat := func(c Candidate) bool { return c.Track.SongKey.Valid && songs[c.Track.SongKey] } + take := func(c Candidate) { + if c.Track.SongKey.Valid { + songs[c.Track.SongKey] = true + } + out = append(out, c) + } for _, s := range scored { if len(out) == limit { break } + if heldBack(s.c) || repeat(s.c) { + continue + } overArtist := caps.MaxPerArtist > 0 && artistCount[s.c.Track.ArtistID] >= caps.MaxPerArtist overAlbum := caps.MaxPerAlbum > 0 && albumCount[s.c.Track.AlbumID] >= caps.MaxPerAlbum if overArtist || overAlbum { @@ -118,7 +139,7 @@ func Shuffle( } artistCount[s.c.Track.ArtistID]++ albumCount[s.c.Track.AlbumID]++ - out = append(out, s.c) + take(s.c) } // Pass two: the caps could not fill the request, so relax them rather @@ -128,7 +149,10 @@ func Shuffle( if len(out) == limit { break } - out = append(out, c) + if repeat(c) { + continue + } + take(c) } return out } diff --git a/internal/recommendation/shuffle_test.go b/internal/recommendation/shuffle_test.go index 8b54107b..9e36fe22 100644 --- a/internal/recommendation/shuffle_test.go +++ b/internal/recommendation/shuffle_test.go @@ -84,3 +84,45 @@ func titles(cs []Candidate) []string { // pure tests; the Track.Title field is the human-readable handle. Track ID // is left as zero pgtype.UUID and never compared. var _ pgtype.UUID + +// One song on two releases is played once, in its better-scoring copy's place, +// and a capped copy held back for pass two does not slip back in either. +func TestShuffle_OneCopyPerSong(t *testing.T) { + song := func(c Candidate, key string) Candidate { + _ = c.Track.SongKey.Scan("00000000-0000-0000-0000-" + key) + return c + } + album := pgtype.UUID{Bytes: [16]byte{9}, Valid: true} + single := song(cand("000000000001", ScoringInputs{IsGeneralLiked: true}), "00000000000a") + albumCut := song(cand("000000000002", ScoringInputs{}), "00000000000a") + other := cand("000000000003", ScoringInputs{}) + single.Track.AlbumID, albumCut.Track.AlbumID = album, pgtype.UUID{Bytes: [16]byte{8}, Valid: true} + + out := Shuffle([]Candidate{albumCut, single, other}, defaultWeights(), time.Now(), fixedRNG(0.5), 10, DiversityCaps{}) + if got := titles(out); len(got) != 2 || got[0] != "000000000001" { + t.Errorf("got %v, want the liked single once and the other song", got) + } + + // The single is capped out of pass one; pass two takes one copy, not both. + capped := cand("000000000004", ScoringInputs{IsGeneralLiked: true}) + capped.Track.AlbumID = album + out = Shuffle([]Candidate{capped, single, albumCut}, defaultWeights(), time.Now(), fixedRNG(0.5), 3, DiversityCaps{MaxPerAlbum: 1}) + if len(out) != 2 { + t.Errorf("got %v, want two tracks: one copy of the song", titles(out)) + } +} + +// A video rip held back from radio (M498 #5439) is never chosen, even to fill +// the request. +func TestShuffle_SkipsHeldBackRips(t *testing.T) { + suspect := "suspect" + fine := "fine" + rip := cand("000000000001", ScoringInputs{IsGeneralLiked: true}) + rip.Track.SourceVerdict = &suspect + cleared := cand("000000000002", ScoringInputs{}) + cleared.Track.SourceVerdict = &fine + out := Shuffle([]Candidate{rip, cleared}, defaultWeights(), time.Now(), fixedRNG(0.5), 10, DiversityCaps{}) + if got := titles(out); len(got) != 1 || got[0] != "000000000002" { + t.Errorf("got %v, want only the track marked fine", got) + } +} 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/subsonic/star.go b/internal/subsonic/star.go index 7d717a3a..2a9fcc12 100644 --- a/internal/subsonic/star.go +++ b/internal/subsonic/star.go @@ -4,6 +4,7 @@ import ( "context" "errors" "net/http" + "slices" "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgtype" @@ -39,12 +40,13 @@ func (m *mediaHandlers) handleStar(w http.ResponseWriter, r *http.Request) { return } if trackID.Valid { - rows, err := q.LikeTrack(r.Context(), dbq.LikeTrackParams{UserID: user.ID, TrackID: trackID}) + // Stars the song: every copy of it (M498 #5438). + liked, err := q.LikeTrack(r.Context(), dbq.LikeTrackParams{UserID: user.ID, TrackID: trackID}) if err != nil { WriteFail(w, r, ErrGeneric, "Could not star track") return } - if rows == 1 { + if slices.Contains(liked, trackID) { _ = playevents.CaptureContextualLikeIfPlaying(r.Context(), q, user.ID, trackID, m.logger) } } @@ -75,7 +77,7 @@ func (m *mediaHandlers) handleUnstar(w http.ResponseWriter, r *http.Request) { params := r.URL.Query() q := dbq.New(m.pool) if t, ok := parseUUID(params.Get("id")); ok { - _ = q.UnlikeTrack(r.Context(), dbq.UnlikeTrackParams{UserID: user.ID, TrackID: t}) + _, _ = q.UnlikeTrack(r.Context(), dbq.UnlikeTrackParams{UserID: user.ID, TrackID: t}) _ = playevents.SoftDeleteContextualLikes(r.Context(), q, user.ID, t) } if a, ok := parseUUID(params.Get("albumId")); ok { 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.suspect-sources.test.ts b/web/src/lib/api/admin.suspect-sources.test.ts index 00caca9c..0d90aee9 100644 --- a/web/src/lib/api/admin.suspect-sources.test.ts +++ b/web/src/lib/api/admin.suspect-sources.test.ts @@ -14,7 +14,9 @@ function track(id: string): AdminSuspectTrack { duration_sec: 180, disc_number: null, track_number: null, - markers: [] + markers: [], + verdict: null, + verdict_at: null }; } diff --git a/web/src/lib/api/admin.ts b/web/src/lib/api/admin.ts index 6056a008..77e1a3b7 100644 --- a/web/src/lib/api/admin.ts +++ b/web/src/lib/api/admin.ts @@ -5,8 +5,11 @@ import type { ActionResult, AdminMissingResponse, AdminSuspectResponse, + SuspectVerdict, AdminDuplicatesResponse, + DuplicatesView, MergeDuplicateResult, + ResolvedDuplicatesResponse, AdminPlaybackError, AdminQuarantineRow, LidarrConfig, @@ -353,6 +356,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 +835,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 }); } @@ -925,6 +962,12 @@ export function createSuspectSourcesQuery() { }); } +// 'fine' lets a flagged track back into radio and the mixes; 'suspect' holds +// it back again (M498 #5439). +export async function setSuspectVerdict(trackId: string, verdict: SuspectVerdict): Promise { + await api.put(`/api/admin/library/suspect-sources/${encodeURIComponent(trackId)}`, { verdict }); +} + // Missing-file re-acquisition (#2527 / milestone #290) ---------------------- export type ReacquisitionSettings = { 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..f752b91e 100644 --- a/web/src/lib/api/types.ts +++ b/web/src/lib/api/types.ts @@ -439,8 +439,14 @@ export type AdminSuspectTrack = { disc_number: number | null; track_number: number | null; markers: string[]; + // 'suspect': held back from radio and the mixes. 'fine': the operator let it + // back. null: the resolver hasn't looked yet (M498 #5439). + verdict: SuspectVerdict | null; + verdict_at: string | null; }; +export type SuspectVerdict = 'suspect' | 'fine'; + // A folder's worth of flagged tracks; the server orders and groups by folder. export type AdminSuspectGroup = { directory: string; @@ -471,8 +477,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 +497,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 +531,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..8bc410e5 100644 --- a/web/src/lib/components/FingerprintSettingsCard.svelte +++ b/web/src/lib/components/FingerprintSettingsCard.svelte @@ -115,6 +115,19 @@ + +