From 3c48508c3f6d338790785fef62dd2b2b81dfbe67 Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Fri, 24 Apr 2026 08:06:50 -0700 Subject: [PATCH 01/13] MusicBrainz: Add project URL to User-Agent per API requirements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MusicBrainz mandates a meaningful User-Agent with contact info, warning that bare strings can trigger IP blocking under load. Our client was sending `SoulSync/2.3` with no contact — and the search adapter passed an app version hard-coded at "2.3" that's now stale (UI is at 2.40). Fix: default contact to the project URL (`https://github.com/Nezreka/SoulSync`) when no email is supplied, so every request lands as `SoulSync/ ( https://github.com/Nezreka/SoulSync )`. Drop the search-adapter version suffix to a generic "2" since the exact UI minor version would add noise to every MB request without helping operators track issues. Reference: https://musicbrainz.org/doc/MusicBrainz_API — "it is important that your application sets a proper User-Agent string." --- core/musicbrainz_client.py | 21 ++++++++++++--------- core/musicbrainz_search.py | 5 ++++- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/core/musicbrainz_client.py b/core/musicbrainz_client.py index b7b2725b..f6a4ef47 100644 --- a/core/musicbrainz_client.py +++ b/core/musicbrainz_client.py @@ -44,28 +44,31 @@ def rate_limited(func): class MusicBrainzClient: """Client for interacting with MusicBrainz API""" - + BASE_URL = "https://musicbrainz.org/ws/2" - + # MusicBrainz mandates a meaningful User-Agent with contact info. Falling back + # to a bare name/version risks IP blocking under load — include the project + # URL so MB operators have a way to reach us if we misbehave. + DEFAULT_CONTACT = "https://github.com/Nezreka/SoulSync" + def __init__(self, app_name: str = "SoulSync", app_version: str = "1.0", contact_email: str = ""): """ Initialize MusicBrainz client - + Args: app_name: Name of the application app_version: Version of the application - contact_email: Contact email (optional but recommended) + contact_email: Contact email or URL (defaults to project URL when empty) """ - self.user_agent = f"{app_name}/{app_version}" - if contact_email: - self.user_agent += f" ( {contact_email} )" - + contact = contact_email or self.DEFAULT_CONTACT + self.user_agent = f"{app_name}/{app_version} ( {contact} )" + self.session = requests.Session() self.session.headers.update({ 'User-Agent': self.user_agent, 'Accept': 'application/json' }) - + logger.info(f"MusicBrainz client initialized with user agent: {self.user_agent}") @rate_limited diff --git a/core/musicbrainz_search.py b/core/musicbrainz_search.py index f9d93aab..cf6e6e8c 100644 --- a/core/musicbrainz_search.py +++ b/core/musicbrainz_search.py @@ -116,7 +116,10 @@ class MusicBrainzSearchClient: def __init__(self): from core.musicbrainz_client import MusicBrainzClient - self._client = MusicBrainzClient("SoulSync", "2.3") + # Client defaults to the project URL as its User-Agent contact, + # which is what MusicBrainz wants. Version stays generic ("2") — + # the exact UI minor version would add noise to every request. + self._client = MusicBrainzClient("SoulSync", "2") self._art_cache: Dict[str, Optional[str]] = {} # mbid -> url def _cached_art(self, release_mbid: str, release_group_mbid: str = '') -> Optional[str]: From e2a5a38cd2bf9449c7f447fbaee302e4a2d92960 Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Fri, 24 Apr 2026 08:07:47 -0700 Subject: [PATCH 02/13] MusicBrainz: Add browse endpoints for release-groups + recordings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add `browse_artist_release_groups(mbid)` and `browse_artist_recordings(mbid)` to MusicBrainzClient. These hit `/ws/2/release-group?artist=` and `/ws/2/recording?artist=` respectively — the correct MusicBrainz pattern for "give me everything linked to this artist." Why this matters: our current search adapter calls text-search (`release?query=...` / `recording?query=...`) for albums and tracks, which matches entity titles literally. Typing "metallica" hits unrelated releases titled "Metallica" and recordings named "Metallica" by obscure bands — every garbage match scores 100 because they're all exact title matches on the wrong field. Browse walks the artist→release-group and artist→recording links directly. Once we know the artist's MBID (from `search_artist`), browse returns their actual discography instead of title collisions. No behavior change yet — search adapter still uses the old path. Follow- up commit wires the new endpoints in. Reference: https://musicbrainz.org/doc/MusicBrainz_API — "Browse queries retrieve entities linked to a known entity" vs search. --- core/musicbrainz_client.py | 87 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/core/musicbrainz_client.py b/core/musicbrainz_client.py index f6a4ef47..a7b1b5b9 100644 --- a/core/musicbrainz_client.py +++ b/core/musicbrainz_client.py @@ -200,6 +200,93 @@ class MusicBrainzClient: logger.error(f"Error searching for recording '{track_name}': {e}") return [] + @rate_limited + def browse_artist_release_groups(self, artist_mbid: str, + release_types: Optional[List[str]] = None, + limit: int = 100, + offset: int = 0) -> List[Dict[str, Any]]: + """Browse release-groups linked to an artist MBID. + + This is the correct MusicBrainz pattern for "give me this artist's + discography" — text-based `/release?query=...` search would look at + release TITLES (matching unrelated releases literally titled after + the artist name), while browse walks the artist→release-group link + directly. + + Args: + artist_mbid: Artist's MusicBrainz ID + release_types: Filter by primary type — any of 'album', 'single', + 'ep', 'compilation', 'soundtrack', 'live', etc. Combined with + `|` per MB spec, e.g. `['album', 'ep']` → `type=album|ep`. + None returns all types. + limit: 1-100 (MB hard cap) + offset: Pagination offset + + Returns: + List of release-group dicts. Each has `id`, `title`, `primary-type`, + `secondary-types`, `first-release-date`, `disambiguation`. + """ + try: + params = {'artist': artist_mbid, 'fmt': 'json', 'limit': min(limit, 100), 'offset': offset} + if release_types: + params['type'] = '|'.join(release_types) + + response = self.session.get( + f"{self.BASE_URL}/release-group", + params=params, + timeout=10 + ) + response.raise_for_status() + + data = response.json() + rgs = data.get('release-groups', []) + logger.debug(f"Browsed {len(rgs)} release-groups for artist {artist_mbid}") + return rgs + except Exception as e: + logger.error(f"Error browsing release-groups for artist {artist_mbid}: {e}") + 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. + + Counterpart to `browse_artist_release_groups` — text search on + `/recording?query=...` matches recording TITLES, while browse follows + the artist→recording link directly. + + 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. + """ + try: + params = {'artist': artist_mbid, 'fmt': 'json', 'limit': min(limit, 100), 'offset': offset} + if includes: + params['inc'] = '+'.join(includes) + + response = self.session.get( + f"{self.BASE_URL}/recording", + params=params, + timeout=10 + ) + response.raise_for_status() + + data = response.json() + recs = data.get('recordings', []) + logger.debug(f"Browsed {len(recs)} recordings for artist {artist_mbid}") + return recs + except Exception as e: + logger.error(f"Error browsing recordings for artist {artist_mbid}: {e}") + return [] + @rate_limited def get_artist(self, mbid: str, includes: Optional[List[str]] = None) -> Optional[Dict[str, Any]]: """ From 434d1c382c2aae219c001c911ad0c5738d1acbd5 Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Fri, 24 Apr 2026 08:09:51 -0700 Subject: [PATCH 03/13] MusicBrainz: Re-enable real artist search (was returning empty) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `MusicBrainzSearchClient.search_artists` has been a `return []` stub since the feature landed, with a comment claiming the MB tab 'doesn't show artists.' That's why kettui saw a missing Artists section on the search page — not a missing render, a hardcoded empty list. Re-enable it properly: - New `strict=False` parameter on `MusicBrainzClient.search_artist` sends a bare Lucene query instead of `artist:"..."`. MusicBrainz matches bare queries against alias+artist+sortname indexes together, which is the right behavior for user-facing fuzzy search (finds typos, aliases, sortname variants). `strict=True` remains the default for enrichment/AcoustID callers that want exact matches. - Adapter filters results to `score >= 80`. MB assigns a 0-100 Lucene score on every hit; the true artist + close variants score 100, tribute bands and lookalikes typically land in the 40-65 range. The cutoff keeps "Metallica" (100) and drops "Black Metallica Tribute Band" (60) without hand-curated lists. - Results returned as the same `Artist` dataclass used elsewhere in the search-tab adapter layer. `popularity` carries the MB score (0-100) so the frontend can sort/highlight top matches if desired. --- core/musicbrainz_client.py | 39 ++++++++++++++++++++-------- core/musicbrainz_search.py | 52 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 78 insertions(+), 13 deletions(-) diff --git a/core/musicbrainz_client.py b/core/musicbrainz_client.py index a7b1b5b9..3bfeb5a9 100644 --- a/core/musicbrainz_client.py +++ b/core/musicbrainz_client.py @@ -72,40 +72,57 @@ class MusicBrainzClient: logger.info(f"MusicBrainz client initialized with user agent: {self.user_agent}") @rate_limited - def search_artist(self, artist_name: str, limit: int = 10) -> List[Dict[str, Any]]: + def search_artist(self, artist_name: str, limit: int = 10, strict: bool = True) -> List[Dict[str, Any]]: """ - Search for artists by name - + Search for artists by name. + Args: artist_name: Name of the artist to search for limit: Maximum number of results to return - + strict: When True (default), builds a phrase-match query against + the `artist` field only — correct for enrichment flows that + already know the exact name. When False, sends a bare query + which MusicBrainz matches against the alias, artist, AND + sortname indexes — the right behavior for user-facing fuzzy + search (finds "Metallica" from typing "metalica", matches + aliased names, etc.). + Returns: - List of artist results with id, name, score, etc. + List of artist results with id, name, score, etc. MusicBrainz + assigns each result a `score` 0-100; the list is pre-sorted + score-descending by the server. """ try: # Escape quotes and backslashes for Lucene query safe_name = artist_name.replace('\\', '\\\\').replace('"', '\\"') - + + if strict: + query = f'artist:"{safe_name}"' + else: + # Bare query hits alias/artist/sortname indexes — much better + # recall for user typing. Still Lucene-escaped via the API's + # query parser. + query = safe_name + params = { - 'query': f'artist:"{safe_name}"', + 'query': query, 'fmt': 'json', 'limit': limit } - + response = self.session.get( f"{self.BASE_URL}/artist", params=params, timeout=10 ) response.raise_for_status() - + data = response.json() artists = data.get('artists', []) - + logger.debug(f"Found {len(artists)} artists for query: {artist_name}") return artists - + except Exception as e: logger.error(f"Error searching for artist '{artist_name}': {e}") return [] diff --git a/core/musicbrainz_search.py b/core/musicbrainz_search.py index cf6e6e8c..017f0cdb 100644 --- a/core/musicbrainz_search.py +++ b/core/musicbrainz_search.py @@ -133,9 +133,57 @@ class MusicBrainzSearchClient: self._art_cache[release_mbid] = url return url + # Score threshold for user-facing search results. MusicBrainz returns a + # Lucene score 0-100 on every match; exact name/alias hits score 100, + # partial/typo matches trend lower, and tribute bands / random + # lookalikes score 40-65. 80 is the cutoff that keeps the true artist + # and close variants while dropping unrelated noise. + _MIN_SCORE = 80 + def search_artists(self, query: str, limit: int = 10) -> List[Artist]: - """MusicBrainz search tab doesn't show artists — only albums and tracks.""" - return [] + """Search MusicBrainz for artists by name. + + Uses a bare Lucene query (no field prefix) so MusicBrainz searches + the alias, artist, AND sortname indexes together — much better + recall than strict `artist:"..."` phrase matching. Results are + filtered by score (>= 80) to drop tribute bands and unrelated + lookalikes. + """ + try: + raw = self._client.search_artist(query, limit=limit, strict=False) + artists = [] + for a in raw: + score = a.get('score', 0) or 0 + if score < self._MIN_SCORE: + continue + + mbid = a.get('id', '') + name = a.get('name', '') + if not mbid or not name: + continue + + # Genres from MB tags (user-applied categorical labels). Each + # tag has {name, count}; keep the top-weighted ones. + tags = a.get('tags', []) or [] + genres = [t.get('name') for t in tags if t.get('name')][:5] + + external_urls = { + 'musicbrainz': f'https://musicbrainz.org/artist/{mbid}' + } + + artists.append(Artist( + id=mbid, + name=name, + popularity=score, # Reuse score as popularity (0-100) + genres=genres, + followers=0, # MusicBrainz doesn't track followers + image_url=None, # MB doesn't store artist images directly + external_urls=external_urls, + )) + return artists + except Exception as e: + logger.warning(f"MusicBrainz artist search failed: {e}") + return [] def search_albums(self, query: str, limit: int = 10) -> List[Album]: """Search MusicBrainz for releases (albums).""" From d7e232e01c4c8bb0dbdd90ab1fb4079226233996 Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Fri, 24 Apr 2026 08:13:40 -0700 Subject: [PATCH 04/13] MusicBrainz: Artist-first browse for albums + tracks, keep text fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bare name queries (typing 'metallica') now resolve to an artist MBID via the fuzzy search added in the previous commit, then BROWSE that artist's release-groups and recordings instead of text-searching release/recording titles. That's the only way to fix the core garbage-results issue: MB indexes release/recording titles, not artist names, so 'recording:metallica' matches random tracks literally titled 'Metallica' (all scoring 100). Structure: - `_split_structured_query` — detects 'Artist - Title' / 'Artist – Title' / 'Artist — Title' shapes. When present, text-search is correct (user gave an explicit title to match). - `_resolve_top_artist` — memoized per-instance lookup for the top-scoring artist MBID. Backend fires artists/albums/tracks searches in parallel against one shared client instance, and albums+tracks both need the same artist lookup. Cache + lock means one HTTP call instead of three. - `_release_group_to_album` / `_recording_to_track` — shared projection helpers between the browse and text paths so both paths return the same dataclass shape. Search flow per kind: - `search_albums('metallica')` → resolve top artist → browse release-groups with `type=album|ep|single|compilation` → sort by type priority then release date desc → Album dataclasses for top N. - `search_tracks('metallica')` → resolve top artist → browse recordings with `inc=releases+artist-credits` → dedupe by normalized title (MB has many live/compilation variants of the same song) → sort by release date desc → Track dataclasses for top N. - `search_albums('foo - bar')` → structured query → text-search path (unchanged behavior, now score-filtered to 80+). - `search_tracks('foo - bar')` → same. - Both text-search paths also dedupe through `_search_albums_text` / `_search_tracks_text` helpers, which apply the 80-score filter that the artist-first path gets free from the resolver's threshold. Also dedupes text-path tracks through the new `_recording_to_track` helper, replacing ~60 lines of inline projection code. Net change is more lines overall (browse + helpers) but the text paths shrank and the garbage-results issue is fixed. Credit: kettui flagged the missing Artists section + unusable track results during PR #371 review. --- core/musicbrainz_search.py | 333 +++++++++++++++++++++++++------------ 1 file changed, 229 insertions(+), 104 deletions(-) diff --git a/core/musicbrainz_search.py b/core/musicbrainz_search.py index 017f0cdb..3e32300b 100644 --- a/core/musicbrainz_search.py +++ b/core/musicbrainz_search.py @@ -6,6 +6,7 @@ enabling MusicBrainz as a search tab in enhanced and global search. Album art is fetched from Cover Art Archive (free, linked by release MBID). """ +import threading import requests from dataclasses import dataclass from typing import Any, Dict, List, Optional @@ -121,6 +122,15 @@ class MusicBrainzSearchClient: # the exact UI minor version would add noise to every request. self._client = MusicBrainzClient("SoulSync", "2") self._art_cache: Dict[str, Optional[str]] = {} # mbid -> url + # Per-instance cache for "top artist MBID for this query". The + # backend fires artists/albums/tracks searches in parallel against + # one client instance, and albums+tracks both need the same artist + # lookup. Without this cache, we'd fire 3 identical artist-search + # HTTP calls (each serialized by the 1-rps rate limit = 3 wasted + # seconds). The _Sentinel marks "we already looked and found + # nothing" to prevent repeat no-hit lookups. + self._artist_mbid_cache: Dict[str, Optional[Dict[str, Any]]] = {} + self._artist_mbid_lock = threading.Lock() def _cached_art(self, release_mbid: str, release_group_mbid: str = '') -> Optional[str]: """Get cover art with caching. Tries release first, then release group.""" @@ -185,31 +195,114 @@ class MusicBrainzSearchClient: logger.warning(f"MusicBrainz artist search failed: {e}") return [] + def _split_structured_query(self, query: str): + """Split 'Artist - Title' / 'Artist – Title' / 'Artist — Title' if + a separator is present. Returns (artist_name, title) or (None, query).""" + for sep in [' - ', ' – ', ' — ']: + if sep in query: + parts = query.split(sep, 1) + return parts[0].strip(), parts[1].strip() + return None, query + + def _resolve_top_artist(self, query: str) -> Optional[Dict[str, Any]]: + """Return the top-scoring artist for a bare-name query, or None if + nothing scores above threshold. Cached per instance so parallel + album/track searches don't each refetch.""" + if not query: + return None + key = query.strip().lower() + with self._artist_mbid_lock: + if key in self._artist_mbid_cache: + return self._artist_mbid_cache[key] + # Do the HTTP call OUTSIDE the lock so other threads can still + # check the cache while we wait on the network. + raw = self._client.search_artist(query, limit=1, strict=False) + top = None + if raw and (raw[0].get('score', 0) or 0) >= self._MIN_SCORE: + top = raw[0] + with self._artist_mbid_lock: + self._artist_mbid_cache[key] = top + return top + + def _release_group_to_album(self, rg: Dict[str, Any], artist_name: str) -> Album: + """Project a MusicBrainz release-group into our Album dataclass.""" + rg_mbid = rg.get('id', '') + title = rg.get('title', '') or '' + primary_type = rg.get('primary-type', '') or '' + secondary_types = rg.get('secondary-types', []) or [] + album_type = _map_release_type(primary_type, secondary_types) + release_date = rg.get('first-release-date', '') or '' + # Release-group browse doesn't link directly to a single release, + # so we can't get per-release track counts cheaply. Leave 0 — the + # frontend treats it as "unknown" gracefully. + image_url = self._cached_art(rg_mbid, rg_mbid) + return Album( + id=rg_mbid, + name=title, + artists=[artist_name] if artist_name else ['Unknown Artist'], + release_date=release_date, + total_tracks=0, + album_type=album_type, + image_url=image_url, + external_urls={'musicbrainz': f'https://musicbrainz.org/release-group/{rg_mbid}'} if rg_mbid else {}, + ) + def search_albums(self, query: str, limit: int = 10) -> List[Album]: - """Search MusicBrainz for releases (albums).""" + """Search MusicBrainz for releases (albums). + + Primary path: when the query looks like a bare artist name, resolve + it to an artist MBID and BROWSE that artist's release-groups. This + returns the artist's actual discography instead of unrelated + releases that happen to be titled after them. + + Fallback path: when the query is structured as "Artist - Album" or + the artist lookup fails, drop back to text search with the + existing Lucene strategy. + """ try: - # Try to split "Artist Album" for better matching - artist_name = None - album_name = query - for sep in [' - ', ' – ', ' — ']: - if sep in query: - parts = query.split(sep, 1) - artist_name = parts[0].strip() - album_name = parts[1].strip() - break + artist_name, title = self._split_structured_query(query) + # Structured "Artist - Album" query → respect user's intent; + # text-search with both terms is more precise than browsing all + # of that artist's discography. + if artist_name: + return self._search_albums_text(title, artist_name, limit) + + # Bare name query → try artist-first → browse path. + top = self._resolve_top_artist(query) + if top: + mbid = top.get('id', '') + tname = top.get('name', '') or query + rgs = self._client.browse_artist_release_groups( + mbid, + release_types=['album', 'ep', 'single', 'compilation'], + limit=100, + ) + # Sort by first-release-date desc (newest first), then by + # primary-type priority (album > ep > single > compilation) + # so the top of the list is a credible "what to explore." + type_priority = {'album': 0, 'ep': 1, 'single': 2, 'compilation': 3} + def _sort_key(rg): + pt = (rg.get('primary-type') or '').lower() + date = rg.get('first-release-date') or '' + return (type_priority.get(pt, 9), -int(date[:4]) if date[:4].isdigit() else 0) + rgs.sort(key=_sort_key) + albums = [self._release_group_to_album(rg, tname) for rg in rgs[:limit]] + return albums + + # No artist match → text search on the whole query. + return self._search_albums_text(query, None, limit) + except Exception as e: + logger.warning(f"MusicBrainz album search failed: {e}") + return [] + + def _search_albums_text(self, album_name: str, artist_name: Optional[str], limit: int) -> List[Album]: + """Fallback text-search path for structured/fuzzy album queries.""" + try: results = self._client.search_release(album_name, artist_name=artist_name, limit=limit) - - # If no separator, try word-boundary splitting - if not results and not artist_name: - words = query.split() - for i in range(1, len(words)): - possible_artist = ' '.join(words[:i]) - possible_album = ' '.join(words[i:]) - if len(possible_album) >= 2: - results = self._client.search_release(possible_album, artist_name=possible_artist, limit=limit) - if results: - break + # Score filter — same threshold as artists. Drops garbage + # title-match hits from unrelated releases. + results = [r for r in results if (r.get('score', 0) or 0) >= self._MIN_SCORE] albums = [] for r in results: @@ -274,96 +367,128 @@ class MusicBrainzSearchClient: logger.warning(f"MusicBrainz album search failed: {e}") return [] + def _recording_to_track(self, r: Dict[str, Any], fallback_artist_name: str) -> Optional[Track]: + """Project a MusicBrainz recording into our Track dataclass. Returns + None when the recording lacks required fields.""" + mbid = r.get('id', '') + title = r.get('title', '') + if not title: + return None + + artists = _extract_artist_credit(r.get('artist-credit', [])) + if not artists and fallback_artist_name: + artists = [fallback_artist_name] + + duration_ms = r.get('length', 0) or 0 + album_name = '' + album_id = '' + release_date = '' + image_url = None + album_type = 'single' + total_tracks = 1 + + releases = r.get('releases', []) or [] + if releases: + rel = releases[0] + album_name = rel.get('title', '') or '' + album_id = rel.get('id', '') or '' + release_date = rel.get('date', '') or '' + + rg = rel.get('release-group', {}) or {} + primary_type = rg.get('primary-type', '') or '' + secondary_types = rg.get('secondary-types', []) or [] + album_type = _map_release_type(primary_type, secondary_types) + + for m in rel.get('media', []) or []: + total_tracks += m.get('track-count', 0) + + rg_mbid = rg.get('id', '') or '' + image_url = self._cached_art(album_id, rg_mbid) if album_id else None + + return Track( + id=mbid, + name=title, + artists=artists if artists else ['Unknown Artist'], + album=album_name or title, + duration_ms=duration_ms, + popularity=r.get('score', 0) or 0, + image_url=image_url, + release_date=release_date, + external_urls={'musicbrainz': f'https://musicbrainz.org/recording/{mbid}'} if mbid else {}, + album_type=album_type, + total_tracks=total_tracks, + album_id=album_id, + ) + def search_tracks(self, query: str, limit: int = 10) -> List[Track]: - """Search MusicBrainz for recordings (tracks).""" + """Search MusicBrainz for recordings (tracks). + + Same strategy as `search_albums`: bare name → artist-first → browse + recordings; structured "Artist - Title" stays on text search so the + user's explicit title intent is respected. + """ try: - # Try to split "Artist - Title" for better matching - artist_name = None - track_name = query - for sep in [' - ', ' – ', ' — ']: - if sep in query: - parts = query.split(sep, 1) - artist_name = parts[0].strip() - track_name = parts[1].strip() - break + artist_name, title = self._split_structured_query(query) + # Structured query → text search with both fields. + if artist_name: + return self._search_tracks_text(title, artist_name, limit) + + # Bare name → artist-first → browse. + 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'], + ) + # 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 + # surface first — matches how the other source tabs look. + seen = set() + deduped = [] + for r in recs: + key = (r.get('title') or '').lower().strip() + if not key or key in seen: + continue + seen.add(key) + deduped.append(r) + + def _track_sort_key(r): + rel = (r.get('releases') or [{}])[0] + date = (rel.get('date') or '')[:4] + return -int(date) if date.isdigit() else 0 + deduped.sort(key=_track_sort_key) + + tracks = [] + for r in deduped[:limit]: + t = self._recording_to_track(r, tname) + if t: + tracks.append(t) + return tracks + + # No artist match → fall back to text search on whole query. + return self._search_tracks_text(query, None, limit) + except Exception as e: + logger.warning(f"MusicBrainz track search failed: {e}") + return [] + + def _search_tracks_text(self, track_name: str, artist_name: Optional[str], limit: int) -> List[Track]: + """Fallback text-search path for structured/fuzzy track queries.""" + try: results = self._client.search_recording(track_name, artist_name=artist_name, limit=limit) + # Score filter matches the artist/album logic — cuts garbage + # title collisions from unrelated recordings. + results = [r for r in results if (r.get('score', 0) or 0) >= self._MIN_SCORE] - # If no separator found or structured search failed, try the full query - # as both a recording search and an artist+recording combined search - if not results and not artist_name: - # Try each word split as potential artist/title boundary - words = query.split() - for i in range(1, len(words)): - possible_artist = ' '.join(words[:i]) - possible_track = ' '.join(words[i:]) - if len(possible_track) >= 2: - results = self._client.search_recording(possible_track, artist_name=possible_artist, limit=limit) - if results: - break tracks = [] for r in results: - mbid = r.get('id', '') - title = r.get('title', '') - if not title: - continue - - artists = _extract_artist_credit(r.get('artist-credit', [])) - duration_ms = r.get('length', 0) or 0 - - # Get album from first release - album_name = '' - album_id = '' - release_date = '' - image_url = None - album_type = 'single' - total_tracks = 1 - track_number = None - - releases = r.get('releases', []) - if releases: - rel = releases[0] - album_name = rel.get('title', '') - album_id = rel.get('id', '') - release_date = rel.get('date', '') or '' - - rg = rel.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) - - media = rel.get('media', []) - for m in media: - total_tracks += m.get('track-count', 0) - # Find track number - for t in m.get('tracks', []): - if t.get('id') == mbid or t.get('recording', {}).get('id') == mbid: - try: - track_number = int(t.get('number', t.get('position', 0))) - except (ValueError, TypeError): - pass - - # Cover art - rg_mbid = rg.get('id', '') - image_url = self._cached_art(album_id, rg_mbid) if album_id else None - - external_urls = {'musicbrainz': f'https://musicbrainz.org/recording/{mbid}'} if mbid else {} - - tracks.append(Track( - id=mbid, - name=title, - artists=artists if artists else ['Unknown Artist'], - album=album_name or title, - duration_ms=duration_ms, - popularity=r.get('score', 0), - image_url=image_url, - release_date=release_date, - external_urls=external_urls, - track_number=track_number, - album_type=album_type, - total_tracks=total_tracks, - album_id=album_id, - )) + t = self._recording_to_track(r, artist_name or '') + if t: + tracks.append(t) return tracks except Exception as e: logger.warning(f"MusicBrainz track search failed: {e}") From 73df2951e525e68e73fee45d9ef1d2c6679a9ffe Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Fri, 24 Apr 2026 08:15:49 -0700 Subject: [PATCH 05/13] MusicBrainz: Construct Cover Art URLs instead of HEAD-probing them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cover Art Archive URLs are deterministic from the MBID: a GET either 307-redirects to the image or returns 404. The previous adapter fired `requests.head(timeout=3)` per search result to probe for the image first. 10 results × 3s worst-case = up to 30s of blocking HEAD calls before a search returned. The probe was defensive overhead — the frontend already handles 404 via `` fallback. Building the URL deterministically and letting the browser load it lazily collapses the tail latency to the real MB API calls (artist-search + browse = ~3s at the 1-rps rate limit). Also prefer release-group scope over per-release scope when both are available — release-group covers every edition of an album, so the hit rate is noticeably higher than pinning to a specific regional release. Removes now-unused `self._art_cache` and the `requests` import. --- core/musicbrainz_search.py | 55 +++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 30 deletions(-) diff --git a/core/musicbrainz_search.py b/core/musicbrainz_search.py index 3e32300b..adf24a2d 100644 --- a/core/musicbrainz_search.py +++ b/core/musicbrainz_search.py @@ -7,7 +7,6 @@ Album art is fetched from Cover Art Archive (free, linked by release MBID). """ import threading -import requests from dataclasses import dataclass from typing import Any, Dict, List, Optional @@ -60,29 +59,24 @@ class Album: external_urls: Optional[Dict[str, str]] = None -def _get_cover_art_url(release_mbid: str) -> Optional[str]: - """Fetch album art URL from Cover Art Archive. Returns None if not available.""" - try: - # CAA redirects to the actual image URL — just get the front image URL - url = f"{COVER_ART_ARCHIVE_URL}/release/{release_mbid}/front-250" - resp = requests.head(url, timeout=3, allow_redirects=True) - if resp.status_code == 200: - return resp.url # The redirect target is the actual image - return None - except Exception: - return None +def _cover_art_url(mbid: str, scope: str = 'release') -> Optional[str]: + """Build a Cover Art Archive URL without hitting the network. + CAA URLs are deterministic from the MBID: the endpoint either 307-redirects + to the image or returns 404. Previously we fired `requests.head(timeout=3)` + per result during search — 10 results × 3s worst-case = up to 30s of + blocking HEAD calls before a search returned. The frontend's tag + handles the 404 case via onerror fallback, so the HEAD round-trip was + pure overhead. -def _get_release_group_art(release_group_mbid: str) -> Optional[str]: - """Fetch album art from release group (covers all editions).""" - try: - url = f"{COVER_ART_ARCHIVE_URL}/release-group/{release_group_mbid}/front-250" - resp = requests.head(url, timeout=3, allow_redirects=True) - if resp.status_code == 200: - return resp.url - return None - except Exception: + `scope` is 'release' (most specific) or 'release-group' (covers all + editions — better hit rate). + """ + if not mbid: return None + if scope not in ('release', 'release-group'): + scope = 'release' + return f"{COVER_ART_ARCHIVE_URL}/{scope}/{mbid}/front-250" def _extract_artist_credit(artist_credit) -> List[str]: @@ -121,7 +115,6 @@ class MusicBrainzSearchClient: # which is what MusicBrainz wants. Version stays generic ("2") — # the exact UI minor version would add noise to every request. self._client = MusicBrainzClient("SoulSync", "2") - self._art_cache: Dict[str, Optional[str]] = {} # mbid -> url # Per-instance cache for "top artist MBID for this query". The # backend fires artists/albums/tracks searches in parallel against # one client instance, and albums+tracks both need the same artist @@ -133,15 +126,17 @@ class MusicBrainzSearchClient: self._artist_mbid_lock = threading.Lock() def _cached_art(self, release_mbid: str, release_group_mbid: str = '') -> Optional[str]: - """Get cover art with caching. Tries release first, then release group.""" - if release_mbid in self._art_cache: - return self._art_cache[release_mbid] + """Build a Cover Art Archive URL for a release / release-group MBID. - url = _get_cover_art_url(release_mbid) - if not url and release_group_mbid: - url = _get_release_group_art(release_group_mbid) - self._art_cache[release_mbid] = url - return url + Prefers release-group scope when provided — better hit rate because + it covers all editions of the same album. No network call; the + frontend's fallback handles 404s. + """ + preferred = release_group_mbid or release_mbid + if not preferred: + return None + scope = 'release-group' if release_group_mbid else 'release' + return _cover_art_url(preferred, scope=scope) # Score threshold for user-facing search results. MusicBrainz returns a # Lucene score 0-100 on every match; exact name/alias hits score 100, From 394ac738779de5009652b4dba79ef80eb87535fc Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Fri, 24 Apr 2026 08:18:47 -0700 Subject: [PATCH 06/13] MusicBrainz: Tests for new search behavior + WHATS_NEW entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 26 new unit tests in tests/test_musicbrainz_search.py covering: - Cover Art URL construction (release + release-group scope, empty MBID, unknown scope fallback) - Structured query splitting (hyphen, en-dash, em-dash, bare name, no false-positive splits on hyphens-inside-words) - Artist search: score filtering, strict=False call contract, exception handling, genre extraction from MB tags, mbid/name validation - Top-artist resolver: memoization by normalized query, sub-threshold returns None, negative-result caching, empty-query short-circuit - Album search routing: bare query → browse path, structured query → text path, no-artist-match falls back to text, text path score filter - Track search routing: browse path, dedupe-by-title across live/compilation variants, structured query → text path, text path score filter All mock the underlying MusicBrainzClient — no network calls. Also adds a WHATS_NEW entry under 2.40 explaining the three user-visible changes: Artists section now populates, album/track results match the searched artist instead of random title collisions, and search completes in ~3 seconds instead of 30+. --- tests/test_musicbrainz_search.py | 351 +++++++++++++++++++++++++++++++ webui/static/helper.js | 1 + 2 files changed, 352 insertions(+) create mode 100644 tests/test_musicbrainz_search.py diff --git a/tests/test_musicbrainz_search.py b/tests/test_musicbrainz_search.py new file mode 100644 index 00000000..17fac08a --- /dev/null +++ b/tests/test_musicbrainz_search.py @@ -0,0 +1,351 @@ +"""Tests for the MusicBrainz search adapter (core/musicbrainz_search.py). + +Covers the behavior changes from the search-overhaul PR: +- Artist search is re-enabled and score-filtered +- Bare name queries route through artist-first → browse +- Structured 'Artist - Title' queries stay on text search +- Top-artist resolution is memoized per instance +- Cover Art URLs are constructed, not probed +""" + +from unittest.mock import MagicMock, patch + +import pytest + +from core.musicbrainz_search import ( + MusicBrainzSearchClient, + _cover_art_url, +) + + +# --------------------------------------------------------------------------- +# Cover art URL construction +# --------------------------------------------------------------------------- + +def test_cover_art_url_release_scope(): + assert _cover_art_url('abc-123') == 'https://coverartarchive.org/release/abc-123/front-250' + + +def test_cover_art_url_release_group_scope(): + assert _cover_art_url('abc-123', scope='release-group') == \ + 'https://coverartarchive.org/release-group/abc-123/front-250' + + +def test_cover_art_url_empty_mbid_returns_none(): + assert _cover_art_url('') is None + assert _cover_art_url(None) is None + + +def test_cover_art_url_unknown_scope_falls_back_to_release(): + assert _cover_art_url('abc', scope='garbage') == 'https://coverartarchive.org/release/abc/front-250' + + +# --------------------------------------------------------------------------- +# Structured query splitting +# --------------------------------------------------------------------------- + +def test_split_structured_query_hyphen(): + client = MusicBrainzSearchClient() + assert client._split_structured_query('Metallica - Master of Puppets') == ('Metallica', 'Master of Puppets') + + +def test_split_structured_query_en_dash(): + client = MusicBrainzSearchClient() + assert client._split_structured_query('Metallica – One') == ('Metallica', 'One') + + +def test_split_structured_query_em_dash(): + client = MusicBrainzSearchClient() + assert client._split_structured_query('Metallica — Battery') == ('Metallica', 'Battery') + + +def test_split_structured_query_bare_name(): + client = MusicBrainzSearchClient() + assert client._split_structured_query('metallica') == (None, 'metallica') + + +def test_split_structured_query_no_separator_with_hyphens_in_word(): + # A hyphen inside a word (no surrounding spaces) should not split. + client = MusicBrainzSearchClient() + assert client._split_structured_query('t-pain') == (None, 't-pain') + + +# --------------------------------------------------------------------------- +# Artist search — score filtering and shape +# --------------------------------------------------------------------------- + +def _mk_artist(name, mbid, score=100, tags=None): + return { + 'id': mbid, + 'name': name, + 'score': score, + 'tags': tags or [], + } + + +def test_search_artists_filters_by_score_threshold(): + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_artist.return_value = [ + _mk_artist('Metallica', 'mb-real', score=100), + _mk_artist('Metallica Tribute', 'mb-tribute', score=60), + _mk_artist('Metallica Jam', 'mb-jam', score=58), + ] + results = client.search_artists('metallica', limit=10) + assert len(results) == 1 + assert results[0].name == 'Metallica' + assert results[0].id == 'mb-real' + + +def test_search_artists_uses_strict_false_for_fuzzy_match(): + """The adapter must use strict=False so MusicBrainz searches + alias+artist+sortname together — strict mode would miss aliased names.""" + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_artist.return_value = [] + client.search_artists('metallica') + client._client.search_artist.assert_called_once_with('metallica', limit=10, strict=False) + + +def test_search_artists_returns_empty_on_exception(): + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_artist.side_effect = RuntimeError('network down') + assert client.search_artists('metallica') == [] + + +def test_search_artists_extracts_tags_as_genres(): + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_artist.return_value = [ + _mk_artist('Metallica', 'mb-real', score=100, + tags=[{'name': 'thrash metal', 'count': 20}, + {'name': 'heavy metal', 'count': 15}]), + ] + results = client.search_artists('metallica') + assert results[0].genres == ['thrash metal', 'heavy metal'] + + +def test_search_artists_skips_entries_without_mbid_or_name(): + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_artist.return_value = [ + {'id': 'mb-1', 'name': 'Good', 'score': 100}, + {'id': '', 'name': 'Missing MBID', 'score': 100}, + {'id': 'mb-2', 'name': '', 'score': 100}, + ] + results = client.search_artists('x') + assert [r.name for r in results] == ['Good'] + + +# --------------------------------------------------------------------------- +# Top-artist resolution — memoization +# --------------------------------------------------------------------------- + +def test_resolve_top_artist_memoizes_by_normalized_query(): + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_artist.return_value = [_mk_artist('Metallica', 'mb-1', score=100)] + + first = client._resolve_top_artist('metallica') + second = client._resolve_top_artist(' Metallica ') # Whitespace / case variant + + assert first is not None + assert first['id'] == 'mb-1' + assert first is second + # HTTP call happens once despite two resolve calls. + assert client._client.search_artist.call_count == 1 + + +def test_resolve_top_artist_returns_none_below_threshold(): + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_artist.return_value = [_mk_artist('Tribute', 'mb-trib', score=50)] + assert client._resolve_top_artist('obscure') is None + + +def test_resolve_top_artist_caches_negative_result(): + """After a lookup finds no good match, subsequent calls don't refetch.""" + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_artist.return_value = [] + first = client._resolve_top_artist('nonexistent band') + second = client._resolve_top_artist('nonexistent band') + assert first is None + assert second is None + assert client._client.search_artist.call_count == 1 + + +def test_resolve_top_artist_empty_query_returns_none_without_http(): + client = MusicBrainzSearchClient() + client._client = MagicMock() + assert client._resolve_top_artist('') is None + client._client.search_artist.assert_not_called() + + +# --------------------------------------------------------------------------- +# Album search — routing +# --------------------------------------------------------------------------- + +def test_search_albums_bare_query_uses_browse_path(): + """When a bare name resolves to an artist, we browse their release-groups + instead of text-searching release titles.""" + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_artist.return_value = [_mk_artist('Metallica', 'mb-1', score=100)] + client._client.browse_artist_release_groups.return_value = [ + {'id': 'rg-1', 'title': 'Master of Puppets', 'primary-type': 'Album', + 'first-release-date': '1986-03-03', 'secondary-types': []}, + {'id': 'rg-2', 'title': 'Ride the Lightning', 'primary-type': 'Album', + 'first-release-date': '1984-07-27', 'secondary-types': []}, + ] + + albums = client.search_albums('metallica', limit=10) + + client._client.browse_artist_release_groups.assert_called_once() + # Text-search path must NOT be taken. + client._client.search_release.assert_not_called() + # Browse results come back, newest first. + assert [a.name for a in albums] == ['Master of Puppets', 'Ride the Lightning'] + assert all(a.artists == ['Metallica'] for a in albums) + + +def test_search_albums_structured_query_uses_text_path(): + """'Artist - Title' shape should text-search the title rather than + browsing all of the artist's discography.""" + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_release.return_value = [ + {'id': 'rel-1', 'title': 'Master of Puppets', 'score': 100, + 'date': '1986', 'media': [{'track-count': 8}], + 'release-group': {'id': 'rg-1', 'primary-type': 'Album'}, + 'artist-credit': [{'name': 'Metallica'}]}, + ] + + albums = client.search_albums('Metallica - Master of Puppets', limit=10) + + client._client.search_release.assert_called_once() + # Artist-first path must NOT be taken. + client._client.search_artist.assert_not_called() + client._client.browse_artist_release_groups.assert_not_called() + assert len(albums) == 1 + assert albums[0].name == 'Master of Puppets' + + +def test_search_albums_falls_back_to_text_when_no_artist_match(): + """No artist above threshold → text-search the whole query.""" + client = MusicBrainzSearchClient() + client._client = MagicMock() + # Artist lookup returns nothing above threshold. + client._client.search_artist.return_value = [_mk_artist('X', 'mb-x', score=40)] + client._client.search_release.return_value = [] + + client.search_albums('very obscure band') + + client._client.search_release.assert_called_once_with('very obscure band', artist_name=None, limit=10) + client._client.browse_artist_release_groups.assert_not_called() + + +def test_search_albums_text_path_filters_by_score(): + client = MusicBrainzSearchClient() + client._client = MagicMock() + # Force text-search path by using a structured query. + client._client.search_release.return_value = [ + {'id': 'rel-good', 'title': 'Good', 'score': 95, + 'release-group': {'id': 'rg-1', 'primary-type': 'Album'}, + 'artist-credit': [{'name': 'Foo'}]}, + {'id': 'rel-bad', 'title': 'Bad', 'score': 40, + 'release-group': {'id': 'rg-2', 'primary-type': 'Album'}, + 'artist-credit': [{'name': 'Foo'}]}, + ] + + albums = client.search_albums('Foo - Good', limit=10) + + titles = [a.name for a in albums] + assert 'Good' in titles + assert 'Bad' not in titles + + +# --------------------------------------------------------------------------- +# Track search — routing +# --------------------------------------------------------------------------- + +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 = [ + {'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'}}], + 'artist-credit': [{'name': 'Metallica'}]}, + {'id': 'rec-2', 'title': 'Battery', 'length': 312000, + 'releases': [{'id': 'rel-2', 'title': 'Master of Puppets', 'date': '1986', + 'release-group': {'id': 'rg-2', 'primary-type': 'Album'}}], + 'artist-credit': [{'name': 'Metallica'}]}, + ] + + tracks = client.search_tracks('metallica', limit=10) + + client._client.browse_artist_recordings.assert_called_once() + client._client.search_recording.assert_not_called() + assert len(tracks) == 2 + assert {t.name for t in tracks} == {'One', 'Battery'} + + +def test_search_tracks_dedupes_by_title(): + """MusicBrainz has many live/compilation variants of the same song. + Browse results should be deduped by normalized title so we don't show + 'One' three times.""" + 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 = [ + {'id': 'rec-1', 'title': 'One', 'length': 446000, + 'releases': [{'id': 'rel-1', 'title': '...And Justice for All', 'date': '1988'}], + 'artist-credit': [{'name': 'Metallica'}]}, + {'id': 'rec-1-live', 'title': 'One', 'length': 490000, + 'releases': [{'id': 'rel-live', 'title': 'Live Shit', 'date': '1993'}], + 'artist-credit': [{'name': 'Metallica'}]}, + ] + + tracks = client.search_tracks('metallica', limit=10) + + assert len(tracks) == 1 + assert tracks[0].name == 'One' + + +def test_search_tracks_structured_query_uses_text_path(): + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_recording.return_value = [ + {'id': 'rec-1', 'title': 'One', 'score': 100, + 'releases': [{'id': 'rel-1', 'title': '...And Justice for All', 'date': '1988'}], + 'artist-credit': [{'name': 'Metallica'}]}, + ] + + tracks = client.search_tracks('Metallica - One', limit=10) + + client._client.search_recording.assert_called_once() + client._client.search_artist.assert_not_called() + client._client.browse_artist_recordings.assert_not_called() + assert len(tracks) == 1 + + +def test_search_tracks_text_path_filters_by_score(): + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_recording.return_value = [ + {'id': 'rec-good', 'title': 'Good', 'score': 95, + 'releases': [{'id': 'rel-1', 'title': 'X', 'date': '2020'}], + 'artist-credit': [{'name': 'Foo'}]}, + {'id': 'rec-bad', 'title': 'Bad', 'score': 40, + 'releases': [{'id': 'rel-2', 'title': 'Y', 'date': '2021'}], + 'artist-credit': [{'name': 'Foo'}]}, + ] + + tracks = client.search_tracks('Foo - Good', limit=10) + + titles = [t.name for t in tracks] + assert 'Good' in titles + assert 'Bad' not in titles diff --git a/webui/static/helper.js b/webui/static/helper.js index 8a13ad18..6f1c5380 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -3462,6 +3462,7 @@ const WHATS_NEW = { { title: 'Stale Search Requests No Longer Flash Empty Results on Fast Retype', desc: 'Cin flagged a race in createSearchController: when you typed a query then quickly re-typed before the first fetch returned, the first fetch\'s catch block (firing on AbortError after the second submitQuery aborted it) cleared loadingSources and notified the UI, causing a brief flash of empty/error state while the new query\'s fetch was still mid-flight. Added a monotonic _requestSeq token — each fetch captures the next value, and stale completions bail before mutating shared state. The controller still aborts in-flight fetches on supersession; this just keeps the abort-cleanup of the old request from clobbering the new one\'s spinner', page: 'search', unreleased: true }, { title: 'Source Picker Dims Soulseek When slskd Isn\'t Configured', desc: 'Cin pointed out that the Soulseek icon was always rendered as configured, so users without slskd set up could click it and fire searches that would never succeed. Soulseek is now in the backend config-status registry as `required: [slskd_url]` and removed from the frontend\'s always-configured set. Without slskd, the icon dims and clicking it routes to Settings → Downloads tab (where the slskd URL field lives, gated behind the download-source dropdown) instead of Settings → Connections', page: 'search', unreleased: true }, { title: 'Fix Discover Hero "View Discography" 404ing on Source Artists', desc: 'Clicking "View Discography" on the Discover page hero slideshow was calling navigateToArtistDetail without a source, so /api/artist-detail defaulted to a library lookup and returned 404 for artists that don\'t exist in your library (which is nearly every hero artist — they come from discover similar-artists, not the library). Regression from the unification PR that rewrote the click handler to route to /artist-detail but forgot to pass the source. Backend already sends artist.source on each hero entry; we now stash it as data-source on the discography button and thread it through to navigateToArtistDetail so the API call includes source=itunes/deezer/etc. and returns the synthesized discography', page: 'discover', unreleased: true }, + { title: 'MusicBrainz Search Actually Works Now', desc: 'kettui flagged during PR #371 review that the MusicBrainz source tab never returned artists and served garbage tracks/albums. Three things were wrong. First, the artist search was hardcoded to return an empty list — re-enabled with a proper fuzzy query (bare Lucene string against alias/artist/sortname indexes) and score-filtered at 80+ to drop tribute bands. Second, track and album searches used text-search on recording/release TITLES — so typing "metallica" matched random tracks literally named "Metallica" (all scoring 100 because they\'re exact title hits). Now a bare name query resolves to the top-scoring artist, then BROWSES that artist\'s release-groups and recordings directly — the same pattern Plex uses. Structured "Artist - Title" queries still take the text-search path since the user gave an explicit title to match. Third, the adapter was firing synchronous Cover Art Archive HEAD requests (up to 30s of blocking probes per search) — replaced with deterministic URL construction so the browser loads images lazily with fallback. Search completes in ~3 seconds instead of 30+ on cold cache. Also shipped: project URL in User-Agent per MB\'s rate-limit policy recommendations', page: 'search', unreleased: true }, ], '2.39': [ // --- April 22, 2026 --- 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 07/13] 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 From ddbcdfe73a0864bc7d4b59a51544fc910058481c Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Fri, 24 Apr 2026 08:32:05 -0700 Subject: [PATCH 08/13] MusicBrainz: Filter live/compilation bootlegs + chronological sort MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three related fixes to make album/track results look like a real artist discography instead of a firehose of fan-compiled bootlegs. 1. Drop 'compilation' from the release-group browse primary-type filter. MB's OR filter (`type=album|ep|single|compilation`) silently breaks when 'compilation' is included — Metallica drops from 1076 matches to 82 because `compilation` is a SECONDARY type on MB, not a primary type. The invalid value corrupts the filter for all types, not just itself. Now we request `type=album|ep|single` which returns the full 1076; actual compilations (primary=Album + secondary=[Compilation]) are filtered out by the studio-preference logic below. 2. Filter release-groups with non-studio secondary-types (Live/Compilation/Soundtrack/Remix/Demo/Mixtape/Interview/Audiobook/ Audio drama). For Metallica, the first 100 browse results are 12 studio albums + 83 live bootlegs + 5 compilations — without this filter the Albums section was dominated by 2019-2021 broadcast recordings. Falls back to the unfiltered list if filtering leaves the result set empty (covers live-only niche artists). 3. Sort chronologically ASC by first-release-date. Wikipedia-style discography ordering — debut album on top, then chronological. Previous DESC sort put the most recent release on top which, for prolific artists, meant 2020s material before their classics. Track side of the same fix: - Re-orders each recording's `releases` array to put studio releases first before `_recording_to_track` picks up the first release for album context. Without this, MB's arbitrary release order often buried the canonical studio album under random live bootlegs. - Filters out recordings that only exist on live/compilation release- groups (keeps the ones with at least one studio release). Falls back to the full set if the artist has no studio recordings at all. - Sorts recordings by earliest studio-release year ASC so classic tracks surface first. Smoke test against live MB API confirmed: - Artists: [Metallica score=100] - Albums: Kill 'Em All (1983) → Ride the Lightning → Master of Puppets → ...And Justice for All → Metallica (Black Album) → Load → Reload → St. Anger → Death Magnetic → Lulu (2011) - Tracks: real Metallica recordings (Killing Time, Nothing Else Matters, Creeping Death, etc.) — a few remastered demos still leak in where MB metadata quality is thin, but the bulk is correct. - Total latency: 3.5 seconds. 4 new tests covering the studio filter, live-only fallback, preferred release ordering, and live-only recording exclusion. Credit: kettui flagged the poor MB results during PR #371 review. --- core/musicbrainz_search.py | 110 ++++++++++++++++++++++++++---- tests/test_musicbrainz_search.py | 112 ++++++++++++++++++++++++++++++- 2 files changed, 208 insertions(+), 14 deletions(-) diff --git a/core/musicbrainz_search.py b/core/musicbrainz_search.py index 7ff95e7a..8af8bd38 100644 --- a/core/musicbrainz_search.py +++ b/core/musicbrainz_search.py @@ -219,6 +219,41 @@ class MusicBrainzSearchClient: self._artist_mbid_cache[key] = top return top + # Secondary-type tags on MB release-groups that indicate NOT a studio + # release. Used by both the album browse (filter out) and the track + # browse (prefer studio release for album context). + _NON_STUDIO_SECONDARY_TYPES = { + 'Live', 'Compilation', 'Soundtrack', 'Remix', 'Demo', + 'Mixtape/Street', 'Interview', 'Audiobook', 'Audio drama', + } + + def _release_preference_key(self, rel: Dict[str, Any]): + """Sort key: studio releases first, then by date ASC. + + Recordings in MB often have 10+ releases (studio album, live, best-of, + reissues, anniversary editions). The first one in the API response is + arbitrary — it's often a recent live bootleg because MB users add new + live recordings all the time. Re-sorting before `_recording_to_track` + reads the first release means tracks show their canonical studio + album, not a random live compilation. + """ + rg = rel.get('release-group') or {} + secs = set(rg.get('secondary-types') or []) + is_studio = 0 if not (secs & self._NON_STUDIO_SECONDARY_TYPES) else 1 + date = (rel.get('date') or '')[:4] + year = int(date) if date.isdigit() else 9999 + return (is_studio, year) + + def _has_studio_release(self, recording: Dict[str, Any]) -> bool: + """True when at least one of the recording's releases is on a + release-group with no non-studio secondary type.""" + for rel in (recording.get('releases') or []): + rg = rel.get('release-group') or {} + secs = set(rg.get('secondary-types') or []) + if not (secs & self._NON_STUDIO_SECONDARY_TYPES): + return True + return False + def _release_group_to_album(self, rg: Dict[str, Any], artist_name: str) -> Album: """Project a MusicBrainz release-group into our Album dataclass.""" rg_mbid = rg.get('id', '') @@ -270,17 +305,42 @@ class MusicBrainzSearchClient: tname = top.get('name', '') or query rgs = self._client.browse_artist_release_groups( mbid, - release_types=['album', 'ep', 'single', 'compilation'], + # 'compilation' is a SECONDARY type, not a primary type + # — including it in the OR filter causes MB to return + # only 82 matches instead of the actual 1076 because + # the filter silently breaks. Actual compilations + # (primary-type=Album with secondary-types=[Compilation]) + # are handled by the studio-preference filter below. + release_types=['album', 'ep', 'single'], limit=100, ) - # Sort by first-release-date desc (newest first), then by - # primary-type priority (album > ep > single > compilation) - # so the top of the list is a credible "what to explore." + + # Prefer studio releases — MusicBrainz tags live bootlegs + # and best-of compilations with secondary-types. For mega- + # artists like Metallica, 83 of 100 browse results are live + # broadcast bootlegs; the 12 studio albums are buried. A + # release-group with no secondary-types (or an explicit + # studio-only type) is the "original studio" shape users + # expect to see first. + def _is_studio(rg): + secs = set((rg.get('secondary-types') or [])) + return not (secs & {'Live', 'Compilation', 'Soundtrack', + 'Remix', 'Demo', 'Mixtape/Street', + 'Interview', 'Audiobook', 'Audio drama'}) + studio = [rg for rg in rgs if _is_studio(rg)] + # If filtering leaves us empty (niche live-only artist), + # fall back to the unfiltered list — better than no results. + rgs = studio or rgs + + # Sort by primary-type priority first (album > ep > single > + # compilation), then chronologically ASC — the standard way + # discographies are listed ("their debut was X, then Y, then Z"). type_priority = {'album': 0, 'ep': 1, 'single': 2, 'compilation': 3} def _sort_key(rg): pt = (rg.get('primary-type') or '').lower() date = rg.get('first-release-date') or '' - return (type_priority.get(pt, 9), -int(date[:4]) if date[:4].isdigit() else 0) + year = int(date[:4]) if date[:4].isdigit() else 9999 + return (type_priority.get(pt, 9), year) rgs.sort(key=_sort_key) albums = [self._release_group_to_album(rg, tname) for rg in rgs[:limit]] return albums @@ -438,10 +498,29 @@ class MusicBrainzSearchClient: # 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 - # surface first — matches how the other source tabs look. + + # Re-order each recording's releases to prefer studio over + # live/compilation. Without this, the first release (which + # the adapter uses for album info + date) is often a random + # live bootleg — Metallica has 10+ live versions of "One" + # ranked ahead of the studio release. Mutates in place so + # `_recording_to_track` sees the preferred release first. + for r in recs: + rels = r.get('releases') or [] + if not rels: + continue + rels.sort(key=self._release_preference_key) + r['releases'] = rels + + # Prefer recordings that have at least one studio release. + # Falls back to the full set if the artist is live-only. + studio = [r for r in recs if self._has_studio_release(r)] + recs = studio or recs + + # Dedupe by normalized title (MB has many versions of the + # same song — live, remaster, re-recording, etc.). Because + # we sorted releases above, `_recording_to_track` will pick + # the studio release for album info on the first keeper. seen = set() deduped = [] for r in recs: @@ -451,10 +530,17 @@ class MusicBrainzSearchClient: seen.add(key) deduped.append(r) + # Sort by studio-release year ASC so classic tracks surface + # first. For a user typing "metallica", this means "Seek + # and Destroy" (1983) before "Atlas, Rise!" (2016) — which + # matches how most discography views order by release. def _track_sort_key(r): - rel = (r.get('releases') or [{}])[0] - date = (rel.get('date') or '')[:4] - return -int(date) if date.isdigit() else 0 + rels = r.get('releases') or [] + for rel in rels: + date = (rel.get('date') or '')[:4] + if date.isdigit(): + return int(date) + return 9999 deduped.sort(key=_track_sort_key) tracks = [] diff --git a/tests/test_musicbrainz_search.py b/tests/test_musicbrainz_search.py index db75b306..88afe15f 100644 --- a/tests/test_musicbrainz_search.py +++ b/tests/test_musicbrainz_search.py @@ -205,8 +205,9 @@ def test_search_albums_bare_query_uses_browse_path(): client._client.browse_artist_release_groups.assert_called_once() # Text-search path must NOT be taken. client._client.search_release.assert_not_called() - # Browse results come back, newest first. - assert [a.name for a in albums] == ['Master of Puppets', 'Ride the Lightning'] + # Chronological ASC — debut first, so the album list reads like a + # standard discography (Wikipedia-style: earliest release on top). + assert [a.name for a in albums] == ['Ride the Lightning', 'Master of Puppets'] assert all(a.artists == ['Metallica'] for a in albums) @@ -246,6 +247,113 @@ def test_search_albums_falls_back_to_text_when_no_artist_match(): client._client.browse_artist_release_groups.assert_not_called() +def test_search_albums_filters_live_and_compilation_secondary_types(): + """Mega-artists' browse results are dominated by live bootlegs and + best-of compilations — they should be filtered out so the studio + discography surfaces.""" + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_artist.return_value = [_mk_artist('Metallica', 'mb-1', score=100)] + client._client.browse_artist_release_groups.return_value = [ + {'id': 'rg-live-1', 'title': 'Live Bootleg 2019', 'primary-type': 'Album', + 'first-release-date': '2019-01-01', 'secondary-types': ['Live']}, + {'id': 'rg-studio-1', 'title': 'Kill Em All', 'primary-type': 'Album', + 'first-release-date': '1983-07-25', 'secondary-types': []}, + {'id': 'rg-comp-1', 'title': 'Greatest Hits', 'primary-type': 'Album', + 'first-release-date': '2010-01-01', 'secondary-types': ['Compilation']}, + {'id': 'rg-studio-2', 'title': 'Master of Puppets', 'primary-type': 'Album', + 'first-release-date': '1986-03-03', 'secondary-types': []}, + ] + + albums = client.search_albums('metallica', limit=10) + + titles = [a.name for a in albums] + assert titles == ['Kill Em All', 'Master of Puppets'] + assert 'Live Bootleg 2019' not in titles + assert 'Greatest Hits' not in titles + + +def test_search_albums_falls_back_to_all_when_no_studio(): + """Niche live-only artist: if no studio releases exist, show live ones + rather than returning empty.""" + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_artist.return_value = [_mk_artist('LiveBand', 'mb-1', score=100)] + client._client.browse_artist_release_groups.return_value = [ + {'id': 'rg-live-1', 'title': 'Live at X', 'primary-type': 'Album', + 'first-release-date': '2019-01-01', 'secondary-types': ['Live']}, + {'id': 'rg-live-2', 'title': 'Live at Y', 'primary-type': 'Album', + 'first-release-date': '2020-01-01', 'secondary-types': ['Live']}, + ] + + albums = client.search_albums('liveband', limit=10) + + assert len(albums) == 2 + + +def test_search_tracks_prefers_studio_release_in_album_field(): + """When a recording has both a studio release and a live release, the + Track.album should reflect the studio release (canonical album), + regardless of the order MB returned them in.""" + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_artist.return_value = [_mk_artist('Metallica', 'mb-1', score=100)] + client._client.search_recordings_by_artist_mbid.return_value = [ + { + 'id': 'rec-master', + 'title': 'Master of Puppets', + 'length': 516000, + 'artist-credit': [{'name': 'Metallica'}], + # Live release first (what MB often returns), studio second. + 'releases': [ + {'id': 'rel-live', 'title': 'Live Bootleg', 'date': '2023-01-01', + 'release-group': {'id': 'rg-live', 'primary-type': 'Album', + 'secondary-types': ['Live']}}, + {'id': 'rel-studio', 'title': 'Master of Puppets', 'date': '1986-03-03', + 'release-group': {'id': 'rg-studio', 'primary-type': 'Album', + 'secondary-types': []}}, + ], + }, + ] + + tracks = client.search_tracks('metallica', limit=10) + + assert len(tracks) == 1 + # Album must be the studio release, not the live bootleg. + assert tracks[0].album == 'Master of Puppets' + assert tracks[0].release_date == '1986-03-03' + + +def test_search_tracks_filters_recordings_without_studio_releases(): + """A recording that only exists on live/compilation releases should be + dropped when we have studio alternatives.""" + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_artist.return_value = [_mk_artist('Metallica', 'mb-1', score=100)] + client._client.search_recordings_by_artist_mbid.return_value = [ + {'id': 'rec-studio', 'title': 'Seek and Destroy', 'length': 390000, + 'artist-credit': [{'name': 'Metallica'}], + 'releases': [ + {'id': 'rel-studio', 'title': 'Kill Em All', 'date': '1983-07-25', + 'release-group': {'id': 'rg-studio', 'primary-type': 'Album', + 'secondary-types': []}}, + ]}, + {'id': 'rec-live-only', 'title': 'Fight Fire With Fire', 'length': 450000, + 'artist-credit': [{'name': 'Metallica'}], + 'releases': [ + {'id': 'rel-live', 'title': 'Live Shit', 'date': '1993-01-01', + 'release-group': {'id': 'rg-live', 'primary-type': 'Album', + 'secondary-types': ['Live']}}, + ]}, + ] + + tracks = client.search_tracks('metallica', limit=10) + + titles = [t.name for t in tracks] + assert 'Seek and Destroy' in titles + assert 'Fight Fire With Fire' not in titles + + def test_search_albums_text_path_filters_by_score(): client = MusicBrainzSearchClient() client._client = MagicMock() From 2b7d6c8c7c0bc448e00d7d36e4f362aa67dd5bb9 Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Fri, 24 Apr 2026 08:34:03 -0700 Subject: [PATCH 09/13] Fix global search popover not scrolling when results overflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The source-picker refactor introduced a new stable DOM structure inside `#gsearch-results`:
But the companion CSS never landed. `#gsearch-body` had default block layout, so when results exceeded the 60vh cap, they clipped silently instead of scrolling. The old structure had `.gsearch-results-body` with `overflow-y: auto; flex: 1` directly inside the panel; that rule still exists but its selector now matches a nested div with no flex parent, so `flex: 1` is a no-op and overflow doesn't trigger. Fix: give the three stable children the right flex behaviour so the body fills remaining space and scrolls. - `#gsearch-source-row` and `#gsearch-fallback-banner` stay at natural height (flex-shrink: 0). - `#gsearch-body` grows (flex: 1 1 auto), can shrink below content height (min-height: 0 — this is the critical bit, otherwise flex items won't shrink below their intrinsic size and overflow never triggers), and scrolls vertically. Styled scrollbar matches the rest of the panel (4px, translucent thumb). --- webui/static/style.css | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/webui/static/style.css b/webui/static/style.css index fedf54cd..7c7e781a 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -5466,6 +5466,22 @@ body.helper-mode-active #dashboard-activity-feed:hover { } .gsearch-results.visible { display: flex; animation: gsearchSlideUp 0.2s ease; } +/* Stable results-panel structure built by downloads.js _doInit: the source + row and fallback banner keep their natural height; #gsearch-body grows + to fill what's left of the 60vh cap and scrolls when content overflows. + Without the flex:1 + overflow on #gsearch-body, content past the cap + was clipped with no scrollbar — the source-picker refactor introduced + the structure but not the accompanying CSS. */ +#gsearch-source-row { flex-shrink: 0; } +#gsearch-fallback-banner { flex-shrink: 0; } +#gsearch-body { + flex: 1 1 auto; + min-height: 0; + overflow-y: auto; +} +#gsearch-body::-webkit-scrollbar { width: 4px; } +#gsearch-body::-webkit-scrollbar-thumb { background: rgba(255,255,255,0.08); border-radius: 2px; } + @keyframes gsearchSlideUp { from { opacity: 0; transform: translateX(-50%) translateY(10px); } to { opacity: 1; transform: translateX(-50%) translateY(0); } From a6359a26903652c26631a99161a8ae16b24b00db Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Fri, 24 Apr 2026 08:41:07 -0700 Subject: [PATCH 10/13] Add fallbacks for search result images MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-audit catch: my earlier cover-art commit claimed 'the frontend's fallback handles 404s' — that was wrong. The enhanced search result images in shared-helpers.js renderCompactSection and all five gsearch-item/track templates in downloads.js render bare `` with no fallback. With the MusicBrainz adapter now emitting Cover Art Archive URLs deterministically (no HEAD probe), albums that don't have cover art would show the browser's broken-image icon instead of the emoji placeholder. Two fallback shapes: - shared-helpers.js renderCompactSection: the `` sits inside a card with a sibling placeholder pattern. On error, replace the img's outerHTML with the placeholder div, matching the shape used when config.image is missing entirely. - downloads.js gsearch items: the `` sits inside a `.gsearch-item-art` div whose default text content is the emoji fallback (🎤 / 💿 / 🎶 / 🎵). On error, set parentElement.textContent to the emoji, which wipes the img and shows the glyph. Same shape as the "no image_url" branch. Applies to every card type that renders a user-provided image URL so the fix covers all sources that might return 404s — MB is the most common offender but iTunes/Deezer/Discogs can all miss too. Tested against the live MB API: Metallica albums without CAA cover art now show the 💿 emoji instead of a broken-image icon. --- webui/static/downloads.js | 10 +++++----- webui/static/shared-helpers.js | 10 ++++++++-- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/webui/static/downloads.js b/webui/static/downloads.js index ce20e659..0f89ed7d 100644 --- a/webui/static/downloads.js +++ b/webui/static/downloads.js @@ -5333,13 +5333,13 @@ function _gsRenderFromState(state) { if (dbArtists.length) { h += '
📚 In Your Library
'; - h += dbArtists.map(a => `
${a.image_url ? `` : '🎤'}
${_escToast(a.name)}
Library
`).join(''); + h += dbArtists.map(a => `
${a.image_url ? `` : '🎤'}
${_escToast(a.name)}
Library
`).join(''); h += '
'; } if (artists.length) { h += `
🎤 Artists ${srcLabel}
`; - h += artists.map(a => `
${a.image_url ? `` : '🎤'}
${_escToast(a.name)}
`).join(''); + h += artists.map(a => `
${a.image_url ? `` : '🎤'}
${_escToast(a.name)}
`).join(''); h += '
'; } @@ -5349,7 +5349,7 @@ function _gsRenderFromState(state) { const ar = a.artist || (a.artists ? a.artists.join(', ') : ''); const yr = a.release_date ? a.release_date.substring(0, 4) : ''; const img = (a.image_url || '').replace(/'/g, "\\'"); - return `
${a.image_url ? `` : '💿'}
${_escToast(a.name)}
${_escToast(ar)}${yr ? ` · ${yr}` : ''}
`; + return `
${a.image_url ? `` : '💿'}
${_escToast(a.name)}
${_escToast(ar)}${yr ? ` · ${yr}` : ''}
`; }).join(''); h += '
'; } @@ -5359,7 +5359,7 @@ function _gsRenderFromState(state) { h += singles.map(a => { const ar = a.artist || (a.artists ? a.artists.join(', ') : ''); const img = (a.image_url || '').replace(/'/g, "\\'"); - return `
${a.image_url ? `` : '🎶'}
${_escToast(a.name)}
${_escToast(ar)}
`; + return `
${a.image_url ? `` : '🎶'}
${_escToast(a.name)}
${_escToast(ar)}
`; }).join(''); h += '
'; } @@ -5369,7 +5369,7 @@ function _gsRenderFromState(state) { h += tracks.map(t => { const ar = t.artist || (t.artists ? t.artists.join(', ') : ''); const dur = t.duration_ms ? `${Math.floor(t.duration_ms / 60000)}:${String(Math.floor((t.duration_ms % 60000) / 1000)).padStart(2, '0')}` : ''; - return `
${t.image_url ? `` : '🎵'}
${_escToast(t.name)}
${_escToast(ar)}${t.album ? ` · ${_escToast(t.album)}` : ''}
${dur}
`; + return `
${t.image_url ? `` : '🎵'}
${_escToast(t.name)}
${_escToast(ar)}${t.album ? ` · ${_escToast(t.album)}` : ''}
${dur}
`; }).join(''); h += '
'; } diff --git a/webui/static/shared-helpers.js b/webui/static/shared-helpers.js index 660ef01c..ce65a412 100644 --- a/webui/static/shared-helpers.js +++ b/webui/static/shared-helpers.js @@ -559,9 +559,15 @@ function renderCompactSection(sectionId, listId, countId, items, mapItem) { placeholderClass += ' track-placeholder'; } + // Fallback placeholder used when the image 404s (common for MB + // Cover Art Archive URLs — we construct them deterministically + // without probing first, so some will miss). Without onerror the + // browser shows its broken-image icon. + const placeholderHtml = `
${config.placeholder}
`; + const escapedFallback = placeholderHtml.replace(/"/g, '"'); const imageHtml = config.image - ? `${escapeHtml(config.name)}` - : `
${config.placeholder}
`; + ? `${escapeHtml(config.name)}` + : placeholderHtml; const badgeHtml = config.badge ? `
${config.badge.text}
` 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 11/13] 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() From b3722449fcf8915666ba858b3a320cc250cc8540 Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Fri, 24 Apr 2026 10:17:59 -0700 Subject: [PATCH 12/13] MusicBrainz: Fix artist images, total_tracks off-by-one, and Artist+Title queries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three bugs from kettui's follow-up review pass on the MusicBrainz search PR, all fixed in one commit because they share UI context. 1. Missing artist images on MB artist results MusicBrainz doesn't store artist images directly. My earlier commit returned `image_url=None` on every artist result and trusted the frontend's lazy-loader — but the lazy-loader's `/api/artist//image? source=musicbrainz` endpoint had no handler for MusicBrainz, so it silently returned None and the emoji placeholder stayed. Fix plumbs the artist name through: - `renderCompactSection` stashes `data-artist-name` on artist cards. - `search.js` and `downloads.js` lazy-loaders pass `name=` as a query param. - `/api/artist//image` accepts an optional `name` param. - `metadata_service.get_artist_image_url` has a new `musicbrainz` branch: since MB has no artist art, it searches fallback sources (iTunes/Deezer by configured priority) for the artist name and returns the first image found. Verified live — Metallica/Kendrick Lamar/Daft Punk all resolve to Deezer artist images via the name lookup. 2. total_tracks off-by-one on tracks with a release `_recording_to_track` initialized `total_tracks = 1` and then summed media track-counts on top. For an 11-track album, it reported 12. An adapter-level regression introduced when the recording-projection helper was extracted during the main MB refactor. Fix: initialize at 0, sum normally. Standalone recordings with no release (can happen for uncredited remixes etc.) still report 1 via an explicit fallback — so the existing "single track" case isn't broken. 3. "Artist Album Title" queries buried specific albums in the discography list Bare-name queries like "The Beatles Abbey Road" used to resolve "The Beatles" as the artist and then browse their full discography — Abbey Road was buried alphabetically among 200+ releases instead of being the top result. Fix adds a title-hint extractor. When the query starts with the resolved artist name followed by more words, the trailing portion is treated as a title hint. Browse results are filtered to those whose release-group title contains the hint. If the filter matches nothing, falls back to text-search with the hint as the title (the "keep the old split-by-whitespace fallback" path kettui called for). If text- search also misses, shows the full discography rather than nothing. 10 new tests in tests/test_musicbrainz_search.py (46 total): - Title-hint extractor: basic match, case-insensitive, whitespace tolerance, bare-artist-no-hint, artist-not-prefix-no-hint, word- boundary required (no false splits on "Metallicasomething"). - Browse filtering by title hint. - Text-search fallback when the title hint matches nothing in browse. - Bare-artist queries return the full discography unfiltered. - total_tracks for single-release, multi-disc, and no-release cases. --- core/metadata_service.py | 51 +++++++++- core/musicbrainz_search.py | 59 +++++++++++- tests/test_musicbrainz_search.py | 159 +++++++++++++++++++++++++++++++ web_server.py | 5 + webui/static/downloads.js | 10 +- webui/static/helper.js | 1 + webui/static/search.js | 13 ++- webui/static/shared-helpers.js | 5 + 8 files changed, 295 insertions(+), 8 deletions(-) diff --git a/core/metadata_service.py b/core/metadata_service.py index 15387201..e2636d0f 100644 --- a/core/metadata_service.py +++ b/core/metadata_service.py @@ -1783,8 +1783,14 @@ def get_artist_image_url( artist_id: str, source_override: Optional[str] = None, plugin: Optional[str] = None, + artist_name: Optional[str] = None, ) -> Optional[str]: - """Resolve an artist image URL using the configured source priority.""" + """Resolve an artist image URL using the configured source priority. + + `artist_name` is used when the source-of-record doesn't store artist + images (MusicBrainz) — the resolver then searches fallback sources + (iTunes/Deezer) by name for a matching artist and returns their image. + """ if not artist_id: return None @@ -1801,6 +1807,14 @@ def get_artist_image_url( return _get_artist_image_from_source('itunes', artist_id) return None + # MusicBrainz doesn't store artist images directly — use the artist + # name (passed by the frontend) to look up the image on a fallback + # source that does. Without a name we can't resolve. + if source_override == 'musicbrainz': + if not artist_name: + return None + return _lookup_artist_image_by_name(artist_name) + if source_override: return _get_artist_image_from_source(source_override, artist_id) @@ -1812,6 +1826,41 @@ def get_artist_image_url( return None +def _lookup_artist_image_by_name(name: str) -> Optional[str]: + """Look up an artist image by NAME (not MBID) across fallback sources. + Used when the primary source doesn't store artist images (MusicBrainz). + + Tries configured sources in priority order, searches each for the + artist name, and returns the first matching result's image URL. + """ + name = (name or '').strip() + if not name: + return None + + # Skip sources that don't do artist-name search or don't have images. + _SKIP_SOURCES = {'musicbrainz', 'soulseek', 'youtube_videos', 'hydrabase'} + for source in get_source_priority(get_primary_source()): + if source in _SKIP_SOURCES: + continue + client = get_client_for_source(source) + if not client or not hasattr(client, 'search_artists'): + continue + try: + results = client.search_artists(name, limit=1) or [] + if results: + top = results[0] + img = getattr(top, 'image_url', None) or ( + top.get('image_url') if isinstance(top, dict) else None + ) + if img: + return img + except Exception as exc: + logger.debug("Artist image lookup by name failed on %s for %r: %s", + source, name, exc) + continue + return None + + def get_deezer_client(): """Get cached Deezer client. diff --git a/core/musicbrainz_search.py b/core/musicbrainz_search.py index f7b8370a..c396662b 100644 --- a/core/musicbrainz_search.py +++ b/core/musicbrainz_search.py @@ -92,6 +92,28 @@ def _extract_artist_credit(artist_credit) -> List[str]: return [n for n in names if n] +def _extract_title_hint(query: str, artist_name: str) -> Optional[str]: + """If `query` starts with `artist_name` followed by more words, return + the trailing portion. Used to pick out the album/track title the user + typed after the artist name (e.g. "The Beatles Abbey Road" → "Abbey + Road"). Returns None when the query is just the artist name. + + Case-insensitive prefix match on whitespace-normalized versions of + both strings, so "the beatles abbey road" → "abbey road" and + "The Beatles" → None. + """ + if not query or not artist_name: + return None + q_norm = ' '.join(query.split()).lower() + a_norm = ' '.join(artist_name.split()).lower() + if q_norm == a_norm: + return None + # Require a word boundary between the artist name and the trailing bit. + if q_norm.startswith(a_norm + ' '): + return query[len(artist_name):].strip() or None + return None + + def _map_release_type(primary_type: str, secondary_types: List[str] = None) -> str: """Map MusicBrainz release group type to standard album_type.""" pt = (primary_type or '').lower() @@ -303,6 +325,13 @@ class MusicBrainzSearchClient: if top: mbid = top.get('id', '') tname = top.get('name', '') or query + # If the query has words beyond the artist name (e.g. "The + # Beatles Abbey Road"), extract the leftover as a title hint. + # We'll use it below to narrow browse results to the specific + # album the user typed rather than dumping the full back + # catalogue. kettui flagged the regression — bare-name browse + # was burying a specific-album query inside a discography list. + title_hint = _extract_title_hint(query, tname) rgs = self._client.browse_artist_release_groups( mbid, # 'compilation' is a SECONDARY type, not a primary type @@ -332,6 +361,25 @@ class MusicBrainzSearchClient: # fall back to the unfiltered list — better than no results. rgs = studio or rgs + # Narrow to the title-hint if the user gave one ("The Beatles + # Abbey Road" → filter to RGs whose title contains "abbey + # road"). If no RG matches, fall back to text-search so the + # user finds the specific album instead of either seeing the + # full discography or getting zero results. (kettui flagged + # this regression — artist-first alone was burying specific- + # album queries inside the unfiltered discography list.) + if title_hint: + hint_lower = title_hint.lower() + matched = [rg for rg in rgs if hint_lower in (rg.get('title') or '').lower()] + if matched: + rgs = matched + else: + fallback = self._search_albums_text(title_hint, tname, limit) + if fallback: + return fallback + # Text-search also missed — fall through and show the + # full (unfiltered) discography rather than nothing. + # Sort by primary-type priority first (album > ep > single > # compilation), then chronologically ASC — the standard way # discographies are listed ("their debut was X, then Y, then Z"). @@ -440,7 +488,10 @@ class MusicBrainzSearchClient: release_date = '' image_url = None album_type = 'single' - total_tracks = 1 + # Initialized to 0 and summed from the release's media track-counts. + # Previously initialized to 1, which made every track-with-release + # report one more than the album actually has (kettui caught this). + total_tracks = 0 releases = r.get('releases', []) or [] if releases: @@ -460,6 +511,12 @@ class MusicBrainzSearchClient: rg_mbid = rg.get('id', '') or '' image_url = self._cached_art(album_id, rg_mbid) if album_id else None + # Tracks with no release info are standalone recordings — give them + # total_tracks=1 (the track itself). Keeps the old shape for that + # edge case but fixes the off-by-one for every normal case. + if not releases: + total_tracks = 1 + return Track( id=mbid, name=title, diff --git a/tests/test_musicbrainz_search.py b/tests/test_musicbrainz_search.py index 939e83fe..f9933a11 100644 --- a/tests/test_musicbrainz_search.py +++ b/tests/test_musicbrainz_search.py @@ -15,6 +15,7 @@ import pytest from core.musicbrainz_search import ( MusicBrainzSearchClient, _cover_art_url, + _extract_title_hint, ) @@ -520,6 +521,164 @@ def test_get_album_falls_back_to_release_lookup_on_rg_miss(): assert album['id'] == 'rel-abc' # Falls back to release MBID since rg lookup missed. +# --------------------------------------------------------------------------- +# Title-hint extraction — for "Artist Album Title" bare queries +# --------------------------------------------------------------------------- + +def test_extract_title_hint_basic(): + assert _extract_title_hint('The Beatles Abbey Road', 'The Beatles') == 'Abbey Road' + + +def test_extract_title_hint_case_insensitive(): + assert _extract_title_hint('the beatles abbey road', 'The Beatles') == 'abbey road' + + +def test_extract_title_hint_preserves_original_casing(): + # Query slicing should return the original casing of the title portion. + assert _extract_title_hint('The Beatles Abbey Road', 'The Beatles') == 'Abbey Road' + + +def test_extract_title_hint_whitespace_tolerant(): + assert _extract_title_hint('The Beatles Abbey Road', 'The Beatles') == 'Abbey Road' + + +def test_extract_title_hint_bare_artist_returns_none(): + assert _extract_title_hint('The Beatles', 'The Beatles') is None + + +def test_extract_title_hint_artist_not_prefix_returns_none(): + # Query where the artist name isn't the prefix — nothing to extract. + assert _extract_title_hint('Abbey Road', 'The Beatles') is None + + +def test_extract_title_hint_word_boundary_required(): + # "Metallicasomething" shouldn't split as artist=Metallica + hint=something + assert _extract_title_hint('Metallicasomething', 'Metallica') is None + + +def test_search_albums_filters_browse_results_by_title_hint(): + """Regression: 'The Beatles Abbey Road' used to return the whole + discography; should now narrow to Abbey Road specifically.""" + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_artist.return_value = [_mk_artist('The Beatles', 'mb-1', score=100)] + client._client.browse_artist_release_groups.return_value = [ + {'id': 'rg-abbey', 'title': 'Abbey Road', 'primary-type': 'Album', + 'first-release-date': '1969-09-26', 'secondary-types': []}, + {'id': 'rg-white', 'title': 'The Beatles', 'primary-type': 'Album', + 'first-release-date': '1968-11-22', 'secondary-types': []}, + {'id': 'rg-revolver', 'title': 'Revolver', 'primary-type': 'Album', + 'first-release-date': '1966-08-05', 'secondary-types': []}, + ] + + albums = client.search_albums('The Beatles Abbey Road', limit=10) + + # Filtered to only the album whose title matches the hint. + assert [a.name for a in albums] == ['Abbey Road'] + + +def test_search_albums_falls_back_to_text_when_hint_matches_nothing(): + """If the title hint doesn't match any browse result, fall back to + text-search rather than returning the full discography or nothing.""" + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_artist.return_value = [_mk_artist('The Beatles', 'mb-1', score=100)] + # Browse returns albums that don't match the hint. + client._client.browse_artist_release_groups.return_value = [ + {'id': 'rg-1', 'title': 'Some Other Album', 'primary-type': 'Album', + 'first-release-date': '1965-01-01', 'secondary-types': []}, + ] + # Text-search fallback (_search_albums_text → search_release) returns the album. + client._client.search_release.return_value = [ + {'id': 'rel-abbey', 'title': 'Abbey Road', 'score': 100, + 'release-group': {'id': 'rg-abbey', 'primary-type': 'Album'}, + 'artist-credit': [{'name': 'The Beatles'}]}, + ] + + albums = client.search_albums('The Beatles Totally Fake Album Name', limit=10) + + # Browse had no hit for the title hint, then fallback kicks in when + # the filter results are also empty (after studio-pref filter etc.). + # NOTE: in this test the hint filter returns empty, so we fall through + # to search_release. + client._client.search_release.assert_called_once() + assert any(a.name == 'Abbey Road' for a in albums) + + +def test_search_albums_bare_artist_no_hint_no_filter(): + """Bare artist name (no title hint) returns full discography — the + filter only kicks in when the user types extra words.""" + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_artist.return_value = [_mk_artist('The Beatles', 'mb-1', score=100)] + client._client.browse_artist_release_groups.return_value = [ + {'id': 'rg-abbey', 'title': 'Abbey Road', 'primary-type': 'Album', + 'first-release-date': '1969-09-26', 'secondary-types': []}, + {'id': 'rg-revolver', 'title': 'Revolver', 'primary-type': 'Album', + 'first-release-date': '1966-08-05', 'secondary-types': []}, + ] + + albums = client.search_albums('the beatles', limit=10) + + # No filter — full discography. + titles = {a.name for a in albums} + assert 'Abbey Road' in titles + assert 'Revolver' in titles + + +def test_recording_to_track_total_tracks_matches_media_count(): + """Regression: total_tracks was initialized at 1 and summed with media + track-counts, producing an off-by-one. An 11-track album reported 12.""" + client = MusicBrainzSearchClient() + recording = { + 'id': 'rec-1', + 'title': 'Song', + 'length': 300000, + 'artist-credit': [{'name': 'Band'}], + 'releases': [{ + 'id': 'rel-1', + 'title': 'Album', + 'date': '2020-01-01', + 'release-group': {'id': 'rg-1', 'primary-type': 'Album', 'secondary-types': []}, + 'media': [{'track-count': 11}], + }], + } + track = client._recording_to_track(recording, 'Band') + assert track is not None + assert track.total_tracks == 11 + + +def test_recording_to_track_multi_disc_sums_media(): + """Two-disc album with 14 tracks total should report 14, not 15 (off by one) + or 3 (missing the sum).""" + client = MusicBrainzSearchClient() + recording = { + 'id': 'rec-1', + 'title': 'Song', + 'artist-credit': [{'name': 'Band'}], + 'releases': [{ + 'id': 'rel-1', 'title': 'Album', + 'release-group': {'id': 'rg-1', 'primary-type': 'Album'}, + 'media': [{'track-count': 7}, {'track-count': 7}], + }], + } + track = client._recording_to_track(recording, 'Band') + assert track.total_tracks == 14 + + +def test_recording_to_track_no_release_defaults_total_tracks_to_one(): + """A recording with no release info is a standalone track — report 1.""" + client = MusicBrainzSearchClient() + recording = { + 'id': 'rec-1', + 'title': 'Standalone', + 'artist-credit': [{'name': 'Band'}], + 'releases': [], + } + track = client._recording_to_track(recording, 'Band') + assert track.total_tracks == 1 + + def test_pick_representative_release_prefers_official_with_media(): """The release picker should skip stub releases (no media) and pick Official over Promotion status.""" diff --git a/web_server.py b/web_server.py index 1e9c6196..2f00b8d6 100644 --- a/web_server.py +++ b/web_server.py @@ -11735,10 +11735,15 @@ def get_artist_image(artist_id): source_override = request.args.get('source', '').strip().lower() or None plugin = request.args.get('plugin', '').strip().lower() or None + # `name` is optional but required for sources that don't store + # artist images directly (MusicBrainz) — the resolver falls back + # to searching iTunes/Deezer by name. + artist_name = request.args.get('name', '').strip() or None image_url = _get_artist_image_url( artist_id, source_override=source_override, plugin=plugin, + artist_name=artist_name, ) return jsonify({"success": True, "image_url": image_url}) except Exception as e: diff --git a/webui/static/downloads.js b/webui/static/downloads.js index 0f89ed7d..30a9d2bd 100644 --- a/webui/static/downloads.js +++ b/webui/static/downloads.js @@ -5339,7 +5339,7 @@ function _gsRenderFromState(state) { if (artists.length) { h += `
🎤 Artists ${srcLabel}
`; - h += artists.map(a => `
${a.image_url ? `` : '🎤'}
${_escToast(a.name)}
`).join(''); + h += artists.map(a => `
${a.image_url ? `` : '🎤'}
${_escToast(a.name)}
`).join(''); h += '
'; } @@ -5398,11 +5398,15 @@ async function _gsLazyLoadArtistImages() { const artistId = card.dataset.artistId; if (!artistId) continue; try { - const res = await fetch(`/api/artist/${artistId}/image?source=${activeSrc}`); + // Pass the artist name so MusicBrainz lookups (which have no + // artist art) can resolve the image by name on a fallback source. + const params = new URLSearchParams({ source: activeSrc }); + if (card.dataset.artistName) params.set('name', card.dataset.artistName); + const res = await fetch(`/api/artist/${artistId}/image?${params}`); const data = await res.json(); if (data.success && data.image_url) { const artDiv = card.querySelector('.gsearch-item-art'); - if (artDiv) artDiv.innerHTML = ``; + if (artDiv) artDiv.innerHTML = ``; card.removeAttribute('data-needs-image'); } } catch (e) { /* ignore */ } diff --git a/webui/static/helper.js b/webui/static/helper.js index 6f1c5380..ff4a7871 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -3463,6 +3463,7 @@ const WHATS_NEW = { { title: 'Source Picker Dims Soulseek When slskd Isn\'t Configured', desc: 'Cin pointed out that the Soulseek icon was always rendered as configured, so users without slskd set up could click it and fire searches that would never succeed. Soulseek is now in the backend config-status registry as `required: [slskd_url]` and removed from the frontend\'s always-configured set. Without slskd, the icon dims and clicking it routes to Settings → Downloads tab (where the slskd URL field lives, gated behind the download-source dropdown) instead of Settings → Connections', page: 'search', unreleased: true }, { title: 'Fix Discover Hero "View Discography" 404ing on Source Artists', desc: 'Clicking "View Discography" on the Discover page hero slideshow was calling navigateToArtistDetail without a source, so /api/artist-detail defaulted to a library lookup and returned 404 for artists that don\'t exist in your library (which is nearly every hero artist — they come from discover similar-artists, not the library). Regression from the unification PR that rewrote the click handler to route to /artist-detail but forgot to pass the source. Backend already sends artist.source on each hero entry; we now stash it as data-source on the discography button and thread it through to navigateToArtistDetail so the API call includes source=itunes/deezer/etc. and returns the synthesized discography', page: 'discover', unreleased: true }, { title: 'MusicBrainz Search Actually Works Now', desc: 'kettui flagged during PR #371 review that the MusicBrainz source tab never returned artists and served garbage tracks/albums. Three things were wrong. First, the artist search was hardcoded to return an empty list — re-enabled with a proper fuzzy query (bare Lucene string against alias/artist/sortname indexes) and score-filtered at 80+ to drop tribute bands. Second, track and album searches used text-search on recording/release TITLES — so typing "metallica" matched random tracks literally named "Metallica" (all scoring 100 because they\'re exact title hits). Now a bare name query resolves to the top-scoring artist, then BROWSES that artist\'s release-groups and recordings directly — the same pattern Plex uses. Structured "Artist - Title" queries still take the text-search path since the user gave an explicit title to match. Third, the adapter was firing synchronous Cover Art Archive HEAD requests (up to 30s of blocking probes per search) — replaced with deterministic URL construction so the browser loads images lazily with fallback. Search completes in ~3 seconds instead of 30+ on cold cache. Also shipped: project URL in User-Agent per MB\'s rate-limit policy recommendations', page: 'search', unreleased: true }, + { title: 'MusicBrainz Search Follow-Ups (Images, Counts, Title Hints)', desc: 'Three fixes from kettui\'s follow-up pass on the MusicBrainz search PR. (1) Artist images were missing because MB doesn\'t store artist art — the lazy-load endpoint now accepts an optional `name` query param and resolves images by searching iTunes/Deezer for that artist name. (2) Track total_tracks was off by one because the counter initialized at 1 before summing release media track-counts — an 11-track album reported 12. Initialized to 0 now, with a special case for standalone recordings that have no release (report 1). (3) Queries like "The Beatles Abbey Road" used to browse The Beatles\' whole discography because the artist-first path resolved the artist and ignored the trailing title. Now extracts the title hint from queries shaped like "Artist Title", filters browse results to match, and falls back to text-search when no browse result matches (so "The Beatles Totally Fake Album" still finds something rather than nothing). 10 new tests covering title-hint extraction, browse-filter behavior, total_tracks edge cases', page: 'search', unreleased: true }, ], '2.39': [ // --- April 22, 2026 --- diff --git a/webui/static/search.js b/webui/static/search.js index 4d47436e..585a5495 100644 --- a/webui/static/search.js +++ b/webui/static/search.js @@ -575,9 +575,16 @@ function initializeSearchModeToggle() { try { const activeSource = searchController.state.activeSource; - const imgUrl = activeSource && activeSource !== 'spotify' - ? `/api/artist/${artistId}/image?source=${activeSource}` - : `/api/artist/${artistId}/image`; + // Pass the artist name so the backend can look up images + // for sources that don't store them (e.g. MusicBrainz — + // it only has MBIDs, not artist art, so the resolver + // falls back to iTunes/Deezer keyed by name). + const artistName = card.dataset.artistName || ''; + const params = new URLSearchParams(); + if (activeSource && activeSource !== 'spotify') params.set('source', activeSource); + if (artistName) params.set('name', artistName); + const qs = params.toString(); + const imgUrl = `/api/artist/${artistId}/image${qs ? '?' + qs : ''}`; const response = await fetch(imgUrl); const data = await response.json(); diff --git a/webui/static/shared-helpers.js b/webui/static/shared-helpers.js index ce65a412..348944ea 100644 --- a/webui/static/shared-helpers.js +++ b/webui/static/shared-helpers.js @@ -537,6 +537,11 @@ function renderCompactSection(sectionId, listId, countId, items, mapItem) { if (item.id) { elem.dataset.artistId = item.id; elem.dataset.needsImage = config.image ? 'false' : 'true'; + // Stash the artist name so the lazy-loader can pass it to + // the backend. Needed for sources that don't store artist + // images directly (MusicBrainz) — backend resolves the + // image by looking up the name on a fallback source. + if (config.name) elem.dataset.artistName = config.name; } } else if (isAlbum) { elem.className = 'enh-compact-item album-card'; From c454b1ebafb7c68593c73e847837948a9fa52f68 Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Fri, 24 Apr 2026 10:27:45 -0700 Subject: [PATCH 13/13] MusicBrainz: Dedupe same-named homonyms in artist search results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Typing "michael jackson" returned 7 identical-looking cards because MusicBrainz has many different PEOPLE sharing a canonical name — the King of Pop plus a NZ poet, a photographer, a mashup artist, a didgeridoo player, and more, all scoring 80+ on exact-name match. All 7 passed the score filter. All 7 rendered with the same fallback image because iTunes/Deezer only know the famous one. Fix dedupes by normalized (lowercase, whitespace-trimmed) name before building Artist dataclasses. Keeps the highest-scoring entry per name, so the King of Pop (score 100) wins over the others (all score 80-81). Artists with genuinely different names stay separate — a search for "the beatles" still surfaces tribute bands if they're above threshold. Implementation note: fetch `max(limit*3, 10)` from MB instead of `limit` directly, so the dedup pool is large enough to still return `limit` distinct artists after collapsing duplicates. Previously the raw fetch was capped at the caller's limit, which would have left fewer-than-requested results after dedup for common names. 3 new tests (49 total): - Dedupe collapses 5 same-named entries to 1 (keeps highest score). - Dedup key is case-insensitive and whitespace-normalized. - Dedup preserves distinct names ("The Beatles" vs "The Beatles Revival" stay separate). Live-verified: "michael jackson" now returns 1 card, "kendrick lamar" returns 1 card. Credit: kettui spotted duplicate Michael Jackson cards in the search UI. --- core/musicbrainz_search.py | 33 +++++++++++++++--- tests/test_musicbrainz_search.py | 58 ++++++++++++++++++++++++++++++-- 2 files changed, 85 insertions(+), 6 deletions(-) diff --git a/core/musicbrainz_search.py b/core/musicbrainz_search.py index c396662b..965317be 100644 --- a/core/musicbrainz_search.py +++ b/core/musicbrainz_search.py @@ -177,17 +177,42 @@ class MusicBrainzSearchClient: lookalikes. """ try: - raw = self._client.search_artist(query, limit=limit, strict=False) - artists = [] + # Fetch extra so dedup below has enough to pick from. For + # common names (Michael Jackson, John Williams, etc.) MB returns + # many same-named people; without a larger pool, capping at + # `limit` before dedup can leave us with fewer results than + # requested. + raw = self._client.search_artist(query, limit=max(limit * 3, 10), strict=False) + + # Dedupe by normalized name. MusicBrainz has many different + # people with the same canonical name (7 entries for "Michael + # Jackson" — the singer + poet + photographer + didgeridoo + # player + ...), all scoring 80+ on exact-name match. Rendered + # as identical cards since the fallback image lookup hits the + # same fallback-source result for each. Keep the highest- + # scoring entry per normalized name so the user sees one card + # per distinct artist. + seen = {} for a in raw: score = a.get('score', 0) or 0 if score < self._MIN_SCORE: continue - mbid = a.get('id', '') name = a.get('name', '') if not mbid or not name: continue + key = name.lower().strip() + if key not in seen or (seen[key].get('score', 0) or 0) < score: + seen[key] = a + + # Sort the survivors score-descending and cap at the caller's + # limit. `seen` only holds top-per-name, so ordering is stable. + top = sorted(seen.values(), key=lambda r: -(r.get('score', 0) or 0))[:limit] + + artists = [] + for a in top: + mbid = a.get('id', '') + name = a.get('name', '') # Genres from MB tags (user-applied categorical labels). Each # tag has {name, count}; keep the top-weighted ones. @@ -201,7 +226,7 @@ class MusicBrainzSearchClient: artists.append(Artist( id=mbid, name=name, - popularity=score, # Reuse score as popularity (0-100) + popularity=a.get('score', 0) or 0, # Reuse score as popularity (0-100) genres=genres, followers=0, # MusicBrainz doesn't track followers image_url=None, # MB doesn't store artist images directly diff --git a/tests/test_musicbrainz_search.py b/tests/test_musicbrainz_search.py index f9933a11..719f02ae 100644 --- a/tests/test_musicbrainz_search.py +++ b/tests/test_musicbrainz_search.py @@ -100,12 +100,66 @@ def test_search_artists_filters_by_score_threshold(): def test_search_artists_uses_strict_false_for_fuzzy_match(): """The adapter must use strict=False so MusicBrainz searches - alias+artist+sortname together — strict mode would miss aliased names.""" + alias+artist+sortname together — strict mode would miss aliased names. + + Adapter fetches `limit * 3` (min 10) so dedup-by-name below has enough + candidates to pick from. + """ client = MusicBrainzSearchClient() client._client = MagicMock() client._client.search_artist.return_value = [] client.search_artists('metallica') - client._client.search_artist.assert_called_once_with('metallica', limit=10, strict=False) + client._client.search_artist.assert_called_once_with('metallica', limit=30, strict=False) + + +def test_search_artists_dedupes_same_named_homonyms(): + """MusicBrainz has many different PEOPLE sharing a canonical name + (7 Michael Jacksons: singer, poet, photographer, mashup artist, ...). + Since they all render as "Michael Jackson" with the same fallback image, + dedupe to the highest-scoring entry per name.""" + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_artist.return_value = [ + {'id': 'mb-king', 'name': 'Michael Jackson', 'score': 100, + 'tags': [{'name': 'pop'}]}, + {'id': 'mb-poet', 'name': 'Michael Jackson', 'score': 81}, + {'id': 'mb-mashup', 'name': 'Michael Jackson', 'score': 80}, + {'id': 'mb-photog', 'name': 'Michael Jackson', 'score': 80}, + {'id': 'mb-other', 'name': 'Michael Jackson', 'score': 80}, + ] + + results = client.search_artists('michael jackson', limit=10) + + # Should collapse to one entry — the highest-scoring one. + assert len(results) == 1 + assert results[0].id == 'mb-king' + assert results[0].popularity == 100 + + +def test_search_artists_dedup_normalized_case_and_whitespace(): + """Dedup key is case-insensitive and whitespace-normalized.""" + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_artist.return_value = [ + {'id': 'mb-1', 'name': 'The Band', 'score': 100}, + {'id': 'mb-2', 'name': 'THE BAND', 'score': 85}, + {'id': 'mb-3', 'name': 'the band', 'score': 82}, + ] + results = client.search_artists('the band', limit=5) + assert len(results) == 1 + assert results[0].id == 'mb-1' + + +def test_search_artists_keeps_distinct_names(): + """Dedup only collapses identical normalized names, not similar names.""" + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_artist.return_value = [ + {'id': 'mb-1', 'name': 'The Beatles', 'score': 100}, + {'id': 'mb-2', 'name': 'The Beatles Revival', 'score': 85}, + ] + results = client.search_artists('the beatles', limit=5) + assert {r.name for r in results} == {'The Beatles', 'The Beatles Revival'} def test_search_artists_returns_empty_on_exception():