From 8523724b03c11487d2c043b23cea165b50981c97 Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Fri, 24 Apr 2026 08:25:09 -0700 Subject: [PATCH] MusicBrainz: Switch track lookup from browse to arid: search MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit's `browse_artist_recordings` call passed `inc=releases+artist-credits` — but MusicBrainz's recording browse endpoint rejects `inc=releases` with HTTP 400. The adapter's error handler returned an empty list, so the Tracks section stayed empty even though the fix was supposed to populate it. Browse without release info is useless for our search UI (tracks would render with no album), so swap to the fielded Lucene search `arid:` on the `/recording` endpoint. That's the canonical MB pattern for "find recordings by this artist WITH release context": - arid: search accepts the artist MBID and returns recordings with `releases` (release-group, date, media) embedded in each result. - One API call per lookup, same as browse would have been. Renamed the method to `search_recordings_by_artist_mbid` so the name matches its behaviour — it's a search, not a browse. Adapter updated to call the new name; tests updated to match. Verified against the live API: Metallica's MBID returns 5 recordings in ~1.8 seconds (vs the previous 400 error). --- core/musicbrainz_client.py | 39 ++++++++++++++++++-------------- core/musicbrainz_search.py | 11 ++++----- tests/test_musicbrainz_search.py | 8 +++---- 3 files changed, 31 insertions(+), 27 deletions(-) diff --git a/core/musicbrainz_client.py b/core/musicbrainz_client.py index 3bfeb5a9..89bac89f 100644 --- a/core/musicbrainz_client.py +++ b/core/musicbrainz_client.py @@ -264,30 +264,35 @@ class MusicBrainzClient: return [] @rate_limited - def browse_artist_recordings(self, artist_mbid: str, - limit: int = 100, - offset: int = 0, - includes: Optional[List[str]] = None) -> List[Dict[str, Any]]: - """Browse recordings (tracks) linked to an artist MBID. + def search_recordings_by_artist_mbid(self, artist_mbid: str, + limit: int = 100) -> List[Dict[str, Any]]: + """Search for recordings linked to an artist via Lucene `arid:` query. - Counterpart to `browse_artist_release_groups` — text search on - `/recording?query=...` matches recording TITLES, while browse follows - the artist→recording link directly. + This is the counterpart to `browse_artist_release_groups` for tracks. + The proper "browse" endpoint (`/recording?artist=`) rejects + `inc=releases`, so we can't get album context per recording from + browse — only the track title/length/MBID. Without release info the + user would see tracks with no album, which is useless. + + The search endpoint with a fielded `arid:` query returns + recordings with the `releases` array already embedded (including + release-group, date, and media info), which is what the search-tab + UI needs. Args: artist_mbid: Artist's MusicBrainz ID limit: 1-100 (MB hard cap) - offset: Pagination offset - includes: e.g. ['releases', 'artist-credits'] to embed linked entities Returns: - List of recording dicts with `id`, `title`, `length`, `disambiguation`, - and optionally `releases` / `artist-credit` per includes. + List of recording dicts with `id`, `title`, `length`, `score`, + `artist-credit`, and `releases` (each with release-group + date). """ try: - params = {'artist': artist_mbid, 'fmt': 'json', 'limit': min(limit, 100), 'offset': offset} - if includes: - params['inc'] = '+'.join(includes) + params = { + 'query': f'arid:{artist_mbid}', + 'fmt': 'json', + 'limit': min(limit, 100), + } response = self.session.get( f"{self.BASE_URL}/recording", @@ -298,10 +303,10 @@ class MusicBrainzClient: data = response.json() recs = data.get('recordings', []) - logger.debug(f"Browsed {len(recs)} recordings for artist {artist_mbid}") + logger.debug(f"Found {len(recs)} recordings for artist {artist_mbid}") return recs except Exception as e: - logger.error(f"Error browsing recordings for artist {artist_mbid}: {e}") + logger.error(f"Error searching recordings for artist {artist_mbid}: {e}") return [] @rate_limited diff --git a/core/musicbrainz_search.py b/core/musicbrainz_search.py index adf24a2d..7ff95e7a 100644 --- a/core/musicbrainz_search.py +++ b/core/musicbrainz_search.py @@ -429,16 +429,15 @@ class MusicBrainzSearchClient: if artist_name: return self._search_tracks_text(title, artist_name, limit) - # Bare name → artist-first → browse. + # Bare name → artist-first → arid: search. top = self._resolve_top_artist(query) if top: mbid = top.get('id', '') tname = top.get('name', '') or query - recs = self._client.browse_artist_recordings( - mbid, - limit=100, - includes=['releases', 'artist-credits'], - ) + # /recording?artist= (browse) rejects inc=releases, + # so we use the fielded Lucene search arid: instead — + # that returns recordings with release context inline. + recs = self._client.search_recordings_by_artist_mbid(mbid, limit=100) # Browse returns recordings unsorted. Dedupe by normalized # title (MB has many live/compilation variants of the same # song), then sort by release date desc so "newest" tracks diff --git a/tests/test_musicbrainz_search.py b/tests/test_musicbrainz_search.py index 17fac08a..db75b306 100644 --- a/tests/test_musicbrainz_search.py +++ b/tests/test_musicbrainz_search.py @@ -274,7 +274,7 @@ def test_search_tracks_bare_query_uses_browse_path(): client = MusicBrainzSearchClient() client._client = MagicMock() client._client.search_artist.return_value = [_mk_artist('Metallica', 'mb-1', score=100)] - client._client.browse_artist_recordings.return_value = [ + client._client.search_recordings_by_artist_mbid.return_value = [ {'id': 'rec-1', 'title': 'One', 'length': 446000, 'releases': [{'id': 'rel-1', 'title': '...And Justice for All', 'date': '1988', 'release-group': {'id': 'rg-1', 'primary-type': 'Album'}}], @@ -287,7 +287,7 @@ def test_search_tracks_bare_query_uses_browse_path(): tracks = client.search_tracks('metallica', limit=10) - client._client.browse_artist_recordings.assert_called_once() + client._client.search_recordings_by_artist_mbid.assert_called_once() client._client.search_recording.assert_not_called() assert len(tracks) == 2 assert {t.name for t in tracks} == {'One', 'Battery'} @@ -300,7 +300,7 @@ def test_search_tracks_dedupes_by_title(): client = MusicBrainzSearchClient() client._client = MagicMock() client._client.search_artist.return_value = [_mk_artist('Metallica', 'mb-1', score=100)] - client._client.browse_artist_recordings.return_value = [ + client._client.search_recordings_by_artist_mbid.return_value = [ {'id': 'rec-1', 'title': 'One', 'length': 446000, 'releases': [{'id': 'rel-1', 'title': '...And Justice for All', 'date': '1988'}], 'artist-credit': [{'name': 'Metallica'}]}, @@ -328,7 +328,7 @@ def test_search_tracks_structured_query_uses_text_path(): client._client.search_recording.assert_called_once() client._client.search_artist.assert_not_called() - client._client.browse_artist_recordings.assert_not_called() + client._client.search_recordings_by_artist_mbid.assert_not_called() assert len(tracks) == 1