feat: video rips and stray copies handle themselves (M498 #5439)
release / govulncheck (push) Successful in 25s
release / web (push) Failing after 26s
release / go (push) Failing after 58s
release / Attach APK to the Release (tag releases only) (push) Canceled after 0s
release / Build + push container image (push) Canceled after 0s
release / Verify release artifacts (tag releases only) (push) Canceled after 0s
release / integration (push) Canceled after 3m25s
release / Build signed APK (releases and dev) (push) Canceled after 3m26s
release / android (push) Canceled after 3m28s

- The resolver also merges cross-release and mismatch groups where Lidarr
  maps exactly one copy: the others fulfil nothing, so removing them opens no
  hole (D-a rule 1). That covers a rip beside the clean copy on another
  release and a wrong-file import Lidarr holds unmapped. With two or more
  mapped copies each fulfils its own release and nothing is removed.
- A track whose file name carries a video-rip marker is held back from radio
  and the system mixes (tracks.source_verdict, migration 0077). It still plays
  when chosen. A renamed file is released; the operator's "fine" sticks.
- Suspect sources shows what was done to each track, with "This one is fine"
  and "Hold back again" (PUT /api/admin/library/suspect-sources/{id}).
- The Liked list prefers a copy that is not held back.

Replacing a rip that has no clean copy is left for the operator to decide.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-08 22:13:37 -04:00
co-authored by Claude Opus 5.5
parent 5b372f61d7
commit 53eb954c86
26 changed files with 638 additions and 63 deletions
+54 -3
View File
@@ -1,7 +1,11 @@
package api
import (
"encoding/json"
"net/http"
"time"
"github.com/go-chi/chi/v5"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
"git.fabledsword.com/bvandeusen/minstrel/internal/library"
@@ -22,6 +26,11 @@ type suspectTrackView struct {
DiscNumber *int32 `json:"disc_number"`
TrackNumber *int32 `json:"track_number"`
Markers []string `json:"markers"`
// Verdict is "suspect" while the track is held back from radio and the
// mixes, "fine" once the operator let it back, null before the resolver's
// first pass (M498 #5439). VerdictAt is when it was last set.
Verdict *string `json:"verdict"`
VerdictAt *string `json:"verdict_at"`
}
// suspectGroupView is a folder's worth of flagged tracks. As on the
@@ -42,9 +51,10 @@ type adminSuspectSourcesResponse struct {
// handleListSuspectSources implements GET /api/admin/library/suspect-sources.
//
// Read-only. A marker is a reason to look, not proof: a band can title a song
// "Music Video". What to do about a flagged file — merge it in Duplicates,
// quarantine it, replace it in Lidarr — stays the operator's call.
// A marker is a reason to look, not proof: a band can title a song "Music
// Video". The duplicate resolver holds a flagged track back from radio and the
// mixes and merges it away where a clean copy exists (M498 #5439); this report
// is where the operator sees that and can say a track is fine.
func (h *handlers) handleListSuspectSources(w http.ResponseWriter, r *http.Request) {
limit, offset, err := parsePaging(r.URL.Query())
if err != nil {
@@ -96,6 +106,11 @@ func groupSuspectByDirectory(rows []dbq.ListSuspectSourceTracksRow) []suspectGro
DiscNumber: row.DiscNumber,
TrackNumber: row.TrackNumber,
Markers: library.SourceMarkersFor(row.FilePath),
Verdict: row.SourceVerdict,
}
if row.SourceVerdictAt.Valid {
at := row.SourceVerdictAt.Time.UTC().Format(time.RFC3339)
t.VerdictAt = &at
}
if n := len(groups); n > 0 && groups[n-1].Directory == row.Directory {
groups[n-1].Tracks = append(groups[n-1].Tracks, t)
@@ -108,3 +123,39 @@ func groupSuspectByDirectory(rows []dbq.ListSuspectSourceTracksRow) []suspectGro
}
return groups
}
type suspectVerdictRequest struct {
Verdict string `json:"verdict"`
}
// handleSetSuspectSourceVerdict implements PUT
// /api/admin/library/suspect-sources/{id}: {"verdict": "fine"} lets a flagged
// track back into radio and the mixes for good, {"verdict": "suspect"} holds
// it back again. 404 suspect_track_not_found when the track was never flagged.
func (h *handlers) handleSetSuspectSourceVerdict(w http.ResponseWriter, r *http.Request) {
id, ok := parseUUID(chi.URLParam(r, "id"))
if !ok {
writeAdminJSONErr(w, http.StatusBadRequest, "invalid_id")
return
}
var req suspectVerdictRequest
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
writeAdminJSONErr(w, http.StatusBadRequest, "invalid_body")
return
}
if req.Verdict != "fine" && req.Verdict != "suspect" {
writeAdminJSONErr(w, http.StatusBadRequest, "invalid_verdict")
return
}
n, err := dbq.New(h.pool).SetTrackSourceVerdict(r.Context(), dbq.SetTrackSourceVerdictParams{Verdict: req.Verdict, ID: id})
if err != nil {
h.logger.Error("admin: set suspect-source verdict", "err", err)
writeAdminJSONErr(w, http.StatusInternalServerError, "server_error")
return
}
if n == 0 {
writeAdminJSONErr(w, http.StatusNotFound, "suspect_track_not_found")
return
}
writeJSON(w, http.StatusOK, map[string]string{"verdict": req.Verdict})
}
+1
View File
@@ -253,6 +253,7 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev
// not be mistaken for it (#2527).
admin.Get("/library/missing", h.handleListMissingTracks)
admin.Get("/library/suspect-sources", h.handleListSuspectSources)
admin.Put("/library/suspect-sources/{id}", h.handleSetSuspectSourceVerdict)
admin.Get("/library/coverage", h.handleGetLibraryCoverage)
admin.Get("/library/fingerprints", h.handleGetFingerprintCoverage)
+3 -1
View File
@@ -261,7 +261,7 @@ func (q *Queries) InsertSkipEvent(ctx context.Context, arg InsertSkipEventParams
}
const listRecentSessionTracks = `-- name: ListRecentSessionTracks :many
SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key FROM tracks t
SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key, t.source_verdict, t.source_verdict_at FROM tracks t
JOIN play_events pe ON pe.track_id = t.id
WHERE pe.session_id = $1
AND pe.started_at < $2
@@ -310,6 +310,8 @@ func (q *Queries) ListRecentSessionTracks(ctx context.Context, arg ListRecentSes
&i.MbidSource,
&i.SongID,
&i.SongKey,
&i.SourceVerdict,
&i.SourceVerdictAt,
); err != nil {
return nil, err
}
+3 -1
View File
@@ -14,7 +14,7 @@ import (
const listUserHistory = `-- name: ListUserHistory :many
SELECT pe.id AS event_id,
pe.started_at,
t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key,
t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key, t.source_verdict, t.source_verdict_at,
albums.title AS album_title,
artists.name AS artist_name
FROM play_events pe
@@ -84,6 +84,8 @@ func (q *Queries) ListUserHistory(ctx context.Context, arg ListUserHistoryParams
&i.Track.MbidSource,
&i.Track.SongID,
&i.Track.SongKey,
&i.Track.SourceVerdict,
&i.Track.SourceVerdictAt,
&i.AlbumTitle,
&i.ArtistName,
); err != nil {
+7 -4
View File
@@ -281,12 +281,13 @@ func (q *Queries) ListLikedTrackIDs(ctx context.Context, userID pgtype.UUID) ([]
}
const listLikedTrackRows = `-- name: ListLikedTrackRows :many
SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key FROM tracks t
SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key, t.source_verdict, t.source_verdict_at FROM tracks t
JOIN (SELECT DISTINCT ON (lt.song_key) l.track_id, l.liked_at
FROM general_likes l
JOIN tracks lt ON lt.id = l.track_id
WHERE l.user_id = $1
ORDER BY lt.song_key, (lt.missing_since IS NOT NULL), l.liked_at, lt.id) l
ORDER BY lt.song_key, (lt.missing_since IS NOT NULL),
(lt.source_verdict IS NOT DISTINCT FROM 'suspect'), l.liked_at, lt.id) l
ON l.track_id = t.id
ORDER BY l.liked_at DESC, t.id
LIMIT $2 OFFSET $3
@@ -298,8 +299,8 @@ type ListLikedTrackRowsParams struct {
Offset int32
}
// One row per liked song: of its copies, the one present on disk, then the
// one liked first.
// One row per liked song: of its copies, the one present on disk, then one
// that is not a held-back video rip, then the one liked first.
func (q *Queries) ListLikedTrackRows(ctx context.Context, arg ListLikedTrackRowsParams) ([]Track, error) {
rows, err := q.db.Query(ctx, listLikedTrackRows, arg.UserID, arg.Limit, arg.Offset)
if err != nil {
@@ -332,6 +333,8 @@ func (q *Queries) ListLikedTrackRows(ctx context.Context, arg ListLikedTrackRows
&i.MbidSource,
&i.SongID,
&i.SongKey,
&i.SourceVerdict,
&i.SourceVerdictAt,
); err != nil {
return nil, err
}
+2
View File
@@ -756,6 +756,8 @@ type Track struct {
MbidSource *string
SongID pgtype.UUID
SongKey pgtype.UUID
SourceVerdict *string
SourceVerdictAt pgtype.Timestamptz
}
type TrackAcoustidLookup struct {
+12 -4
View File
@@ -208,7 +208,7 @@ WITH plays AS (
WHERE user_id = $2 AND was_skipped = false
GROUP BY track_id
)
SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key,
SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key, t.source_verdict, t.source_verdict_at,
albums.title AS album_title,
artists.name AS artist_name
FROM plays p
@@ -273,6 +273,8 @@ func (q *Queries) ListMostPlayedTracksForArtist(ctx context.Context, arg ListMos
&i.Track.MbidSource,
&i.Track.SongID,
&i.Track.SongKey,
&i.Track.SourceVerdict,
&i.Track.SourceVerdictAt,
&i.AlbumTitle,
&i.ArtistName,
); err != nil {
@@ -293,7 +295,7 @@ WITH plays AS (
WHERE user_id = $1 AND was_skipped = false
GROUP BY track_id
)
SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key,
SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key, t.source_verdict, t.source_verdict_at,
albums.title AS album_title,
artists.name AS artist_name
FROM plays p
@@ -360,6 +362,8 @@ func (q *Queries) ListMostPlayedTracksForUser(ctx context.Context, arg ListMostP
&i.Track.MbidSource,
&i.Track.SongID,
&i.Track.SongKey,
&i.Track.SourceVerdict,
&i.Track.SourceVerdictAt,
&i.AlbumTitle,
&i.ArtistName,
); err != nil {
@@ -699,7 +703,7 @@ func (q *Queries) ListRediscoverArtistsForUser(ctx context.Context, arg ListRedi
const loadRadioCandidates = `-- name: LoadRadioCandidates :many
SELECT
t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key,
t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key, t.source_verdict, t.source_verdict_at,
(l.user_id IS NOT NULL)::bool AS is_liked,
pe.last_played_at::timestamptz AS last_played_at,
pe.play_count,
@@ -783,6 +787,8 @@ func (q *Queries) LoadRadioCandidates(ctx context.Context, arg LoadRadioCandidat
&i.Track.MbidSource,
&i.Track.SongID,
&i.Track.SongKey,
&i.Track.SourceVerdict,
&i.Track.SourceVerdictAt,
&i.IsLiked,
&i.LastPlayedAt,
&i.PlayCount,
@@ -931,7 +937,7 @@ random_fill AS (
LIMIT $9
)
SELECT
t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key,
t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key, t.source_verdict, t.source_verdict_at,
(l.user_id IS NOT NULL)::bool AS is_liked,
pe.last_played_at::timestamptz AS last_played_at,
pe.play_count,
@@ -1061,6 +1067,8 @@ func (q *Queries) LoadRadioCandidatesV2(ctx context.Context, arg LoadRadioCandid
&i.Track.MbidSource,
&i.Track.SongID,
&i.Track.SongKey,
&i.Track.SourceVerdict,
&i.Track.SourceVerdictAt,
&i.IsLiked,
&i.LastPlayedAt,
&i.PlayCount,
+8 -4
View File
@@ -46,14 +46,18 @@ func (q *Queries) LinkTracksAsSong(ctx context.Context, trackIds []pgtype.UUID)
}
const listTrackSongKeys = `-- name: ListTrackSongKeys :many
SELECT id, song_key FROM tracks WHERE id = ANY($1::uuid[])
SELECT id, song_key, (source_verdict IS NOT DISTINCT FROM 'suspect')::boolean AS held_back
FROM tracks WHERE id = ANY($1::uuid[])
`
type ListTrackSongKeysRow struct {
ID pgtype.UUID
SongKey pgtype.UUID
ID pgtype.UUID
SongKey pgtype.UUID
HeldBack bool
}
// What the mix writer needs to place a track: its song, and whether it is a
// video rip held back from the mixes (#5439).
func (q *Queries) ListTrackSongKeys(ctx context.Context, ids []pgtype.UUID) ([]ListTrackSongKeysRow, error) {
rows, err := q.db.Query(ctx, listTrackSongKeys, ids)
if err != nil {
@@ -63,7 +67,7 @@ func (q *Queries) ListTrackSongKeys(ctx context.Context, ids []pgtype.UUID) ([]L
var items []ListTrackSongKeysRow
for rows.Next() {
var i ListTrackSongKeysRow
if err := rows.Scan(&i.ID, &i.SongKey); err != nil {
if err := rows.Scan(&i.ID, &i.SongKey, &i.HeldBack); err != nil {
return nil, err
}
items = append(items, i)
+100 -20
View File
@@ -39,6 +39,23 @@ func (q *Queries) AdoptTrackPath(ctx context.Context, arg AdoptTrackPathParams)
return result.RowsAffected(), nil
}
const clearStaleSuspectSourceTracks = `-- name: ClearStaleSuspectSourceTracks :execrows
UPDATE tracks
SET source_verdict = NULL, source_verdict_at = NULL
WHERE source_verdict = 'suspect'
AND NOT regexp_replace(file_path, '^.*/', '') ~* $1::text
`
// A held-back track whose name no longer carries a marker (renamed, retagged
// by Lidarr) is released again.
func (q *Queries) ClearStaleSuspectSourceTracks(ctx context.Context, pattern string) (int64, error) {
result, err := q.db.Exec(ctx, clearStaleSuspectSourceTracks, pattern)
if err != nil {
return 0, err
}
return result.RowsAffected(), nil
}
const clearTracksMissing = `-- name: ClearTracksMissing :execrows
UPDATE tracks
SET missing_since = NULL
@@ -246,8 +263,27 @@ func (q *Queries) FindMissingTrackByMbid(ctx context.Context, mbid string) ([]Fi
return items, nil
}
const flagSuspectSourceTracks = `-- name: FlagSuspectSourceTracks :execrows
UPDATE tracks
SET source_verdict = 'suspect', source_verdict_at = now()
WHERE missing_since IS NULL
AND source_verdict IS NULL
AND regexp_replace(file_path, '^.*/', '') ~* $1::text
`
// Holds back present tracks whose basename carries a video-rip marker (M498
// #5439), the same pattern the report lists. A track the operator called fine
// keeps that verdict.
func (q *Queries) FlagSuspectSourceTracks(ctx context.Context, pattern string) (int64, error) {
result, err := q.db.Exec(ctx, flagSuspectSourceTracks, pattern)
if err != nil {
return 0, err
}
return result.RowsAffected(), nil
}
const getTrackByID = `-- name: GetTrackByID :one
SELECT id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at, tag_source, tag_sources_version, tag_read_version, missing_since, mbid_source, song_id, song_key FROM tracks WHERE id = $1
SELECT id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at, tag_source, tag_sources_version, tag_read_version, missing_since, mbid_source, song_id, song_key, source_verdict, source_verdict_at FROM tracks WHERE id = $1
`
func (q *Queries) GetTrackByID(ctx context.Context, id pgtype.UUID) (Track, error) {
@@ -276,12 +312,14 @@ func (q *Queries) GetTrackByID(ctx context.Context, id pgtype.UUID) (Track, erro
&i.MbidSource,
&i.SongID,
&i.SongKey,
&i.SourceVerdict,
&i.SourceVerdictAt,
)
return i, err
}
const getTrackByPath = `-- name: GetTrackByPath :one
SELECT id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at, tag_source, tag_sources_version, tag_read_version, missing_since, mbid_source, song_id, song_key FROM tracks WHERE file_path = $1
SELECT id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at, tag_source, tag_sources_version, tag_read_version, missing_since, mbid_source, song_id, song_key, source_verdict, source_verdict_at FROM tracks WHERE file_path = $1
`
func (q *Queries) GetTrackByPath(ctx context.Context, filePath string) (Track, error) {
@@ -310,12 +348,14 @@ func (q *Queries) GetTrackByPath(ctx context.Context, filePath string) (Track, e
&i.MbidSource,
&i.SongID,
&i.SongKey,
&i.SourceVerdict,
&i.SourceVerdictAt,
)
return i, err
}
const getTracksByIDs = `-- name: GetTracksByIDs :many
SELECT id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at, tag_source, tag_sources_version, tag_read_version, missing_since, mbid_source, song_id, song_key FROM tracks WHERE id = ANY($1::uuid[])
SELECT id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at, tag_source, tag_sources_version, tag_read_version, missing_since, mbid_source, song_id, song_key, source_verdict, source_verdict_at FROM tracks WHERE id = ANY($1::uuid[])
`
// Batched lookup used by /api/library/sync to hydrate upsert payloads
@@ -352,6 +392,8 @@ func (q *Queries) GetTracksByIDs(ctx context.Context, dollar_1 []pgtype.UUID) ([
&i.MbidSource,
&i.SongID,
&i.SongKey,
&i.SourceVerdict,
&i.SourceVerdictAt,
); err != nil {
return nil, err
}
@@ -364,7 +406,7 @@ func (q *Queries) GetTracksByIDs(ctx context.Context, dollar_1 []pgtype.UUID) ([
}
const listArtistTracksForUser = `-- name: ListArtistTracksForUser :many
SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key,
SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key, t.source_verdict, t.source_verdict_at,
albums.title AS album_title,
artists.name AS artist_name
FROM tracks t
@@ -427,6 +469,8 @@ func (q *Queries) ListArtistTracksForUser(ctx context.Context, arg ListArtistTra
&i.Track.MbidSource,
&i.Track.SongID,
&i.Track.SongKey,
&i.Track.SourceVerdict,
&i.Track.SourceVerdictAt,
&i.AlbumTitle,
&i.ArtistName,
); err != nil {
@@ -527,7 +571,7 @@ func (q *Queries) ListMissingTracks(ctx context.Context, arg ListMissingTracksPa
}
const listRandomTracksForUser = `-- name: ListRandomTracksForUser :many
SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key,
SELECT t.id, t.title, t.album_id, t.artist_id, t.track_number, t.disc_number, t.duration_ms, t.file_path, t.file_size, t.file_format, t.bitrate, t.mbid, t.genre, t.added_at, t.updated_at, t.tag_source, t.tag_sources_version, t.tag_read_version, t.missing_since, t.mbid_source, t.song_id, t.song_key, t.source_verdict, t.source_verdict_at,
albums.title AS album_title,
artists.name AS artist_name
FROM tracks t
@@ -587,6 +631,8 @@ func (q *Queries) ListRandomTracksForUser(ctx context.Context, arg ListRandomTra
&i.Track.MbidSource,
&i.Track.SongID,
&i.Track.SongKey,
&i.Track.SourceVerdict,
&i.Track.SourceVerdictAt,
&i.AlbumTitle,
&i.ArtistName,
); err != nil {
@@ -611,7 +657,9 @@ SELECT t.id,
albums.id AS album_id,
albums.title AS album_title,
artists.id AS artist_id,
artists.name AS artist_name
artists.name AS artist_name,
t.source_verdict,
t.source_verdict_at
FROM tracks t
JOIN albums ON albums.id = t.album_id
JOIN artists ON artists.id = t.artist_id
@@ -628,17 +676,19 @@ type ListSuspectSourceTracksParams struct {
}
type ListSuspectSourceTracksRow struct {
ID pgtype.UUID
Title string
FilePath string
Directory string
DurationMs int32
DiscNumber *int32
TrackNumber *int32
AlbumID pgtype.UUID
AlbumTitle string
ArtistID pgtype.UUID
ArtistName string
ID pgtype.UUID
Title string
FilePath string
Directory string
DurationMs int32
DiscNumber *int32
TrackNumber *int32
AlbumID pgtype.UUID
AlbumTitle string
ArtistID pgtype.UUID
ArtistName string
SourceVerdict *string
SourceVerdictAt pgtype.Timestamptz
}
// The admin report of present tracks whose filename looks like a video rip
@@ -672,6 +722,8 @@ func (q *Queries) ListSuspectSourceTracks(ctx context.Context, arg ListSuspectSo
&i.AlbumTitle,
&i.ArtistID,
&i.ArtistName,
&i.SourceVerdict,
&i.SourceVerdictAt,
); err != nil {
return nil, err
}
@@ -719,7 +771,7 @@ func (q *Queries) ListTrackPathsForReconcile(ctx context.Context) ([]ListTrackPa
}
const listTracksByAlbum = `-- name: ListTracksByAlbum :many
SELECT id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at, tag_source, tag_sources_version, tag_read_version, missing_since, mbid_source, song_id, song_key FROM tracks
SELECT id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at, tag_source, tag_sources_version, tag_read_version, missing_since, mbid_source, song_id, song_key, source_verdict, source_verdict_at FROM tracks
WHERE album_id = $1
AND NOT EXISTS (
SELECT 1 FROM lidarr_quarantine q
@@ -768,6 +820,8 @@ func (q *Queries) ListTracksByAlbum(ctx context.Context, arg ListTracksByAlbumPa
&i.MbidSource,
&i.SongID,
&i.SongKey,
&i.SourceVerdict,
&i.SourceVerdictAt,
); err != nil {
return nil, err
}
@@ -841,7 +895,7 @@ func (q *Queries) MarkTracksMissing(ctx context.Context, ids []pgtype.UUID) (int
}
const searchTracks = `-- name: SearchTracks :many
SELECT id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at, tag_source, tag_sources_version, tag_read_version, missing_since, mbid_source, song_id, song_key FROM tracks
SELECT id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at, tag_source, tag_sources_version, tag_read_version, missing_since, mbid_source, song_id, song_key, source_verdict, source_verdict_at FROM tracks
WHERE title ILIKE '%' || $1::text || '%'
AND NOT EXISTS (
SELECT 1 FROM lidarr_quarantine q
@@ -897,6 +951,8 @@ func (q *Queries) SearchTracks(ctx context.Context, arg SearchTracksParams) ([]T
&i.MbidSource,
&i.SongID,
&i.SongKey,
&i.SourceVerdict,
&i.SourceVerdictAt,
); err != nil {
return nil, err
}
@@ -926,6 +982,28 @@ func (q *Queries) SetTrackMbidIfNull(ctx context.Context, arg SetTrackMbidIfNull
return err
}
const setTrackSourceVerdict = `-- name: SetTrackSourceVerdict :execrows
UPDATE tracks
SET source_verdict = $1::text, source_verdict_at = now()
WHERE id = $2
AND source_verdict IS NOT NULL
`
type SetTrackSourceVerdictParams struct {
Verdict string
ID pgtype.UUID
}
// The operator's call on a flagged track: 'fine' lets it back into radio and
// the mixes for good; 'suspect' holds it back again.
func (q *Queries) SetTrackSourceVerdict(ctx context.Context, arg SetTrackSourceVerdictParams) (int64, error) {
result, err := q.db.Exec(ctx, setTrackSourceVerdict, arg.Verdict, arg.ID)
if err != nil {
return 0, err
}
return result.RowsAffected(), nil
}
const upsertTrack = `-- name: UpsertTrack :one
INSERT INTO tracks (
title, album_id, artist_id, track_number, disc_number,
@@ -953,7 +1031,7 @@ ON CONFLICT (file_path) DO UPDATE SET
-- next scan can short-circuit them again (#2499).
tag_read_version = EXCLUDED.tag_read_version,
updated_at = now()
RETURNING id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at, tag_source, tag_sources_version, tag_read_version, missing_since, mbid_source, song_id, song_key
RETURNING id, title, album_id, artist_id, track_number, disc_number, duration_ms, file_path, file_size, file_format, bitrate, mbid, genre, added_at, updated_at, tag_source, tag_sources_version, tag_read_version, missing_since, mbid_source, song_id, song_key, source_verdict, source_verdict_at
`
type UpsertTrackParams struct {
@@ -1015,6 +1093,8 @@ func (q *Queries) UpsertTrack(ctx context.Context, arg UpsertTrackParams) (Track
&i.MbidSource,
&i.SongID,
&i.SongKey,
&i.SourceVerdict,
&i.SourceVerdictAt,
)
return i, err
}
@@ -0,0 +1,3 @@
DROP INDEX IF EXISTS tracks_source_suspect_idx;
ALTER TABLE tracks DROP COLUMN IF EXISTS source_verdict_at;
ALTER TABLE tracks DROP COLUMN IF EXISTS source_verdict;
@@ -0,0 +1,15 @@
-- 0077_track_source_verdict.up.sql — video rips are held back from mixes and
-- radio until the operator says otherwise (Scribe milestone #498, #5439;
-- operator decision D-d).
--
-- A file whose name carries a video-rip marker ("(Official Video)", "[HD]",
-- #5410) holds the audio of an upload, which can carry intros, skits or a
-- different take. The duplicate resolver sets 'suspect' on such a present
-- track each pass, and clears it when the name no longer matches. 'suspect'
-- keeps the track out of radio and the system mixes; it still plays when
-- chosen. 'fine' is the operator's "this one is fine" and is never
-- overwritten by the resolver.
ALTER TABLE tracks ADD COLUMN source_verdict text
CONSTRAINT tracks_source_verdict_check CHECK (source_verdict IN ('suspect', 'fine'));
ALTER TABLE tracks ADD COLUMN source_verdict_at timestamptz;
CREATE INDEX tracks_source_suspect_idx ON tracks (id) WHERE source_verdict = 'suspect';
+4 -3
View File
@@ -20,14 +20,15 @@ DELETE FROM general_likes
RETURNING track_id;
-- name: ListLikedTrackRows :many
-- One row per liked song: of its copies, the one present on disk, then the
-- one liked first.
-- One row per liked song: of its copies, the one present on disk, then one
-- that is not a held-back video rip, then the one liked first.
SELECT t.* FROM tracks t
JOIN (SELECT DISTINCT ON (lt.song_key) l.track_id, l.liked_at
FROM general_likes l
JOIN tracks lt ON lt.id = l.track_id
WHERE l.user_id = $1
ORDER BY lt.song_key, (lt.missing_since IS NOT NULL), l.liked_at, lt.id) l
ORDER BY lt.song_key, (lt.missing_since IS NOT NULL),
(lt.source_verdict IS NOT DISTINCT FROM 'suspect'), l.liked_at, lt.id) l
ON l.track_id = t.id
ORDER BY l.liked_at DESC, t.id
LIMIT $2 OFFSET $3;
+4 -1
View File
@@ -40,7 +40,10 @@ UPDATE tracks SET song_id = NULL
AND id IN (SELECT track_id FROM duplicate_group_members WHERE group_id = sqlc.arg(group_id));
-- name: ListTrackSongKeys :many
SELECT id, song_key FROM tracks WHERE id = ANY(sqlc.arg(ids)::uuid[]);
-- What the mix writer needs to place a track: its song, and whether it is a
-- video rip held back from the mixes (#5439).
SELECT id, song_key, (source_verdict IS NOT DISTINCT FROM 'suspect')::boolean AS held_back
FROM tracks WHERE id = ANY(sqlc.arg(ids)::uuid[]);
-- name: MergeInheritSongLink :exec
-- A merge keeps the removed copy's song link: when the copy being removed was
+29 -1
View File
@@ -285,7 +285,9 @@ SELECT t.id,
albums.id AS album_id,
albums.title AS album_title,
artists.id AS artist_id,
artists.name AS artist_name
artists.name AS artist_name,
t.source_verdict,
t.source_verdict_at
FROM tracks t
JOIN albums ON albums.id = t.album_id
JOIN artists ON artists.id = t.artist_id
@@ -299,3 +301,29 @@ SELECT t.id,
SELECT COUNT(*) FROM tracks t
WHERE t.missing_since IS NULL
AND regexp_replace(t.file_path, '^.*/', '') ~* sqlc.arg(pattern)::text;
-- name: FlagSuspectSourceTracks :execrows
-- Holds back present tracks whose basename carries a video-rip marker (M498
-- #5439), the same pattern the report lists. A track the operator called fine
-- keeps that verdict.
UPDATE tracks
SET source_verdict = 'suspect', source_verdict_at = now()
WHERE missing_since IS NULL
AND source_verdict IS NULL
AND regexp_replace(file_path, '^.*/', '') ~* sqlc.arg(pattern)::text;
-- name: ClearStaleSuspectSourceTracks :execrows
-- A held-back track whose name no longer carries a marker (renamed, retagged
-- by Lidarr) is released again.
UPDATE tracks
SET source_verdict = NULL, source_verdict_at = NULL
WHERE source_verdict = 'suspect'
AND NOT regexp_replace(file_path, '^.*/', '') ~* sqlc.arg(pattern)::text;
-- name: SetTrackSourceVerdict :execrows
-- The operator's call on a flagged track: 'fine' lets it back into radio and
-- the mixes for good; 'suspect' holds it back again.
UPDATE tracks
SET source_verdict = sqlc.arg(verdict)::text, source_verdict_at = now()
WHERE id = sqlc.arg(id)
AND source_verdict IS NOT NULL;
+43 -3
View File
@@ -91,6 +91,9 @@ type DuplicateResolveResult struct {
ReleaseChanges []ReleaseChange
// SongsLinked counts cross-release groups newly linked as one song.
SongsLinked int
// HeldBack counts tracks newly held back from radio and the mixes as video
// rips; Released counts held-back tracks whose name no longer says so.
HeldBack, Released int64
}
// LidarrPathKey is the part of a path Minstrel and Lidarr agree on: the last
@@ -115,6 +118,35 @@ type resolveGroup struct {
states []string // per member: tracked, unmapped, or "" when Lidarr was not asked
}
// autoMergeable says whether the resolver may merge the group on its own: when
// every copy it would remove is one Lidarr does not map (D-a rule 1). On one
// album that is any group with at most one mapped copy. Across releases each
// mapped copy fulfils its own release (D-b), so only a group with exactly one
// mapped copy qualifies: the others fulfil nothing — a rip beside the clean
// copy on another release, or a wrong-file import Lidarr holds unmapped
// (#5439) — and the mapped copy is the clear one to keep.
func autoMergeable(g *resolveGroup) bool {
switch g.class {
case ClassSameRelease:
return g.tracked() <= 1
case ClassCrossRelease, ClassMismatch:
return g.tracked() == 1
default:
return false
}
}
// settling says whether any of the group's albums had its Lidarr release
// changed recently, so Lidarr may still be remapping its files.
func (g *resolveGroup) settling(albums map[string]bool) bool {
for _, m := range g.members {
if albums[syncpkg.FormatUUID(m.AlbumID)] {
return true
}
}
return false
}
func (g *resolveGroup) tracked() int {
n := 0
for _, s := range g.states {
@@ -183,7 +215,14 @@ func ResolveDuplicates(
return res, nil
}
// Linking removes nothing and needs nothing from Lidarr.
// Holding rips back and linking songs remove nothing and need nothing from
// Lidarr.
if res.HeldBack, err = q.FlagSuspectSourceTracks(ctx, SuspectSourcePattern); err != nil {
return res, fmt.Errorf("hold back video rips: %w", err)
}
if res.Released, err = q.ClearStaleSuspectSourceTracks(ctx, SuspectSourcePattern); err != nil {
return res, fmt.Errorf("release renamed tracks: %w", err)
}
linked, err := linkCrossRelease(ctx, pool, logger, groups)
res.SongsLinked = linked
if err != nil {
@@ -208,7 +247,7 @@ func ResolveDuplicates(
if ctx.Err() != nil {
return res, ctx.Err()
}
if g.class != ClassSameRelease || g.tracked() > 1 || settling[syncpkg.FormatUUID(g.members[0].AlbumID)] {
if !autoMergeable(g) || g.settling(settling) {
continue
}
if err := autoMerge(ctx, pool, logger, dataDir, g, removable); err != nil {
@@ -747,5 +786,6 @@ func (w *DuplicateResolveWorker) tickOnce(ctx context.Context) {
}
w.logger.Info("duplicate resolve complete",
"groups", res.Groups, "lidarr", res.LidarrConsulted, "merged", res.Merged,
"merge_failed", res.MergeFailed, "release_changes", len(res.ReleaseChanges), "songs_linked", res.SongsLinked)
"merge_failed", res.MergeFailed, "release_changes", len(res.ReleaseChanges), "songs_linked", res.SongsLinked,
"held_back", res.HeldBack, "released", res.Released)
}
+155
View File
@@ -347,3 +347,158 @@ func TestMergeDuplicateGroupGuarded_RefusesATrackedCopy_Integration(t *testing.T
t.Errorf("group status = %s, want pending", status)
}
}
// crossReleaseRipFixture is a video rip on one release with the clean copy on
// another: the album cut, and the same song's video filed as a single.
type crossReleaseRipFixture struct {
pool *pgxpool.Pool
clean, rip dbq.Track
cleanPath, ripPath string
groupID pgtype.UUID
}
func newCrossReleaseRipFixture(t *testing.T) crossReleaseRipFixture {
t.Helper()
pool := newPool(t)
ctx := context.Background()
q := dbq.New(pool)
root := t.TempDir()
f := crossReleaseRipFixture{pool: pool}
f.cleanPath = filepath.Join(root, "Gorillaz", "Demon Days (2005)", "06 - Feel Good Inc.mp3")
f.ripPath = filepath.Join(root, "Gorillaz", "Feel Good Inc (2005)", "01 - Feel Good Inc. (Official Video).mp3")
artist, err := q.UpsertArtist(ctx, dbq.UpsertArtistParams{Name: "Gorillaz", SortName: "Gorillaz"})
if err != nil {
t.Fatal(err)
}
track := func(albumTitle, title, path string) dbq.Track {
t.Helper()
if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(path, []byte("audio"), 0o644); err != nil {
t.Fatal(err)
}
album, err := q.UpsertAlbum(ctx, dbq.UpsertAlbumParams{Title: albumTitle, SortTitle: albumTitle, ArtistID: artist.ID})
if err != nil {
t.Fatal(err)
}
tr, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{
Title: title, AlbumID: album.ID, ArtistID: artist.ID,
DurationMs: 1000, FilePath: path, FileSize: 100, FileFormat: "mp3",
})
if err != nil {
t.Fatal(err)
}
return tr
}
f.clean = track("Demon Days", "Feel Good Inc.", f.cleanPath)
f.rip = track("Feel Good Inc.", "Feel Good Inc. (Official Video)", f.ripPath)
if err := pool.QueryRow(ctx,
`INSERT INTO duplicate_groups (member_key, tier) VALUES ('cross-rip', 'acoustic') RETURNING id`,
).Scan(&f.groupID); err != nil {
t.Fatal(err)
}
if _, err := pool.Exec(ctx,
`INSERT INTO duplicate_group_members (group_id, track_id) VALUES ($1, $2), ($1, $3)`,
f.groupID, f.clean.ID, f.rip.ID); err != nil {
t.Fatal(err)
}
return f
}
// The rip fulfils nothing in Lidarr while the clean copy on the album does:
// the rip goes and the album copy stays (#5439, D-a rule 1).
func TestResolveDuplicates_MergesAnUnmappedRipAcrossReleases_Integration(t *testing.T) {
f := newCrossReleaseRipFixture(t)
lid := &fakeLidarr{unmapped: []string{lidarrPath(f.ripPath)}}
res, err := ResolveDuplicates(context.Background(), f.pool, nil, "", lid, true)
if err != nil {
t.Fatal(err)
}
if res.Classes[ClassCrossRelease] != 1 || res.Merged != 1 {
t.Fatalf("classes %v, merged %d; want the cross-release group merged", res.Classes, res.Merged)
}
if exists(f.ripPath) || !exists(f.cleanPath) {
t.Errorf("rip exists %v, clean exists %v; want only the clean copy", exists(f.ripPath), exists(f.cleanPath))
}
}
// When Lidarr maps both copies, each fulfils its own release: nothing is
// removed, and the copies are linked as one song instead.
func TestResolveDuplicates_KeepsCopiesLidarrMapsOnTwoReleases_Integration(t *testing.T) {
f := newCrossReleaseRipFixture(t)
res, err := ResolveDuplicates(context.Background(), f.pool, nil, "", &fakeLidarr{}, true)
if err != nil {
t.Fatal(err)
}
if res.Merged != 0 || !exists(f.ripPath) {
t.Errorf("merged %d; want both copies kept", res.Merged)
}
if res.SongsLinked != 1 {
t.Errorf("linked %d, want the pair linked as one song", res.SongsLinked)
}
}
// A rip is held back from radio and the mixes; the operator's "fine" sticks
// through later passes; a renamed file is released.
func TestResolveDuplicates_HoldsBackRips_Integration(t *testing.T) {
f := newCrossReleaseRipFixture(t)
ctx := context.Background()
q := dbq.New(f.pool)
verdict := func(id pgtype.UUID) *string {
t.Helper()
var v *string
if err := f.pool.QueryRow(ctx, `SELECT source_verdict FROM tracks WHERE id = $1`, id).Scan(&v); err != nil {
t.Fatal(err)
}
return v
}
resolve := func() DuplicateResolveResult {
t.Helper()
res, err := ResolveDuplicates(ctx, f.pool, nil, "", &fakeLidarr{}, true)
if err != nil {
t.Fatal(err)
}
return res
}
if res := resolve(); res.HeldBack != 1 {
t.Errorf("held back %d, want the rip", res.HeldBack)
}
if v := verdict(f.rip.ID); v == nil || *v != "suspect" {
t.Errorf("rip verdict %v, want suspect", v)
}
if v := verdict(f.clean.ID); v != nil {
t.Errorf("clean copy verdict %v, want none", *v)
}
if n, err := q.SetTrackSourceVerdict(ctx, dbq.SetTrackSourceVerdictParams{Verdict: "fine", ID: f.rip.ID}); err != nil || n != 1 {
t.Fatalf("set fine: %d, %v", n, err)
}
if res := resolve(); res.HeldBack != 0 {
t.Errorf("held back %d after the operator's fine, want 0", res.HeldBack)
}
if v := verdict(f.rip.ID); v == nil || *v != "fine" {
t.Errorf("rip verdict %v, want fine kept", v)
}
// Held back again, then renamed: the next pass lets it go.
if _, err := q.SetTrackSourceVerdict(ctx, dbq.SetTrackSourceVerdictParams{Verdict: "suspect", ID: f.rip.ID}); err != nil {
t.Fatal(err)
}
if _, err := f.pool.Exec(ctx, `UPDATE tracks SET file_path = $2 WHERE id = $1`,
f.rip.ID, filepath.Join(filepath.Dir(f.ripPath), "01 - Feel Good Inc.mp3")); err != nil {
t.Fatal(err)
}
if res := resolve(); res.Released != 1 {
t.Errorf("released %d, want the renamed track", res.Released)
}
if v := verdict(f.rip.ID); v != nil {
t.Errorf("renamed track verdict %v, want none", *v)
}
// The clean copy can't be flagged by hand: there is nothing to judge.
if n, err := q.SetTrackSourceVerdict(ctx, dbq.SetTrackSourceVerdictParams{Verdict: "fine", ID: f.clean.ID}); err != nil || n != 0 {
t.Errorf("set on an unflagged track: %d, %v; want 0 rows", n, err)
}
}
+49
View File
@@ -0,0 +1,49 @@
package playlists
import (
"context"
"testing"
"github.com/jackc/pgx/v5/pgtype"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
)
// A mix holds a song once (M498 #5438) and never a held-back video rip
// (#5439): of a single and its album cut, the better-ranked copy stays; a rip
// is left out and the next copy of its song takes the place.
func TestOneCopyPerSong_Integration(t *testing.T) {
pool := newPool(t)
ctx := context.Background()
q := dbq.New(pool)
single := seedTrack(t, pool, "copies-single", "copies-artist")
albumCut := seedTrack(t, pool, "copies-album-cut", "copies-artist")
rip := seedTrack(t, pool, "copies-rip", "copies-artist")
clean := seedTrack(t, pool, "copies-clean", "copies-artist")
other := seedTrack(t, pool, "copies-other", "copies-artist")
for _, pair := range [][]pgtype.UUID{{single.ID, albumCut.ID}, {rip.ID, clean.ID}} {
if _, err := q.LinkTracksAsSong(ctx, pair); err != nil {
t.Fatal(err)
}
}
if _, err := pool.Exec(ctx, `UPDATE tracks SET source_verdict = 'suspect' WHERE id = $1`, rip.ID); err != nil {
t.Fatal(err)
}
in := []rankedCandidate{
{TrackID: single.ID}, {TrackID: rip.ID}, {TrackID: albumCut.ID}, {TrackID: other.ID}, {TrackID: clean.ID},
}
out, err := oneCopyPerSong(ctx, q, in)
if err != nil {
t.Fatal(err)
}
want := []pgtype.UUID{single.ID, other.ID, clean.ID}
if len(out) != len(want) {
t.Fatalf("got %d tracks, want %d", len(out), len(want))
}
for i, w := range want {
if out[i].TrackID != w {
t.Errorf("position %d = %s, want %s", i, uuidString(out[i].TrackID), uuidString(w))
}
}
}
+10 -4
View File
@@ -1101,7 +1101,9 @@ func insertSystemPlaylist(ctx context.Context, qtx *dbq.Queries, userID pgtype.U
}
// oneCopyPerSong keeps the first of a song's copies on different releases
// (M498 #5438): a mix holds the song once, in its best-ranked place.
// (M498 #5438): a mix holds the song once, in its best-ranked place. A video
// rip held back from the mixes (#5439) is left out, and a clean copy of the
// same song can take its place.
func oneCopyPerSong(ctx context.Context, qtx *dbq.Queries, tracks []rankedCandidate) ([]rankedCandidate, error) {
ids := make([]pgtype.UUID, len(tracks))
for i, t := range tracks {
@@ -1111,14 +1113,18 @@ func oneCopyPerSong(ctx context.Context, qtx *dbq.Queries, tracks []rankedCandid
if err != nil {
return nil, fmt.Errorf("song keys: %w", err)
}
songOf := make(map[pgtype.UUID]pgtype.UUID, len(rows))
songOf := make(map[pgtype.UUID]dbq.ListTrackSongKeysRow, len(rows))
for _, r := range rows {
songOf[r.ID] = r.SongKey
songOf[r.ID] = r
}
seen := make(map[pgtype.UUID]bool, len(tracks))
out := tracks[:0:0]
for _, t := range tracks {
song, ok := songOf[t.TrackID]
row, ok := songOf[t.TrackID]
if ok && row.HeldBack {
continue
}
song := row.SongKey
if ok && seen[song] {
continue
}
+10 -2
View File
@@ -52,6 +52,12 @@ func RadioDiversityCaps(limit int) DiversityCaps {
return DiversityCaps{MaxPerArtist: artist, MaxPerAlbum: album}
}
// heldBack says whether the candidate is a video rip held back from radio
// (M498 #5439): it plays when chosen, but radio does not choose it.
func heldBack(c Candidate) bool {
return c.Track.SourceVerdict != nil && *c.Track.SourceVerdict == "suspect"
}
// Shuffle scores each candidate, sorts descending by score, and returns the
// top `limit` candidates, preferring artist/album diversity. limit <= 0
// returns nil; nil input returns nil. Pure — no IO, no global state beyond
@@ -61,7 +67,9 @@ func RadioDiversityCaps(limit int) DiversityCaps {
// first takes candidates that fit under the caps, and the second fills any
// remaining slots from those the first pass skipped, still in score order.
// So the result holds min(limit, len(candidates)) either way — the caps
// change WHICH tracks are chosen, never HOW MANY.
// change WHICH tracks are chosen, never HOW MANY. (What is not a candidate at
// all is dropped first: a held-back video rip, and a second copy of a song
// already chosen from another release, M498.)
//
// That two-pass shape is the whole design, and a hard cap would have been
// the easy mistake. Radio asks for 50 tracks by default and 200 at most; a
@@ -119,7 +127,7 @@ func Shuffle(
if len(out) == limit {
break
}
if repeat(s.c) {
if heldBack(s.c) || repeat(s.c) {
continue
}
overArtist := caps.MaxPerArtist > 0 && artistCount[s.c.Track.ArtistID] >= caps.MaxPerArtist
+15
View File
@@ -111,3 +111,18 @@ func TestShuffle_OneCopyPerSong(t *testing.T) {
t.Errorf("got %v, want two tracks: one copy of the song", titles(out))
}
}
// A video rip held back from radio (M498 #5439) is never chosen, even to fill
// the request.
func TestShuffle_SkipsHeldBackRips(t *testing.T) {
suspect := "suspect"
fine := "fine"
rip := cand("000000000001", ScoringInputs{IsGeneralLiked: true})
rip.Track.SourceVerdict = &suspect
cleared := cand("000000000002", ScoringInputs{})
cleared.Track.SourceVerdict = &fine
out := Shuffle([]Candidate{rip, cleared}, defaultWeights(), time.Now(), fixedRNG(0.5), 10, DiversityCaps{})
if got := titles(out); len(got) != 1 || got[0] != "000000000002" {
t.Errorf("got %v, want only the track marked fine", got)
}
}