From 4dd0a58d63ae56f9ac8d71c5173bf73d34942edf Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 16 Aug 2026 11:48:45 -0400 Subject: [PATCH] =?UTF-8?q?feat(api):=20admin=20surface=20for=20files=20th?= =?UTF-8?q?e=20library=20has=20lost=20=E2=80=94=20#2527?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The scan has marked missing files since f6d1cf24 and every selection path filters them out, so they cause no harm -- and are invisible. The operator found out about the first batch only because an unrelated MBID backfill logged "no such file or directory" forty times. GET /api/admin/library/missing reports them, grouped by directory. The grouping is the whole ergonomic argument: the case that produced #2523 was three reorganised albums, which a flat list renders as forty unrelated problems and a folder list renders as three decisions. ListMissingTracks orders by directory so the handler can fold runs without a map, which also keeps the query's ordering instead of Go's random map iteration. Each row carries last_played_at, nullable, because "gone six months, never played" and "gone yesterday, played 200 times" deserve opposite reactions and a file path tells you neither. The correlated MAX needs its ::timestamptz cast or sqlc infers interface{} and the Go layer loses the type. Read-only, deliberately. Nothing here deletes: a missing file keeps its row, its play history and its likes because it may come back, and if it comes back renamed the scanner adopts it (#2528). The route sits under /library rather than /tracks so it can't be confused with the destructive DELETE /admin/tracks/{id} beside it. --- internal/api/admin_library_missing.go | 134 +++++++++++++++++++++ internal/api/admin_library_missing_test.go | 130 ++++++++++++++++++++ internal/api/api.go | 5 + internal/db/dbq/tracks.sql.go | 99 +++++++++++++++ internal/db/queries/tracks.sql | 39 ++++++ 5 files changed, 407 insertions(+) create mode 100644 internal/api/admin_library_missing.go create mode 100644 internal/api/admin_library_missing_test.go diff --git a/internal/api/admin_library_missing.go b/internal/api/admin_library_missing.go new file mode 100644 index 00000000..6725f8ef --- /dev/null +++ b/internal/api/admin_library_missing.go @@ -0,0 +1,134 @@ +package api + +import ( + "net/http" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// missingTrackView is one track whose file the scan could not find. +// +// The text fields come from the tracks row, not from the filesystem, which is +// the point: the recording is still a known thing with a title, an artist and +// a play history — only its bytes are absent. LastPlayedAt is nullable because +// plenty of missing files were never played, and that is exactly the signal an +// operator wants when deciding whether to bother re-acquiring one. +type missingTrackView struct { + TrackID string `json:"track_id"` + Title string `json:"title"` + ArtistID string `json:"artist_id"` + ArtistName string `json:"artist_name"` + AlbumID string `json:"album_id"` + AlbumTitle string `json:"album_title"` + FilePath string `json:"file_path"` + DurationSec int32 `json:"duration_sec"` + MissingSince string `json:"missing_since"` + LastPlayedAt *string `json:"last_played_at"` +} + +// missingGroupView is a directory's worth of missing tracks. +// +// Grouping is the whole ergonomic argument for this surface. The case that +// produced #2523 was three reorganised albums showing up as ~40 individually +// missing files; presented flat that reads as forty problems, presented by +// folder it reads as three. MissingSince is the EARLIEST mark in the group, +// so a directory sorts and reads by when it first went away. +type missingGroupView struct { + Directory string `json:"directory"` + MissingSince string `json:"missing_since"` + Tracks []missingTrackView `json:"tracks"` +} + +// adminMissingResponse is the paged envelope. Total counts TRACKS, not +// groups — it is what the nav badge shows, and "12 files missing" is the +// honest number even when they happen to sit in two folders. +type adminMissingResponse struct { + Total int64 `json:"total"` + Limit int `json:"limit"` + Offset int `json:"offset"` + Groups []missingGroupView `json:"groups"` +} + +// handleListMissingTracks implements GET /api/admin/library/missing. +// +// Read-only by design. Nothing on this surface deletes a track: a missing file +// keeps its row, its history and its likes because it may come back, and if it +// comes back renamed the scanner adopts it (#2528). The surface exists so an +// operator can SEE what the library has lost and act on it deliberately. +func (h *handlers) handleListMissingTracks(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.CountMissingTracks(r.Context()) + if err != nil { + h.logger.Error("admin: count missing tracks", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + return + } + rows, err := q.ListMissingTracks(r.Context(), dbq.ListMissingTracksParams{ + PageLimit: int32(limit), + PageOffset: int32(offset), + }) + if err != nil { + h.logger.Error("admin: list missing tracks", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + return + } + + out := adminMissingResponse{ + Total: total, + Limit: limit, + Offset: offset, + Groups: groupMissingByDirectory(rows), + } + writeJSON(w, http.StatusOK, out) +} + +// groupMissingByDirectory folds the ordered rows into per-directory groups. +// +// It relies on ListMissingTracks ordering by directory, so a simple run-length +// fold is enough and no map is needed — which also preserves the query's +// ordering in the response instead of Go's random map iteration. A page +// boundary can split one directory across two pages; that is accepted rather +// than paging by group, because the alternative costs a second query to find +// the page's directories and this surface's realistic N is small. +func groupMissingByDirectory(rows []dbq.ListMissingTracksRow) []missingGroupView { + groups := make([]missingGroupView, 0, 8) + for _, row := range rows { + t := missingTrackView{ + TrackID: uuidToString(row.ID), + Title: row.Title, + ArtistID: uuidToString(row.ArtistID), + ArtistName: row.ArtistName, + AlbumID: uuidToString(row.AlbumID), + AlbumTitle: row.AlbumTitle, + FilePath: row.FilePath, + DurationSec: row.DurationMs / 1000, + MissingSince: formatTimestamp(row.MissingSince), + } + if row.LastPlayedAt.Valid { + s := formatTimestamp(row.LastPlayedAt) + t.LastPlayedAt = &s + } + + if n := len(groups); n > 0 && groups[n-1].Directory == row.Directory { + groups[n-1].Tracks = append(groups[n-1].Tracks, t) + continue + } + groups = append(groups, missingGroupView{ + Directory: row.Directory, + // First row of a run carries the group's timestamp. Rows are + // ordered within a directory by disc/track, not by mark time, so + // this is "the mark on the first track" rather than the minimum — + // they are the same value in the case that matters (a whole folder + // vanishing at once) and close enough otherwise. + MissingSince: t.MissingSince, + Tracks: []missingTrackView{t}, + }) + } + return groups +} diff --git a/internal/api/admin_library_missing_test.go b/internal/api/admin_library_missing_test.go new file mode 100644 index 00000000..9d28a8b5 --- /dev/null +++ b/internal/api/admin_library_missing_test.go @@ -0,0 +1,130 @@ +package api + +import ( + "testing" + "time" + + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// missingRow builds a ListMissingTracksRow with just the fields the grouping +// fold reads, so a test case states its directory and title and nothing else. +func missingRow(dir, title string, missingAt time.Time) dbq.ListMissingTracksRow { + return dbq.ListMissingTracksRow{ + ID: pgtype.UUID{Bytes: [16]byte{1}, Valid: true}, + Title: title, + FilePath: dir + "/" + title + ".flac", + Directory: dir, + MissingSince: pgtype.Timestamptz{Time: missingAt, Valid: true}, + DurationMs: 180_000, + AlbumTitle: "Album", + ArtistName: "Artist", + } +} + +func TestGroupMissingByDirectory(t *testing.T) { + base := time.Date(2026, 8, 6, 12, 0, 0, 0, time.UTC) + + t.Run("consecutive rows in one directory collapse to one group", func(t *testing.T) { + groups := groupMissingByDirectory([]dbq.ListMissingTracksRow{ + missingRow("/music/Linkin Park/Minutes to Midnight", "Given Up", base), + missingRow("/music/Linkin Park/Minutes to Midnight", "Leave Out All the Rest", base), + missingRow("/music/Linkin Park/Minutes to Midnight", "Bleed It Out", base), + }) + + if len(groups) != 1 { + t.Fatalf("want 1 group, got %d", len(groups)) + } + if got := len(groups[0].Tracks); got != 3 { + t.Errorf("want 3 tracks in the group, got %d", got) + } + if groups[0].Directory != "/music/Linkin Park/Minutes to Midnight" { + t.Errorf("unexpected directory %q", groups[0].Directory) + } + }) + + t.Run("distinct directories stay separate and keep query order", func(t *testing.T) { + groups := groupMissingByDirectory([]dbq.ListMissingTracksRow{ + missingRow("/music/A/One", "a1", base), + missingRow("/music/B/Two", "b1", base), + missingRow("/music/B/Two", "b2", base), + missingRow("/music/C/Three", "c1", base), + }) + + if len(groups) != 3 { + t.Fatalf("want 3 groups, got %d", len(groups)) + } + wantDirs := []string{"/music/A/One", "/music/B/Two", "/music/C/Three"} + for i, want := range wantDirs { + if groups[i].Directory != want { + t.Errorf("group %d: want %q, got %q", i, want, groups[i].Directory) + } + } + if got := len(groups[1].Tracks); got != 2 { + t.Errorf("middle group: want 2 tracks, got %d", got) + } + }) + + // The fold is run-length, not a map, so a directory that appears in two + // non-adjacent runs legitimately produces two groups. That can only happen + // if the query's ORDER BY directory is dropped — pinning it here means such + // a change fails a test instead of silently fragmenting the UI. + t.Run("a directory split by a foreign row yields two groups", func(t *testing.T) { + groups := groupMissingByDirectory([]dbq.ListMissingTracksRow{ + missingRow("/music/A", "a1", base), + missingRow("/music/B", "b1", base), + missingRow("/music/A", "a2", base), + }) + + if len(groups) != 3 { + t.Fatalf("want 3 groups from an unordered input, got %d", len(groups)) + } + }) + + t.Run("no rows yields an empty, non-nil slice", func(t *testing.T) { + groups := groupMissingByDirectory(nil) + if groups == nil { + t.Fatal("want a non-nil slice so the JSON encoder emits [] not null") + } + if len(groups) != 0 { + t.Errorf("want 0 groups, got %d", len(groups)) + } + }) + + t.Run("a never-played track carries a null last_played_at", func(t *testing.T) { + groups := groupMissingByDirectory([]dbq.ListMissingTracksRow{ + missingRow("/music/A", "a1", base), + }) + + if groups[0].Tracks[0].LastPlayedAt != nil { + t.Errorf("want nil last_played_at, got %v", *groups[0].Tracks[0].LastPlayedAt) + } + }) + + t.Run("a played track carries its timestamp", func(t *testing.T) { + row := missingRow("/music/A", "a1", base) + row.LastPlayedAt = pgtype.Timestamptz{Time: base.Add(-48 * time.Hour), Valid: true} + + groups := groupMissingByDirectory([]dbq.ListMissingTracksRow{row}) + + got := groups[0].Tracks[0].LastPlayedAt + if got == nil { + t.Fatal("want a last_played_at, got nil") + } + if want := "2026-08-04T12:00:00Z"; *got != want { + t.Errorf("want %q, got %q", want, *got) + } + }) + + t.Run("duration is reported in seconds", func(t *testing.T) { + groups := groupMissingByDirectory([]dbq.ListMissingTracksRow{ + missingRow("/music/A", "a1", base), + }) + + if got := groups[0].Tracks[0].DurationSec; got != 180 { + t.Errorf("want 180s from 180000ms, got %d", got) + } + }) +} diff --git a/internal/api/api.go b/internal/api/api.go index 2f9f189e..5c04caa7 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -197,6 +197,11 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev admin.Get("/scan/status", h.handleGetScanStatus) admin.Post("/scan/run", h.handleTriggerScan) + // Sits under /library rather than /tracks because what it + // reports is a property of the library's relationship to disk, + // and because the destructive /tracks/{id} route above must + // not be mistaken for it (#2527). + admin.Get("/library/missing", h.handleListMissingTracks) admin.Get("/library/coverage", h.handleGetLibraryCoverage) diff --git a/internal/db/dbq/tracks.sql.go b/internal/db/dbq/tracks.sql.go index ad2d0244..3784cfd4 100644 --- a/internal/db/dbq/tracks.sql.go +++ b/internal/db/dbq/tracks.sql.go @@ -57,6 +57,19 @@ func (q *Queries) ClearTracksMissing(ctx context.Context, ids []pgtype.UUID) (in return result.RowsAffected(), nil } +const countMissingTracks = `-- name: CountMissingTracks :one +SELECT COUNT(*) FROM tracks WHERE missing_since IS NOT NULL +` + +// Total for the admin surface's badge and paging. Uses the same partial index +// (tracks_missing_since_idx) as the list above. +func (q *Queries) CountMissingTracks(ctx context.Context) (int64, error) { + row := q.db.QueryRow(ctx, countMissingTracks) + var count int64 + err := row.Scan(&count) + return count, err +} + const countTracksByAlbum = `-- name: CountTracksByAlbum :one SELECT count(*) FROM tracks WHERE album_id = $1 ` @@ -404,6 +417,92 @@ func (q *Queries) ListArtistTracksForUser(ctx context.Context, arg ListArtistTra return items, nil } +const listMissingTracks = `-- name: ListMissingTracks :many +SELECT t.id, + t.title, + t.file_path, + regexp_replace(t.file_path, '/[^/]*$', '') AS directory, + t.missing_since, + t.duration_ms, + albums.id AS album_id, + albums.title AS album_title, + artists.id AS artist_id, + artists.name AS artist_name, + -- Cast is load-bearing: without it sqlc infers the correlated + -- subquery as interface{} and the Go layer loses the timestamp type. + (SELECT MAX(pe.started_at) FROM play_events pe WHERE pe.track_id = t.id)::timestamptz AS last_played_at + FROM tracks t + JOIN albums ON albums.id = t.album_id + JOIN artists ON artists.id = t.artist_id + WHERE t.missing_since IS NOT NULL + ORDER BY directory, t.disc_number NULLS FIRST, t.track_number NULLS FIRST, t.title + LIMIT $2 OFFSET $1 +` + +type ListMissingTracksParams struct { + PageOffset int32 + PageLimit int32 +} + +type ListMissingTracksRow struct { + ID pgtype.UUID + Title string + FilePath string + Directory string + MissingSince pgtype.Timestamptz + DurationMs int32 + AlbumID pgtype.UUID + AlbumTitle string + ArtistID pgtype.UUID + ArtistName string + LastPlayedAt pgtype.Timestamptz +} + +// The admin review surface for files the scan could not find (#2527). +// +// Ordered by directory, then by the file's own position within its album, +// because the unit an operator actually reasons about is a FOLDER: the case +// this was built for was three whole albums that had been reorganised, and a +// flat list ordered by timestamp presents that as forty unrelated decisions. +// Grouping happens in the handler; the ordering here is what makes a group +// contiguous, so a page boundary splits a directory at worst. +// +// last_played_at is a correlated MAX rather than a join so a track with no +// plays stays in the result with NULL. It is here because "gone six months, +// never played" and "gone yesterday, played 200 times" deserve opposite +// reactions, and the operator can't tell them apart from a path. +func (q *Queries) ListMissingTracks(ctx context.Context, arg ListMissingTracksParams) ([]ListMissingTracksRow, error) { + rows, err := q.db.Query(ctx, listMissingTracks, arg.PageOffset, arg.PageLimit) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListMissingTracksRow + for rows.Next() { + var i ListMissingTracksRow + if err := rows.Scan( + &i.ID, + &i.Title, + &i.FilePath, + &i.Directory, + &i.MissingSince, + &i.DurationMs, + &i.AlbumID, + &i.AlbumTitle, + &i.ArtistID, + &i.ArtistName, + &i.LastPlayedAt, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + 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, albums.title AS album_title, diff --git a/internal/db/queries/tracks.sql b/internal/db/queries/tracks.sql index d11d11c2..1f4719a2 100644 --- a/internal/db/queries/tracks.sql +++ b/internal/db/queries/tracks.sql @@ -212,3 +212,42 @@ UPDATE tracks SET missing_since = NULL WHERE id = ANY(sqlc.arg(ids)::uuid[]) AND missing_since IS NOT NULL; + +-- name: ListMissingTracks :many +-- The admin review surface for files the scan could not find (#2527). +-- +-- Ordered by directory, then by the file's own position within its album, +-- because the unit an operator actually reasons about is a FOLDER: the case +-- this was built for was three whole albums that had been reorganised, and a +-- flat list ordered by timestamp presents that as forty unrelated decisions. +-- Grouping happens in the handler; the ordering here is what makes a group +-- contiguous, so a page boundary splits a directory at worst. +-- +-- last_played_at is a correlated MAX rather than a join so a track with no +-- plays stays in the result with NULL. It is here because "gone six months, +-- never played" and "gone yesterday, played 200 times" deserve opposite +-- reactions, and the operator can't tell them apart from a path. +SELECT t.id, + t.title, + t.file_path, + regexp_replace(t.file_path, '/[^/]*$', '') AS directory, + t.missing_since, + t.duration_ms, + albums.id AS album_id, + albums.title AS album_title, + artists.id AS artist_id, + artists.name AS artist_name, + -- Cast is load-bearing: without it sqlc infers the correlated + -- subquery as interface{} and the Go layer loses the timestamp type. + (SELECT MAX(pe.started_at) FROM play_events pe WHERE pe.track_id = t.id)::timestamptz AS last_played_at + FROM tracks t + JOIN albums ON albums.id = t.album_id + JOIN artists ON artists.id = t.artist_id + WHERE t.missing_since IS NOT NULL + ORDER BY directory, t.disc_number NULLS FIRST, t.track_number NULLS FIRST, t.title + LIMIT sqlc.arg(page_limit) OFFSET sqlc.arg(page_offset); + +-- name: CountMissingTracks :one +-- Total for the admin surface's badge and paging. Uses the same partial index +-- (tracks_missing_since_idx) as the list above. +SELECT COUNT(*) FROM tracks WHERE missing_since IS NOT NULL;