diff options
Diffstat (limited to 'packages/meshbay-node')
| -rw-r--r-- | packages/meshbay-node/src/meshbay_node/musicbrainz.py | 74 | ||||
| -rw-r--r-- | packages/meshbay-node/tests/test_musicbrainz.py | 98 |
2 files changed, 159 insertions, 13 deletions
diff --git a/packages/meshbay-node/src/meshbay_node/musicbrainz.py b/packages/meshbay-node/src/meshbay_node/musicbrainz.py index f3d9b4f..6ca1cf4 100644 --- a/packages/meshbay-node/src/meshbay_node/musicbrainz.py +++ b/packages/meshbay-node/src/meshbay_node/musicbrainz.py @@ -64,23 +64,49 @@ def _normalize(s: str) -> str: return re.sub(r"\s+", " ", s).strip() -def _best_match(query: str, results: list[dict], key: str) -> tuple[dict | None, float]: +# Lucene/Solr query-syntax characters (the parser MusicBrainz's `ws/2` search +# runs on). A tag or folder-derived artist/album string is untrusted free +# text as far as this parser is concerned — a stray "(", ":" or bare '"' +# either breaks the surrounding quoted phrase or gets read as field/grouping +# syntax rather than a literal character. Backslash-escaping each one keeps +# it literal without changing what the analyzer tokenizes. +_LUCENE_SPECIAL_RE = re.compile(r'([+\-&|!(){}\[\]^"~*?:\\/])') + + +def _escape_lucene(s: str) -> str: + return _LUCENE_SPECIAL_RE.sub(r"\\\1", s) + + +def _similarity(query: str, val: str | None) -> float: + return (difflib.SequenceMatcher(None, _normalize(query), _normalize(str(val))).ratio() + if val else 0.0) + + +def _best_match_release(artist: str, album: str, results: list[dict]) -> tuple[dict | None, float]: """ Same "trust the search's own ranking" shape as tmdb.py's `_best_match` (§3.3 of mediacenter.md found a locally-recomputed re-rank pick a coincidentally closer-looking wrong result once — no reason to expect MusicBrainz's own scored search to fare differently under the same treatment). MusicBrainz already returns results ordered by its own - `score`; only the top one is considered, and the similarity ratio is a - confidence signal for the caller's fallback decision, not a re-rank. + `score`; only the top one is considered. + + Confidence is the average of the album/title and artist similarity, + not the title alone: `search_release`'s loose fallback query has no + field scoping at all, so a title-only ratio would happily call a + same-titled album by an unrelated artist a good match. Averaging both + still lets a strong single-field match (e.g. the artist matches + exactly but the local album string carries an edition suffix) clear + the caller's threshold, while an unrelated same-name result does not. """ if not results: return None, 0.0 top = results[0] - val = top.get(key) - ratio = (difflib.SequenceMatcher(None, _normalize(query), _normalize(str(val))).ratio() - if val else 0.0) - return top, ratio + title_ratio = _similarity(album, top.get("title")) + credit = top.get("artist-credit") or [] + artist_name = credit[0].get("name") if credit else None + artist_ratio = _similarity(artist, artist_name) + return top, (title_ratio + artist_ratio) / 2 class MusicBrainzClient: @@ -132,11 +158,37 @@ class MusicBrainzClient: return None async def search_release(self, artist: str, album: str) -> tuple[dict | None, float]: - """Top MusicBrainz release match for an (artist, album) pair.""" - query = f'artist:"{artist}" AND release:"{album}"' - data = await self._get(_BASE_URL + "release", {"query": query}) + """ + Top MusicBrainz release match for an (artist, album) pair. + + Tries a field-scoped exact-phrase query first — cheap, and the + common case since tag data usually already matches MusicBrainz's + own spelling. Verified live against the real service: real tag/ + folder text routinely carries something an exact phrase does not + tolerate at all — a year suffix, an edition tag, a stray + punctuation mark, or an artist credited under an older/aliased + name on that specific release — and MusicBrainz's phrase parser + then returns **zero** hits, not a low-scored one, so the caller's + confidence check never gets a chance to run at all. + `artist:"Groundation" AND release:"Hebron Gate"` finds it, score + 100; appending "(2003)" — exactly how many rip folders name a + release — drops it to zero results outright. A second, unscoped, + unquoted query lets MusicBrainz's own relevance ranking find the + same release regardless of the extra text — confirmed with the + same pair, buried among 6000+ candidates by the plain query, still + ranked first. Only tried when the strict query comes back empty, + to keep the common case at one request. + """ + strict_query = f'artist:"{_escape_lucene(artist)}" AND release:"{_escape_lucene(album)}"' + data = await self._get(_BASE_URL + "release", {"query": strict_query}) + results = (data or {}).get("releases", []) + if results: + return _best_match_release(artist, album, results) + + loose_query = f"{_escape_lucene(artist)} {_escape_lucene(album)}" + data = await self._get(_BASE_URL + "release", {"query": loose_query}) results = (data or {}).get("releases", []) - return _best_match(album, results, "title") + return _best_match_release(artist, album, results) async def release_details(self, mbid: str) -> dict | None: """Full release details, including recordings (tracklist).""" diff --git a/packages/meshbay-node/tests/test_musicbrainz.py b/packages/meshbay-node/tests/test_musicbrainz.py index 482395d..4d075d3 100644 --- a/packages/meshbay-node/tests/test_musicbrainz.py +++ b/packages/meshbay-node/tests/test_musicbrainz.py @@ -4,8 +4,7 @@ import time import httpx import pytest - -from meshbay_node.musicbrainz import MusicBrainzClient, _MIN_INTERVAL_SECS +from meshbay_node.musicbrainz import _MIN_INTERVAL_SECS, MusicBrainzClient, _escape_lucene class FakeRoster: @@ -55,6 +54,101 @@ async def test_no_results_returns_none_and_zero_confidence(): await client.close() +def test_escape_lucene_escapes_query_syntax_characters(): + # Verified live against musicbrainz.org before writing the fallback + # below: a literal "(" inside a quoted phrase is read as query syntax, + # not a character to match, and silently drops the phrase to zero + # results rather than raising. + assert _escape_lucene("Hebron Gate (2003)") == r"Hebron Gate \(2003\)" + assert _escape_lucene('Say "It" Loud') == r"Say \"It\" Loud" + assert _escape_lucene("Rock & Roll: Live") == r"Rock \& Roll\: Live" + assert _escape_lucene("no specials here") == "no specials here" + + +@pytest.mark.asyncio +async def test_strict_match_does_not_pay_for_a_second_request(): + """A tag that already matches MusicBrainz's own spelling should cost + exactly one request — the fallback below exists for the case that + doesn't, not for every call.""" + calls = [] + + def handle(request: httpx.Request) -> httpx.Response: + calls.append(str(request.url)) + return httpx.Response(200, json={ + "releases": [{"id": "abc-123", "title": "The Great Album", + "artist-credit": [{"name": "Some Artist"}]}], + }) + + client = MusicBrainzClient( + roster=FakeRoster(), + transport=httpx.MockTransport(handle), + ) + await client.search_release("Some Artist", "The Great Album") + + assert len(calls) == 1 + await client.close() + + +@pytest.mark.asyncio +async def test_falls_back_to_a_loose_query_when_the_strict_one_finds_nothing(): + """ + Regression for the real failure mode found against the live service: + `artist:"Groundation" AND release:"Hebron Gate"` finds the release + (score 100), but a local album tag/folder carrying a trailing year — + "Hebron Gate (2003)", a common rip-folder shape — drops the strict + exact-phrase query to zero hits, not a low-scored one. The loose, + unscoped query must be tried next instead of stopping at "no match". + """ + calls = [] + + def handle(request: httpx.Request) -> httpx.Response: + query = request.url.params.get("query", "") + calls.append(query) + if query.startswith("artist:"): + return httpx.Response(200, json={"releases": []}) + return httpx.Response(200, json={ + "releases": [{"id": "xyz-789", "title": "Hebron Gate", + "artist-credit": [{"name": "Groundation"}]}], + }) + + client = MusicBrainzClient( + roster=FakeRoster(), + transport=httpx.MockTransport(handle), + ) + result, ratio = await client.search_release("Groundation", "Hebron Gate (2003)") + + assert result is not None + assert result["id"] == "xyz-789" + assert ratio > 0.5 + assert len(calls) == 2, "must retry with a loose query after the strict one finds nothing" + await client.close() + + +@pytest.mark.asyncio +async def test_confidence_reflects_a_wrong_artist_on_a_same_titled_release(): + """ + The loose fallback query has no field scoping at all, so a same- + titled release by an unrelated artist must not read as confidently as + one that also matches on artist — otherwise the fallback trades + "never finds a mismatch" for "sometimes confirms a wrong one". + """ + def handle(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, json={ + "releases": [{"id": "wrong-1", "title": "Live", + "artist-credit": [{"name": "An Entirely Different Band"}]}], + }) + + client = MusicBrainzClient( + roster=FakeRoster(), + transport=httpx.MockTransport(handle), + ) + result, ratio = await client.search_release("Groundation", "Live") + + assert result is not None + assert ratio < 0.6, "a title-only match against the wrong artist must not read as confident" + await client.close() + + @pytest.mark.asyncio async def test_no_contact_configured_makes_no_request(monkeypatch): monkeypatch.delenv("MESHBAY_MUSICBRAINZ_CONTACT_DEFAULT", raising=False) |