diff --git a/internal/api/admin_duplicates.go b/internal/api/admin_duplicates.go new file mode 100644 index 00000000..05fc1be7 --- /dev/null +++ b/internal/api/admin_duplicates.go @@ -0,0 +1,230 @@ +package api + +import ( + "context" + "errors" + "net/http" + + "github.com/go-chi/chi/v5" + "github.com/jackc/pgx/v5" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/library" +) + +// duplicateMemberView is one copy in a proposed duplicate group. LikeCount and +// PlayCount span every user: the report is admin-only, and what a copy carries +// is the fact the operator weighs when choosing which to keep. +type duplicateMemberView struct { + TrackID string `json:"track_id"` + Title string `json:"title"` + ArtistName string `json:"artist_name"` + AlbumID string `json:"album_id"` + AlbumTitle string `json:"album_title"` + FilePath string `json:"file_path"` + FileFormat string `json:"file_format"` + FileSize int64 `json:"file_size"` + DurationSec int32 `json:"duration_sec"` + AddedAt string `json:"added_at"` + LikeCount int64 `json:"like_count"` + PlayCount int64 `json:"play_count"` +} + +// duplicateGroupView is one proposal. SurvivorTrackID and SurvivorReason are +// the copy the report proposes keeping and the rule that chose it +// (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"` +} + +// duplicateSweepView is the latest sweep. State is "never" when none has run, +// which is what lets the page tell an empty report apart from a sweep that +// found nothing. +type duplicateSweepView struct { + State string `json:"state"` + StartedAt *string `json:"started_at"` + FinishedAt *string `json:"finished_at"` + Candidates *int32 `json:"candidates"` + GroupsFound *int32 `json:"groups_found"` + OversizeClusters *int32 `json:"oversize_clusters"` + ErrorMessage *string `json:"error_message"` +} + +// adminDuplicatesResponse is the paged report. Total counts groups. +type adminDuplicatesResponse struct { + Sweep duplicateSweepView `json:"sweep"` + Fingerprints fingerprintCoverageResp `json:"fingerprints"` + Total int64 `json:"total"` + Limit int `json:"limit"` + Offset int `json:"offset"` + Groups []duplicateGroupView `json:"groups"` +} + +// handleListDuplicates implements GET /api/admin/library/duplicates (#3912). +// +// 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 +// fingerprinting, never swept, or swept and clean — and the page has to say which. +func (h *handlers) handleListDuplicates(w http.ResponseWriter, r *http.Request) { + limit, offset, err := parsePaging(r.URL.Query()) + if err != nil { + writeAdminJSONErr(w, http.StatusBadRequest, "invalid_paging") + return + } + ctx := r.Context() + q := dbq.New(h.pool) + + sweep := duplicateSweepView{State: "never"} + last, err := q.GetLatestDuplicateSweep(ctx) + switch { + case err == nil: + sweep = duplicateSweepViewOf(last) + case !errors.Is(err, pgx.ErrNoRows): + h.logger.Error("admin: latest duplicate sweep", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + return + } + + cov, err := library.FingerprintCoverage(ctx, h.pool) + if err != nil { + h.logger.Error("admin: fingerprint coverage", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + return + } + total, err := q.CountPendingDuplicateGroups(ctx) + if err != nil { + h.logger.Error("admin: count duplicate groups", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + return + } + rows, err := q.ListPendingDuplicateGroupMembers(ctx, dbq.ListPendingDuplicateGroupMembersParams{ + PageLimit: int32(limit), PageOffset: int32(offset), + }) + if err != nil { + h.logger.Error("admin: list duplicate groups", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + return + } + + writeJSON(w, http.StatusOK, adminDuplicatesResponse{ + Sweep: sweep, + Fingerprints: fingerprintCoverageResp{ + Total: cov.Total, Fingerprinted: cov.Fingerprinted, Rejected: cov.Rejected, Pending: cov.Pending, + }, + Total: total, + Limit: limit, + Offset: offset, + Groups: foldDuplicateGroups(rows), + }) +} + +func duplicateSweepViewOf(s dbq.DuplicateSweep) duplicateSweepView { + v := duplicateSweepView{ + State: "running", + Candidates: s.Candidates, + GroupsFound: s.GroupsFound, + OversizeClusters: s.OversizeClusters, + ErrorMessage: s.ErrorMessage, + } + started := formatTimestamp(s.StartedAt) + v.StartedAt = &started + if s.FinishedAt.Valid { + finished := formatTimestamp(s.FinishedAt) + v.FinishedAt = &finished + v.State = "finished" + } + return v +} + +// foldDuplicateGroups folds the one-row-per-member query result into groups and +// proposes each group's survivor. It relies on the query ordering members of a +// group together, so a run-length fold is enough and the page order holds. +func foldDuplicateGroups(rows []dbq.ListPendingDuplicateGroupMembersRow) []duplicateGroupView { + groups := make([]duplicateGroupView, 0, 8) + var candidates [][]library.SurvivorCandidate + for _, row := range rows { + id := uuidToString(row.GroupID) + if n := len(groups); n == 0 || groups[n-1].ID != id { + groups = append(groups, duplicateGroupView{ + ID: id, + Tier: row.Tier, + WorstBitErrorRate: row.WorstBitErrorRate, + DetectedAt: formatTimestamp(row.DetectedAt), + }) + candidates = append(candidates, nil) + } + n := len(groups) - 1 + trackID := uuidToString(row.TrackID) + groups[n].Members = append(groups[n].Members, duplicateMemberView{ + TrackID: trackID, + Title: row.Title, + ArtistName: row.ArtistName, + AlbumID: uuidToString(row.AlbumID), + AlbumTitle: row.AlbumTitle, + FilePath: row.FilePath, + FileFormat: row.FileFormat, + FileSize: row.FileSize, + DurationSec: row.DurationMs / 1000, + AddedAt: formatTimestamp(row.AddedAt), + LikeCount: row.LikeCount, + PlayCount: row.PlayCount, + }) + candidates[n] = append(candidates[n], library.SurvivorCandidate{ + TrackID: trackID, FileFormat: row.FileFormat, FileSize: row.FileSize, AddedAt: row.AddedAt.Time, + }) + } + for i := range groups { + groups[i].SurvivorTrackID, groups[i].SurvivorReason = library.ProposeSurvivor(candidates[i]) + } + return groups +} + +// handleRunDuplicateSweep implements POST /api/admin/library/duplicates/sweep: +// 202 when a sweep starts, 409 sweep_in_progress when one is already running. +// The sweep outlives the request, so it runs on a background context, as +// handleTriggerScan's scan does. +func (h *handlers) handleRunDuplicateSweep(w http.ResponseWriter, _ *http.Request) { + started, err := library.TryStartDuplicateSweep( + context.Background(), h.pool, h.logger.With("source", "manual"), + ) + if err != nil { + h.logger.Error("admin: start duplicate sweep", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + return + } + if !started { + writeAdminJSONErr(w, http.StatusConflict, "sweep_in_progress") + return + } + writeJSON(w, http.StatusAccepted, map[string]bool{"started": true}) +} + +// 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. +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) + if err != nil { + h.logger.Error("admin: dismiss duplicate group", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + return + } + if n == 0 { + writeAdminJSONErr(w, http.StatusNotFound, "duplicate_group_not_pending") + return + } + writeJSON(w, http.StatusOK, map[string]string{"status": "dismissed"}) +} diff --git a/internal/api/admin_duplicates_test.go b/internal/api/admin_duplicates_test.go new file mode 100644 index 00000000..a1c8a332 --- /dev/null +++ b/internal/api/admin_duplicates_test.go @@ -0,0 +1,65 @@ +package api + +import ( + "testing" + "time" + + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +func dupUUID(b byte) pgtype.UUID { + var u pgtype.UUID + u.Bytes[15] = b + u.Valid = true + return u +} + +func dupTS(t time.Time) pgtype.Timestamptz { return pgtype.Timestamptz{Time: t, Valid: true} } + +// Rows arrive one per member, members of a group together. The fold must keep +// groups apart, keep the query's order, and propose each group's survivor from +// its own members only. +func TestFoldDuplicateGroups(t *testing.T) { + older := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC) + newer := older.Add(48 * time.Hour) + ber := float32(0.04) + rows := []dbq.ListPendingDuplicateGroupMembersRow{ + // Group 1: identical audio, sizes tie, the older copy should be kept. + {GroupID: dupUUID(1), Tier: "exact", DetectedAt: dupTS(newer), TrackID: dupUUID(10), + Title: "WWW", FileFormat: "mp3", FileSize: 6_900_000, DurationMs: 215_400, AddedAt: dupTS(newer), PlayCount: 3}, + {GroupID: dupUUID(1), Tier: "exact", DetectedAt: dupTS(newer), TrackID: dupUUID(11), + Title: "WWW", FileFormat: "mp3", FileSize: 6_900_000, DurationMs: 215_400, AddedAt: dupTS(older), LikeCount: 1}, + // Group 2: the same recording, FLAC against MP3. + {GroupID: dupUUID(2), Tier: "acoustic", WorstBitErrorRate: &ber, DetectedAt: dupTS(older), TrackID: dupUUID(20), + Title: "Lovesick", FileFormat: "mp3", FileSize: 9_000_000, DurationMs: 198_000, AddedAt: dupTS(older)}, + {GroupID: dupUUID(2), Tier: "acoustic", WorstBitErrorRate: &ber, DetectedAt: dupTS(older), TrackID: dupUUID(21), + Title: "Lovesick", FileFormat: "flac", FileSize: 30_000_000, DurationMs: 198_000, AddedAt: dupTS(newer)}, + } + + got := foldDuplicateGroups(rows) + if len(got) != 2 { + t.Fatalf("folded %d groups, want 2", len(got)) + } + + g1, g2 := got[0], got[1] + if g1.ID != uuidToString(dupUUID(1)) || len(g1.Members) != 2 || g1.WorstBitErrorRate != nil { + t.Fatalf("group 1 = %+v, want the exact pair with no score", g1) + } + if g1.SurvivorTrackID != uuidToString(dupUUID(11)) || g1.SurvivorReason != "in the library longest" { + t.Errorf("group 1 survivor = (%s, %q), want the older copy", g1.SurvivorTrackID, g1.SurvivorReason) + } + if g1.Members[0].DurationSec != 215 || g1.Members[0].PlayCount != 3 || g1.Members[1].LikeCount != 1 { + t.Errorf("group 1 members lost their facts: %+v", g1.Members) + } + + if g2.Tier != "acoustic" || g2.WorstBitErrorRate == nil || *g2.WorstBitErrorRate != ber { + t.Fatalf("group 2 = %+v, want the acoustic pair with its score", g2) + } + // Chosen from group 2's own members: a survivor leaking across groups is + // exactly what a wrong fold boundary would produce. + if g2.SurvivorTrackID != uuidToString(dupUUID(21)) || g2.SurvivorReason != "lossless (flac)" { + t.Errorf("group 2 survivor = (%s, %q), want the FLAC copy", g2.SurvivorTrackID, g2.SurvivorReason) + } +} diff --git a/internal/api/api.go b/internal/api/api.go index b2e76c2d..d3018b9d 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -216,6 +216,11 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev admin.Get("/library/coverage", h.handleGetLibraryCoverage) admin.Get("/library/fingerprints", h.handleGetFingerprintCoverage) + // Duplicates report (#3912): proposals from the duplicate sweep, a + // trigger to sweep now, and dismissal. Nothing here merges or deletes. + admin.Get("/library/duplicates", h.handleListDuplicates) + admin.Post("/library/duplicates/sweep", h.handleRunDuplicateSweep) + admin.Post("/library/duplicates/{id}/dismiss", h.handleDismissDuplicateGroup) admin.Get("/invites", h.handleListInvites) admin.Post("/invites", h.handleCreateInvite) diff --git a/internal/db/dbq/duplicates.sql.go b/internal/db/dbq/duplicates.sql.go index 94a8cb0e..0f979e34 100644 --- a/internal/db/dbq/duplicates.sql.go +++ b/internal/db/dbq/duplicates.sql.go @@ -27,6 +27,23 @@ func (q *Queries) AddDuplicateGroupMember(ctx context.Context, arg AddDuplicateG return err } +const countPendingDuplicateGroups = `-- name: CountPendingDuplicateGroups :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 +` + +// Proposals awaiting review. A group left with one member — its other tracks +// deleted since the sweep — is no proposal at all and is not counted; the next +// sweep retires it. +func (q *Queries) CountPendingDuplicateGroups(ctx context.Context) (int64, error) { + row := q.db.QueryRow(ctx, countPendingDuplicateGroups) + 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' @@ -51,6 +68,22 @@ func (q *Queries) DeleteStalePendingDuplicateGroups(ctx context.Context, sweepID return result.RowsAffected(), nil } +const dismissDuplicateGroup = `-- name: DismissDuplicateGroup :execrows +UPDATE duplicate_groups + SET status = 'dismissed', resolved_at = now() + WHERE id = $1 AND status = 'pending' +` + +// "These are not duplicates." Only a pending group can be dismissed; zero rows +// means it was already resolved or no longer exists. +func (q *Queries) DismissDuplicateGroup(ctx context.Context, id pgtype.UUID) (int64, error) { + result, err := q.db.Exec(ctx, dismissDuplicateGroup, id) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + const finishDuplicateSweep = `-- name: FinishDuplicateSweep :exec UPDATE duplicate_sweeps SET finished_at = now(), @@ -270,6 +303,104 @@ func (q *Queries) ListExactDuplicateHashes(ctx context.Context, currentVersion i return items, nil } +const listPendingDuplicateGroupMembers = `-- name: ListPendingDuplicateGroupMembers :many +WITH page AS ( + SELECT g.id, g.tier, g.worst_bit_error_rate, g.detected_at + FROM duplicate_groups g + WHERE g.status = 'pending' + AND (SELECT count(*) FROM duplicate_group_members m WHERE m.group_id = g.id) >= 2 + ORDER BY g.detected_at DESC, g.id + LIMIT $2 OFFSET $1 +) +SELECT p.id AS group_id, + p.tier, + p.worst_bit_error_rate, + p.detected_at, + t.id AS track_id, + t.title, + artists.name AS artist_name, + albums.id AS album_id, + albums.title AS album_title, + t.file_path, + t.file_format, + t.file_size, + t.duration_ms, + t.added_at, + (SELECT count(*) FROM general_likes l WHERE l.track_id = t.id)::bigint AS like_count, + (SELECT count(*) FROM play_events e WHERE e.track_id = t.id)::bigint AS play_count + FROM page p + JOIN duplicate_group_members m ON m.group_id = p.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 + ORDER BY p.detected_at DESC, p.id, t.id +` + +type ListPendingDuplicateGroupMembersParams struct { + PageOffset int32 + PageLimit int32 +} + +type ListPendingDuplicateGroupMembersRow struct { + GroupID pgtype.UUID + Tier string + WorstBitErrorRate *float32 + DetectedAt pgtype.Timestamptz + TrackID pgtype.UUID + Title string + ArtistName string + AlbumID pgtype.UUID + AlbumTitle string + FilePath string + FileFormat string + FileSize int64 + DurationMs int32 + AddedAt pgtype.Timestamptz + LikeCount int64 + 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. +func (q *Queries) ListPendingDuplicateGroupMembers(ctx context.Context, arg ListPendingDuplicateGroupMembersParams) ([]ListPendingDuplicateGroupMembersRow, error) { + rows, err := q.db.Query(ctx, listPendingDuplicateGroupMembers, arg.PageOffset, arg.PageLimit) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListPendingDuplicateGroupMembersRow + for rows.Next() { + var i ListPendingDuplicateGroupMembersRow + if err := rows.Scan( + &i.GroupID, + &i.Tier, + &i.WorstBitErrorRate, + &i.DetectedAt, + &i.TrackID, + &i.Title, + &i.ArtistName, + &i.AlbumID, + &i.AlbumTitle, + &i.FilePath, + &i.FileFormat, + &i.FileSize, + &i.DurationMs, + &i.AddedAt, + &i.LikeCount, + &i.PlayCount, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const startDuplicateSweep = `-- name: StartDuplicateSweep :one INSERT INTO duplicate_sweeps DEFAULT VALUES RETURNING id, started_at ` diff --git a/internal/db/migrations/0060_play_events_track_index.down.sql b/internal/db/migrations/0060_play_events_track_index.down.sql new file mode 100644 index 00000000..af749a3f --- /dev/null +++ b/internal/db/migrations/0060_play_events_track_index.down.sql @@ -0,0 +1 @@ +DROP INDEX IF EXISTS play_events_track_idx; diff --git a/internal/db/migrations/0060_play_events_track_index.up.sql b/internal/db/migrations/0060_play_events_track_index.up.sql new file mode 100644 index 00000000..35036982 --- /dev/null +++ b/internal/db/migrations/0060_play_events_track_index.up.sql @@ -0,0 +1,8 @@ +-- 0060_play_events_track_index.up.sql — play_events by track (Scribe #3912, #3911). +-- +-- play_events is indexed by (user_id, started_at) and (user_id, track_id), both +-- led by user. Nothing reached it by track alone until the duplicates report, +-- which shows each copy's play count — a scan of the whole table per copy — and +-- the merge (#3911), which repoints a duplicate's play history onto the copy +-- being kept. Both ask "every play of this track", whoever played it. +CREATE INDEX play_events_track_idx ON play_events (track_id); diff --git a/internal/db/queries/duplicates.sql b/internal/db/queries/duplicates.sql index def52620..22c3b1fe 100644 --- a/internal/db/queries/duplicates.sql +++ b/internal/db/queries/duplicates.sql @@ -98,3 +98,55 @@ DELETE FROM duplicate_groups g AND (g.last_seen_sweep_id IS NULL OR (SELECT s.started_at FROM duplicate_sweeps s WHERE s.id = g.last_seen_sweep_id) < (SELECT s.started_at FROM duplicate_sweeps s WHERE s.id = sqlc.arg(sweep_id))); + +-- name: CountPendingDuplicateGroups :one +-- Proposals awaiting review. A group left with one member — its other tracks +-- deleted since the sweep — is no proposal at all and is not counted; the next +-- sweep retires it. +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; + +-- 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. +WITH page AS ( + SELECT g.id, g.tier, g.worst_bit_error_rate, g.detected_at + FROM duplicate_groups g + WHERE g.status = 'pending' + AND (SELECT count(*) FROM duplicate_group_members m WHERE m.group_id = g.id) >= 2 + ORDER BY g.detected_at DESC, g.id + LIMIT sqlc.arg(page_limit) OFFSET sqlc.arg(page_offset) +) +SELECT p.id AS group_id, + p.tier, + p.worst_bit_error_rate, + p.detected_at, + t.id AS track_id, + t.title, + artists.name AS artist_name, + albums.id AS album_id, + albums.title AS album_title, + t.file_path, + t.file_format, + t.file_size, + t.duration_ms, + t.added_at, + (SELECT count(*) FROM general_likes l WHERE l.track_id = t.id)::bigint AS like_count, + (SELECT count(*) FROM play_events e WHERE e.track_id = t.id)::bigint AS play_count + FROM page p + JOIN duplicate_group_members m ON m.group_id = p.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 + ORDER BY p.detected_at DESC, p.id, t.id; + +-- name: DismissDuplicateGroup :execrows +-- "These are not duplicates." Only a pending group can be dismissed; zero rows +-- means it was already resolved or no longer exists. +UPDATE duplicate_groups + SET status = 'dismissed', resolved_at = now() + WHERE id = sqlc.arg(id) AND status = 'pending'; diff --git a/internal/library/duplicate_survivor.go b/internal/library/duplicate_survivor.go new file mode 100644 index 00000000..6f90e64f --- /dev/null +++ b/internal/library/duplicate_survivor.go @@ -0,0 +1,77 @@ +package library + +import ( + "sort" + "strings" + "time" +) + +// SurvivorCandidate is what choosing which copy to keep needs to know about one +// member of a duplicate group. +type SurvivorCandidate struct { + TrackID string + FileFormat string + FileSize int64 + AddedAt time.Time +} + +// losslessFormats are the scanned extensions that are lossless by definition. +// m4a is left out on purpose: it holds either ALAC or AAC, and the scanner +// records only the extension, so calling it lossless would sometimes prefer an +// AAC copy over a FLAC one. +var losslessFormats = map[string]bool{"flac": true, "wav": true} + +// ProposeSurvivor picks which copy of a duplicate group to keep, and gives the +// reason in words the operator reads beside it. It is a default, not a verdict: +// the report shows it and the merge (#3911) lets the operator choose another. +// +// In order: +// 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 +// filled), so file size is the signal that actually exists +// 3. the copy in the library longest — the one most likely to carry the play +// history and likes, so the merge moves the least +// 4. the lowest track id, so the choice is stable between page loads +func ProposeSurvivor(cands []SurvivorCandidate) (trackID, reason string) { + if len(cands) == 0 { + return "", "" + } + ranked := append([]SurvivorCandidate(nil), cands...) + sort.SliceStable(ranked, func(i, j int) bool { return survivorBefore(ranked[i], ranked[j]) }) + best := ranked[0] + if len(ranked) == 1 { + return best.TrackID, "the only copy" + } + + // The reason names the first rule that separated the best copy from the + // runner-up — the rule that actually decided, not every rule it passed. + next := ranked[1] + switch { + case isLossless(best) != isLossless(next): + return best.TrackID, "lossless (" + strings.ToLower(best.FileFormat) + ")" + case best.FileSize != next.FileSize: + return best.TrackID, "largest file" + case !best.AddedAt.Equal(next.AddedAt): + return best.TrackID, "in the library longest" + default: + return best.TrackID, "copies are otherwise identical" + } +} + +func survivorBefore(a, b SurvivorCandidate) bool { + if isLossless(a) != isLossless(b) { + return isLossless(a) + } + if a.FileSize != b.FileSize { + return a.FileSize > b.FileSize + } + if !a.AddedAt.Equal(b.AddedAt) { + return a.AddedAt.Before(b.AddedAt) + } + return a.TrackID < b.TrackID +} + +func isLossless(c SurvivorCandidate) bool { + return losslessFormats[strings.ToLower(c.FileFormat)] +} diff --git a/internal/library/duplicate_survivor_test.go b/internal/library/duplicate_survivor_test.go new file mode 100644 index 00000000..4e770d7e --- /dev/null +++ b/internal/library/duplicate_survivor_test.go @@ -0,0 +1,91 @@ +package library + +import ( + "testing" + "time" +) + +func TestProposeSurvivor(t *testing.T) { + older := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC) + newer := older.Add(24 * time.Hour) + cases := []struct { + name string + cands []SurvivorCandidate + wantID string + wantReason string + }{ + { + // Lossless wins even against a much larger lossy file, and even + // when the lossy copy has been in the library longer. + name: "lossless beats larger and older", + cands: []SurvivorCandidate{ + {TrackID: "mp3", FileFormat: "mp3", FileSize: 90_000_000, AddedAt: older}, + {TrackID: "flac", FileFormat: "FLAC", FileSize: 30_000_000, AddedAt: newer}, + }, + wantID: "flac", wantReason: "lossless (flac)", + }, + { + // m4a may be AAC; it must not outrank an mp3 just for being m4a. + name: "m4a is not treated as lossless", + cands: []SurvivorCandidate{ + {TrackID: "m4a", FileFormat: "m4a", FileSize: 5_000_000, AddedAt: older}, + {TrackID: "mp3", FileFormat: "mp3", FileSize: 9_000_000, AddedAt: newer}, + }, + wantID: "mp3", wantReason: "largest file", + }, + { + name: "larger file wins among lossy copies", + cands: []SurvivorCandidate{ + {TrackID: "128k", FileFormat: "mp3", FileSize: 3_400_000, AddedAt: older}, + {TrackID: "320k", FileFormat: "mp3", FileSize: 8_600_000, AddedAt: newer}, + }, + wantID: "320k", wantReason: "largest file", + }, + { + // The #3885 pair: identical audio, sizes equal but for the tags. + name: "the longest-standing copy wins when size ties", + cands: []SurvivorCandidate{ + {TrackID: "www-02", FileFormat: "mp3", FileSize: 6_900_000, AddedAt: newer}, + {TrackID: "www-01", FileFormat: "mp3", FileSize: 6_900_000, AddedAt: older}, + }, + wantID: "www-01", wantReason: "in the library longest", + }, + { + name: "a full tie falls back to the lowest id, stably", + cands: []SurvivorCandidate{ + {TrackID: "b", FileFormat: "mp3", FileSize: 1, AddedAt: older}, + {TrackID: "a", FileFormat: "mp3", FileSize: 1, AddedAt: older}, + }, + wantID: "a", wantReason: "copies are otherwise identical", + }, + { + name: "one copy", + cands: []SurvivorCandidate{{TrackID: "only", FileFormat: "mp3", FileSize: 1, AddedAt: older}}, + wantID: "only", wantReason: "the only copy", + }, + } + 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) + } + }) + } +} + +// The reason must name the rule that decided. Across three copies that is the +// comparison between first and second place, not the first rule any pair +// differs on: here the lossy copy differs from the others by format, but the +// two FLACs are separated by size. +func TestProposeSurvivor_ReasonIsTheDecidingRule(t *testing.T) { + at := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC) + id, reason := ProposeSurvivor([]SurvivorCandidate{ + {TrackID: "mp3", FileFormat: "mp3", FileSize: 99_000_000, AddedAt: at}, + {TrackID: "flac-small", FileFormat: "flac", FileSize: 20_000_000, AddedAt: at}, + {TrackID: "flac-big", FileFormat: "flac", FileSize: 40_000_000, AddedAt: at}, + }) + if id != "flac-big" || reason != "largest file" { + t.Fatalf("got (%q, %q), want (flac-big, largest file)", id, reason) + } +} diff --git a/web/src/lib/api/admin.duplicates.test.ts b/web/src/lib/api/admin.duplicates.test.ts new file mode 100644 index 00000000..f3a2da1c --- /dev/null +++ b/web/src/lib/api/admin.duplicates.test.ts @@ -0,0 +1,30 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { dismissDuplicateGroup, listDuplicates, runDuplicateSweep } from './admin'; + +vi.mock('./client', () => ({ + api: { get: vi.fn(), post: vi.fn() } +})); + +import { api } from './client'; + +describe('admin duplicates API', () => { + beforeEach(() => vi.clearAllMocks()); + + it('listDuplicates GETs 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'); + }); + + it('runDuplicateSweep POSTs the trigger', async () => { + (api.post as unknown as ReturnType).mockResolvedValueOnce({ started: true }); + await runDuplicateSweep(); + expect(api.post).toHaveBeenCalledWith('/api/admin/library/duplicates/sweep', {}); + }); + + it('dismissDuplicateGroup POSTs to the group', async () => { + (api.post as unknown as ReturnType).mockResolvedValueOnce(undefined); + await dismissDuplicateGroup('g/1'); + expect(api.post).toHaveBeenCalledWith('/api/admin/library/duplicates/g%2F1/dismiss', {}); + }); +}); diff --git a/web/src/lib/api/admin.ts b/web/src/lib/api/admin.ts index f4e209e0..a560b7df 100644 --- a/web/src/lib/api/admin.ts +++ b/web/src/lib/api/admin.ts @@ -4,6 +4,7 @@ import { qk } from './queries'; import type { ActionResult, AdminMissingResponse, + AdminDuplicatesResponse, AdminPlaybackError, AdminQuarantineRow, LidarrConfig, @@ -695,6 +696,37 @@ export async function updateNetworkSettings(hops: number): Promise { + return api.get( + `/api/admin/library/duplicates?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), + staleTime: 30_000, + refetchInterval: 15_000 + }); +} + +export async function runDuplicateSweep(): Promise<{ started: boolean }> { + return api.post<{ started: boolean }>('/api/admin/library/duplicates/sweep', {}); +} + +export async function dismissDuplicateGroup(id: string): Promise { + await api.post(`/api/admin/library/duplicates/${encodeURIComponent(id)}/dismiss`, {}); +} + // Missing files (#2527) ----------------------------------------------------- export async function listMissingFiles( diff --git a/web/src/lib/api/queries.ts b/web/src/lib/api/queries.ts index 7e62f0f4..105a308e 100644 --- a/web/src/lib/api/queries.ts +++ b/web/src/lib/api/queries.ts @@ -56,6 +56,8 @@ 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, adminDiagnosticDevices: (userId?: string) => ['adminDiagnosticDevices', { userId: userId ?? 'all' }] as const, smtpConfig: () => ['smtpConfig'] as const, diff --git a/web/src/lib/api/types.ts b/web/src/lib/api/types.ts index 51cab7fa..2d5ecef0 100644 --- a/web/src/lib/api/types.ts +++ b/web/src/lib/api/types.ts @@ -418,3 +418,53 @@ export type AdminMissingResponse = { offset: number; groups: AdminMissingGroup[]; }; + +// Duplicates report (#3912) ------------------------------------------------- + +// One copy in a proposed duplicate group. like_count and play_count cover every +// user: they are what the operator weighs when choosing which copy to keep. +export type AdminDuplicateMember = { + track_id: string; + title: string; + artist_name: string; + album_id: string; + album_title: string; + file_path: string; + file_format: string; + file_size: number; + duration_sec: number; + added_at: string; + like_count: number; + play_count: number; +}; + +// 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 = { + id: string; + tier: 'exact' | 'acoustic'; + worst_bit_error_rate: number | null; + detected_at: string; + survivor_track_id: string; + survivor_reason: string; + members: AdminDuplicateMember[]; +}; + +export type AdminDuplicateSweep = { + state: 'never' | 'running' | 'finished'; + started_at: string | null; + finished_at: string | null; + candidates: number | null; + groups_found: number | null; + oversize_clusters: number | null; + error_message: string | null; +}; + +export type AdminDuplicatesResponse = { + sweep: AdminDuplicateSweep; + fingerprints: { total: number; fingerprinted: number; rejected: number; pending: number }; + total: number; + limit: number; + offset: number; + groups: AdminDuplicateGroup[]; +}; diff --git a/web/src/lib/components/AdminTabs.svelte b/web/src/lib/components/AdminTabs.svelte index 1be389cf..9dff0ef1 100644 --- a/web/src/lib/components/AdminTabs.svelte +++ b/web/src/lib/components/AdminTabs.svelte @@ -9,6 +9,7 @@ { href: '/admin/requests', label: 'Requests' }, { href: '/admin/quarantine', label: 'Quarantine' }, { href: '/admin/missing-files', label: 'Missing files' }, + { href: '/admin/duplicates', label: 'Duplicates' }, { href: '/admin/playback-errors', label: 'Playback errors' }, { href: '/admin/diagnostics', label: 'Diagnostics' }, { href: '/admin/tuning', label: 'Tuning' }, diff --git a/web/src/lib/components/AdminTabs.test.ts b/web/src/lib/components/AdminTabs.test.ts index 85065bbc..c6e42c89 100644 --- a/web/src/lib/components/AdminTabs.test.ts +++ b/web/src/lib/components/AdminTabs.test.ts @@ -52,7 +52,7 @@ describe('AdminTabs', () => { ); }); - test('renders all nine tabs in order', () => { + test('renders all ten tabs in order', () => { state.pageUrl = new URL('http://localhost/admin'); render(AdminTabs); const links = screen.getAllByRole('link'); @@ -64,6 +64,9 @@ describe('AdminTabs', () => { // Missing files sits with Quarantine and Playback errors: the three // surfaces that show tracks needing an operator's attention. 'Missing files', + // Duplicates follows Missing files: both are library-health reports on + // what the library holds, rather than a queue of user reports. + 'Duplicates', 'Playback errors', 'Diagnostics', 'Tuning', diff --git a/web/src/lib/styles/error-copy.json b/web/src/lib/styles/error-copy.json index 80b94860..260b3060 100644 --- a/web/src/lib/styles/error-copy.json +++ b/web/src/lib/styles/error-copy.json @@ -42,6 +42,8 @@ "track_not_found": "That track no longer exists.", "library_not_writable": "The music library isn't writable by the server.", "file_delete_failed": "The file couldn't be deleted.", + "sweep_in_progress": "A duplicate sweep is already running.", + "duplicate_group_not_pending": "That group has already been resolved.", "album_not_found": "That album no longer exists.", "artist_not_found": "That artist no longer exists.", "playlist_not_found": "That playlist no longer exists.", diff --git a/web/src/routes/admin/duplicates/+page.svelte b/web/src/routes/admin/duplicates/+page.svelte new file mode 100644 index 00000000..2e0881cb --- /dev/null +++ b/web/src/routes/admin/duplicates/+page.svelte @@ -0,0 +1,251 @@ + + +{pageTitle('Admin · Duplicates')} + +
+
+
+
+

Duplicates

+ {#if total > 0} + + {total} + + {/if} +
+ +
+

+ Tracks that hold the same recording more than once. Review each group; nothing is + merged or removed from here. +

+
+ + {#if sweep} +

+ {#if sweep.state === 'never'} + The duplicate sweep hasn't run yet. + {:else if sweep.state === 'running'} + Sweeping now — started {relativeTime(sweep.started_at ?? '')}. + {:else} + Last swept {relativeTime(sweep.finished_at ?? '')}, comparing + {(sweep.candidates ?? 0).toLocaleString()} tracks. + {#if sweep.error_message} + It stopped early: {sweep.error_message} + {/if} + {/if} + {#if prints && prints.pending > 0} + {prints.pending.toLocaleString()} tracks are still waiting for a fingerprint and join + the comparison once they have one. + {/if} +

+ {/if} + + {#if query.isPending} +

Loading duplicates…

+ {:else if query.isError} +

Couldn't load the duplicates report.

+ {:else if groups.length === 0} + +
+ + {#if prints && prints.total > 0 && prints.fingerprinted === 0} +

Nothing to compare yet.

+

+ The library is still being fingerprinted — {prints.pending.toLocaleString()} tracks to go. + Duplicates appear here as the sweep finds them. +

+ {:else if sweep?.state === 'never'} +

The sweep hasn't run yet.

+

+ It runs on its own whenever new fingerprints arrive, or now if you start it. +

+ {:else if sweep?.state === 'running'} +

Sweeping…

+

Anything it finds will appear here.

+ {:else} +

No duplicates found.

+

+ A group appears when two tracks hold identical audio, or the same recording in a + different encoding. Groups you dismiss don't come back. +

+ {/if} +
+ {:else} +
    + {#each groups as group (group.id)} +
  • +
    +
    +

    {tierLabel(group)}

    +

    Found {relativeTime(group.detected_at)}

    +
    + +
    + +
      + {#each group.members as m (m.track_id)} + {@const keep = m.track_id === group.survivor_track_id} +
    • +
      +
      + {m.title} + {#if keep} + + + Keep · {group.survivor_reason} + + {/if} +
      +
      + {m.artist_name} · {m.album_title} +
      +
      + {m.file_path} +
      +
      +
      +
      {m.file_format.toUpperCase()} · {sizeLabel(m.file_size)} · {durationLabel(m.duration_sec)}
      +
      {historyLabel(m)}
      +
      +
      • + {/each} +
    +
    • + {/each} +
+ + {#if hasMore || offset > 0} + + {/if} + {/if} +
diff --git a/web/src/routes/admin/duplicates/duplicates.test.ts b/web/src/routes/admin/duplicates/duplicates.test.ts new file mode 100644 index 00000000..c8701d30 --- /dev/null +++ b/web/src/routes/admin/duplicates/duplicates.test.ts @@ -0,0 +1,147 @@ +import { afterEach, describe, expect, test, vi } from 'vitest'; +import { fireEvent, render, screen } from '@testing-library/svelte'; +import { mockQuery } from '../../../test-utils/query'; +import type { AdminDuplicatesResponse } from '$lib/api/types'; + +vi.mock('$lib/api/admin', () => ({ + createDuplicatesQuery: vi.fn(), + runDuplicateSweep: vi.fn().mockResolvedValue({ started: true }), + dismissDuplicateGroup: vi.fn().mockResolvedValue(undefined) +})); + +import AdminDuplicatesPage from './+page.svelte'; +import { createDuplicatesQuery, dismissDuplicateGroup, runDuplicateSweep } from '$lib/api/admin'; + +const HOUR = 3_600_000; +const ago = (ms: number) => new Date(Date.now() - ms).toISOString(); + +const finishedSweep = { + state: 'finished' as const, + started_at: ago(2 * HOUR), + finished_at: ago(HOUR), + candidates: 1200, + groups_found: 1, + oversize_clusters: 0, + error_message: null +}; +const allFingerprinted = { total: 1200, fingerprinted: 1200, rejected: 0, pending: 0 }; + +function member(id: string, extra: Partial = {}) { + return { + track_id: id, + title: 'WWW', + artist_name: 'Moe Shop', + album_id: 'al-1', + album_title: 'WWW', + file_path: `/music/Moe Shop/WWW (2020)/${id}.mp3`, + file_format: 'mp3', + file_size: 6_900_000, + duration_sec: 215, + added_at: ago(500 * HOUR), + like_count: 0, + play_count: 0, + ...extra + }; +} + +function response(over: Partial = {}): AdminDuplicatesResponse { + return { + sweep: finishedSweep, + fingerprints: allFingerprinted, + total: 1, + limit: 25, + offset: 0, + groups: [ + { + id: 'g-1', + tier: 'exact', + worst_bit_error_rate: null, + detected_at: ago(HOUR), + survivor_track_id: 'www-01', + survivor_reason: 'in the library longest', + members: [member('www-01', { like_count: 2, play_count: 14 }), member('www-02')] + } + ], + ...over + }; +} + +function renderWith(data: AdminDuplicatesResponse | undefined) { + vi.mocked(createDuplicatesQuery).mockReturnValue( + mockQuery({ data }) as ReturnType + ); + return render(AdminDuplicatesPage); +} + +function text(el: HTMLElement): string { + return (el.textContent ?? '').replace(/\s+/g, ' ').trim(); +} + +afterEach(() => vi.clearAllMocks()); + +describe('admin duplicates', () => { + test('shows each group with its members, what they carry, and the copy to keep', () => { + renderWith(response()); + expect(screen.getAllByTestId('duplicate-group')).toHaveLength(1); + expect(screen.getAllByTestId('duplicate-member')).toHaveLength(2); + expect(text(screen.getByTestId('duplicate-tier'))).toBe('Identical audio'); + expect(text(screen.getByTestId('survivor-badge'))).toBe('Keep · in the library longest'); + const history = screen.getAllByTestId('member-history').map(text); + expect(history).toEqual(['2 likes · 14 plays', 'no likes or plays']); + }); + + // The badge must sit on the proposed survivor, not merely appear somewhere. + test('the keep badge is on the survivor row', () => { + renderWith(response()); + const rows = screen.getAllByTestId('duplicate-member'); + expect(rows[0].querySelector('[data-testid="survivor-badge"]')).not.toBeNull(); + expect(rows[1].querySelector('[data-testid="survivor-badge"]')).toBeNull(); + }); + + test('an acoustic group reads as a match percentage', () => { + const r = response(); + r.groups[0] = { ...r.groups[0], tier: 'acoustic', worst_bit_error_rate: 0.04 }; + renderWith(r); + expect(text(screen.getByTestId('duplicate-tier'))).toBe('Same recording · 96% match'); + }); + + // Three states all show zero groups. Each must say which it is. + test('empty while still fingerprinting says so', () => { + renderWith( + response({ + groups: [], + total: 0, + fingerprints: { total: 1200, fingerprinted: 0, rejected: 0, pending: 1200 } + }) + ); + expect(text(screen.getByTestId('empty-state'))).toContain('still being fingerprinted'); + }); + + test('empty before any sweep says the sweep has not run', () => { + renderWith( + response({ + groups: [], + total: 0, + sweep: { ...finishedSweep, state: 'never', started_at: null, finished_at: null, candidates: null } + }) + ); + expect(text(screen.getByTestId('empty-state'))).toContain("hasn't run yet"); + }); + + test('empty after a sweep says no duplicates were found', () => { + renderWith(response({ groups: [], total: 0 })); + expect(text(screen.getByTestId('empty-state'))).toContain('No duplicates found'); + }); + + test('Not duplicates dismisses that group', async () => { + renderWith(response()); + await fireEvent.click(screen.getByRole('button', { name: 'Not duplicates' })); + expect(dismissDuplicateGroup).toHaveBeenCalledWith('g-1'); + }); + + test('Sweep now starts a sweep', async () => { + renderWith(response()); + await fireEvent.click(screen.getByRole('button', { name: 'Sweep now' })); + expect(runDuplicateSweep).toHaveBeenCalledTimes(1); + }); +});