diff --git a/internal/db/dbq/browse.sql.go b/internal/db/dbq/browse.sql.go index be5de0f4..bf428524 100644 --- a/internal/db/dbq/browse.sql.go +++ b/internal/db/dbq/browse.sql.go @@ -18,6 +18,7 @@ WHERE EXISTS ( FROM tracks JOIN LATERAL regexp_split_to_table(coalesce(tracks.genre, ''), '[;,]') AS g(genre) ON true WHERE tracks.album_id = albums.id + AND tracks.missing_since IS NULL AND trim(g.genre) = trim($1::text) ) ` @@ -97,6 +98,7 @@ WHERE EXISTS ( FROM tracks JOIN LATERAL regexp_split_to_table(coalesce(tracks.genre, ''), '[;,]') AS g(genre) ON true WHERE tracks.album_id = albums.id + AND tracks.missing_since IS NULL AND trim(g.genre) = trim($1::text) ) ORDER BY albums.sort_title, albums.id @@ -219,6 +221,7 @@ SELECT DISTINCT trim(g.genre) AS genre FROM tracks JOIN LATERAL regexp_split_to_table(coalesce(tracks.genre, ''), '[;,]') AS g(genre) ON true WHERE tracks.album_id = $1 AND trim(g.genre) <> '' + AND tracks.missing_since IS NULL ORDER BY trim(g.genre) ` @@ -251,6 +254,7 @@ SELECT DISTINCT trim(g.genre) AS genre FROM tracks JOIN LATERAL regexp_split_to_table(coalesce(tracks.genre, ''), '[;,]') AS g(genre) ON true WHERE tracks.artist_id = $1 AND trim(g.genre) <> '' + AND tracks.missing_since IS NULL ORDER BY trim(g.genre) ` @@ -278,10 +282,12 @@ func (q *Queries) ListGenresForArtist(ctx context.Context, artistID pgtype.UUID) } const listGenresWithCount = `-- name: ListGenresWithCount :many + SELECT trim(g.genre) AS genre, COUNT(DISTINCT tracks.id)::bigint AS track_count FROM tracks JOIN LATERAL regexp_split_to_table(coalesce(tracks.genre, ''), '[;,]') AS g(genre) ON true WHERE trim(g.genre) <> '' + AND tracks.missing_since IS NULL GROUP BY trim(g.genre) ORDER BY track_count DESC, trim(g.genre) ` @@ -291,6 +297,17 @@ type ListGenresWithCountRow struct { TrackCount int64 } +// Every query in this file filters `tracks.missing_since IS NULL` (#2523). +// A row whose file has vanished keeps its genre forever — the scanner walks the +// filesystem, so it never revisits a path that no longer exists — which is how +// pre-#2499 welded genres survived a full re-scan and kept showing in the index. +// Browsing is a way of finding something to play, so a track that cannot play +// should not shape it. +// +// Year queries below join albums only and are deliberately left alone: an album +// is still a real release even if some of its tracks are gone. An album whose +// EVERY track is missing will linger on the year axis; that's a narrower case, +// tracked with the rest of the cleanup work. // Genre browse index (#367). // // Genres live inline on tracks.genre as a delimited string, so this splits on diff --git a/internal/db/dbq/discover.sql.go b/internal/db/dbq/discover.sql.go index 69161ec1..7abd362a 100644 --- a/internal/db/dbq/discover.sql.go +++ b/internal/db/dbq/discover.sql.go @@ -15,7 +15,8 @@ const listCrossUserLikedTracksForDiscover = `-- name: ListCrossUserLikedTracksFo SELECT t.id, t.album_id, t.artist_id FROM general_likes gl JOIN tracks t ON t.id = gl.track_id - WHERE gl.user_id != $1 + WHERE t.missing_since IS NULL -- #2523: never offer a file that is gone + AND gl.user_id != $1 AND NOT EXISTS ( SELECT 1 FROM play_events pe WHERE pe.user_id = $1 @@ -95,7 +96,8 @@ dormant_artists AS ( SELECT t.id, t.album_id, t.artist_id FROM tracks t JOIN dormant_artists da ON da.id = t.artist_id - WHERE NOT EXISTS ( + WHERE t.missing_since IS NULL -- #2523: never offer a file that is gone + AND NOT EXISTS ( SELECT 1 FROM play_events pe WHERE pe.user_id = $1 AND pe.track_id = t.id @@ -159,7 +161,8 @@ func (q *Queries) ListDormantArtistTracksForDiscover(ctx context.Context, arg Li const listRandomUnheardTracksForDiscover = `-- name: ListRandomUnheardTracksForDiscover :many SELECT t.id, t.album_id, t.artist_id FROM tracks t - WHERE NOT EXISTS ( + WHERE t.missing_since IS NULL -- #2523: never offer a file that is gone + AND NOT EXISTS ( SELECT 1 FROM play_events pe WHERE pe.user_id = $1 AND pe.track_id = t.id @@ -217,7 +220,8 @@ SELECT t.id, t.album_id, t.artist_id FROM tracks t JOIN LATERAL regexp_split_to_table(coalesce(t.genre, ''), '[;,]') AS g_split(g) ON true JOIN taste_profile_tags nt ON nt.user_id = $1 AND trim(g_split.g) = nt.tag - WHERE nt.weight > 0 + WHERE t.missing_since IS NULL -- #2523: never offer a file that is gone + AND nt.weight > 0 AND trim(g_split.g) <> '' AND NOT EXISTS ( SELECT 1 FROM play_events pe diff --git a/internal/db/dbq/events.sql.go b/internal/db/dbq/events.sql.go index 837edfe1..64b35b72 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 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 FROM tracks t JOIN play_events pe ON pe.track_id = t.id WHERE pe.session_id = $1 AND pe.started_at < $2 @@ -306,6 +306,7 @@ func (q *Queries) ListRecentSessionTracks(ctx context.Context, arg ListRecentSes &i.TagSource, &i.TagSourcesVersion, &i.TagReadVersion, + &i.MissingSince, ); err != nil { return nil, err } diff --git a/internal/db/dbq/history.sql.go b/internal/db/dbq/history.sql.go index ef652a45..26f99fc9 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.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, artists.name AS artist_name FROM play_events pe @@ -80,6 +80,7 @@ func (q *Queries) ListUserHistory(ctx context.Context, arg ListUserHistoryParams &i.Track.TagSource, &i.Track.TagSourcesVersion, &i.Track.TagReadVersion, + &i.Track.MissingSince, &i.AlbumTitle, &i.ArtistName, ); err != nil { diff --git a/internal/db/dbq/likes.sql.go b/internal/db/dbq/likes.sql.go index 369eddb1..f5eaa5e6 100644 --- a/internal/db/dbq/likes.sql.go +++ b/internal/db/dbq/likes.sql.go @@ -259,7 +259,7 @@ 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 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 FROM tracks t JOIN general_likes l ON l.track_id = t.id WHERE l.user_id = $1 ORDER BY l.liked_at DESC @@ -300,6 +300,7 @@ func (q *Queries) ListLikedTrackRows(ctx context.Context, arg ListLikedTrackRows &i.TagSource, &i.TagSourcesVersion, &i.TagReadVersion, + &i.MissingSince, ); err != nil { return nil, err } diff --git a/internal/db/dbq/models.go b/internal/db/dbq/models.go index e8ae8630..e4e01981 100644 --- a/internal/db/dbq/models.go +++ b/internal/db/dbq/models.go @@ -643,6 +643,7 @@ type Track struct { TagSource *string TagSourcesVersion int32 TagReadVersion int16 + MissingSince pgtype.Timestamptz } type TrackSimilarity struct { diff --git a/internal/db/dbq/recommendation.sql.go b/internal/db/dbq/recommendation.sql.go index 96f4947b..f290ec35 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, +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, artists.name AS artist_name FROM plays p @@ -216,6 +216,7 @@ JOIN tracks t ON t.id = p.track_id JOIN albums ON albums.id = t.album_id JOIN artists ON artists.id = t.artist_id WHERE t.artist_id = $1 + AND t.missing_since IS NULL -- #2523: never offer a file that is gone AND NOT EXISTS ( SELECT 1 FROM lidarr_quarantine q WHERE q.user_id = $2 AND q.track_id = t.id @@ -268,6 +269,7 @@ func (q *Queries) ListMostPlayedTracksForArtist(ctx context.Context, arg ListMos &i.Track.TagSource, &i.Track.TagSourcesVersion, &i.Track.TagReadVersion, + &i.Track.MissingSince, &i.AlbumTitle, &i.ArtistName, ); err != nil { @@ -288,14 +290,15 @@ 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, +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, artists.name AS artist_name FROM plays p JOIN tracks t ON t.id = p.track_id JOIN albums ON albums.id = t.album_id JOIN artists ON artists.id = t.artist_id -WHERE NOT EXISTS ( +WHERE t.missing_since IS NULL -- #2523: never offer a file that is gone + AND NOT EXISTS ( SELECT 1 FROM lidarr_quarantine q WHERE q.user_id = $1 AND q.track_id = t.id ) @@ -350,6 +353,7 @@ func (q *Queries) ListMostPlayedTracksForUser(ctx context.Context, arg ListMostP &i.Track.TagSource, &i.Track.TagSourcesVersion, &i.Track.TagReadVersion, + &i.Track.MissingSince, &i.AlbumTitle, &i.ArtistName, ); err != nil { @@ -687,7 +691,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.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, (l.user_id IS NOT NULL)::bool AS is_liked, pe.last_played_at::timestamptz AS last_played_at, pe.play_count, @@ -705,6 +709,7 @@ LEFT JOIN LATERAL ( WHERE user_id = $1 AND track_id = t.id ) pe ON true WHERE t.id <> $2 + AND t.missing_since IS NULL -- #2523: never offer a file that is gone AND NOT EXISTS ( SELECT 1 FROM play_events WHERE user_id = $1 AND track_id = t.id @@ -766,6 +771,7 @@ func (q *Queries) LoadRadioCandidates(ctx context.Context, arg LoadRadioCandidat &i.Track.TagSource, &i.Track.TagSourcesVersion, &i.Track.TagReadVersion, + &i.Track.MissingSince, &i.IsLiked, &i.LastPlayedAt, &i.PlayCount, @@ -898,7 +904,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.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, (l.user_id IS NOT NULL)::bool AS is_liked, pe.last_played_at::timestamptz AS last_played_at, pe.play_count, @@ -914,7 +920,7 @@ FROM ( UNION ALL SELECT track_id, sim_score FROM coplay_artists UNION ALL SELECT track_id, sim_score FROM random_fill ) u -JOIN tracks t ON t.id = u.track_id +JOIN tracks t ON t.id = u.track_id AND t.missing_since IS NULL -- #2523: never offer a file that is gone JOIN albums al ON al.id = t.album_id LEFT JOIN general_likes l ON l.user_id = $1 AND l.track_id = t.id LEFT JOIN LATERAL ( @@ -1008,6 +1014,7 @@ func (q *Queries) LoadRadioCandidatesV2(ctx context.Context, arg LoadRadioCandid &i.Track.TagSource, &i.Track.TagSourcesVersion, &i.Track.TagReadVersion, + &i.Track.MissingSince, &i.IsLiked, &i.LastPlayedAt, &i.PlayCount, diff --git a/internal/db/dbq/system_mixes.sql.go b/internal/db/dbq/system_mixes.sql.go index ce98bd19..d8046f1b 100644 --- a/internal/db/dbq/system_mixes.sql.go +++ b/internal/db/dbq/system_mixes.sql.go @@ -39,7 +39,8 @@ SELECT t.id, t.album_id, t.artist_id JOIN affinity_artists aa ON aa.artist_id = t.artist_id LEFT JOIN play_counts pc ON pc.track_id = t.id LEFT JOIN skip_counts sc ON sc.track_id = t.id - WHERE COALESCE(pc.c, 0) <= 2 + WHERE t.missing_since IS NULL -- #2523: never offer a file that is gone + AND COALESCE(pc.c, 0) <= 2 AND COALESCE(sc.c, 0) < 2 AND NOT EXISTS ( SELECT 1 FROM lidarr_quarantine q @@ -124,7 +125,8 @@ SELECT t.id, t.album_id, t.artist_id, alt.tier::int AS tier FROM tracks t JOIN albums al ON al.id = t.album_id JOIN albums_tiered alt ON alt.album_id = al.id - WHERE alt.tier IS NOT NULL + WHERE t.missing_since IS NULL -- #2523: never offer a file that is gone + AND alt.tier IS NOT NULL AND NOT EXISTS (SELECT 1 FROM attempted a WHERE a.track_id = t.id) AND NOT EXISTS ( SELECT 1 FROM lidarr_quarantine q @@ -225,7 +227,8 @@ albums_tiered AS ( SELECT t.id, t.album_id, t.artist_id, alt.tier::int AS tier FROM tracks t JOIN albums_tiered alt ON alt.album_id = t.album_id - WHERE NOT EXISTS ( + WHERE t.missing_since IS NULL -- #2523: never offer a file that is gone + AND NOT EXISTS ( SELECT 1 FROM lidarr_quarantine q WHERE q.user_id = $1 AND q.track_id = t.id ) @@ -303,7 +306,8 @@ WITH windowed AS ( SELECT t.id, t.album_id, t.artist_id FROM tracks t JOIN windowed w ON w.track_id = t.id - WHERE NOT EXISTS ( + WHERE t.missing_since IS NULL -- #2523: never offer a file that is gone + AND NOT EXISTS ( SELECT 1 FROM lidarr_quarantine q WHERE q.user_id = $1 AND q.track_id = t.id ) @@ -367,7 +371,8 @@ WITH stats AS ( SELECT t.id, t.album_id, t.artist_id FROM tracks t JOIN stats s ON s.track_id = t.id - WHERE s.c >= 3 + WHERE t.missing_since IS NULL -- #2523: never offer a file that is gone + AND s.c >= 3 AND s.last_at <= now() - interval '30 days' AND NOT EXISTS ( SELECT 1 FROM lidarr_quarantine q diff --git a/internal/db/dbq/system_playlists.sql.go b/internal/db/dbq/system_playlists.sql.go index a86564a9..f76d10ff 100644 --- a/internal/db/dbq/system_playlists.sql.go +++ b/internal/db/dbq/system_playlists.sql.go @@ -189,6 +189,7 @@ func (q *Queries) GetSystemPlaylistRun(ctx context.Context, userID pgtype.UUID) const listActiveUsersForSystemPlaylists = `-- name: ListActiveUsersForSystemPlaylists :many + SELECT u.id FROM users u WHERE EXISTS ( SELECT 1 FROM play_events pe @@ -197,6 +198,13 @@ SELECT u.id FROM users u ) ` +// Track picks here join `tracks ... AND t.missing_since IS NULL` (#2523): a +// seed or For-You candidate has to be something that can actually play. Note +// this only affects newly GENERATED playlists — already-stored system +// playlists keep their rows until the next daily rebuild, which is why the +// shared ListPlaylistTracks read path is deliberately left unfiltered (it +// also serves user-curated playlists, where hiding a track the user added +// themselves would be wrong). // M7 #352 slice 2: system-generated playlist queries. // Active = had a play in the last 7 days. The cron iterates this list. func (q *Queries) ListActiveUsersForSystemPlaylists(ctx context.Context) ([]pgtype.UUID, error) { @@ -298,7 +306,7 @@ recent7 AS ( COUNT(*) FILTER (WHERE pe.was_skipped = false) AS play_count, 0 AS tier FROM play_events pe - JOIN tracks t ON t.id = pe.track_id + JOIN tracks t ON t.id = pe.track_id AND t.missing_since IS NULL WHERE pe.user_id = $1 AND pe.started_at > now() - INTERVAL '7 days' AND t.artist_id IS NOT NULL @@ -309,7 +317,7 @@ recent30 AS ( COUNT(*) FILTER (WHERE pe.was_skipped = false) AS play_count, 1 AS tier FROM play_events pe - JOIN tracks t ON t.id = pe.track_id + JOIN tracks t ON t.id = pe.track_id AND t.missing_since IS NULL WHERE pe.user_id = $1 AND pe.started_at > now() - INTERVAL '30 days' AND t.artist_id IS NOT NULL @@ -320,7 +328,7 @@ alltime AS ( COUNT(*) FILTER (WHERE pe.was_skipped = false) AS play_count, 2 AS tier FROM play_events pe - JOIN tracks t ON t.id = pe.track_id + JOIN tracks t ON t.id = pe.track_id AND t.missing_since IS NULL WHERE pe.user_id = $1 AND t.artist_id IS NOT NULL GROUP BY t.artist_id @@ -432,7 +440,7 @@ const pickTopPlayedTrackForArtistByUser = `-- name: PickTopPlayedTrackForArtistB SELECT COALESCE( (SELECT t.id FROM play_events pe - JOIN tracks t ON t.id = pe.track_id + JOIN tracks t ON t.id = pe.track_id AND t.missing_since IS NULL WHERE pe.user_id = $1 AND t.artist_id = $2 AND pe.started_at > now() - INTERVAL '7 days' @@ -472,7 +480,7 @@ const pickTopPlayedTracksForUser = `-- name: PickTopPlayedTracksForUser :many WITH recent AS ( SELECT t.id, COUNT(*) AS c, 0 AS tier FROM play_events pe - JOIN tracks t ON t.id = pe.track_id + JOIN tracks t ON t.id = pe.track_id AND t.missing_since IS NULL WHERE pe.user_id = $1 AND pe.started_at > now() - INTERVAL '30 days' AND pe.was_skipped = false @@ -481,7 +489,7 @@ WITH recent AS ( alltime AS ( SELECT t.id, COUNT(*) AS c, 1 AS tier FROM play_events pe - JOIN tracks t ON t.id = pe.track_id + JOIN tracks t ON t.id = pe.track_id AND t.missing_since IS NULL WHERE pe.user_id = $1 AND pe.was_skipped = false GROUP BY t.id diff --git a/internal/db/dbq/tracks.sql.go b/internal/db/dbq/tracks.sql.go index 9c3b0daa..a6266b08 100644 --- a/internal/db/dbq/tracks.sql.go +++ b/internal/db/dbq/tracks.sql.go @@ -11,6 +11,24 @@ import ( "github.com/jackc/pgx/v5/pgtype" ) +const clearTracksMissing = `-- name: ClearTracksMissing :execrows +UPDATE tracks + SET missing_since = NULL + WHERE id = ANY($1::uuid[]) + AND missing_since IS NOT NULL +` + +// Clears the mark on rows whose file is back. Runs independently of the mtime +// skip check, so a file that reappears unchanged is un-marked even though the +// scanner skips re-reading its tags. +func (q *Queries) ClearTracksMissing(ctx context.Context, ids []pgtype.UUID) (int64, error) { + result, err := q.db.Exec(ctx, clearTracksMissing, ids) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + const countTracksByAlbum = `-- name: CountTracksByAlbum :one SELECT count(*) FROM tracks WHERE album_id = $1 ` @@ -90,7 +108,7 @@ func (q *Queries) DeleteTrack(ctx context.Context, id pgtype.UUID) (DeleteTrackR } 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 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 FROM tracks WHERE id = $1 ` func (q *Queries) GetTrackByID(ctx context.Context, id pgtype.UUID) (Track, error) { @@ -115,12 +133,13 @@ func (q *Queries) GetTrackByID(ctx context.Context, id pgtype.UUID) (Track, erro &i.TagSource, &i.TagSourcesVersion, &i.TagReadVersion, + &i.MissingSince, ) 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 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 FROM tracks WHERE file_path = $1 ` func (q *Queries) GetTrackByPath(ctx context.Context, filePath string) (Track, error) { @@ -145,12 +164,13 @@ func (q *Queries) GetTrackByPath(ctx context.Context, filePath string) (Track, e &i.TagSource, &i.TagSourcesVersion, &i.TagReadVersion, + &i.MissingSince, ) 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 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 FROM tracks WHERE id = ANY($1::uuid[]) ` // Batched lookup used by /api/library/sync to hydrate upsert payloads @@ -183,6 +203,7 @@ func (q *Queries) GetTracksByIDs(ctx context.Context, dollar_1 []pgtype.UUID) ([ &i.TagSource, &i.TagSourcesVersion, &i.TagReadVersion, + &i.MissingSince, ); err != nil { return nil, err } @@ -195,7 +216,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, +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, artists.name AS artist_name FROM tracks t @@ -254,6 +275,7 @@ func (q *Queries) ListArtistTracksForUser(ctx context.Context, arg ListArtistTra &i.Track.TagSource, &i.Track.TagSourcesVersion, &i.Track.TagReadVersion, + &i.Track.MissingSince, &i.AlbumTitle, &i.ArtistName, ); err != nil { @@ -268,7 +290,7 @@ func (q *Queries) ListArtistTracksForUser(ctx context.Context, arg ListArtistTra } 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, +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, artists.name AS artist_name FROM tracks t @@ -324,6 +346,7 @@ func (q *Queries) ListRandomTracksForUser(ctx context.Context, arg ListRandomTra &i.Track.TagSource, &i.Track.TagSourcesVersion, &i.Track.TagReadVersion, + &i.Track.MissingSince, &i.AlbumTitle, &i.ArtistName, ); err != nil { @@ -337,8 +360,43 @@ func (q *Queries) ListRandomTracksForUser(ctx context.Context, arg ListRandomTra return items, nil } +const listTrackPathsForReconcile = `-- name: ListTrackPathsForReconcile :many +SELECT id, file_path, missing_since FROM tracks +` + +type ListTrackPathsForReconcileRow struct { + ID pgtype.UUID + FilePath string + MissingSince pgtype.Timestamptz +} + +// Every row's path + current missing mark, for the scanner's reconcile pass +// (#2523). Deliberately unfiltered and unpaged: reconcile has to compare the +// WHOLE table against what the walk saw, and a filtered subset would let rows +// outside it drift forever. Three narrow columns keep it cheap even on a +// library of a few hundred thousand tracks. +func (q *Queries) ListTrackPathsForReconcile(ctx context.Context) ([]ListTrackPathsForReconcileRow, error) { + rows, err := q.db.Query(ctx, listTrackPathsForReconcile) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListTrackPathsForReconcileRow + for rows.Next() { + var i ListTrackPathsForReconcileRow + if err := rows.Scan(&i.ID, &i.FilePath, &i.MissingSince); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + 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 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 FROM tracks WHERE album_id = $1 AND NOT EXISTS ( SELECT 1 FROM lidarr_quarantine q @@ -383,6 +441,7 @@ func (q *Queries) ListTracksByAlbum(ctx context.Context, arg ListTracksByAlbumPa &i.TagSource, &i.TagSourcesVersion, &i.TagReadVersion, + &i.MissingSince, ); err != nil { return nil, err } @@ -429,8 +488,31 @@ func (q *Queries) ListTracksMissingMbidWithPath(ctx context.Context, limit int32 return items, nil } +const markTracksMissing = `-- name: MarkTracksMissing :execrows +UPDATE tracks + SET missing_since = now() + WHERE id = ANY($1::uuid[]) + AND missing_since IS NULL +` + +// Marks rows whose file the walk did not see. `missing_since IS NULL` in the +// predicate makes this idempotent: a row already marked keeps its ORIGINAL +// timestamp, so "how long has it been gone" survives repeated scans. Losing +// that would make any age-based cleanup policy meaningless. +// +// updated_at is deliberately NOT touched. It tracks content changes and gates +// the scanner's mtime skip; moving it here would make a returning file look +// newer than its own mtime and stop its tags being re-read. +func (q *Queries) MarkTracksMissing(ctx context.Context, ids []pgtype.UUID) (int64, error) { + result, err := q.db.Exec(ctx, markTracksMissing, ids) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + 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 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 FROM tracks WHERE title ILIKE '%' || $1::text || '%' AND NOT EXISTS ( SELECT 1 FROM lidarr_quarantine q @@ -482,6 +564,7 @@ func (q *Queries) SearchTracks(ctx context.Context, arg SearchTracksParams) ([]T &i.TagSource, &i.TagSourcesVersion, &i.TagReadVersion, + &i.MissingSince, ); err != nil { return nil, err } @@ -533,7 +616,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 +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 ` type UpsertTrackParams struct { @@ -589,6 +672,7 @@ func (q *Queries) UpsertTrack(ctx context.Context, arg UpsertTrackParams) (Track &i.TagSource, &i.TagSourcesVersion, &i.TagReadVersion, + &i.MissingSince, ) return i, err } diff --git a/internal/db/migrations/0055_track_missing_since.down.sql b/internal/db/migrations/0055_track_missing_since.down.sql new file mode 100644 index 00000000..d9bc5bc1 --- /dev/null +++ b/internal/db/migrations/0055_track_missing_since.down.sql @@ -0,0 +1,4 @@ +DROP INDEX IF EXISTS tracks_missing_since_idx; + +ALTER TABLE tracks + DROP COLUMN missing_since; diff --git a/internal/db/migrations/0055_track_missing_since.up.sql b/internal/db/migrations/0055_track_missing_since.up.sql new file mode 100644 index 00000000..11952ade --- /dev/null +++ b/internal/db/migrations/0055_track_missing_since.up.sql @@ -0,0 +1,25 @@ +-- Marks a track whose file the scanner could no longer find (#2523). +-- +-- NULL means present. A timestamp means the file was absent as of that scan, +-- and is the point from which "how long has this been gone" is measured — which +-- is what a later cleanup pass needs in order to require a grace period rather +-- than deleting on a single missed stat. +-- +-- Deliberately a nullable timestamp rather than a boolean: "missing" is not a +-- state we want to act on immediately, and the age is the only thing that makes +-- an automated deletion safe to reason about. +-- +-- No default and no backfill. Existing rows start NULL (present) and the next +-- full scan sets the mark where it belongs — a migration cannot check the +-- filesystem, and guessing here would mark the whole library on a server whose +-- media volume happens to be detached at upgrade time. +ALTER TABLE tracks + ADD COLUMN missing_since timestamptz; + +-- Partial index: the only query that filters on this column positively is the +-- admin "what's missing" list, which is a small set. Playback and browse +-- queries filter `missing_since IS NULL`, which matches nearly every row and is +-- better served by a sequential scan than an index lookup. +CREATE INDEX tracks_missing_since_idx + ON tracks (missing_since) + WHERE missing_since IS NOT NULL; diff --git a/internal/db/queries/browse.sql b/internal/db/queries/browse.sql index 3a1e3385..00d47d29 100644 --- a/internal/db/queries/browse.sql +++ b/internal/db/queries/browse.sql @@ -1,3 +1,15 @@ +-- Every query in this file filters `tracks.missing_since IS NULL` (#2523). +-- A row whose file has vanished keeps its genre forever — the scanner walks the +-- filesystem, so it never revisits a path that no longer exists — which is how +-- pre-#2499 welded genres survived a full re-scan and kept showing in the index. +-- Browsing is a way of finding something to play, so a track that cannot play +-- should not shape it. +-- +-- Year queries below join albums only and are deliberately left alone: an album +-- is still a real release even if some of its tracks are gone. An album whose +-- EVERY track is missing will linger on the year axis; that's a narrower case, +-- tracked with the rest of the cleanup work. + -- name: ListGenresWithCount :many -- Genre browse index (#367). -- @@ -22,6 +34,7 @@ SELECT trim(g.genre) AS genre, COUNT(DISTINCT tracks.id)::bigint AS track_count FROM tracks JOIN LATERAL regexp_split_to_table(coalesce(tracks.genre, ''), '[;,]') AS g(genre) ON true WHERE trim(g.genre) <> '' + AND tracks.missing_since IS NULL GROUP BY trim(g.genre) -- Ordered by the expression, not the output alias: `ORDER BY genre` is -- ambiguous between the alias and tracks.genre, and sqlc rejects it. @@ -41,6 +54,7 @@ WHERE EXISTS ( FROM tracks JOIN LATERAL regexp_split_to_table(coalesce(tracks.genre, ''), '[;,]') AS g(genre) ON true WHERE tracks.album_id = albums.id + AND tracks.missing_since IS NULL AND trim(g.genre) = trim(sqlc.arg(genre)::text) ) ORDER BY albums.sort_title, albums.id @@ -56,6 +70,7 @@ WHERE EXISTS ( FROM tracks JOIN LATERAL regexp_split_to_table(coalesce(tracks.genre, ''), '[;,]') AS g(genre) ON true WHERE tracks.album_id = albums.id + AND tracks.missing_since IS NULL AND trim(g.genre) = trim(sqlc.arg(genre)::text) ); @@ -96,6 +111,7 @@ SELECT DISTINCT trim(g.genre) AS genre FROM tracks JOIN LATERAL regexp_split_to_table(coalesce(tracks.genre, ''), '[;,]') AS g(genre) ON true WHERE tracks.album_id = $1 AND trim(g.genre) <> '' + AND tracks.missing_since IS NULL ORDER BY trim(g.genre); -- name: ListGenresForArtist :many @@ -106,4 +122,5 @@ SELECT DISTINCT trim(g.genre) AS genre FROM tracks JOIN LATERAL regexp_split_to_table(coalesce(tracks.genre, ''), '[;,]') AS g(genre) ON true WHERE tracks.artist_id = $1 AND trim(g.genre) <> '' + AND tracks.missing_since IS NULL ORDER BY trim(g.genre); diff --git a/internal/db/queries/discover.sql b/internal/db/queries/discover.sql index ed44c378..86385609 100644 --- a/internal/db/queries/discover.sql +++ b/internal/db/queries/discover.sql @@ -29,7 +29,8 @@ dormant_artists AS ( SELECT t.id, t.album_id, t.artist_id FROM tracks t JOIN dormant_artists da ON da.id = t.artist_id - WHERE NOT EXISTS ( + WHERE t.missing_since IS NULL -- #2523: never offer a file that is gone + AND NOT EXISTS ( SELECT 1 FROM play_events pe WHERE pe.user_id = $1 AND pe.track_id = t.id @@ -60,7 +61,8 @@ SELECT t.id, t.album_id, t.artist_id SELECT t.id, t.album_id, t.artist_id FROM general_likes gl JOIN tracks t ON t.id = gl.track_id - WHERE gl.user_id != $1 + WHERE t.missing_since IS NULL -- #2523: never offer a file that is gone + AND gl.user_id != $1 AND NOT EXISTS ( SELECT 1 FROM play_events pe WHERE pe.user_id = $1 @@ -86,7 +88,8 @@ SELECT t.id, t.album_id, t.artist_id -- $1 = user_id, $2 = date string for md5 ordering. SELECT t.id, t.album_id, t.artist_id FROM tracks t - WHERE NOT EXISTS ( + WHERE t.missing_since IS NULL -- #2523: never offer a file that is gone + AND NOT EXISTS ( SELECT 1 FROM play_events pe WHERE pe.user_id = $1 AND pe.track_id = t.id @@ -117,7 +120,8 @@ SELECT t.id, t.album_id, t.artist_id FROM tracks t JOIN LATERAL regexp_split_to_table(coalesce(t.genre, ''), '[;,]') AS g_split(g) ON true JOIN taste_profile_tags nt ON nt.user_id = $1 AND trim(g_split.g) = nt.tag - WHERE nt.weight > 0 + WHERE t.missing_since IS NULL -- #2523: never offer a file that is gone + AND nt.weight > 0 AND trim(g_split.g) <> '' AND NOT EXISTS ( SELECT 1 FROM play_events pe diff --git a/internal/db/queries/recommendation.sql b/internal/db/queries/recommendation.sql index fcb35e8b..20fbf7f7 100644 --- a/internal/db/queries/recommendation.sql +++ b/internal/db/queries/recommendation.sql @@ -24,6 +24,7 @@ LEFT JOIN LATERAL ( WHERE user_id = $1 AND track_id = t.id ) pe ON true WHERE t.id <> $2 + AND t.missing_since IS NULL -- #2523: never offer a file that is gone AND NOT EXISTS ( SELECT 1 FROM play_events WHERE user_id = $1 AND track_id = t.id @@ -177,7 +178,7 @@ FROM ( UNION ALL SELECT track_id, sim_score FROM coplay_artists UNION ALL SELECT track_id, sim_score FROM random_fill ) u -JOIN tracks t ON t.id = u.track_id +JOIN tracks t ON t.id = u.track_id AND t.missing_since IS NULL -- #2523: never offer a file that is gone JOIN albums al ON al.id = t.album_id LEFT JOIN general_likes l ON l.user_id = $1 AND l.track_id = t.id LEFT JOIN LATERAL ( @@ -382,7 +383,8 @@ FROM plays p JOIN tracks t ON t.id = p.track_id JOIN albums ON albums.id = t.album_id JOIN artists ON artists.id = t.artist_id -WHERE NOT EXISTS ( +WHERE t.missing_since IS NULL -- #2523: never offer a file that is gone + AND NOT EXISTS ( SELECT 1 FROM lidarr_quarantine q WHERE q.user_id = $1 AND q.track_id = t.id ) @@ -408,6 +410,7 @@ JOIN tracks t ON t.id = p.track_id JOIN albums ON albums.id = t.album_id JOIN artists ON artists.id = t.artist_id WHERE t.artist_id = sqlc.arg(artist_id) + AND t.missing_since IS NULL -- #2523: never offer a file that is gone AND NOT EXISTS ( SELECT 1 FROM lidarr_quarantine q WHERE q.user_id = sqlc.arg(user_id) AND q.track_id = t.id diff --git a/internal/db/queries/system_mixes.sql b/internal/db/queries/system_mixes.sql index 7503a122..6469ae06 100644 --- a/internal/db/queries/system_mixes.sql +++ b/internal/db/queries/system_mixes.sql @@ -40,7 +40,8 @@ SELECT t.id, t.album_id, t.artist_id JOIN affinity_artists aa ON aa.artist_id = t.artist_id LEFT JOIN play_counts pc ON pc.track_id = t.id LEFT JOIN skip_counts sc ON sc.track_id = t.id - WHERE COALESCE(pc.c, 0) <= 2 + WHERE t.missing_since IS NULL -- #2523: never offer a file that is gone + AND COALESCE(pc.c, 0) <= 2 AND COALESCE(sc.c, 0) < 2 AND NOT EXISTS ( SELECT 1 FROM lidarr_quarantine q @@ -70,7 +71,8 @@ WITH stats AS ( SELECT t.id, t.album_id, t.artist_id FROM tracks t JOIN stats s ON s.track_id = t.id - WHERE s.c >= 3 + WHERE t.missing_since IS NULL -- #2523: never offer a file that is gone + AND s.c >= 3 AND s.last_at <= now() - interval '30 days' AND NOT EXISTS ( SELECT 1 FROM lidarr_quarantine q @@ -149,7 +151,8 @@ albums_tiered AS ( SELECT t.id, t.album_id, t.artist_id, alt.tier::int AS tier FROM tracks t JOIN albums_tiered alt ON alt.album_id = t.album_id - WHERE NOT EXISTS ( + WHERE t.missing_since IS NULL -- #2523: never offer a file that is gone + AND NOT EXISTS ( SELECT 1 FROM lidarr_quarantine q WHERE q.user_id = $1 AND q.track_id = t.id ) @@ -187,7 +190,8 @@ WITH windowed AS ( SELECT t.id, t.album_id, t.artist_id FROM tracks t JOIN windowed w ON w.track_id = t.id - WHERE NOT EXISTS ( + WHERE t.missing_since IS NULL -- #2523: never offer a file that is gone + AND NOT EXISTS ( SELECT 1 FROM lidarr_quarantine q WHERE q.user_id = $1 AND q.track_id = t.id ) @@ -240,7 +244,8 @@ SELECT t.id, t.album_id, t.artist_id, alt.tier::int AS tier FROM tracks t JOIN albums al ON al.id = t.album_id JOIN albums_tiered alt ON alt.album_id = al.id - WHERE alt.tier IS NOT NULL + WHERE t.missing_since IS NULL -- #2523: never offer a file that is gone + AND alt.tier IS NOT NULL AND NOT EXISTS (SELECT 1 FROM attempted a WHERE a.track_id = t.id) AND NOT EXISTS ( SELECT 1 FROM lidarr_quarantine q diff --git a/internal/db/queries/system_playlists.sql b/internal/db/queries/system_playlists.sql index b57fd079..41c1cff5 100644 --- a/internal/db/queries/system_playlists.sql +++ b/internal/db/queries/system_playlists.sql @@ -1,3 +1,11 @@ +-- Track picks here join `tracks ... AND t.missing_since IS NULL` (#2523): a +-- seed or For-You candidate has to be something that can actually play. Note +-- this only affects newly GENERATED playlists — already-stored system +-- playlists keep their rows until the next daily rebuild, which is why the +-- shared ListPlaylistTracks read path is deliberately left unfiltered (it +-- also serves user-curated playlists, where hiding a track the user added +-- themselves would be wrong). + -- M7 #352 slice 2: system-generated playlist queries. -- name: ListActiveUsersForSystemPlaylists :many @@ -72,7 +80,7 @@ recent7 AS ( COUNT(*) FILTER (WHERE pe.was_skipped = false) AS play_count, 0 AS tier FROM play_events pe - JOIN tracks t ON t.id = pe.track_id + JOIN tracks t ON t.id = pe.track_id AND t.missing_since IS NULL WHERE pe.user_id = $1 AND pe.started_at > now() - INTERVAL '7 days' AND t.artist_id IS NOT NULL @@ -83,7 +91,7 @@ recent30 AS ( COUNT(*) FILTER (WHERE pe.was_skipped = false) AS play_count, 1 AS tier FROM play_events pe - JOIN tracks t ON t.id = pe.track_id + JOIN tracks t ON t.id = pe.track_id AND t.missing_since IS NULL WHERE pe.user_id = $1 AND pe.started_at > now() - INTERVAL '30 days' AND t.artist_id IS NOT NULL @@ -94,7 +102,7 @@ alltime AS ( COUNT(*) FILTER (WHERE pe.was_skipped = false) AS play_count, 2 AS tier FROM play_events pe - JOIN tracks t ON t.id = pe.track_id + JOIN tracks t ON t.id = pe.track_id AND t.missing_since IS NULL WHERE pe.user_id = $1 AND t.artist_id IS NOT NULL GROUP BY t.artist_id @@ -139,7 +147,7 @@ SELECT c.artist_id, WITH recent AS ( SELECT t.id, COUNT(*) AS c, 0 AS tier FROM play_events pe - JOIN tracks t ON t.id = pe.track_id + JOIN tracks t ON t.id = pe.track_id AND t.missing_since IS NULL WHERE pe.user_id = $1 AND pe.started_at > now() - INTERVAL '30 days' AND pe.was_skipped = false @@ -148,7 +156,7 @@ WITH recent AS ( alltime AS ( SELECT t.id, COUNT(*) AS c, 1 AS tier FROM play_events pe - JOIN tracks t ON t.id = pe.track_id + JOIN tracks t ON t.id = pe.track_id AND t.missing_since IS NULL WHERE pe.user_id = $1 AND pe.was_skipped = false GROUP BY t.id @@ -181,7 +189,7 @@ SELECT id SELECT COALESCE( (SELECT t.id FROM play_events pe - JOIN tracks t ON t.id = pe.track_id + JOIN tracks t ON t.id = pe.track_id AND t.missing_since IS NULL WHERE pe.user_id = $1 AND t.artist_id = $2 AND pe.started_at > now() - INTERVAL '7 days' diff --git a/internal/db/queries/tracks.sql b/internal/db/queries/tracks.sql index c16e7108..3f59044d 100644 --- a/internal/db/queries/tracks.sql +++ b/internal/db/queries/tracks.sql @@ -137,3 +137,34 @@ RETURNING id, album_id, artist_id, file_path, mbid; -- Batched lookup used by /api/library/sync to hydrate upsert payloads -- (#357). Mirror of GetArtistsByIDs. SELECT * FROM tracks WHERE id = ANY($1::uuid[]); + +-- name: ListTrackPathsForReconcile :many +-- Every row's path + current missing mark, for the scanner's reconcile pass +-- (#2523). Deliberately unfiltered and unpaged: reconcile has to compare the +-- WHOLE table against what the walk saw, and a filtered subset would let rows +-- outside it drift forever. Three narrow columns keep it cheap even on a +-- library of a few hundred thousand tracks. +SELECT id, file_path, missing_since FROM tracks; + +-- name: MarkTracksMissing :execrows +-- Marks rows whose file the walk did not see. `missing_since IS NULL` in the +-- predicate makes this idempotent: a row already marked keeps its ORIGINAL +-- timestamp, so "how long has it been gone" survives repeated scans. Losing +-- that would make any age-based cleanup policy meaningless. +-- +-- updated_at is deliberately NOT touched. It tracks content changes and gates +-- the scanner's mtime skip; moving it here would make a returning file look +-- newer than its own mtime and stop its tags being re-read. +UPDATE tracks + SET missing_since = now() + WHERE id = ANY(sqlc.arg(ids)::uuid[]) + AND missing_since IS NULL; + +-- name: ClearTracksMissing :execrows +-- Clears the mark on rows whose file is back. Runs independently of the mtime +-- skip check, so a file that reappears unchanged is un-marked even though the +-- scanner skips re-reading its tags. +UPDATE tracks + SET missing_since = NULL + WHERE id = ANY(sqlc.arg(ids)::uuid[]) + AND missing_since IS NOT NULL; diff --git a/internal/library/reconcile.go b/internal/library/reconcile.go new file mode 100644 index 00000000..6c406cf2 --- /dev/null +++ b/internal/library/reconcile.go @@ -0,0 +1,155 @@ +package library + +import ( + "context" + "errors" + "fmt" + "os" + + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// trackReconciler is the slice of dbq.Queries reconcileMissing needs. Narrowed +// to an interface so the guard logic — which is the part that can do damage — +// is unit-testable against a fake without a database. +type trackReconciler interface { + ListTrackPathsForReconcile(ctx context.Context) ([]dbq.ListTrackPathsForReconcileRow, error) + MarkTracksMissing(ctx context.Context, ids []pgtype.UUID) (int64, error) + ClearTracksMissing(ctx context.Context, ids []pgtype.UUID) (int64, error) +} + +// Reconcile marks tracks whose files have disappeared (#2523). +// +// Why this exists: nothing in Minstrel used to notice a deleted file. The walk +// only visits paths that exist, so a row whose file is gone was never scanned, +// never errored, never counted — permanently invisible. The watcher ignores +// removals by design (see classifyEvent), and the safety-net scan is the same +// walk, so it covers additions only. Rows accumulated forever, kept being +// offered to recommendations, and failed at playback. +// +// Why it MARKS rather than deletes: a missing file is a claim about the +// filesystem, and the filesystem lies transiently — an unmounted volume, a +// network-storage blip, a container that started before its media mount +// attached. Every other sweep in this codebase (internal/gc) resolves a truth +// *inside* the database and is safe to run blind. This one isn't, so the +// destructive step is deliberately not here. Marking is reversible: the next +// good scan clears it. + +// missingMarkMaxFraction caps how much of the library one reconcile may newly +// mark missing. A partially-attached mount is the failure this defends against: +// the roots resolve, the walk succeeds, and it legitimately sees only part of +// the library — evidence indistinguishable from a mass deletion. +// +// A quarter is deliberately conservative. A genuine bulk deletion trips it and +// gets logged rather than applied, which needs a second scan (or operator +// action) to take effect. That's the right trade: the cost of over-refusing is +// a stale row and a log line, and the cost of over-marking is a chunk of the +// library silently vanishing from every mix. +const missingMarkMaxFraction = 0.25 + +// reconcileMissing diffs the paths the walk saw against every row in the table. +// Rows not seen get marked; rows seen that carry a mark get cleared. +// +// seen must come from a COMPLETE walk of every configured root. Callers with a +// partial view must not call this. +func (s *Scanner) reconcileMissing( + ctx context.Context, q trackReconciler, seen map[string]struct{}, stats *Stats, +) error { + if err := s.verifyRootsPresent(); err != nil { + return err + } + // Roots resolved but the walk found nothing. Either the library is genuinely + // empty — in which case there is nothing to reconcile — or the mount is + // hollow. Both mean: don't act. + if len(seen) == 0 { + return errors.New("walk saw no audio files; refusing to reconcile") + } + + rows, err := q.ListTrackPathsForReconcile(ctx) + if err != nil { + return fmt.Errorf("list track paths: %w", err) + } + if len(rows) == 0 { + return nil + } + + var toMark, toClear []pgtype.UUID + for _, row := range rows { + _, present := seen[row.FilePath] + switch { + case !present && !row.MissingSince.Valid: + toMark = append(toMark, row.ID) + case present && row.MissingSince.Valid: + toClear = append(toClear, row.ID) + } + } + + // Clear before marking, and unconditionally. Restoring a file is never the + // dangerous direction, so it must not be blocked by the guard below — + // otherwise a library that tripped the cap once could never recover its + // marks even after the mount came back. + if len(toClear) > 0 { + n, err := q.ClearTracksMissing(ctx, toClear) + if err != nil { + return fmt.Errorf("clear missing marks: %w", err) + } + stats.Restored = int(n) + s.logger.Info("library scan: files returned", "count", n) + } + + if len(toMark) == 0 { + return nil + } + if fraction := float64(len(toMark)) / float64(len(rows)); fraction > missingMarkMaxFraction { + return fmt.Errorf( + "refusing to mark %d of %d tracks missing (%.0f%% > %.0f%% cap): "+ + "this looks like an unavailable mount rather than a deletion", + len(toMark), len(rows), fraction*100, missingMarkMaxFraction*100, + ) + } + + n, err := q.MarkTracksMissing(ctx, toMark) + if err != nil { + return fmt.Errorf("mark tracks missing: %w", err) + } + stats.Missing = int(n) + // Warn, not Info: every one of these is a library entry the operator + // probably didn't intend to lose, and the only place it surfaces today is + // this line. + s.logger.Warn("library scan: tracks marked missing (files not found)", + "count", n, "library_total", len(rows)) + return nil +} + +// verifyRootsPresent is the first and most important guard. If a configured root +// doesn't resolve to a readable directory, the walk beneath it found nothing and +// every row under it would look deleted. An unmounted media volume is the +// obvious case, and it is common enough — a container restart racing its volume +// mount does exactly this. +func (s *Scanner) verifyRootsPresent() error { + if len(s.paths) == 0 { + return errors.New("no scan roots configured") + } + for _, root := range s.paths { + info, err := os.Stat(root) + if err != nil { + return fmt.Errorf("scan root %q unavailable: %w", root, err) + } + if !info.IsDir() { + return fmt.Errorf("scan root %q is not a directory", root) + } + entries, err := os.ReadDir(root) + if err != nil { + return fmt.Errorf("scan root %q unreadable: %w", root, err) + } + // An empty root is the signature of a mount point with nothing mounted + // on it. `os.Stat` succeeds on the bare directory, so this is the only + // cheap way to tell the two apart. + if len(entries) == 0 { + return fmt.Errorf("scan root %q is empty; refusing to reconcile", root) + } + } + return nil +} diff --git a/internal/library/reconcile_test.go b/internal/library/reconcile_test.go new file mode 100644 index 00000000..a5248f73 --- /dev/null +++ b/internal/library/reconcile_test.go @@ -0,0 +1,326 @@ +package library + +import ( + "context" + "errors" + "fmt" + "io" + "log/slog" + "os" + "path/filepath" + "testing" + + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// fakeReconciler records what reconcileMissing decided to do, so the guards can +// be tested without a database. The guards are the whole point of this pass — +// they are what stands between an unmounted volume and the library disappearing +// from every mix — so they get tested directly rather than via integration. +type fakeReconciler struct { + rows []dbq.ListTrackPathsForReconcileRow + marked []pgtype.UUID + cleared []pgtype.UUID + listErr error + markErr error + clearErr error +} + +func (f *fakeReconciler) ListTrackPathsForReconcile(context.Context) ([]dbq.ListTrackPathsForReconcileRow, error) { + return f.rows, f.listErr +} + +func (f *fakeReconciler) MarkTracksMissing(_ context.Context, ids []pgtype.UUID) (int64, error) { + if f.markErr != nil { + return 0, f.markErr + } + f.marked = append(f.marked, ids...) + return int64(len(ids)), nil +} + +func (f *fakeReconciler) ClearTracksMissing(_ context.Context, ids []pgtype.UUID) (int64, error) { + if f.clearErr != nil { + return 0, f.clearErr + } + f.cleared = append(f.cleared, ids...) + return int64(len(ids)), nil +} + +// Compile-time proof the real queries still satisfy what reconcile needs — the +// interface exists to narrow dbq.Queries, not to diverge from it. +var _ trackReconciler = (*dbq.Queries)(nil) + +func testUUID(n byte) pgtype.UUID { + var u pgtype.UUID + u.Bytes[15] = n + u.Valid = true + return u +} + +func markedAt() pgtype.Timestamptz { + return pgtype.Timestamptz{Valid: true} +} + +func row(n byte, path string, missing bool) dbq.ListTrackPathsForReconcileRow { + r := dbq.ListTrackPathsForReconcileRow{ID: testUUID(n), FilePath: path} + if missing { + r.MissingSince = markedAt() + } + return r +} + +// populatedRoot returns a directory containing one file, so verifyRootsPresent +// treats it as a real, mounted library root. +func populatedRoot(t *testing.T) string { + t.Helper() + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "a.mp3"), []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + return dir +} + +func testScanner(t *testing.T, roots ...string) *Scanner { + t.Helper() + return &Scanner{ + logger: slog.New(slog.NewTextHandler(io.Discard, nil)), + paths: roots, + } +} + +func TestReconcileMissing_MarksRowsTheWalkDidNotSee(t *testing.T) { + root := populatedRoot(t) + s := testScanner(t, root) + + // 10 rows with 2 absent — 20%, deliberately under missingMarkMaxFraction so + // this exercises marking rather than the cap. (An earlier version of this + // test used 2-of-4 and was really testing the guard by accident.) + rows := make([]dbq.ListTrackPathsForReconcileRow, 0, 10) + seen := map[string]struct{}{} + for i := 0; i < 10; i++ { + p := fmt.Sprintf("/music/track-%02d.mp3", i) + rows = append(rows, row(byte(i), p, false)) + if i >= 2 { + seen[p] = struct{}{} + } + } + q := &fakeReconciler{rows: rows} + + var stats Stats + if err := s.reconcileMissing(context.Background(), q, seen, &stats); err != nil { + t.Fatalf("reconcile: %v", err) + } + if len(q.marked) != 2 { + t.Fatalf("marked %d rows, want 2", len(q.marked)) + } + if q.marked[0] != testUUID(0) || q.marked[1] != testUUID(1) { + t.Errorf("marked the wrong rows: %v", q.marked) + } + if stats.Missing != 2 { + t.Errorf("stats.Missing = %d, want 2", stats.Missing) + } + if len(q.cleared) != 0 { + t.Errorf("cleared %d rows, want 0", len(q.cleared)) + } +} + +func TestReconcileMissing_ClearsRowsWhoseFileReturned(t *testing.T) { + root := populatedRoot(t) + s := testScanner(t, root) + q := &fakeReconciler{rows: []dbq.ListTrackPathsForReconcileRow{ + row(1, "/music/back.mp3", true), + row(2, "/music/still-here.mp3", false), + }} + seen := map[string]struct{}{ + "/music/back.mp3": {}, + "/music/still-here.mp3": {}, + } + + var stats Stats + if err := s.reconcileMissing(context.Background(), q, seen, &stats); err != nil { + t.Fatalf("reconcile: %v", err) + } + if len(q.cleared) != 1 || q.cleared[0] != testUUID(1) { + t.Fatalf("cleared = %v, want just row 1", q.cleared) + } + if stats.Restored != 1 { + t.Errorf("stats.Restored = %d, want 1", stats.Restored) + } + if len(q.marked) != 0 { + t.Errorf("marked %d rows, want 0", len(q.marked)) + } +} + +// An already-marked row must not be re-marked: the timestamp is the "how long +// has this been gone" clock that any future cleanup policy depends on. +func TestReconcileMissing_DoesNotRemarkAlreadyMissingRows(t *testing.T) { + root := populatedRoot(t) + s := testScanner(t, root) + q := &fakeReconciler{rows: []dbq.ListTrackPathsForReconcileRow{ + row(1, "/music/long-gone.mp3", true), + row(2, "/music/present.mp3", false), + }} + seen := map[string]struct{}{"/music/present.mp3": {}} + + var stats Stats + if err := s.reconcileMissing(context.Background(), q, seen, &stats); err != nil { + t.Fatalf("reconcile: %v", err) + } + if len(q.marked) != 0 { + t.Errorf("re-marked an already-missing row: %v", q.marked) + } + if len(q.cleared) != 0 { + t.Errorf("cleared = %v, want none", q.cleared) + } +} + +// The guard that matters most. A half-attached mount makes the walk succeed +// while seeing only part of the library — evidence indistinguishable from a mass +// deletion, so reconcile must refuse rather than guess. +func TestReconcileMissing_RefusesWhenTooMuchWouldBeMarked(t *testing.T) { + root := populatedRoot(t) + s := testScanner(t, root) + rows := make([]dbq.ListTrackPathsForReconcileRow, 0, 100) + seen := map[string]struct{}{} + for i := 0; i < 100; i++ { + p := fmt.Sprintf("/music/track-%03d.mp3", i) + rows = append(rows, row(byte(i), p, false)) + // Only 60 of 100 present -> 40% would be marked, over the 25% cap. + if i < 60 { + seen[p] = struct{}{} + } + } + q := &fakeReconciler{rows: rows} + + var stats Stats + err := s.reconcileMissing(context.Background(), q, seen, &stats) + if err == nil { + t.Fatal("expected reconcile to refuse, got nil error") + } + if len(q.marked) != 0 { + t.Errorf("marked %d rows despite refusing", len(q.marked)) + } + if stats.Missing != 0 { + t.Errorf("stats.Missing = %d, want 0", stats.Missing) + } +} + +// Restoring is never the dangerous direction, so it must survive the cap — +// otherwise a library that tripped the cap once could never clear its marks +// even after the volume came back. +func TestReconcileMissing_ClearsEvenWhenMarkCapTrips(t *testing.T) { + root := populatedRoot(t) + s := testScanner(t, root) + rows := []dbq.ListTrackPathsForReconcileRow{row(1, "/music/back.mp3", true)} + seen := map[string]struct{}{"/music/back.mp3": {}} + // Add enough absent rows to blow the cap. + for i := 2; i < 10; i++ { + rows = append(rows, row(byte(i), fmt.Sprintf("/music/absent-%02d.mp3", i), false)) + } + q := &fakeReconciler{rows: rows} + + var stats Stats + if err := s.reconcileMissing(context.Background(), q, seen, &stats); err == nil { + t.Fatal("expected the mark cap to trip") + } + if len(q.cleared) != 1 { + t.Errorf("cleared %d rows, want 1 — restores must not be blocked by the cap", len(q.cleared)) + } + if stats.Restored != 1 { + t.Errorf("stats.Restored = %d, want 1", stats.Restored) + } +} + +func TestReconcileMissing_RefusesOnEmptyWalk(t *testing.T) { + root := populatedRoot(t) + s := testScanner(t, root) + q := &fakeReconciler{rows: []dbq.ListTrackPathsForReconcileRow{ + row(1, "/music/a.mp3", false), + }} + + var stats Stats + if err := s.reconcileMissing(context.Background(), q, map[string]struct{}{}, &stats); err == nil { + t.Fatal("expected refusal when the walk saw no files") + } + if len(q.marked) != 0 { + t.Errorf("marked rows on an empty walk: %v", q.marked) + } +} + +// The unmounted-volume case: the configured root doesn't exist at all. +func TestReconcileMissing_RefusesWhenRootMissing(t *testing.T) { + s := testScanner(t, filepath.Join(t.TempDir(), "not-mounted")) + q := &fakeReconciler{rows: []dbq.ListTrackPathsForReconcileRow{ + row(1, "/music/a.mp3", false), + }} + + var stats Stats + if err := s.reconcileMissing(context.Background(), q, map[string]struct{}{"/x": {}}, &stats); err == nil { + t.Fatal("expected refusal when a scan root is absent") + } + if len(q.marked) != 0 { + t.Errorf("marked rows with an absent root: %v", q.marked) + } +} + +// A mount point that exists but has nothing mounted on it: os.Stat succeeds on +// the bare directory, which is why emptiness is checked separately. +func TestReconcileMissing_RefusesWhenRootEmpty(t *testing.T) { + s := testScanner(t, t.TempDir()) + q := &fakeReconciler{rows: []dbq.ListTrackPathsForReconcileRow{ + row(1, "/music/a.mp3", false), + }} + + var stats Stats + if err := s.reconcileMissing(context.Background(), q, map[string]struct{}{"/x": {}}, &stats); err == nil { + t.Fatal("expected refusal when a scan root is empty") + } +} + +// Several roots, one detached. Marking must not proceed on partial evidence just +// because the other roots looked fine. +func TestReconcileMissing_RefusesWhenAnyRootMissing(t *testing.T) { + good := populatedRoot(t) + s := testScanner(t, good, filepath.Join(t.TempDir(), "detached")) + q := &fakeReconciler{rows: []dbq.ListTrackPathsForReconcileRow{ + row(1, "/music/a.mp3", false), + }} + + var stats Stats + if err := s.reconcileMissing(context.Background(), q, map[string]struct{}{"/x": {}}, &stats); err == nil { + t.Fatal("expected refusal when one of several roots is absent") + } +} + +func TestReconcileMissing_NoRowsIsNotAnError(t *testing.T) { + root := populatedRoot(t) + s := testScanner(t, root) + q := &fakeReconciler{} + + var stats Stats + if err := s.reconcileMissing(context.Background(), q, map[string]struct{}{"/x": {}}, &stats); err != nil { + t.Fatalf("empty library should reconcile cleanly, got %v", err) + } +} + +func TestReconcileMissing_PropagatesListError(t *testing.T) { + root := populatedRoot(t) + s := testScanner(t, root) + sentinel := errors.New("boom") + q := &fakeReconciler{listErr: sentinel} + + var stats Stats + err := s.reconcileMissing(context.Background(), q, map[string]struct{}{"/x": {}}, &stats) + if !errors.Is(err, sentinel) { + t.Fatalf("err = %v, want it to wrap %v", err, sentinel) + } +} + +func TestVerifyRootsPresent_NoRootsConfigured(t *testing.T) { + s := testScanner(t) + if err := s.verifyRootsPresent(); err == nil { + t.Fatal("expected an error with no scan roots configured") + } +} diff --git a/internal/library/scanner.go b/internal/library/scanner.go index bd988986..a7116db6 100644 --- a/internal/library/scanner.go +++ b/internal/library/scanner.go @@ -60,6 +60,11 @@ type Stats struct { Updated int `json:"updated"` Skipped int `json:"skipped"` Errored int `json:"errored"` + // Missing / Restored come from the reconcile pass, not the walk (#2523): + // rows whose file the walk didn't find, and rows whose file came back. + // Only a full Scan sets these — see reconcileMissing. + Missing int `json:"missing"` + Restored int `json:"restored"` } type Scanner struct { @@ -76,6 +81,12 @@ func New(pool *pgxpool.Pool, logger *slog.Logger, paths []string) *Scanner { // newer than the existing row's updated_at. Walk errors and per-file errors // are logged + counted; the scan keeps going. // +// It then reconciles: rows whose file the walk never saw get marked missing, +// and rows whose file has come back get un-marked (#2523). Only a FULL scan may +// do this — the walk's set of seen paths is the evidence, and a partial +// (watcher-driven) scan has no basis for concluding anything about files it +// didn't look at. That's why ScanFiles does not reconcile. +// // progressCb (may be nil) receives the current Stats snapshot after each // processed file. Used by the orchestrator to drive partial-tally writes. func (s *Scanner) Scan(ctx context.Context, progressCb func(Stats)) (Stats, error) { @@ -83,6 +94,12 @@ func (s *Scanner) Scan(ctx context.Context, progressCb func(Stats)) (Stats, erro q := dbq.New(s.pool) start := time.Now() + // Every audio path the walk visited. Reconcile diffs this against the table, + // so it costs no extra filesystem I/O — the walk already established which + // files exist. ~100 bytes/path, so a 250k-track library is ~25MB, which is + // worth it to avoid a second stat pass over the whole library. + seen := make(map[string]struct{}, 8192) + for _, root := range s.paths { if err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { if ctx.Err() != nil { @@ -102,6 +119,11 @@ func (s *Scanner) Scan(ctx context.Context, progressCb func(Stats)) (Stats, erro if !audioExtensions[strings.ToLower(filepath.Ext(path))] { return nil } + // Recorded before scanFile so a file that exists but fails to parse + // still counts as present. It's a broken file, not a missing one, + // and marking it missing would hide it from the operator behind the + // wrong explanation. + seen[path] = struct{}{} if _, _, err := s.scanFile(ctx, q, path, &stats); err != nil { s.logger.Warn("library scan file error", "path", path, "err", err) stats.Errored++ @@ -115,12 +137,26 @@ func (s *Scanner) Scan(ctx context.Context, progressCb func(Stats)) (Stats, erro } } + // Reconcile only after a COMPLETE walk. A cancelled scan has a partial + // `seen` set, which would mark everything it hadn't reached yet. + if err := ctx.Err(); err != nil { + return stats, err + } + if err := s.reconcileMissing(ctx, q, seen, &stats); err != nil { + // Not fatal: the walk's results are already persisted and useful. The + // guards deliberately refuse to act on ambiguous evidence, and that + // refusal arrives here as an error. + s.logger.Warn("library scan: reconcile skipped", "err", err) + } + s.logger.Info("library scan complete", "scanned", stats.Scanned, "added", stats.Added, "updated", stats.Updated, "skipped", stats.Skipped, "errored", stats.Errored, + "missing", stats.Missing, + "restored", stats.Restored, "duration_ms", time.Since(start).Milliseconds(), ) if err := ctx.Err(); err != nil { diff --git a/internal/library/scanrun.go b/internal/library/scanrun.go index 5f42ecb5..e907eaab 100644 --- a/internal/library/scanrun.go +++ b/internal/library/scanrun.go @@ -24,6 +24,11 @@ type LibraryStageTallies struct { Updated int `json:"updated"` Skipped int `json:"skipped"` Errored int `json:"errored"` + // Reconcile results (#2523). Surfaced in the scan record because a track + // disappearing from the library is something the operator should be able to + // see happened, rather than discovering it when a mix comes up short. + Missing int `json:"missing"` + Restored int `json:"restored"` } // MBIDBackfillStageTallies wires BackfillMBIDsResult into the scan_runs jsonb column.