fix: a second copy of a track counts once, joining words don't part two titles, and a newer matcher retries an older one's refusals (M502 #5569)
release / govulncheck (push) Successful in 15s
release / web (push) Successful in 1m18s
release / go (push) Successful in 1m35s
release / integration (push) Failing after 5m33s
release / android (push) Successful in 5m47s
release / Build signed APK (releases and dev) (push) Successful in 6m4s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Skipped
release / Verify release artifacts (tag releases only) (push) Skipped
release / govulncheck (push) Successful in 15s
release / web (push) Successful in 1m18s
release / go (push) Successful in 1m35s
release / integration (push) Failing after 5m33s
release / android (push) Successful in 5m47s
release / Build signed APK (releases and dev) (push) Successful in 6m4s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Skipped
release / Verify release artifacts (tag releases only) (push) Skipped
The second run's refusals were mostly albums holding a track twice (a two-track single with three files), which no edition can pair, and release names that differ only in "+" against "/". Tracks are now read once per disc, number and title, and "and", "&" and "+" count for nothing when titles are compared; an edition's own title also counts as the album's name. library_album_matches records which matcher version decided each album. Refusals by an older version are matched again, even after the listing is done, so an improvement reaches them without a migration. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -494,6 +494,7 @@ type LibraryAlbumMatch struct {
|
|||||||
ReleaseGroupMbid string
|
ReleaseGroupMbid string
|
||||||
Detail string
|
Detail string
|
||||||
DecidedAt pgtype.Timestamptz
|
DecidedAt pgtype.Timestamptz
|
||||||
|
Matcher int32
|
||||||
}
|
}
|
||||||
|
|
||||||
type LibraryChange struct {
|
type LibraryChange struct {
|
||||||
|
|||||||
@@ -20,6 +20,19 @@ func (q *Queries) ClearDefaultReleaseTypeProfile(ctx context.Context) error {
|
|||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const countAlbumsToRematch = `-- name: CountAlbumsToRematch :one
|
||||||
|
SELECT COUNT(*) FROM library_album_matches
|
||||||
|
WHERE outcome = 'unmatched' AND matcher < $1::int
|
||||||
|
`
|
||||||
|
|
||||||
|
// Refusals by an older matcher, which a finished listing goes back to.
|
||||||
|
func (q *Queries) CountAlbumsToRematch(ctx context.Context, matcher int32) (int64, error) {
|
||||||
|
row := q.db.QueryRow(ctx, countAlbumsToRematch, matcher)
|
||||||
|
var count int64
|
||||||
|
err := row.Scan(&count)
|
||||||
|
return count, err
|
||||||
|
}
|
||||||
|
|
||||||
const countWants = `-- name: CountWants :one
|
const countWants = `-- name: CountWants :one
|
||||||
SELECT COUNT(*) FILTER (WHERE fulfilled_at IS NULL AND reason <> 'missing')::bigint AS wanted,
|
SELECT COUNT(*) FILTER (WHERE fulfilled_at IS NULL AND reason <> 'missing')::bigint AS wanted,
|
||||||
COUNT(*) FILTER (WHERE fulfilled_at IS NULL AND reason = 'missing')::bigint AS missing,
|
COUNT(*) FILTER (WHERE fulfilled_at IS NULL AND reason = 'missing')::bigint AS missing,
|
||||||
@@ -126,9 +139,9 @@ func (q *Queries) DeleteWant(ctx context.Context, id pgtype.UUID) (int64, error)
|
|||||||
|
|
||||||
const finishLibraryListing = `-- name: FinishLibraryListing :one
|
const finishLibraryListing = `-- name: FinishLibraryListing :one
|
||||||
UPDATE acquisition_settings
|
UPDATE acquisition_settings
|
||||||
SET library_listed_at = now(),
|
SET library_listed_at = COALESCE(library_listed_at, now()),
|
||||||
library_unlisted = (SELECT COUNT(*) FROM library_album_matches WHERE outcome = 'unmatched')
|
library_unlisted = (SELECT COUNT(*) FROM library_album_matches WHERE outcome = 'unmatched')
|
||||||
WHERE id = true AND library_listed_at IS NULL
|
WHERE id = true
|
||||||
RETURNING library_listed_at, library_unlisted
|
RETURNING library_listed_at, library_unlisted
|
||||||
`
|
`
|
||||||
|
|
||||||
@@ -137,7 +150,8 @@ type FinishLibraryListingRow struct {
|
|||||||
LibraryUnlisted int32
|
LibraryUnlisted int32
|
||||||
}
|
}
|
||||||
|
|
||||||
// The listing is done: count the albums no match was found for.
|
// The listing is done, or done again after a newer matcher's retries:
|
||||||
|
// count the albums no match was found for. The first finish is kept.
|
||||||
func (q *Queries) FinishLibraryListing(ctx context.Context) (FinishLibraryListingRow, error) {
|
func (q *Queries) FinishLibraryListing(ctx context.Context) (FinishLibraryListingRow, error) {
|
||||||
row := q.db.QueryRow(ctx, finishLibraryListing)
|
row := q.db.QueryRow(ctx, finishLibraryListing)
|
||||||
var i FinishLibraryListingRow
|
var i FinishLibraryListingRow
|
||||||
@@ -405,13 +419,15 @@ SELECT a.id, a.title, a.mbid, a.release_group_mbid, ar.name AS artist_name, ar.m
|
|||||||
JOIN artists ar ON ar.id = a.artist_id
|
JOIN artists ar ON ar.id = a.artist_id
|
||||||
WHERE a.created_at <= $1::timestamptz
|
WHERE a.created_at <= $1::timestamptz
|
||||||
AND EXISTS (SELECT 1 FROM tracks t WHERE t.album_id = a.id)
|
AND EXISTS (SELECT 1 FROM tracks t WHERE t.album_id = a.id)
|
||||||
AND NOT EXISTS (SELECT 1 FROM library_album_matches m WHERE m.album_id = a.id)
|
AND NOT EXISTS (SELECT 1 FROM library_album_matches m
|
||||||
|
WHERE m.album_id = a.id AND (m.outcome = 'listed' OR m.matcher >= $2::int))
|
||||||
ORDER BY a.created_at, a.id
|
ORDER BY a.created_at, a.id
|
||||||
LIMIT $2
|
LIMIT $3
|
||||||
`
|
`
|
||||||
|
|
||||||
type ListAlbumsToMatchParams struct {
|
type ListAlbumsToMatchParams struct {
|
||||||
Cutoff pgtype.Timestamptz
|
Cutoff pgtype.Timestamptz
|
||||||
|
Matcher int32
|
||||||
MaxRows int32
|
MaxRows int32
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -424,11 +440,11 @@ type ListAlbumsToMatchRow struct {
|
|||||||
ArtistMbid *string
|
ArtistMbid *string
|
||||||
}
|
}
|
||||||
|
|
||||||
// Library albums from before the cutoff that the listing has not decided:
|
// Library albums from before the cutoff that the listing has not decided,
|
||||||
// each is matched to MusicBrainz (#5569). An album row with no tracks is
|
// or that an older matcher refused: each is matched to MusicBrainz (#5569).
|
||||||
// not an album in the library.
|
// An album row with no tracks is not an album in the library.
|
||||||
func (q *Queries) ListAlbumsToMatch(ctx context.Context, arg ListAlbumsToMatchParams) ([]ListAlbumsToMatchRow, error) {
|
func (q *Queries) ListAlbumsToMatch(ctx context.Context, arg ListAlbumsToMatchParams) ([]ListAlbumsToMatchRow, error) {
|
||||||
rows, err := q.db.Query(ctx, listAlbumsToMatch, arg.Cutoff, arg.MaxRows)
|
rows, err := q.db.Query(ctx, listAlbumsToMatch, arg.Cutoff, arg.Matcher, arg.MaxRows)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
@@ -937,11 +953,12 @@ func (q *Queries) MarkGroupSeen(ctx context.Context, arg MarkGroupSeenParams) er
|
|||||||
}
|
}
|
||||||
|
|
||||||
const recordAlbumMatch = `-- name: RecordAlbumMatch :exec
|
const recordAlbumMatch = `-- name: RecordAlbumMatch :exec
|
||||||
INSERT INTO library_album_matches (album_id, outcome, method, release_group_mbid, detail)
|
INSERT INTO library_album_matches (album_id, outcome, method, release_group_mbid, detail, matcher)
|
||||||
VALUES ($1, $2, $3, $4, $5)
|
VALUES ($1, $2, $3, $4, $5,
|
||||||
|
$6)
|
||||||
ON CONFLICT (album_id) DO UPDATE
|
ON CONFLICT (album_id) DO UPDATE
|
||||||
SET outcome = EXCLUDED.outcome, method = EXCLUDED.method,
|
SET outcome = EXCLUDED.outcome, method = EXCLUDED.method, release_group_mbid = EXCLUDED.release_group_mbid,
|
||||||
release_group_mbid = EXCLUDED.release_group_mbid, detail = EXCLUDED.detail, decided_at = now()
|
detail = EXCLUDED.detail, matcher = EXCLUDED.matcher, decided_at = now()
|
||||||
`
|
`
|
||||||
|
|
||||||
type RecordAlbumMatchParams struct {
|
type RecordAlbumMatchParams struct {
|
||||||
@@ -950,6 +967,7 @@ type RecordAlbumMatchParams struct {
|
|||||||
Method string
|
Method string
|
||||||
ReleaseGroupMbid string
|
ReleaseGroupMbid string
|
||||||
Detail string
|
Detail string
|
||||||
|
Matcher int32
|
||||||
}
|
}
|
||||||
|
|
||||||
func (q *Queries) RecordAlbumMatch(ctx context.Context, arg RecordAlbumMatchParams) error {
|
func (q *Queries) RecordAlbumMatch(ctx context.Context, arg RecordAlbumMatchParams) error {
|
||||||
@@ -959,6 +977,7 @@ func (q *Queries) RecordAlbumMatch(ctx context.Context, arg RecordAlbumMatchPara
|
|||||||
arg.Method,
|
arg.Method,
|
||||||
arg.ReleaseGroupMbid,
|
arg.ReleaseGroupMbid,
|
||||||
arg.Detail,
|
arg.Detail,
|
||||||
|
arg.Matcher,
|
||||||
)
|
)
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1 @@
|
|||||||
|
ALTER TABLE library_album_matches DROP COLUMN matcher;
|
||||||
@@ -0,0 +1,4 @@
|
|||||||
|
-- Which version of the matching (#5569) decided an album. A refusal by an
|
||||||
|
-- older matcher is tried again by a newer one, so improving the matching
|
||||||
|
-- reaches the albums it refused without a migration each time.
|
||||||
|
ALTER TABLE library_album_matches ADD COLUMN matcher integer NOT NULL DEFAULT 0;
|
||||||
@@ -272,18 +272,24 @@ SELECT a.id, 'listed', 'tags', a.release_group_mbid
|
|||||||
ON CONFLICT (album_id) DO NOTHING;
|
ON CONFLICT (album_id) DO NOTHING;
|
||||||
|
|
||||||
-- name: ListAlbumsToMatch :many
|
-- name: ListAlbumsToMatch :many
|
||||||
-- Library albums from before the cutoff that the listing has not decided:
|
-- Library albums from before the cutoff that the listing has not decided,
|
||||||
-- each is matched to MusicBrainz (#5569). An album row with no tracks is
|
-- or that an older matcher refused: each is matched to MusicBrainz (#5569).
|
||||||
-- not an album in the library.
|
-- An album row with no tracks is not an album in the library.
|
||||||
SELECT a.id, a.title, a.mbid, a.release_group_mbid, ar.name AS artist_name, ar.mbid AS artist_mbid
|
SELECT a.id, a.title, a.mbid, a.release_group_mbid, ar.name AS artist_name, ar.mbid AS artist_mbid
|
||||||
FROM albums a
|
FROM albums a
|
||||||
JOIN artists ar ON ar.id = a.artist_id
|
JOIN artists ar ON ar.id = a.artist_id
|
||||||
WHERE a.created_at <= sqlc.arg(cutoff)::timestamptz
|
WHERE a.created_at <= sqlc.arg(cutoff)::timestamptz
|
||||||
AND EXISTS (SELECT 1 FROM tracks t WHERE t.album_id = a.id)
|
AND EXISTS (SELECT 1 FROM tracks t WHERE t.album_id = a.id)
|
||||||
AND NOT EXISTS (SELECT 1 FROM library_album_matches m WHERE m.album_id = a.id)
|
AND NOT EXISTS (SELECT 1 FROM library_album_matches m
|
||||||
|
WHERE m.album_id = a.id AND (m.outcome = 'listed' OR m.matcher >= sqlc.arg(matcher)::int))
|
||||||
ORDER BY a.created_at, a.id
|
ORDER BY a.created_at, a.id
|
||||||
LIMIT sqlc.arg(max_rows);
|
LIMIT sqlc.arg(max_rows);
|
||||||
|
|
||||||
|
-- name: CountAlbumsToRematch :one
|
||||||
|
-- Refusals by an older matcher, which a finished listing goes back to.
|
||||||
|
SELECT COUNT(*) FROM library_album_matches
|
||||||
|
WHERE outcome = 'unmatched' AND matcher < sqlc.arg(matcher)::int;
|
||||||
|
|
||||||
-- name: ListAlbumTracksToMatch :many
|
-- name: ListAlbumTracksToMatch :many
|
||||||
-- What an album's tracks say about themselves, to check a match against.
|
-- What an album's tracks say about themselves, to check a match against.
|
||||||
SELECT title, disc_number, track_number, duration_ms, mbid
|
SELECT title, disc_number, track_number, duration_ms, mbid
|
||||||
@@ -308,18 +314,20 @@ VALUES (sqlc.arg(release_group_mbid), sqlc.arg(artist_mbid), sqlc.arg(artist_nam
|
|||||||
ON CONFLICT (release_group_mbid) DO NOTHING;
|
ON CONFLICT (release_group_mbid) DO NOTHING;
|
||||||
|
|
||||||
-- name: RecordAlbumMatch :exec
|
-- name: RecordAlbumMatch :exec
|
||||||
INSERT INTO library_album_matches (album_id, outcome, method, release_group_mbid, detail)
|
INSERT INTO library_album_matches (album_id, outcome, method, release_group_mbid, detail, matcher)
|
||||||
VALUES (sqlc.arg(album_id), sqlc.arg(outcome), sqlc.arg(method), sqlc.arg(release_group_mbid), sqlc.arg(detail))
|
VALUES (sqlc.arg(album_id), sqlc.arg(outcome), sqlc.arg(method), sqlc.arg(release_group_mbid), sqlc.arg(detail),
|
||||||
|
sqlc.arg(matcher))
|
||||||
ON CONFLICT (album_id) DO UPDATE
|
ON CONFLICT (album_id) DO UPDATE
|
||||||
SET outcome = EXCLUDED.outcome, method = EXCLUDED.method,
|
SET outcome = EXCLUDED.outcome, method = EXCLUDED.method, release_group_mbid = EXCLUDED.release_group_mbid,
|
||||||
release_group_mbid = EXCLUDED.release_group_mbid, detail = EXCLUDED.detail, decided_at = now();
|
detail = EXCLUDED.detail, matcher = EXCLUDED.matcher, decided_at = now();
|
||||||
|
|
||||||
-- name: FinishLibraryListing :one
|
-- name: FinishLibraryListing :one
|
||||||
-- The listing is done: count the albums no match was found for.
|
-- The listing is done, or done again after a newer matcher's retries:
|
||||||
|
-- count the albums no match was found for. The first finish is kept.
|
||||||
UPDATE acquisition_settings
|
UPDATE acquisition_settings
|
||||||
SET library_listed_at = now(),
|
SET library_listed_at = COALESCE(library_listed_at, now()),
|
||||||
library_unlisted = (SELECT COUNT(*) FROM library_album_matches WHERE outcome = 'unmatched')
|
library_unlisted = (SELECT COUNT(*) FROM library_album_matches WHERE outcome = 'unmatched')
|
||||||
WHERE id = true AND library_listed_at IS NULL
|
WHERE id = true
|
||||||
RETURNING library_listed_at, library_unlisted;
|
RETURNING library_listed_at, library_unlisted;
|
||||||
|
|
||||||
-- name: ListUnmatchedLibraryAlbums :many
|
-- name: ListUnmatchedLibraryAlbums :many
|
||||||
|
|||||||
@@ -23,6 +23,15 @@ const libraryMatchBudget = 2 * time.Minute
|
|||||||
// libraryMatchBatch is how many albums one query hands the matching.
|
// libraryMatchBatch is how many albums one query hands the matching.
|
||||||
const libraryMatchBatch = 25
|
const libraryMatchBatch = 25
|
||||||
|
|
||||||
|
// libraryMatcher is the matching's version. Raise it when the matching
|
||||||
|
// changes in a way that could match an album it refused: albums an older
|
||||||
|
// version refused are matched again, even after the listing is done.
|
||||||
|
//
|
||||||
|
// 1 tags, release, discography and search, checked by title and length
|
||||||
|
// 2 titles alone under the album's own name; a refusal names the track
|
||||||
|
// 3 a second copy of a track counts once; joining words count for nothing
|
||||||
|
const libraryMatcher = 3
|
||||||
|
|
||||||
// listLibrary puts the library's albums on the wanted list as held, once
|
// listLibrary puts the library's albums on the wanted list as held, once
|
||||||
// (operator, 2026-10-10: "make a record saying that the import has
|
// (operator, 2026-10-10: "make a record saying that the import has
|
||||||
// happened so later changes like unwanting hold and dont auto import from
|
// happened so later changes like unwanting hold and dont auto import from
|
||||||
@@ -39,7 +48,8 @@ const libraryMatchBatch = 25
|
|||||||
// of the albums no match was found for.
|
// of the albums no match was found for.
|
||||||
//
|
//
|
||||||
// Each album decided is recorded in library_album_matches, so an album taken
|
// Each album decided is recorded in library_album_matches, so an album taken
|
||||||
// off the list stays off, and once done the listing never runs again.
|
// off the list stays off. Once done the listing runs again only for albums
|
||||||
|
// an older matcher refused (libraryMatcher).
|
||||||
func (s *Service) listLibrary(ctx context.Context, res *PassResult) error {
|
func (s *Service) listLibrary(ctx context.Context, res *PassResult) error {
|
||||||
q := dbq.New(s.pool)
|
q := dbq.New(s.pool)
|
||||||
state, err := q.GetLibraryListing(ctx)
|
state, err := q.GetLibraryListing(ctx)
|
||||||
@@ -47,7 +57,13 @@ func (s *Service) listLibrary(ctx context.Context, res *PassResult) error {
|
|||||||
return fmt.Errorf("wanted: read the library listing: %w", err)
|
return fmt.Errorf("wanted: read the library listing: %w", err)
|
||||||
}
|
}
|
||||||
if state.LibraryListedAt.Valid {
|
if state.LibraryListedAt.Valid {
|
||||||
return nil
|
stale, err := q.CountAlbumsToRematch(ctx, libraryMatcher)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("wanted: count refusals to retry: %w", err)
|
||||||
|
}
|
||||||
|
if stale == 0 {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
}
|
}
|
||||||
cutoff := state.LibraryListingStartedAt
|
cutoff := state.LibraryListingStartedAt
|
||||||
if !cutoff.Valid {
|
if !cutoff.Valid {
|
||||||
@@ -67,7 +83,8 @@ func (s *Service) listLibrary(ctx context.Context, res *PassResult) error {
|
|||||||
|
|
||||||
deadline := s.now().Add(libraryMatchBudget)
|
deadline := s.now().Add(libraryMatchBudget)
|
||||||
for {
|
for {
|
||||||
albums, err := q.ListAlbumsToMatch(ctx, dbq.ListAlbumsToMatchParams{Cutoff: cutoff, MaxRows: libraryMatchBatch})
|
albums, err := q.ListAlbumsToMatch(ctx, dbq.ListAlbumsToMatchParams{
|
||||||
|
Cutoff: cutoff, Matcher: libraryMatcher, MaxRows: libraryMatchBatch})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("wanted: albums to match: %w", err)
|
return fmt.Errorf("wanted: albums to match: %w", err)
|
||||||
}
|
}
|
||||||
@@ -107,7 +124,8 @@ func (s *Service) listAlbum(ctx context.Context, q *dbq.Queries, a dbq.ListAlbum
|
|||||||
}
|
}
|
||||||
if !m.OK {
|
if !m.OK {
|
||||||
s.logger.Info("wanted: no MusicBrainz match for a library album", "album", a.Title, "artist", a.ArtistName, "why", m.Detail)
|
s.logger.Info("wanted: no MusicBrainz match for a library album", "album", a.Title, "artist", a.ArtistName, "why", m.Detail)
|
||||||
return 0, q.RecordAlbumMatch(ctx, dbq.RecordAlbumMatchParams{AlbumID: a.ID, Outcome: "unmatched", Detail: m.Detail})
|
return 0, q.RecordAlbumMatch(ctx, dbq.RecordAlbumMatchParams{
|
||||||
|
AlbumID: a.ID, Outcome: "unmatched", Detail: m.Detail, Matcher: libraryMatcher})
|
||||||
}
|
}
|
||||||
var listed int64
|
var listed int64
|
||||||
err = pgx.BeginFunc(ctx, s.pool, func(tx pgx.Tx) error {
|
err = pgx.BeginFunc(ctx, s.pool, func(tx pgx.Tx) error {
|
||||||
@@ -124,7 +142,7 @@ func (s *Service) listAlbum(ctx context.Context, q *dbq.Queries, a dbq.ListAlbum
|
|||||||
}
|
}
|
||||||
listed = n
|
listed = n
|
||||||
return tq.RecordAlbumMatch(ctx, dbq.RecordAlbumMatchParams{
|
return tq.RecordAlbumMatch(ctx, dbq.RecordAlbumMatchParams{
|
||||||
AlbumID: a.ID, Outcome: "listed", Method: m.Method, ReleaseGroupMbid: m.Group,
|
AlbumID: a.ID, Outcome: "listed", Method: m.Method, ReleaseGroupMbid: m.Group, Matcher: libraryMatcher,
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
@@ -268,7 +268,48 @@ func TestTrackAgreement(t *testing.T) {
|
|||||||
if _, miss := trackAgreement([]libraryTrack{{Title: "Other", LengthMs: 61000}}, edition, true); miss != `no track of its edition is like "Other" (1:01)` {
|
if _, miss := trackAgreement([]libraryTrack{{Title: "Other", LengthMs: 61000}}, edition, true); miss != `no track of its edition is like "Other" (1:01)` {
|
||||||
t.Errorf("unplaced: %q", miss)
|
t.Errorf("unplaced: %q", miss)
|
||||||
}
|
}
|
||||||
|
if !sameTitle("Brain Stew + Jaded", "Brain Stew / Jaded") || sameTitle("+++", "Grind & Hustle") || sameTitle("Blur", "The Best of Blur") {
|
||||||
|
t.Error("sameTitle: joining words count for nothing, and a title of none is no title")
|
||||||
|
}
|
||||||
|
n := int32(1)
|
||||||
|
copies := tracksToMatch([]dbq.ListAlbumTracksToMatchRow{
|
||||||
|
{Title: "Song", TrackNumber: &n, DurationMs: 1}, {Title: "song", TrackNumber: &n, DurationMs: 2}, {Title: "Other", TrackNumber: &n}})
|
||||||
|
if len(copies) != 2 {
|
||||||
|
t.Errorf("a second copy of a track is read once: %+v", copies)
|
||||||
|
}
|
||||||
if agrees(3, 4) || !agrees(4, 5) || agrees(0, 0) {
|
if agrees(3, 4) || !agrees(4, 5) || agrees(0, 0) {
|
||||||
t.Error("agrees: 80% of the album's tracks, and an album has tracks")
|
t.Error("agrees: 80% of the album's tracks, and an album has tracks")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A refusal by an older matcher is matched again once the matching improves,
|
||||||
|
// even after the listing is done; a refusal by this one is not.
|
||||||
|
func TestListLibrary_RetriesAnOlderMatchersRefusals_Integration(t *testing.T) {
|
||||||
|
pool := newPool(t)
|
||||||
|
ctx := context.Background()
|
||||||
|
q := dbq.New(pool)
|
||||||
|
s := newService(t, pool, newFakeMB(t))
|
||||||
|
ballad := seedAlbum(t, q, blur, "The Ballad of Darren", "", []string{"The Ballad", "St. Charles Square"})
|
||||||
|
bootleg := seedAlbum(t, q, blur, "Bootleg", "", []string{"Intro"})
|
||||||
|
for _, a := range []pgtype.UUID{ballad.ID, bootleg.ID} {
|
||||||
|
if err := q.RecordAlbumMatch(ctx, dbq.RecordAlbumMatchParams{AlbumID: a, Outcome: "unmatched", Detail: "old", Matcher: 1}); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if _, err := pool.Exec(ctx, `UPDATE library_album_matches SET matcher = $1 WHERE album_id = $2`, libraryMatcher, bootleg.ID); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
// The reset leaves the listing done.
|
||||||
|
if err := s.listLibrary(ctx, &PassResult{}); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if outcome, method, _, _ := matchOf(t, pool, ballad.ID); outcome != "listed" || method != matchDiscography {
|
||||||
|
t.Errorf("the older refusal was decided %s by %s", outcome, method)
|
||||||
|
}
|
||||||
|
if _, _, _, detail := matchOf(t, pool, bootleg.ID); detail != "old" {
|
||||||
|
t.Errorf("this matcher's own refusal was retried: %q", detail)
|
||||||
|
}
|
||||||
|
if state, _ := q.GetLibraryListing(ctx); state.LibraryUnlisted != 1 {
|
||||||
|
t.Errorf("unlisted = %d, want the bootleg alone", state.LibraryUnlisted)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -85,8 +85,13 @@ type albumMatch struct {
|
|||||||
Detail string
|
Detail string
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// tracksToMatch reads an album's tracks, each once: a second copy of a
|
||||||
|
// track (the same disc, number and title, as a duplicate file in another
|
||||||
|
// format is) would otherwise count against the match as a track no edition
|
||||||
|
// has.
|
||||||
func tracksToMatch(rows []dbq.ListAlbumTracksToMatchRow) []libraryTrack {
|
func tracksToMatch(rows []dbq.ListAlbumTracksToMatchRow) []libraryTrack {
|
||||||
out := make([]libraryTrack, 0, len(rows))
|
out := make([]libraryTrack, 0, len(rows))
|
||||||
|
seen := make(map[string]bool, len(rows))
|
||||||
for _, r := range rows {
|
for _, r := range rows {
|
||||||
t := libraryTrack{Title: r.Title, LengthMs: int(r.DurationMs), Recording: deref(r.Mbid)}
|
t := libraryTrack{Title: r.Title, LengthMs: int(r.DurationMs), Recording: deref(r.Mbid)}
|
||||||
if r.DiscNumber != nil {
|
if r.DiscNumber != nil {
|
||||||
@@ -95,6 +100,11 @@ func tracksToMatch(rows []dbq.ListAlbumTracksToMatchRow) []libraryTrack {
|
|||||||
if r.TrackNumber != nil {
|
if r.TrackNumber != nil {
|
||||||
t.Number = int(*r.TrackNumber)
|
t.Number = int(*r.TrackNumber)
|
||||||
}
|
}
|
||||||
|
key := fmt.Sprintf("%d/%d/%s", max(t.Disc, 1), t.Number, strings.Join(release.Tokens(t.Title), " "))
|
||||||
|
if seen[key] {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
seen[key] = true
|
||||||
out = append(out, t)
|
out = append(out, t)
|
||||||
}
|
}
|
||||||
return out
|
return out
|
||||||
@@ -165,9 +175,15 @@ func agrees(n, tracks int) bool {
|
|||||||
return tracks > 0 && n*10 >= tracks*matchAgreement
|
return tracks > 0 && n*10 >= tracks*matchAgreement
|
||||||
}
|
}
|
||||||
|
|
||||||
// sameTitle: the same words, not merely one holding the other.
|
// sameTitle: the same words, not merely one holding the other. "And",
|
||||||
|
// "&" and "+" join two titles however a release spells it ("Brain Stew +
|
||||||
|
// Jaded", "Brain Stew / Jaded"), so they count for nothing.
|
||||||
func sameTitle(a, b string) bool {
|
func sameTitle(a, b string) bool {
|
||||||
return slices.Equal(release.Tokens(a), release.Tokens(b))
|
words := func(s string) []string {
|
||||||
|
return slices.DeleteFunc(release.Tokens(s), func(w string) bool { return w == "and" })
|
||||||
|
}
|
||||||
|
wa, wb := words(a), words(b)
|
||||||
|
return len(wa) > 0 && slices.Equal(wa, wb)
|
||||||
}
|
}
|
||||||
|
|
||||||
// matchAlbum finds the release group a library album is. A MusicBrainz
|
// matchAlbum finds the release group a library album is. A MusicBrainz
|
||||||
@@ -204,7 +220,9 @@ func (s *Service) matchAlbum(ctx context.Context, a dbq.ListAlbumsToMatchRow, tr
|
|||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
tried[c.Group] = true
|
tried[c.Group] = true
|
||||||
n, size, miss, err := s.agreementWith(ctx, c.Group, tracks, sameTitle(c.Title, a.Title))
|
n, size, miss, err := s.agreementWith(ctx, c.Group, tracks, func(edition string) bool {
|
||||||
|
return sameTitle(c.Title, a.Title) || sameTitle(edition, a.Title)
|
||||||
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
@@ -328,7 +346,9 @@ func (s *Service) searchCandidates(ctx context.Context, a dbq.ListAlbumsToMatchR
|
|||||||
// agreementWith is how many of the album's tracks the group's best edition
|
// agreementWith is how many of the album's tracks the group's best edition
|
||||||
// agrees with, that edition's length, and how it differs. A group
|
// agrees with, that edition's length, and how it differs. A group
|
||||||
// MusicBrainz no longer has, or lists no edition of, agrees with none (-1).
|
// MusicBrainz no longer has, or lists no edition of, agrees with none (-1).
|
||||||
func (s *Service) agreementWith(ctx context.Context, group string, tracks []libraryTrack, titlesEnough bool) (n, size int, miss string, err error) {
|
// ownName says whether an edition, by its title, is called what the album
|
||||||
|
// is, so that its track titles are enough.
|
||||||
|
func (s *Service) agreementWith(ctx context.Context, group string, tracks []libraryTrack, ownName func(edition string) bool) (n, size int, miss string, err error) {
|
||||||
eds, err := s.cat.Releases(ctx, group)
|
eds, err := s.cat.Releases(ctx, group)
|
||||||
switch {
|
switch {
|
||||||
case errors.Is(err, catalogue.ErrNotFound):
|
case errors.Is(err, catalogue.ErrNotFound):
|
||||||
@@ -343,7 +363,7 @@ func (s *Service) agreementWith(ctx context.Context, group string, tracks []libr
|
|||||||
edition = append(edition, editionTrack{Medium: int(t.MediumPosition), Position: int(t.Position),
|
edition = append(edition, editionTrack{Medium: int(t.MediumPosition), Position: int(t.Position),
|
||||||
Title: t.Title, LengthMs: int(t.LengthMs), Recording: t.RecordingMbid})
|
Title: t.Title, LengthMs: int(t.LengthMs), Recording: t.RecordingMbid})
|
||||||
}
|
}
|
||||||
if got, m := trackAgreement(tracks, edition, titlesEnough); got > n || (got == n && abs(len(edition)-len(tracks)) < abs(size-len(tracks))) {
|
if got, m := trackAgreement(tracks, edition, ownName(e.Release.Title)); got > n || (got == n && abs(len(edition)-len(tracks)) < abs(size-len(tracks))) {
|
||||||
n, size, miss = got, len(edition), m
|
n, size, miss = got, len(edition), m
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user