diff --git a/worker/lyra_worker/_musicbrainz.py b/worker/lyra_worker/_musicbrainz.py index 49fade4..f34180d 100644 --- a/worker/lyra_worker/_musicbrainz.py +++ b/worker/lyra_worker/_musicbrainz.py @@ -13,15 +13,39 @@ def _is_album(g: dict) -> bool: return (g.get("primary-type") or "").casefold() == "album" -def _best_release_group(album: str, groups: list) -> dict | None: - """Choose the best release-group match for `album`. Highest title similarity wins; - an Album primary-type breaks ties so a famous album (e.g. Michael Jackson's "Off the - Wall") isn't resolved to its same-named single, whose short tracklist would then be - mapped positionally onto the full album's files. Returns None below the title threshold. - Pure — no network I/O.""" +def _credited_artist(g: dict) -> str: + """Primary credited artist name of a release-group search result, or '' if absent.""" + credit = g.get("artist-credit") + if isinstance(credit, list) and credit and isinstance(credit[0], dict): + return (credit[0].get("artist") or {}).get("name", "") or "" + return "" + + +def _best_release_group(artist: str, album: str, groups: list) -> dict | None: + """Choose the best release-group match for `artist`/`album`. + + Ranked, highest first, on `(artist_ratio, title_ratio, is_album)`: + * artist match dominates — MusicBrainz relevance ties same-titled release groups by + *different* artists (e.g. "Fun Machine" exists as a Lake Street Dive EP and an + unrelated band's Album), and the artist we followed is trustworthy, so a credited + artist that matches the request outranks everything else; + * title similarity is next; + * an Album primary-type only breaks ties *within* the same artist+title, so a famous + album (Michael Jackson's "Off the Wall") still isn't resolved to its same-named + single whose short tracklist would map positionally onto the album's files. + Artist is a ranking signal, never a hard filter — a low match sinks a candidate but + never drops the release, so credited-name variations (feat., punctuation) still resolve. + Returns None below the title threshold. Pure — no network I/O.""" if not groups: return None - best = max(groups, key=lambda g: (_ratio(album, g.get("title", "")), _is_album(g))) + best = max( + groups, + key=lambda g: ( + _ratio(artist, _credited_artist(g)), + _ratio(album, g.get("title", "")), + _is_album(g), + ), + ) if _ratio(album, best.get("title", "")) < _MIN_TITLE_RATIO: return None return best @@ -46,7 +70,7 @@ class MusicBrainzResolver: res = musicbrainzngs.search_release_groups(query=album, artist=artist, limit=5) groups = res.get("release-group-list", []) - rg = _best_release_group(album, groups) + rg = _best_release_group(artist, album, groups) if rg is None: return None diff --git a/worker/tests/test_musicbrainz.py b/worker/tests/test_musicbrainz.py index 90ee381..6d1821c 100644 --- a/worker/tests/test_musicbrainz.py +++ b/worker/tests/test_musicbrainz.py @@ -4,33 +4,59 @@ from lyra_worker._musicbrainz import _best_release_group # path (network) is covered by test_musicbrainz_live.py. -def _rg(title, primary_type): - return {"id": title.lower(), "title": title, "primary-type": primary_type} +def _rg(title, primary_type, artist=""): + g = {"id": title.lower(), "title": title, "primary-type": primary_type} + if artist: + g["artist-credit"] = [{"artist": {"name": artist, "id": artist.lower()}}] + return g def test_prefers_album_over_same_titled_single(): # Real-world "Off the Wall" bug: MusicBrainz returns the Single release-group # first, tied on title with the Album. We must pick the Album so a 10-track # download isn't tagged from the 2-track single's tracklist. - groups = [_rg("Off the Wall", "Single"), _rg("Off the Wall", "Album")] - assert _best_release_group("Off the Wall", groups)["primary-type"] == "Album" + groups = [ + _rg("Off the Wall", "Single", "Michael Jackson"), + _rg("Off the Wall", "Album", "Michael Jackson"), + ] + assert _best_release_group("Michael Jackson", "Off the Wall", groups)["primary-type"] == "Album" def test_album_preference_is_order_independent(): - groups = [_rg("Off the Wall", "Album"), _rg("Off the Wall", "Single")] - assert _best_release_group("Off the Wall", groups)["primary-type"] == "Album" + groups = [ + _rg("Off the Wall", "Album", "Michael Jackson"), + _rg("Off the Wall", "Single", "Michael Jackson"), + ] + assert _best_release_group("Michael Jackson", "Off the Wall", groups)["primary-type"] == "Album" def test_closer_title_still_wins_over_album_type(): - # Title similarity dominates; the album-type preference only breaks ties. - groups = [_rg("Continuum", "Single"), _rg("Continuum Deluxe Reissue", "Album")] - assert _best_release_group("Continuum", groups)["title"] == "Continuum" + # Title similarity dominates the album-type preference (for a single artist). + groups = [ + _rg("Continuum", "Single", "John Mayer"), + _rg("Continuum Deluxe Reissue", "Album", "John Mayer"), + ] + assert _best_release_group("John Mayer", "Continuum", groups)["title"] == "Continuum" def test_below_threshold_returns_none(): - groups = [_rg("Something Totally Different", "Album")] - assert _best_release_group("Off the Wall", groups) is None + groups = [_rg("Something Totally Different", "Album", "Michael Jackson")] + assert _best_release_group("Michael Jackson", "Off the Wall", groups) is None def test_empty_groups_returns_none(): - assert _best_release_group("Off the Wall", []) is None + assert _best_release_group("Michael Jackson", "Off the Wall", []) is None + + +def test_prefers_credited_artist_over_same_titled_other_artist_album(): + # Real-world "Fun Machine" bug: Lake Street Dive's EP is what we followed, but two + # unrelated bands also have a release group literally titled "Fun Machine" as an Album. + # All three tie on title (1.0), so the old is_album tiebreak wrongly grabbed a foreign + # Album. The followed artist must win over title-tied albums by other artists. + groups = [ + _rg("Fun Machine", "EP", "Lake Street Dive"), + _rg("Fun Machine", "Album", "Bastards of Melody"), + _rg("Fun Machine", "Album", "Teledildonics 5000"), + ] + best = _best_release_group("Lake Street Dive", "Fun Machine", groups) + assert best["artist-credit"][0]["artist"]["name"] == "Lake Street Dive"