feat(library): match own-state on artist MBID, not just name
Own-state ("in library") relied on an exact artist-NAME match, so an owned
album whose name differed from the MB canonical (punctuation/locale/feat.)
showed as not-owned even when followed. Capture the MusicBrainz artist MBID
on LibraryItem and prefer it for matching.
- New LibraryItem.artistMbid (migration add_library_artist_mbid). Populated at
import (pipeline) + scan, both of which already hold the resolved target;
the pipeline also now stores rgMbid (it previously didn't). COALESCE on
conflict backfills MBIDs on re-scan without clobbering.
- /api/artists "in library, not followed" matches by artistMbid first, name
case-insensitively as fallback (rows predating the column). Library route
prefers the item's own artistMbid over the release join.
Last.fm/discover own-state still use name-match (they work) — a smaller
follow-up. worker 233, web 186 tests.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,2 @@
|
||||
-- AlterTable
|
||||
ALTER TABLE "LibraryItem" ADD COLUMN "artistMbid" TEXT;
|
||||
@@ -89,6 +89,10 @@ model LibraryItem {
|
||||
// MusicBrainz release-group MBID when known (resolved at manual import) — gives the album
|
||||
// cover art without depending on a matching MonitoredRelease row.
|
||||
rgMbid String?
|
||||
// MusicBrainz ARTIST MBID when known (captured at import + scan). Own-state ("in library")
|
||||
// prefers this over the fragile exact artist-name match; null rows fall back to name-match
|
||||
// (re-scan to populate).
|
||||
artistMbid String?
|
||||
// The album's on-disk audio filenames ("## Title.ext"), captured at import + scan, so the
|
||||
// Library modal can show the REAL tracks on disk (revealing incomplete/edition mismatches)
|
||||
// instead of MusicBrainz's canonical listing. Empty for items imported before this existed
|
||||
|
||||
@@ -98,6 +98,16 @@ describe("artists collection API", () => {
|
||||
expect(body.ownedNotFollowed).not.toContain("followed artist"); // case-insensitive match
|
||||
});
|
||||
|
||||
it("GET matches owned artists to the watched roster by MBID despite a name mismatch", async () => {
|
||||
// followed under the canonical MB name, but the library row's name differs (feat./locale)
|
||||
await prisma.watchedArtist.create({ data: { mbid: "mb-beyonce", name: "Beyoncé" } });
|
||||
await prisma.libraryItem.create({
|
||||
data: { artist: "Beyonce", album: "Lemonade", artistMbid: "mb-beyonce", path: "/l", source: "scan", format: "FLAC", qualityClass: 2 },
|
||||
});
|
||||
const body = await (await GET(listReq())).json();
|
||||
expect(body.ownedNotFollowed).not.toContain("Beyonce"); // matched by MBID → not "unfollowed"
|
||||
});
|
||||
|
||||
it("GET ?mbid= returns the follow state for a single artist", async () => {
|
||||
await prisma.watchedArtist.create({ data: { mbid: "followed-1", name: "Followed" } });
|
||||
expect((await (await GET(listReq("?mbid=followed-1"))).json()).followed).toBe(true);
|
||||
|
||||
@@ -20,16 +20,21 @@ export async function GET(request: Request) {
|
||||
},
|
||||
},
|
||||
}),
|
||||
prisma.libraryItem.findMany({ select: { artist: true }, distinct: ["artist"] }),
|
||||
prisma.libraryItem.findMany({ select: { artist: true, artistMbid: true } }),
|
||||
]);
|
||||
|
||||
// Artists you own music by (LibraryItem) but don't follow yet — name-matched
|
||||
// case-insensitively against the watched roster (the have-via-scan name-match caveat).
|
||||
// Artists you own music by (LibraryItem) but don't follow yet. Match on the MusicBrainz
|
||||
// ARTIST MBID first (robust to name punctuation/locale/feat. variants), falling back to a
|
||||
// case-insensitive name match for library rows that predate the artistMbid column.
|
||||
const followedMbids = new Set(artists.map((a) => a.mbid));
|
||||
const followedNames = new Set(artists.map((a) => a.name.trim().toLowerCase()));
|
||||
const ownedNotFollowed = libraryArtists
|
||||
.map((r) => r.artist)
|
||||
.filter((name) => name.trim() && !followedNames.has(name.trim().toLowerCase()))
|
||||
.sort((a, b) => a.localeCompare(b));
|
||||
const isFollowed = (r: { artist: string; artistMbid: string | null }) =>
|
||||
(r.artistMbid && followedMbids.has(r.artistMbid)) || followedNames.has(r.artist.trim().toLowerCase());
|
||||
const ownedNotFollowed = [
|
||||
...new Set(
|
||||
libraryArtists.filter((r) => r.artist.trim() && !isFollowed(r)).map((r) => r.artist),
|
||||
),
|
||||
].sort((a, b) => a.localeCompare(b));
|
||||
|
||||
return Response.json({
|
||||
artists: artists.map((a) => {
|
||||
|
||||
Binary file not shown.
@@ -245,15 +245,17 @@ def _import(conn: psycopg.Connection, job_id: str, target: MBTarget,
|
||||
with conn.cursor() as cur:
|
||||
cur.execute(
|
||||
'INSERT INTO "LibraryItem" (id, "requestId", artist, album, path, source, '
|
||||
'format, "qualityClass", "trackNames", "importedAt") '
|
||||
"VALUES (gen_random_uuid()::text, %s, %s, %s, %s, %s, %s, %s, %s, now()) "
|
||||
'format, "qualityClass", "trackNames", "rgMbid", "artistMbid", "importedAt") '
|
||||
"VALUES (gen_random_uuid()::text, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, now()) "
|
||||
'ON CONFLICT (artist, album) DO UPDATE SET '
|
||||
' "requestId" = EXCLUDED."requestId", path = EXCLUDED.path, source = EXCLUDED.source, '
|
||||
' format = EXCLUDED.format, "qualityClass" = EXCLUDED."qualityClass", '
|
||||
' "trackNames" = EXCLUDED."trackNames", "importedAt" = now() '
|
||||
' "trackNames" = EXCLUDED."trackNames", "rgMbid" = COALESCE(EXCLUDED."rgMbid", "LibraryItem"."rgMbid"), '
|
||||
' "artistMbid" = COALESCE(EXCLUDED."artistMbid", "LibraryItem"."artistMbid"), "importedAt" = now() '
|
||||
'WHERE EXCLUDED."qualityClass" > "LibraryItem"."qualityClass"',
|
||||
(request_id, target.artist, target.album, path, winner.source,
|
||||
winner.quality.fmt, quality_class(winner.quality), track_names),
|
||||
winner.quality.fmt, quality_class(winner.quality), track_names,
|
||||
target.rg_mbid or None, target.artist_mbid or None),
|
||||
)
|
||||
cur.execute(
|
||||
"UPDATE \"Request\" SET status = 'completed' WHERE id = %s", (request_id,)
|
||||
|
||||
@@ -84,13 +84,16 @@ def _record_owned(conn, target, quality_class: int, fmt: str, path: str, watched
|
||||
track_names = list_audio_files(path) # capture the real on-disk tracks
|
||||
cur.execute(
|
||||
'INSERT INTO "LibraryItem" (id, "requestId", artist, album, path, source, format, '
|
||||
'"qualityClass", "trackNames", "importedAt") '
|
||||
"VALUES (gen_random_uuid()::text, NULL, %s, %s, %s, 'scan', %s, %s, %s, now()) "
|
||||
# refresh trackNames on re-scan (populates items imported before this column);
|
||||
# xmax=0 ⇒ this was an INSERT (a genuinely new library item) vs an UPDATE.
|
||||
'ON CONFLICT (artist, album) DO UPDATE SET "trackNames" = EXCLUDED."trackNames" '
|
||||
'"qualityClass", "trackNames", "rgMbid", "artistMbid", "importedAt") '
|
||||
"VALUES (gen_random_uuid()::text, NULL, %s, %s, %s, 'scan', %s, %s, %s, %s, %s, now()) "
|
||||
# refresh trackNames + MBIDs on re-scan (populates items recorded before these
|
||||
# columns); xmax=0 ⇒ this was an INSERT (a genuinely new library item) vs an UPDATE.
|
||||
'ON CONFLICT (artist, album) DO UPDATE SET "trackNames" = EXCLUDED."trackNames", '
|
||||
' "rgMbid" = COALESCE(EXCLUDED."rgMbid", "LibraryItem"."rgMbid"), '
|
||||
' "artistMbid" = COALESCE(EXCLUDED."artistMbid", "LibraryItem"."artistMbid") '
|
||||
"RETURNING (xmax = 0) AS inserted",
|
||||
(target.artist, target.album, path, fmt, quality_class, track_names),
|
||||
(target.artist, target.album, path, fmt, quality_class, track_names,
|
||||
target.rg_mbid or None, target.artist_mbid or None),
|
||||
)
|
||||
newly = cur.fetchone()[0] # a new LibraryItem means this album wasn't recorded before
|
||||
conn.commit()
|
||||
|
||||
@@ -49,9 +49,9 @@ def test_scan_records_have_monitored_and_follows_artist(conn, tmp_path):
|
||||
assert (result.imported, result.skipped) == (1, 0)
|
||||
assert _counts(conn) == {"LibraryItem": 1, "MonitoredRelease": 1, "WatchedArtist": 1}
|
||||
with conn.cursor() as cur:
|
||||
cur.execute('SELECT source, "qualityClass" FROM "LibraryItem" WHERE artist=%s AND album=%s',
|
||||
("John Mayer", "Continuum"))
|
||||
assert cur.fetchone() == ("scan", 3)
|
||||
cur.execute('SELECT source, "qualityClass", "artistMbid", "rgMbid" FROM "LibraryItem" '
|
||||
'WHERE artist=%s AND album=%s', ("John Mayer", "Continuum"))
|
||||
assert cur.fetchone() == ("scan", 3, "a1", "rg1") # MBIDs captured for own-state matching
|
||||
cur.execute('SELECT monitored, state, "currentQualityClass" FROM "MonitoredRelease" WHERE "rgMbid"=%s',
|
||||
("rg1",))
|
||||
assert cur.fetchone() == (True, "fulfilled", 3)
|
||||
|
||||
Reference in New Issue
Block a user