From 7dfe1ae88d7d378d22c74ba665d1ba6924734950 Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Fri, 24 Apr 2026 08:48:02 -0700 Subject: [PATCH] MusicBrainz: Resolve release-group MBIDs to a release on album click MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clicking a MusicBrainz album returned 404 because the browse-based search path now stores release-GROUP MBIDs in Album.id, but `get_album` still hit `/ws/2/release/` directly. Release-group MBIDs don't resolve as release MBIDs — MB 404s. User log: GET /api/spotify/album/b88655ba...?source=musicbrainz → 404 Error fetching release b88655ba...: 404 Client Error The fix requires a two-step resolution for the new browse path: 1. Look up the release-group with `inc=releases+artist-credits` to get the list of releases inside (original + reissues + regional + promo editions). MB release-groups routinely hold 5-20 releases. 2. Pick a representative release: prefer Official status over Promo, prefer releases with a real tracklist over stubs, then earliest date. 3. Fetch that release's full tracklist via `get_release`. Two extra seconds at the 1-rps rate limit, but it's on click, not on search results rendering. Structure: - New `MusicBrainzClient.get_release_group(mbid, includes)` method. - New `_pick_representative_release(releases)` helper encapsulates the ranking logic. - Tracklist projection extracted into `_render_release_as_album` so both paths share the same shape construction. - `get_album` tries release-group first; falls back to direct release lookup when the MBID turns out to be a release from the text-search fallback path. - Canonical Album.id stays the release-group MBID so a re-fetch with the same URL hits the same code path idempotently. 3 new tests (now 33 total): - End-to-end release-group → release resolution with mocked client - Fallback to direct release lookup when rg lookup misses - Representative-release picker ranks correctly Verified against live API with the exact MBID that 404'd for the user (b88655ba... for DAMN. by Kendrick Lamar): now returns in 1.2s with the full 14-track listing (BLOOD., DNA., YAH., ELEMENT., FEEL., ...). --- core/musicbrainz_client.py | 32 ++++++ core/musicbrainz_search.py | 191 +++++++++++++++++++++---------- tests/test_musicbrainz_search.py | 95 +++++++++++++++ 3 files changed, 257 insertions(+), 61 deletions(-) diff --git a/core/musicbrainz_client.py b/core/musicbrainz_client.py index 89bac89f..b152f28f 100644 --- a/core/musicbrainz_client.py +++ b/core/musicbrainz_client.py @@ -369,6 +369,38 @@ class MusicBrainzClient: logger.error(f"Error fetching release {mbid}: {e}") return None + @rate_limited + def get_release_group(self, mbid: str, includes: Optional[List[str]] = None) -> Optional[Dict[str, Any]]: + """Get full release-group details by MBID. + + Release-groups are the 'canonical album' entity in MusicBrainz — + they group every edition/reissue/region-specific release of the + same logical album under one MBID. Use `inc=releases` to list the + individual releases this group contains (each with its own + tracklist); use `inc=artist-credits` for artist info. + + Args: + mbid: Release-group's MusicBrainz ID + includes: Optional list, e.g. ['releases', 'artist-credits'] + + Returns: + Release-group data or None if not found. + """ + try: + params = {'fmt': 'json'} + if includes: + params['inc'] = '+'.join(includes) + response = self.session.get( + f"{self.BASE_URL}/release-group/{mbid}", + params=params, + timeout=10 + ) + response.raise_for_status() + return response.json() + except Exception as e: + logger.error(f"Error fetching release-group {mbid}: {e}") + return None + @rate_limited def get_recording(self, mbid: str, includes: Optional[List[str]] = None) -> Optional[Dict[str, Any]]: """ diff --git a/core/musicbrainz_search.py b/core/musicbrainz_search.py index 8af8bd38..f7b8370a 100644 --- a/core/musicbrainz_search.py +++ b/core/musicbrainz_search.py @@ -574,70 +574,139 @@ class MusicBrainzSearchClient: logger.warning(f"MusicBrainz track search failed: {e}") return [] - def get_album(self, release_mbid: str) -> Optional[Dict[str, Any]]: - """Get full album details with track listing for download modal.""" - try: - release = self._client.get_release(release_mbid, includes=['recordings', 'artist-credits', 'release-groups']) - if not release: - return None + def _pick_representative_release(self, releases: List[Dict[str, Any]]) -> Optional[Dict[str, Any]]: + """Pick the best release out of a release-group's editions. - title = release.get('title', '') - artists_raw = _extract_artist_credit(release.get('artist-credit', [])) - release_date = release.get('date', '') or '' - - rg = release.get('release-group', {}) - primary_type = rg.get('primary-type', '') or '' - secondary_types = rg.get('secondary-types', []) or [] - album_type = _map_release_type(primary_type, secondary_types) - - # Cover art - rg_mbid = rg.get('id', '') - image_url = self._cached_art(release_mbid, rg_mbid) - - # Build tracks from media - tracks = [] - total_tracks = 0 - media_list = release.get('media', []) - for media_idx, media in enumerate(media_list): - disc_number = media.get('position', media_idx + 1) - for track in media.get('tracks', []): - total_tracks += 1 - recording = track.get('recording', {}) - track_artists = _extract_artist_credit(recording.get('artist-credit', [])) - if not track_artists: - track_artists = artists_raw - - try: - track_num = int(track.get('number', track.get('position', total_tracks))) - except (ValueError, TypeError): - track_num = total_tracks - - tracks.append({ - 'id': recording.get('id', track.get('id', '')), - 'name': recording.get('title', track.get('title', '')), - 'artists': [{'name': a} for a in track_artists], - 'duration_ms': recording.get('length', 0) or track.get('length', 0) or 0, - 'track_number': track_num, - 'disc_number': disc_number, - }) - - images = [{'url': image_url, 'height': 250, 'width': 250}] if image_url else [] - - return { - 'id': release_mbid, - 'name': title, - 'artists': [{'name': a, 'id': ''} for a in (artists_raw or ['Unknown Artist'])], - 'release_date': release_date, - 'total_tracks': total_tracks, - 'album_type': album_type, - 'images': images, - 'tracks': tracks, - 'external_urls': {'musicbrainz': f'https://musicbrainz.org/release/{release_mbid}'}, - } - except Exception as e: - logger.error(f"MusicBrainz album detail failed for {release_mbid}: {e}") + Release-groups often contain 5-20+ releases (original, reissues, + remasters, regional editions, bonus-track editions). We want a + single canonical version to show the user as 'the album.' Prefer: + 1. Official releases (not promo/bootleg) + 2. Earliest date (the original) + 3. Any release with media (skip entries that are just stubs) + """ + if not releases: return None + def _key(r): + status = (r.get('status') or '').lower() + status_rank = 0 if status == 'official' else 1 # Official first + has_media = 0 if r.get('media') else 1 # Real tracklists first + date = (r.get('date') or '9999-99-99')[:10] + return (has_media, status_rank, date) + + return sorted(releases, key=_key)[0] + + def get_album(self, album_mbid: str) -> Optional[Dict[str, Any]]: + """Get full album details with track listing for download modal. + + The MBID passed in could be either: + - A release-group MBID (from `search_albums` browse path — the + common case now that bare-name searches route artist-first → + browse), or + - A release MBID (from the text-search fallback path). + + Try release-group first since that's the majority; if it 404s, + fall back to direct release lookup. Release-group resolution adds + one extra API call (~1s at the 1-rps rate limit) to pick a + representative release and then fetch its tracklist. + """ + try: + # Path A: release-group MBID (new browse-based search default) + rg = self._client.get_release_group( + album_mbid, includes=['releases', 'artist-credits'] + ) + if rg: + releases = rg.get('releases') or [] + rep = self._pick_representative_release(releases) + if rep and rep.get('id'): + album = self._render_release_as_album( + rep['id'], + rg_fallback=rg, + ) + if album: + # Keep the release-group MBID as the canonical + # Album.id so downstream code can re-fetch with + # the same URL. + album['id'] = album_mbid + album['external_urls'] = { + 'musicbrainz': f'https://musicbrainz.org/release-group/{album_mbid}' + } + return album + + # Path B: release MBID (text-search fallback path) + return self._render_release_as_album(album_mbid) + except Exception as e: + logger.error(f"MusicBrainz album detail failed for {album_mbid}: {e}") + return None + + def _render_release_as_album(self, release_mbid: str, + rg_fallback: Optional[Dict[str, Any]] = None) -> Optional[Dict[str, Any]]: + """Fetch a specific release and project it to the album-detail dict + shape the download modal expects. `rg_fallback` supplies release-group + metadata (type, artist credits) when resolving from a release-group + whose releases may be lightly populated.""" + release = self._client.get_release( + release_mbid, includes=['recordings', 'artist-credits', 'release-groups'] + ) + if not release: + return None + + title = release.get('title', '') + artists_raw = _extract_artist_credit(release.get('artist-credit', [])) + if not artists_raw and rg_fallback: + artists_raw = _extract_artist_credit(rg_fallback.get('artist-credit', [])) + release_date = release.get('date', '') or '' + if not release_date and rg_fallback: + release_date = rg_fallback.get('first-release-date', '') or '' + + rg = release.get('release-group', rg_fallback or {}) or {} + primary_type = rg.get('primary-type', '') or '' + secondary_types = rg.get('secondary-types', []) or [] + album_type = _map_release_type(primary_type, secondary_types) + + rg_mbid = rg.get('id', '') + image_url = self._cached_art(release_mbid, rg_mbid) + + tracks = [] + total_tracks = 0 + media_list = release.get('media', []) + for media_idx, media in enumerate(media_list): + disc_number = media.get('position', media_idx + 1) + for track in media.get('tracks', []): + total_tracks += 1 + recording = track.get('recording', {}) + track_artists = _extract_artist_credit(recording.get('artist-credit', [])) + if not track_artists: + track_artists = artists_raw + + try: + track_num = int(track.get('number', track.get('position', total_tracks))) + except (ValueError, TypeError): + track_num = total_tracks + + tracks.append({ + 'id': recording.get('id', track.get('id', '')), + 'name': recording.get('title', track.get('title', '')), + 'artists': [{'name': a} for a in track_artists], + 'duration_ms': recording.get('length', 0) or track.get('length', 0) or 0, + 'track_number': track_num, + 'disc_number': disc_number, + }) + + images = [{'url': image_url, 'height': 250, 'width': 250}] if image_url else [] + + return { + 'id': release_mbid, + 'name': title, + 'artists': [{'name': a, 'id': ''} for a in (artists_raw or ['Unknown Artist'])], + 'release_date': release_date, + 'total_tracks': total_tracks, + 'album_type': album_type, + 'images': images, + 'tracks': tracks, + 'external_urls': {'musicbrainz': f'https://musicbrainz.org/release/{release_mbid}'}, + } + def get_artist_albums(self, artist_mbid: str, album_type: str = 'album,single') -> List: """Get artist's releases for discography view.""" try: diff --git a/tests/test_musicbrainz_search.py b/tests/test_musicbrainz_search.py index 88afe15f..939e83fe 100644 --- a/tests/test_musicbrainz_search.py +++ b/tests/test_musicbrainz_search.py @@ -440,6 +440,101 @@ def test_search_tracks_structured_query_uses_text_path(): assert len(tracks) == 1 +def test_get_album_resolves_release_group_mbid_to_release(): + """When the album ID is a release-group MBID (from the browse path), + get_album must look up the release-group, pick a canonical release, + and fetch that release's tracklist. Fetching /release/ + directly 404s.""" + client = MusicBrainzSearchClient() + client._client = MagicMock() + # Release-group lookup returns two editions — an Official release and + # a promo. The Official earlier release should win. + client._client.get_release_group.return_value = { + 'id': 'rg-damn', + 'title': 'DAMN.', + 'primary-type': 'Album', + 'secondary-types': [], + 'first-release-date': '2017-04-14', + 'artist-credit': [{'name': 'Kendrick Lamar'}], + 'releases': [ + {'id': 'rel-promo', 'status': 'Promotion', 'date': '2017-04-01', + 'media': [{'track-count': 14, 'tracks': []}]}, + {'id': 'rel-official', 'status': 'Official', 'date': '2017-04-14', + 'media': [{'track-count': 14, 'tracks': []}]}, + ], + } + # Release lookup returns a full release with tracklist. + client._client.get_release.return_value = { + 'id': 'rel-official', + 'title': 'DAMN.', + 'date': '2017-04-14', + 'artist-credit': [{'name': 'Kendrick Lamar'}], + 'release-group': {'id': 'rg-damn', 'primary-type': 'Album', 'secondary-types': []}, + 'media': [ + {'position': 1, 'tracks': [ + {'id': 't1', 'number': '1', 'position': 1, 'length': 50000, + 'recording': {'id': 'rec-1', 'title': 'BLOOD.', + 'artist-credit': [{'name': 'Kendrick Lamar'}], 'length': 50000}}, + ]}, + ], + } + + album = client.get_album('rg-damn') + + # Must have called release-group first, then release for the picked edition. + client._client.get_release_group.assert_called_once_with( + 'rg-damn', includes=['releases', 'artist-credits'] + ) + client._client.get_release.assert_called_once_with( + 'rel-official', includes=['recordings', 'artist-credits', 'release-groups'] + ) + assert album is not None + assert album['id'] == 'rg-damn' # Canonical ID stays the release-group MBID. + assert album['name'] == 'DAMN.' + assert len(album['tracks']) == 1 + assert album['tracks'][0]['name'] == 'BLOOD.' + assert 'release-group' in album['external_urls']['musicbrainz'] + + +def test_get_album_falls_back_to_release_lookup_on_rg_miss(): + """When the MBID is a release (from the text-search fallback path) the + release-group lookup 404s, but the direct release lookup works.""" + client = MusicBrainzSearchClient() + client._client = MagicMock() + # Release-group lookup returns None (simulating 404). + client._client.get_release_group.return_value = None + client._client.get_release.return_value = { + 'id': 'rel-abc', + 'title': 'Some Album', + 'date': '2020-01-01', + 'artist-credit': [{'name': 'Some Artist'}], + 'release-group': {'id': 'rg-abc', 'primary-type': 'Album', 'secondary-types': []}, + 'media': [{'position': 1, 'tracks': []}], + } + + album = client.get_album('rel-abc') + + client._client.get_release_group.assert_called_once() + client._client.get_release.assert_called_once() + assert album is not None + assert album['id'] == 'rel-abc' # Falls back to release MBID since rg lookup missed. + + +def test_pick_representative_release_prefers_official_with_media(): + """The release picker should skip stub releases (no media) and pick + Official over Promotion status.""" + client = MusicBrainzSearchClient() + releases = [ + {'id': 'stub', 'status': 'Official', 'date': '2020-01-01'}, # No media + {'id': 'promo', 'status': 'Promotion', 'date': '2019-12-01', + 'media': [{'track-count': 10}]}, + {'id': 'official', 'status': 'Official', 'date': '2020-01-05', + 'media': [{'track-count': 10}]}, + ] + picked = client._pick_representative_release(releases) + assert picked['id'] == 'official' + + def test_search_tracks_text_path_filters_by_score(): client = MusicBrainzSearchClient() client._client = MagicMock()