diff --git a/internal/api/admin_suspect_sources.go b/internal/api/admin_suspect_sources.go index cca97934..6161d0c4 100644 --- a/internal/api/admin_suspect_sources.go +++ b/internal/api/admin_suspect_sources.go @@ -31,6 +31,9 @@ type suspectTrackView struct { // first pass (M498 #5439). VerdictAt is when it was last set. Verdict *string `json:"verdict"` VerdictAt *string `json:"verdict_at"` + // SearchedAt is when the resolver last asked Lidarr to search the album for + // a better copy (#5447): null when it has not. + SearchedAt *string `json:"searched_at"` } // suspectGroupView is a folder's worth of flagged tracks. As on the @@ -112,6 +115,10 @@ func groupSuspectByDirectory(rows []dbq.ListSuspectSourceTracksRow) []suspectGro at := row.SourceVerdictAt.Time.UTC().Format(time.RFC3339) t.VerdictAt = &at } + if row.SearchedAt.Valid { + at := row.SearchedAt.Time.UTC().Format(time.RFC3339) + t.SearchedAt = &at + } if n := len(groups); n > 0 && groups[n-1].Directory == row.Directory { groups[n-1].Tracks = append(groups[n-1].Tracks, t) continue diff --git a/internal/audit/audit.go b/internal/audit/audit.go index 3097bc23..f574daf5 100644 --- a/internal/audit/audit.go +++ b/internal/audit/audit.go @@ -66,6 +66,11 @@ const ( // Written with no actor; the metadata names the album and both releases. ActionLidarrReleaseChange Action = "lidarr_release_change" + // Lidarr rip search (M498 #5447): the resolver asked Lidarr to search an + // album whose only copy of a song is a video rip, for a better release. + // Written with no actor; the metadata names the album and how many rips. + ActionLidarrRipSearch Action = "lidarr_rip_search" + // 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/tracks.sql.go b/internal/db/dbq/tracks.sql.go index 6a9de912..9cc88467 100644 --- a/internal/db/dbq/tracks.sql.go +++ b/internal/db/dbq/tracks.sql.go @@ -405,6 +405,78 @@ func (q *Queries) GetTracksByIDs(ctx context.Context, dollar_1 []pgtype.UUID) ([ return items, nil } +const listAlbumsWithSoleCopyRips = `-- name: ListAlbumsWithSoleCopyRips :many +SELECT a.id AS album_id, + a.release_group_mbid::text AS release_group_mbid, + a.title AS album_title, + ar.name AS artist_name, + count(*)::bigint AS rips + FROM tracks t + JOIN albums a ON a.id = t.album_id + JOIN artists ar ON ar.id = a.artist_id + WHERE t.source_verdict = 'suspect' + AND t.missing_since IS NULL + AND a.release_group_mbid IS NOT NULL + AND NOT EXISTS ( + SELECT 1 FROM tracks c + WHERE c.song_key = t.song_key AND c.id <> t.id + AND c.missing_since IS NULL AND c.source_verdict IS DISTINCT FROM 'suspect') + AND NOT EXISTS ( + SELECT 1 FROM duplicate_group_members m + JOIN duplicate_groups g ON g.id = m.group_id AND g.status = 'pending' + JOIN duplicate_group_members m2 ON m2.group_id = g.id AND m2.track_id <> t.id + JOIN tracks c ON c.id = m2.track_id + WHERE m.track_id = t.id + AND c.missing_since IS NULL AND c.source_verdict IS DISTINCT FROM 'suspect') + AND NOT EXISTS ( + SELECT 1 FROM audit_log l + WHERE l.action = 'lidarr_rip_search' + AND l.metadata->>'album_id' = a.id::text + AND l.created_at > now() - interval '7 days') + GROUP BY a.id, a.release_group_mbid, a.title, ar.name + ORDER BY count(*) DESC, a.id + LIMIT $1 +` + +type ListAlbumsWithSoleCopyRipsRow struct { + AlbumID pgtype.UUID + ReleaseGroupMbid string + AlbumTitle string + ArtistName string + Rips int64 +} + +// Albums holding a held-back video rip that is the only copy of its song +// (#5447): no present copy that is not a rip shares its song, and no pending +// duplicate group pairs it with one. Lidarr counts such a track fulfilled, so +// the resolver asks it to search the album for a better release. Albums +// searched within the last week are left out, most rips first. +func (q *Queries) ListAlbumsWithSoleCopyRips(ctx context.Context, pageLimit int32) ([]ListAlbumsWithSoleCopyRipsRow, error) { + rows, err := q.db.Query(ctx, listAlbumsWithSoleCopyRips, pageLimit) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListAlbumsWithSoleCopyRipsRow + for rows.Next() { + var i ListAlbumsWithSoleCopyRipsRow + if err := rows.Scan( + &i.AlbumID, + &i.ReleaseGroupMbid, + &i.AlbumTitle, + &i.ArtistName, + &i.Rips, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + 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, t.song_id, t.song_key, t.source_verdict, t.source_verdict_at, albums.title AS album_title, @@ -659,10 +731,17 @@ SELECT t.id, artists.id AS artist_id, artists.name AS artist_name, t.source_verdict, - t.source_verdict_at + t.source_verdict_at, + searched.at AS searched_at FROM tracks t JOIN albums ON albums.id = t.album_id JOIN artists ON artists.id = t.artist_id + -- When the resolver last asked Lidarr to search the album for a better copy + -- (#5447). + LEFT JOIN LATERAL ( + SELECT max(l.created_at)::timestamptz AS at FROM audit_log l + WHERE l.action = 'lidarr_rip_search' AND l.metadata->>'album_id' = albums.id::text + ) searched ON true WHERE t.missing_since IS NULL AND regexp_replace(t.file_path, '^.*/', '') ~* $1::text ORDER BY directory, t.disc_number NULLS FIRST, t.track_number NULLS FIRST, t.title @@ -689,6 +768,7 @@ type ListSuspectSourceTracksRow struct { ArtistName string SourceVerdict *string SourceVerdictAt pgtype.Timestamptz + SearchedAt pgtype.Timestamptz } // The admin report of present tracks whose filename looks like a video rip @@ -724,6 +804,7 @@ func (q *Queries) ListSuspectSourceTracks(ctx context.Context, arg ListSuspectSo &i.ArtistName, &i.SourceVerdict, &i.SourceVerdictAt, + &i.SearchedAt, ); err != nil { return nil, err } diff --git a/internal/db/queries/tracks.sql b/internal/db/queries/tracks.sql index b8e7d9d3..dd7b367d 100644 --- a/internal/db/queries/tracks.sql +++ b/internal/db/queries/tracks.sql @@ -287,10 +287,17 @@ SELECT t.id, artists.id AS artist_id, artists.name AS artist_name, t.source_verdict, - t.source_verdict_at + t.source_verdict_at, + searched.at AS searched_at FROM tracks t JOIN albums ON albums.id = t.album_id JOIN artists ON artists.id = t.artist_id + -- When the resolver last asked Lidarr to search the album for a better copy + -- (#5447). + LEFT JOIN LATERAL ( + SELECT max(l.created_at)::timestamptz AS at FROM audit_log l + WHERE l.action = 'lidarr_rip_search' AND l.metadata->>'album_id' = albums.id::text + ) searched ON true WHERE t.missing_since IS NULL AND regexp_replace(t.file_path, '^.*/', '') ~* sqlc.arg(pattern)::text ORDER BY directory, t.disc_number NULLS FIRST, t.track_number NULLS FIRST, t.title @@ -327,3 +334,40 @@ UPDATE tracks SET source_verdict = sqlc.arg(verdict)::text, source_verdict_at = now() WHERE id = sqlc.arg(id) AND source_verdict IS NOT NULL; + +-- name: ListAlbumsWithSoleCopyRips :many +-- Albums holding a held-back video rip that is the only copy of its song +-- (#5447): no present copy that is not a rip shares its song, and no pending +-- duplicate group pairs it with one. Lidarr counts such a track fulfilled, so +-- the resolver asks it to search the album for a better release. Albums +-- searched within the last week are left out, most rips first. +SELECT a.id AS album_id, + a.release_group_mbid::text AS release_group_mbid, + a.title AS album_title, + ar.name AS artist_name, + count(*)::bigint AS rips + FROM tracks t + JOIN albums a ON a.id = t.album_id + JOIN artists ar ON ar.id = a.artist_id + WHERE t.source_verdict = 'suspect' + AND t.missing_since IS NULL + AND a.release_group_mbid IS NOT NULL + AND NOT EXISTS ( + SELECT 1 FROM tracks c + WHERE c.song_key = t.song_key AND c.id <> t.id + AND c.missing_since IS NULL AND c.source_verdict IS DISTINCT FROM 'suspect') + AND NOT EXISTS ( + SELECT 1 FROM duplicate_group_members m + JOIN duplicate_groups g ON g.id = m.group_id AND g.status = 'pending' + JOIN duplicate_group_members m2 ON m2.group_id = g.id AND m2.track_id <> t.id + JOIN tracks c ON c.id = m2.track_id + WHERE m.track_id = t.id + AND c.missing_since IS NULL AND c.source_verdict IS DISTINCT FROM 'suspect') + AND NOT EXISTS ( + SELECT 1 FROM audit_log l + WHERE l.action = 'lidarr_rip_search' + AND l.metadata->>'album_id' = a.id::text + AND l.created_at > now() - interval '7 days') + GROUP BY a.id, a.release_group_mbid, a.title, ar.name + ORDER BY count(*) DESC, a.id + LIMIT sqlc.arg(page_limit); diff --git a/internal/library/duplicate_resolve.go b/internal/library/duplicate_resolve.go index 5dd4b9ee..b4e79da5 100644 --- a/internal/library/duplicate_resolve.go +++ b/internal/library/duplicate_resolve.go @@ -50,6 +50,7 @@ type LidarrLibrary interface { 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 + SearchAlbums(ctx context.Context, albumIDs []int) error } const ( @@ -94,6 +95,9 @@ type DuplicateResolveResult struct { // 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 + // RipSearches counts albums Lidarr was asked to search for a better copy + // of a rip that has no clean copy (#5447). + RipSearches int } // LidarrPathKey is the part of a path Minstrel and Lidarr agree on: the last @@ -265,6 +269,13 @@ func ResolveDuplicates( return res, err } + // Last, rips with no clean copy anywhere: ask Lidarr for something better. + searched, err := searchSoleCopyRips(ctx, q, pool, logger, lid) + res.RipSearches = searched + if err != nil { + logger.Warn("duplicate resolve: rip search failed", "err", err) + } + if res.Merged > 0 || len(changes) > 0 { notifyAdmins(ctx, notifications.KindDuplicatesResolved, notifications.Payload{ Count: int64(res.Merged + len(changes)), @@ -334,6 +345,61 @@ func linkSong(ctx context.Context, pool *pgxpool.Pool, ids []pgtype.UUID) (bool, return true, tx.Commit(ctx) } +// ripSearchCap bounds the albums one pass asks Lidarr to search, so the first +// pass after deploy does not queue the whole backlog of rips at once. +const ripSearchCap = 10 + +// searchSoleCopyRips asks Lidarr to search each album holding a video rip that +// is the only copy of its song (#5447, operator's choice: option 2). While +// Lidarr maps the rip it counts the track fulfilled, so nothing else asks for +// better. An album search grabs only when the quality profile allows an +// upgrade, so this removes nothing and may change nothing: the rip stays held +// back either way. An album is searched at most once a week (the query reads +// the audit log), and an album Lidarr does not hold is skipped. +func searchSoleCopyRips( + ctx context.Context, q *dbq.Queries, pool *pgxpool.Pool, logger *slog.Logger, lid LidarrLibrary, +) (int, error) { + albums, err := q.ListAlbumsWithSoleCopyRips(ctx, ripSearchCap) + if err != nil { + return 0, fmt.Errorf("list albums with sole-copy rips: %w", err) + } + type found struct { + row dbq.ListAlbumsWithSoleCopyRipsRow + lidarrID int + } + var targets []found + for _, a := range albums { + la, err := lid.LookupAlbumByMBID(ctx, a.ReleaseGroupMbid) + if errors.Is(err, lidarr.ErrNotFound) { + continue + } + if err != nil { + return 0, fmt.Errorf("look up %s: %w", a.ReleaseGroupMbid, err) + } + targets = append(targets, found{row: a, lidarrID: la.ID}) + } + if len(targets) == 0 { + return 0, nil + } + ids := make([]int, len(targets)) + for i, t := range targets { + ids[i] = t.lidarrID + } + if err := lid.SearchAlbums(ctx, ids); err != nil { + return 0, fmt.Errorf("search albums: %w", err) + } + for _, t := range targets { + audit.WriteOrLog(ctx, pool, logger, pgtype.UUID{}, pgtype.UUID{}, audit.ActionLidarrRipSearch, map[string]any{ + "album_id": syncpkg.FormatUUID(t.row.AlbumID), + "album_title": t.row.AlbumTitle, + "artist_name": t.row.ArtistName, + "lidarr_album_id": t.lidarrID, + "rips": t.row.Rips, + }) + } + return len(targets), nil +} + func foldResolveGroups(rows []dbq.ListDuplicateGroupsForResolveRow) []*resolveGroup { var groups []*resolveGroup for _, r := range rows { @@ -787,5 +853,5 @@ func (w *DuplicateResolveWorker) tickOnce(ctx context.Context) { 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) + "held_back", res.HeldBack, "released", res.Released, "rip_searches", res.RipSearches) } diff --git a/internal/library/duplicate_resolve_test.go b/internal/library/duplicate_resolve_test.go index d1b67a83..20778fc6 100644 --- a/internal/library/duplicate_resolve_test.go +++ b/internal/library/duplicate_resolve_test.go @@ -82,6 +82,7 @@ type fakeLidarr struct { releases []lidarr.AlbumRelease releaseTracks map[int][]lidarr.ReleaseTrack setCalls [][2]int + searchCalls [][]int } func (f *fakeLidarr) ListUnmappedTrackFiles(context.Context) ([]lidarr.TrackFile, error) { @@ -116,6 +117,11 @@ func (f *fakeLidarr) ListReleaseTracks(_ context.Context, releaseID int) ([]lida return f.releaseTracks[releaseID], nil } +func (f *fakeLidarr) SearchAlbums(_ context.Context, albumIDs []int) error { + f.searchCalls = append(f.searchCalls, albumIDs) + return 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 { @@ -502,3 +508,60 @@ func TestResolveDuplicates_HoldsBackRips_Integration(t *testing.T) { t.Errorf("set on an unflagged track: %d, %v; want 0 rows", n, err) } } + +// A rip that is the only copy of its song: Lidarr is asked to search its album +// for a better release (#5447), once a week at most, and the rip is not touched. +// A rip with a clean copy elsewhere is not searched for. +func TestResolveDuplicates_SearchesForASoleCopyRip_Integration(t *testing.T) { + f := newCrossReleaseRipFixture(t) + ctx := context.Background() + exec := func(sql string, args ...any) { + t.Helper() + if _, err := f.pool.Exec(ctx, sql, args...); err != nil { + t.Fatalf("exec %q: %v", sql, err) + } + } + // The single's rip has a clean copy on the album: never searched for. + exec(`UPDATE albums SET release_group_mbid = 'rg-single' WHERE id = $1`, f.rip.AlbumID) + lid := &fakeLidarr{album: lidarr.LidarrAlbum{ID: 77, ForeignAlbumID: "rg-single"}} + if _, err := ResolveDuplicates(ctx, f.pool, nil, "", lid, true); err != nil { + t.Fatal(err) + } + if len(lid.searchCalls) != 0 { + t.Fatalf("searched %v while a clean copy exists", lid.searchCalls) + } + + // Without the group and the song link, the rip is the only copy. + exec(`DELETE FROM duplicate_groups WHERE id = $1`, f.groupID) + exec(`UPDATE tracks SET song_id = NULL WHERE id IN ($1, $2)`, f.rip.ID, f.clean.ID) + res, err := ResolveDuplicates(ctx, f.pool, nil, "", lid, true) + if err != nil { + t.Fatal(err) + } + if res.RipSearches != 1 || len(lid.searchCalls) != 1 || len(lid.searchCalls[0]) != 1 || lid.searchCalls[0][0] != 77 { + t.Fatalf("rip searches %d, calls %v; want album 77 searched once", res.RipSearches, lid.searchCalls) + } + if !exists(f.ripPath) { + t.Error("the rip was removed; a search must not touch it") + } + + // The next pass, inside the week, leaves the album alone. + if res, err := ResolveDuplicates(ctx, f.pool, nil, "", lid, true); err != nil || res.RipSearches != 0 { + t.Errorf("second pass searched %d (%v), want 0", res.RipSearches, err) + } + var searchedAt pgtype.Timestamptz + rows, err := dbq.New(f.pool).ListSuspectSourceTracks(ctx, dbq.ListSuspectSourceTracksParams{ + Pattern: SuspectSourcePattern, PageLimit: 10, + }) + if err != nil { + t.Fatal(err) + } + for _, r := range rows { + if r.ID == f.rip.ID { + searchedAt = r.SearchedAt + } + } + if !searchedAt.Valid { + t.Error("the suspect-sources report doesn't show the search") + } +} diff --git a/internal/lidarr/releases.go b/internal/lidarr/releases.go index 2513af12..a952d52e 100644 --- a/internal/lidarr/releases.go +++ b/internal/lidarr/releases.go @@ -152,3 +152,24 @@ func (c *Client) SetMonitoredRelease(ctx context.Context, albumID, releaseID int _ = putResp.Body.Close() return nil } + +// SearchAlbums asks Lidarr to search its indexers for the albums +// (POST /api/v1/command, AlbumSearch). Lidarr grabs a release only when it +// beats the files the album already has under the quality profile, so for an +// album whose tracks are all present this is an upgrade search: harmless when +// nothing better exists. The command is queued; the call does not wait for it. +func (c *Client) SearchAlbums(ctx context.Context, albumIDs []int) error { + if len(albumIDs) == 0 { + return nil + } + body, err := json.Marshal(map[string]any{"name": "AlbumSearch", "albumIds": albumIDs}) + if err != nil { + return fmt.Errorf("%w: marshal command: %v", ErrInvalidPayload, err) + } + resp, err := c.post(ctx, "/api/v1/command", body) + if err != nil { + return err + } + _ = resp.Body.Close() + return nil +} diff --git a/internal/lidarr/releases_test.go b/internal/lidarr/releases_test.go index 5e3c769e..e89a39d2 100644 --- a/internal/lidarr/releases_test.go +++ b/internal/lidarr/releases_test.go @@ -114,3 +114,25 @@ func TestSetMonitoredRelease_UnknownReleaseSendsNothing(t *testing.T) { t.Errorf("err = %v, want ErrNotFound", err) } } + +func TestSearchAlbums_PostsTheCommand(t *testing.T) { + var got map[string]any + c, srv := newTestClient(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost || r.URL.Path != "/api/v1/command" { + t.Errorf("request = %s %s, want POST /api/v1/command", r.Method, r.URL.Path) + } + b, _ := io.ReadAll(r.Body) + _ = json.Unmarshal(b, &got) + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{"id":1,"name":"AlbumSearch"}`)) + }) + defer srv.Close() + + if err := c.SearchAlbums(context.Background(), []int{4723, 12}); err != nil { + t.Fatalf("SearchAlbums: %v", err) + } + ids, _ := got["albumIds"].([]any) + if got["name"] != "AlbumSearch" || len(ids) != 2 || ids[0] != float64(4723) || ids[1] != float64(12) { + t.Errorf("command = %v", got) + } +} diff --git a/web/src/lib/api/admin.suspect-sources.test.ts b/web/src/lib/api/admin.suspect-sources.test.ts index 0d90aee9..59017a66 100644 --- a/web/src/lib/api/admin.suspect-sources.test.ts +++ b/web/src/lib/api/admin.suspect-sources.test.ts @@ -16,7 +16,8 @@ function track(id: string): AdminSuspectTrack { track_number: null, markers: [], verdict: null, - verdict_at: null + verdict_at: null, + searched_at: null }; } diff --git a/web/src/lib/api/types.ts b/web/src/lib/api/types.ts index f752b91e..2fef805d 100644 --- a/web/src/lib/api/types.ts +++ b/web/src/lib/api/types.ts @@ -443,6 +443,8 @@ export type AdminSuspectTrack = { // back. null: the resolver hasn't looked yet (M498 #5439). verdict: SuspectVerdict | null; verdict_at: string | null; + // When Lidarr was last asked to search the album for a better copy (#5447). + searched_at: string | null; }; export type SuspectVerdict = 'suspect' | 'fine'; diff --git a/web/src/routes/admin/suspect-sources/+page.svelte b/web/src/routes/admin/suspect-sources/+page.svelte index 101362fb..db39fd60 100644 --- a/web/src/routes/admin/suspect-sources/+page.svelte +++ b/web/src/routes/admin/suspect-sources/+page.svelte @@ -17,7 +17,8 @@ // The duplicate resolver holds each one back from radio and the mixes, and // merges it away where Lidarr keeps a clean copy (M498 #5439). A marker is a // reason to look, not proof, so each row says what was done and offers the - // undo: "This one is fine" lets the track back for good. + // undo: "This one is fine" lets the track back for good. A rip that is the + // only copy of its song has its album searched in Lidarr (#5447). const queryStore = createSuspectSourcesQuery(); const query = $derived($queryStore); @@ -101,7 +102,8 @@ a video site, and can be the wrong audio: a live take, a music-video edit, or something else entirely. Each one is kept out of radio and the mixes, though it still plays when you choose it, and it is removed where Lidarr keeps a clean copy of the same - song. If one is fine, say so and it goes back in. + song. Where it is the only copy, Lidarr is asked once a week to search the album for a + better release. If one is fine, say so and it goes back in.

@@ -178,6 +180,11 @@ Held back{#if t.verdict_at && !changed[t.track_id]} {relativeTime(t.verdict_at)}{/if} + {#if t.searched_at} + + Lidarr searched for a better copy {relativeTime(t.searched_at)} + + {/if}