diff --git a/core/artists/quality.py b/core/artists/quality.py index de2a660f..e9d5f16e 100644 --- a/core/artists/quality.py +++ b/core/artists/quality.py @@ -32,10 +32,14 @@ Per-track flow (source-agnostic): The flow originally had Spotify-only logic for steps 1 and 2 with iTunes hardcoded as the only fallback. That broke for users with neither Spotify nor Deezer connected — iTunes returned sparse / no matches and -the failure mode was silent. Now everything dispatches through the -configured primary source via ``deps.get_metadata_fallback_client()`` -(which respects `metadata.fallback_source` in config). Spotify keeps -its direct-lookup optimization; everything else goes through search. +the failure mode was silent. The Track Redownload modal had been doing +parallel multi-source search across every configured source the whole +time; enhance now uses the same shared module +(``core.metadata.multi_source_search``) so both endpoints dispatch +identically. Spotify keeps its direct-lookup fast-path optimization +(only Spotify exposes ``get_track_details(id)`` returning a rich +wishlist-shaped payload); when that doesn't fire, the multi-source +parallel search picks the cross-source best match. Returns `(payload_dict, http_status_code)` so the route wrapper can `jsonify()` and return. @@ -60,8 +64,12 @@ class ArtistQualityDeps: get_wishlist_service: Callable[[], Any] get_current_profile_id: Callable[[], int] get_quality_tier_from_extension: Callable - get_metadata_fallback_client: Callable[[], Any] - get_metadata_fallback_source: Callable[[], str] + # Returns ``[(source_name, client), ...]`` for every metadata source + # the user has configured (not just the active primary). The Track + # Redownload modal already searches multi-source in parallel — + # Enhance Quality now matches that contract via the shared + # ``core.metadata.multi_source_search`` module. + get_metadata_search_sources: Callable[[], list] def _has_complete_metadata(payload: Optional[dict]) -> bool: @@ -191,79 +199,44 @@ def _spotify_direct_lookup(spotify_client, spotify_tid: str, return None -def _search_match(client, matching_engine, title: str, artist_name: str, - album_title: str) -> Optional[dict]: - """Search the configured primary source for a track matching - title + artist. Confidence threshold 0.7; album-tracks get a - small bonus over singles. Returns a wishlist payload built from - the best match, or None if nothing clears threshold. +# Minimum match-score threshold for accepting a match without user +# confirmation. Mirrors the legacy single-source threshold the enhance +# flow has always used. +_AUTO_ACCEPT_SCORE_THRESHOLD = 0.7 - Source-agnostic — works for any client implementing - ``search_tracks(query, limit)`` returning Track-shaped objects. + +def _try_upgrade_to_rich_payload(client: Any, track_id: str) -> Optional[dict]: + """Spotify-only optimization — ``get_track_details(id)`` returns + rich raw_data already in the wishlist payload shape. Other sources + don't expose this; caller falls back to ``_build_payload_from_track``. """ - if not client: + if not hasattr(client, 'get_track_details'): return None - - temp_track = type('TempTrack', (), { - 'name': title, 'artists': [artist_name], 'album': album_title, - })() try: - queries = matching_engine.generate_download_queries(temp_track) + details = client.get_track_details(track_id) + if details and details.get('raw_data'): + return details['raw_data'] except Exception: - return None - - best = None - best_conf = 0.0 - for query in queries[:3]: - try: - results = client.search_tracks(query, limit=5) - except Exception: - continue - if not results: - continue - for cand in results: - artist_conf = max( - (matching_engine.similarity_score( - matching_engine.normalize_string(artist_name), - matching_engine.normalize_string(a), - ) for a in (cand.artists or [artist_name])), - default=0, - ) - title_conf = matching_engine.similarity_score( - matching_engine.normalize_string(title), - matching_engine.normalize_string(cand.name), - ) - combined = artist_conf * 0.5 + title_conf * 0.5 - # Small bonus for album tracks over singles. - album_type = getattr(cand, 'album_type', None) or '' - if album_type == 'album': - combined += 0.02 - elif album_type == 'ep': - combined += 0.01 - if combined > best_conf and combined >= 0.7: - best_conf = combined - best = cand - if best_conf >= 0.9: - break - - if not best: - return None - - # Spotify search returns a richer dict via get_track_details — try - # to upgrade the match if the search-results client exposes that. - if hasattr(client, 'get_track_details'): - try: - full_details = client.get_track_details(best.id) - if full_details and full_details.get('raw_data'): - return full_details['raw_data'] - except Exception: - pass - - return _build_payload_from_track(best) + pass + return None def enhance_artist_quality(artist_id, track_ids, deps: ArtistQualityDeps): - """Add selected tracks to wishlist for quality enhancement re-download.""" + """Add selected tracks to wishlist for quality enhancement re-download. + + Per-track flow now mirrors the Track Redownload modal: searches every + configured metadata source in parallel via the shared + ``core.metadata.multi_source_search`` module, picks the best cross-source + match if it clears the auto-accept threshold, validates the match + has non-empty fields, and queues the wishlist payload. + + Pre-refactor this was Spotify-only with an iTunes fallback — + Discogs / Hydrabase / Deezer-primary users got far worse coverage + than redownload despite both flows asking the same question + ("find me the metadata for this library track"). + """ + from core.metadata.multi_source_search import TrackQuery, search_all_sources + try: if not track_ids: return {"success": False, "error": "No track IDs provided"}, 400 @@ -289,11 +262,9 @@ def enhance_artist_quality(artist_id, track_ids, deps: ArtistQualityDeps): track['_album_id'] = album.get('id') track_lookup[tid] = track - # Resolve the primary metadata source ONCE up front. All matching - # routes through this. Replaces the legacy hardcoded - # Spotify-direct → Spotify-search → iTunes-fallback chain. - primary_source = deps.get_metadata_fallback_source() - primary_client = deps.get_metadata_fallback_client() + # Resolve every configured metadata source up front — the same + # parallel multi-source search the Track Redownload modal uses. + search_sources = deps.get_metadata_search_sources() enhanced_count = 0 failed_count = 0 @@ -320,47 +291,81 @@ def enhance_artist_quality(artist_id, track_ids, deps: ArtistQualityDeps): album_title = track.get('_album_title', '') matched_track_data = None + chosen_source = None - # 1. Spotify-only direct-lookup optimization. Other sources - # don't have a stored-ID-to-track API today. - if primary_source == 'spotify' and deps.spotify_client: + # 1. Spotify-only direct-lookup optimization. If Spotify is + # configured AND we have a stored Spotify ID, fast path + # straight to the rich raw payload (no search required). + if deps.spotify_client: spotify_tid = track.get('spotify_track_id') if spotify_tid: matched_track_data = _spotify_direct_lookup( deps.spotify_client, spotify_tid, artist_name, album_title, title, ) + if matched_track_data: + chosen_source = 'spotify' - # 2. Search match against the primary source (works for any - # source that implements search_tracks — Spotify, iTunes, - # Deezer, Discogs, Hydrabase). - if not matched_track_data: + # 2. Multi-source parallel search across every configured + # metadata source. Picks the highest-scoring match across + # all sources (Spotify / iTunes / Deezer / Discogs / + # Hydrabase — whichever the user has configured). + if not matched_track_data and search_sources: try: - matched_track_data = _search_match( - primary_client, deps.matching_engine, - title, artist_name, album_title, + track_query = TrackQuery( + title=title, + artist=artist_name, + album=album_title, + duration_ms=track.get('duration', 0) or 0, + spotify_track_id=track.get('spotify_track_id'), + deezer_id=track.get('deezer_id'), ) + multi_result = search_all_sources(track_query, search_sources) + if multi_result.best_match and multi_result.best_match['score'] >= _AUTO_ACCEPT_SCORE_THRESHOLD: + chosen_source = multi_result.best_match['source'] + best_track_obj = multi_result.best_track() + if best_track_obj: + # Try Spotify's rich-payload upgrade first; fall + # back to building from the source-native Track. + client = next( + (c for n, c in search_sources if n == chosen_source), + None, + ) + if client: + matched_track_data = _try_upgrade_to_rich_payload( + client, str(best_track_obj.id), + ) + if not matched_track_data: + matched_track_data = _build_payload_from_track(best_track_obj) except Exception as exc: - logger.error(f"[Enhance] {primary_source} search failed for {title}: {exc}") + logger.error(f"[Enhance] Multi-source search failed for {title}: {exc}") # 3. Reject matches with empty / missing core fields. if not _has_complete_metadata(matched_track_data): if matched_track_data: logger.warning( - f"[Enhance] {primary_source} match for '{title}' rejected — " + f"[Enhance] {chosen_source} match for '{title}' rejected — " f"empty title / album / artists (would render as 'unknown')" ) matched_track_data = None if not matched_track_data: failed_count += 1 + source_list = ', '.join(name for name, _ in (search_sources or [])) + if not source_list: + reason = ( + 'No metadata source configured — connect Spotify / ' + 'iTunes / Deezer / Discogs / Hydrabase to enable enhance' + ) + else: + reason = ( + f'No usable match across {source_list} — ' + f'try connecting an additional metadata source' + ) failed_tracks.append({ 'track_id': track_id, 'title': title, - 'reason': ( - f'No usable {primary_source} match — ' - f'connect another metadata source for better coverage' - ), + 'reason': reason, }) continue diff --git a/core/metadata/multi_source_search.py b/core/metadata/multi_source_search.py new file mode 100644 index 00000000..19b78c77 --- /dev/null +++ b/core/metadata/multi_source_search.py @@ -0,0 +1,240 @@ +"""Multi-source parallel metadata search. + +Both the Track Redownload modal and the Artist Enhance Quality flow +need to find the best metadata match for a known track (we have the +title + artist + duration from the user's library; we want to find +the matching entry in Spotify / iTunes / Deezer / Discogs / Hydrabase +to drive the wishlist re-download). + +Pre-extraction, redownload had a fully-fledged multi-source parallel +search (parallel ThreadPoolExecutor, per-source query optimization, +"current match" flagging via stored source IDs, per-result scoring) +while enhance had a hardcoded Spotify-direct → Spotify-search → +iTunes-fallback chain that only searched ONE source. That's why +redownload "worked" for users without Spotify (it'd find matches via +iTunes / Deezer in parallel) and enhance silently failed (single +fallback returned junk). + +This module owns the search logic. Both endpoints call +``search_all_sources`` and get back the same shape — same scoring, +same source-optimized queries, same "current match" semantics. UI +behavior diverges per-endpoint (redownload renders a picker, enhance +auto-picks the best across all sources) but the metadata-search +contract is shared. +""" + +from __future__ import annotations + +import logging +from concurrent.futures import ThreadPoolExecutor, as_completed +from dataclasses import dataclass, field +from difflib import SequenceMatcher +from typing import Any, Dict, List, Optional, Tuple + +logger = logging.getLogger(__name__) + + +@dataclass +class TrackQuery: + """Inputs needed to run a multi-source metadata search for one track.""" + title: str + artist: str + album: str = '' + # Library-side duration in milliseconds. Used for the duration + # similarity component of scoring; pass 0 when unknown (scoring + # falls back to a neutral 0.5 weight). + duration_ms: int = 0 + # Source-native track IDs already stored on the library track, + # used for the "is_current_match" flag in the per-result rendering + # so the UI can highlight the entry that produced the existing file. + spotify_track_id: Optional[str] = None + deezer_id: Optional[str] = None + + +@dataclass +class MultiSourceResult: + """Aggregated output from ``search_all_sources``.""" + # source_name → list of result dicts (already per-source sorted: + # is_current_match first, then descending match_score). Dict shape + # is JSON-serializable for direct return to the frontend (the + # redownload picker uses these as-is). + metadata_results: Dict[str, List[dict]] = field(default_factory=dict) + # source_name → list of source-native Track objects, parallel-indexed + # to ``metadata_results[source_name]``. Used by callers (Enhance + # Quality) that need to build a wishlist payload from the chosen + # match — the dict shape lacks per-source fields like full album + # data / external_urls / popularity that the wishlist needs. + raw_tracks: Dict[str, List[Any]] = field(default_factory=dict) + # Best match across all sources, or None if every source returned + # nothing. Shape: ``{'source': str, 'index': int, 'score': float}``. + best_match: Optional[dict] = None + + def best_track(self) -> Optional[Any]: + """Convenience: return the source-native Track object for the + cross-source best match, or None if no match was found.""" + if not self.best_match: + return None + source = self.best_match['source'] + index = self.best_match['index'] + tracks = self.raw_tracks.get(source) or [] + return tracks[index] if index < len(tracks) else None + + +def _score_match(query: TrackQuery, result: dict) -> float: + """Score one result dict against the query. + + Weights: title 0.5, artist 0.35, duration 0.15. Duration component + is neutral (0.5) when the library track has no duration on file. + + These weights match the redownload pre-extraction implementation + so existing callers keep their scoring behavior identical. + """ + title_sim = SequenceMatcher( + None, query.title.lower(), (result.get('name') or '').lower() + ).ratio() + artist_sim = SequenceMatcher( + None, query.artist.lower(), (result.get('artist') or '').lower() + ).ratio() + if query.duration_ms: + dur_diff = abs(query.duration_ms - (result.get('duration_ms') or 0)) + dur_score = max(0.0, 1.0 - dur_diff / 30000.0) + else: + dur_score = 0.5 + return round((title_sim * 0.5 + artist_sim * 0.35 + dur_score * 0.15), 3) + + +def _build_source_query(source_name: str, query: TrackQuery, clean_title: str) -> str: + """Build the source-optimized search query string. + + Deezer's API responds best to its native field-prefixed syntax + (``artist:"X" track:"Y"``) — empirically returns better matches + than a plain query for ambiguous track names. Other sources use + the artist + clean-title concatenation. + """ + if source_name == 'deezer': + return f'artist:"{query.artist}" track:"{clean_title}"' + return f"{query.artist} {clean_title}" + + +def _search_one_source(source_name: str, client: Any, + query: TrackQuery, clean_title: str + ) -> Tuple[str, List[dict], List[Any]]: + """Run one source's search with three-tier query fallback. + + Tier 1: source-optimized query (Deezer's structured form, others' plain). + Tier 2: plain ``artist + title`` if tier 1 returned nothing. + Tier 3: title-only as last resort. + + Returns ``(source_name, results, raw_tracks)``: + - ``results`` are the JSON-serializable dicts (id / name / artist / + etc.), sorted by is_current_match first, then descending match_score + - ``raw_tracks`` are the source-native Track objects, parallel-indexed + to ``results``, for callers that need richer per-source fields + than the dict surface (album_type, external_urls, etc). + """ + try: + primary_q = _build_source_query(source_name, query, clean_title) + plain_q = f"{query.artist} {clean_title}" + title_q = clean_title + + logger.info(f"[MultiSourceSearch] Searching {source_name} for: {primary_q}") + track_objs = client.search_tracks(primary_q, limit=10) + if not track_objs and primary_q != plain_q: + track_objs = client.search_tracks(plain_q, limit=10) + if not track_objs and clean_title != plain_q: + track_objs = client.search_tracks(title_q, limit=10) + logger.info(f"[MultiSourceSearch] {source_name} returned {len(track_objs)} results") + + scored: List[Tuple[dict, Any]] = [] + for t in track_objs: + r = { + 'id': str(getattr(t, 'id', '')), + 'name': getattr(t, 'name', '') or '', + 'artist': ', '.join(t.artists) if getattr(t, 'artists', None) else '', + 'album': getattr(t, 'album', '') or '', + 'duration_ms': getattr(t, 'duration_ms', 0) or 0, + 'image_url': getattr(t, 'image_url', '') or '', + 'is_current_match': False, + } + # Flag the result that backs the user's existing library + # track so the UI can highlight it. + if source_name == 'spotify' and query.spotify_track_id and r['id'] == str(query.spotify_track_id): + r['is_current_match'] = True + elif source_name == 'deezer' and query.deezer_id and r['id'] == str(query.deezer_id): + r['is_current_match'] = True + r['match_score'] = _score_match(query, r) + scored.append((r, t)) + + # Sort dict + raw track in lockstep so raw_tracks[i] is the + # source-native object behind metadata_results[source][i]. + scored.sort(key=lambda pair: (-int(pair[0]['is_current_match']), -pair[0]['match_score'])) + results = [pair[0] for pair in scored] + raw_tracks = [pair[1] for pair in scored] + return source_name, results, raw_tracks + except Exception as exc: + logger.error( + f"[MultiSourceSearch] Search failed for {source_name}: {exc}", + exc_info=True, + ) + return source_name, [], [] + + +def search_all_sources(query: TrackQuery, + sources: List[Tuple[str, Any]], + clean_title: Optional[str] = None, + max_workers: int = 3) -> MultiSourceResult: + """Run a parallel metadata search across every source in ``sources``. + + Args: + query: TrackQuery describing the library track we want to match. + sources: List of ``(name, client)`` pairs. Each client must + implement ``search_tracks(query: str, limit: int) -> List[Track]`` + where each Track has ``.id``, ``.name``, ``.artists`` (list), + ``.album``, ``.duration_ms``, ``.image_url`` attributes. + All five primary metadata clients (Spotify / iTunes / + Deezer / Discogs / Hydrabase) satisfy this contract. + clean_title: Optional pre-cleaned track title (e.g. with + "(Remastered)" / "(Single Version)" suffixes stripped). + Defaults to ``query.title`` if not supplied. + max_workers: ThreadPoolExecutor pool size. Default 3 matches + the redownload endpoint's pre-extraction default — bumping + higher rate-limits on slower sources without speeding up + the slowest source's response. + + Returns: + MultiSourceResult with per-source results + cross-source best match. + """ + if clean_title is None: + clean_title = query.title + + if not sources: + return MultiSourceResult() + + metadata_results: Dict[str, List[dict]] = {} + raw_tracks: Dict[str, List[Any]] = {} + with ThreadPoolExecutor(max_workers=max_workers) as pool: + futures = { + pool.submit(_search_one_source, name, client, query, clean_title): name + for name, client in sources + } + for future in as_completed(futures): + source_name, results, raws = future.result() + metadata_results[source_name] = results + raw_tracks[source_name] = raws + + best_match: Optional[dict] = None + for source, results in metadata_results.items(): + if results: + top = results[0] + if best_match is None or top['match_score'] > best_match['score']: + best_match = { + 'source': source, + 'index': 0, + 'score': top['match_score'], + } + + return MultiSourceResult( + metadata_results=metadata_results, + raw_tracks=raw_tracks, + best_match=best_match, + ) diff --git a/tests/artists/test_quality.py b/tests/artists/test_quality.py index bdba465d..7b80a1ef 100644 --- a/tests/artists/test_quality.py +++ b/tests/artists/test_quality.py @@ -92,20 +92,28 @@ def _build_deps( wishlist=None, fallback_client=None, fallback_source=None, + search_sources=None, profile_id=1, quality_tier=('mp3_320', 4), ): - # Default source mirrors the runtime helper: 'spotify' when a Spotify - # client is wired, otherwise 'itunes'. Tests override via the - # ``fallback_source`` kwarg when they need a specific source name. - if fallback_source is None: - fallback_source = 'spotify' if spotify is not None else 'itunes' - # When primary is Spotify, the Spotify client serves both the direct - # lookup AND the search match — so the fallback_client doubles as the - # primary search client. Other sources use whatever fallback_client - # the test passes in. - if fallback_client is None and fallback_source == 'spotify': - fallback_client = spotify + """Build deps for tests. + + ``search_sources`` is the new contract — list of ``(name, client)`` + pairs the multi-source search dispatches across. For convenience, + ``fallback_client`` + ``fallback_source`` are still supported and + auto-translate to a single-source list when ``search_sources`` + isn't passed (preserves the older test ergonomics). + """ + if search_sources is None: + # Default: build a single-source list from fallback_client + + # fallback_source, mirroring the legacy single-source contract. + if fallback_source is None: + fallback_source = 'spotify' if spotify is not None else 'itunes' + if fallback_client is None and fallback_source == 'spotify': + fallback_client = spotify + search_sources = ( + [(fallback_source, fallback_client)] if fallback_client else [] + ) deps = aq.ArtistQualityDeps( spotify_client=spotify, matching_engine=matching_engine or _FakeMatchingEngine(), @@ -113,8 +121,7 @@ def _build_deps( get_wishlist_service=lambda: wishlist or _FakeWishlist(), get_current_profile_id=lambda: profile_id, get_quality_tier_from_extension=lambda fp: quality_tier, - get_metadata_fallback_client=lambda: fallback_client, - get_metadata_fallback_source=lambda: fallback_source, + get_metadata_search_sources=lambda: list(search_sources), ) return deps @@ -289,11 +296,18 @@ def test_dispatches_through_primary_source_not_spotify_specific(): assert wishlist.added[0]['spotify_track_data']['id'] == 'dc-1' -def test_spotify_direct_lookup_not_used_when_spotify_not_primary(): - """Architectural pin: even if the user has Spotify configured AND a - track has a stored spotify_track_id, the direct-lookup optimization - only runs when Spotify is the ACTIVE primary source. Otherwise it - must search the active primary instead. +def test_spotify_direct_lookup_runs_as_fast_path_then_falls_back(): + """Architectural pin: Spotify direct-lookup is a fast-path + optimization, NOT a primary-source gate. When the user has Spotify + configured AND a stored spotify_track_id, direct lookup runs first + regardless of which other sources are wired. If it returns nothing, + the multi-source parallel search runs and the cross-source best + match wins (in this test, Discogs). + + This pins the post-refactor behavior — pre-refactor direct lookup + was gated on Spotify being the active primary source, which broke + enhance for users without Spotify primary even when they had a + stored Spotify ID. """ spotify_calls = [] @@ -301,7 +315,7 @@ def test_spotify_direct_lookup_not_used_when_spotify_not_primary(): def get_track_details(self, tid): spotify_calls.append(('details', tid)) return None - def search_tracks(self, q, limit=5): + def search_tracks(self, q, limit=10): spotify_calls.append(('search', q)) return [] @@ -312,7 +326,7 @@ def test_spotify_direct_lookup_not_used_when_spotify_not_primary(): discogs_track.album = 'Album X' class _DiscogsStub: - def search_tracks(self, q, limit=5): + def search_tracks(self, q, limit=10): return [discogs_track] wishlist = _FakeWishlist() @@ -326,10 +340,11 @@ def test_spotify_direct_lookup_not_used_when_spotify_not_primary(): payload, _ = aq.enhance_artist_quality('artist-1', ['t1'], deps) - # Spotify direct lookup must NOT be invoked when primary is discogs. - assert ('details', 'sp-stored') not in spotify_calls - # Discogs match was queued instead. + # Fast path was attempted (Spotify configured + stored ID present). + assert ('details', 'sp-stored') in spotify_calls + # Fast path returned None → multi-source search ran → Discogs won. assert payload['enhanced_count'] == 1 + assert wishlist.added[0]['spotify_track_data']['id'] == 'dc-1' # --------------------------------------------------------------------------- @@ -358,8 +373,8 @@ def test_track_with_no_file_path_marked_failed(): def test_no_match_anywhere_marked_failed(): - """No primary-source match → failed reason names the source so the - user knows which one returned nothing.""" + """No source returns a usable match → failed reason lists the + sources that were searched so user knows what was tried.""" spotify = _FakeSpotify(track_details=None, search_results=[]) deps = _build_deps( spotify=spotify, @@ -370,13 +385,14 @@ def test_no_match_anywhere_marked_failed(): assert payload['failed_count'] == 1 reason = payload['failed_tracks'][0]['reason'] - assert 'No usable spotify match' in reason - assert 'connect another metadata source' in reason + assert 'No usable match across' in reason + assert 'spotify' in reason + assert 'try connecting an additional metadata source' in reason -def test_no_match_without_spotify_uses_source_specific_reason(): - """When Spotify isn't connected and the active primary (e.g. iTunes) - finds nothing, the failure reason names iTunes specifically. Discord +def test_no_match_without_spotify_lists_searched_sources(): + """When Spotify isn't connected and the configured sources find + nothing, the failure reason lists every searched source. Discord case: user with no Spotify / Deezer saw enhance silently produce 'unknown artist - unknown album - unknown track' wishlist entries instead of a clear failure.""" @@ -393,8 +409,23 @@ def test_no_match_without_spotify_uses_source_specific_reason(): payload, _ = aq.enhance_artist_quality('artist-1', ['t1'], deps) assert payload['failed_count'] == 1 reason = payload['failed_tracks'][0]['reason'] - assert 'No usable itunes match' in reason - assert 'connect another metadata source' in reason + assert 'No usable match across' in reason + assert 'itunes' in reason + + +def test_no_match_with_no_sources_configured_prompts_setup(): + """User with literally zero metadata sources configured gets a + clear prompt to connect one — instead of a generic 'no match'.""" + deps = _build_deps( + spotify=None, + artist_detail=_artist_with_track(), + search_sources=[], + ) + + payload, _ = aq.enhance_artist_quality('artist-1', ['t1'], deps) + assert payload['failed_count'] == 1 + reason = payload['failed_tracks'][0]['reason'] + assert 'No metadata source configured' in reason def test_fallback_match_with_empty_artist_rejected(): diff --git a/web_server.py b/web_server.py index 0c212ff6..b8d87886 100644 --- a/web_server.py +++ b/web_server.py @@ -9403,6 +9403,37 @@ def _build_artist_quality_deps(): """Build the ArtistQualityDeps bundle from web_server.py globals on each call.""" from core.wishlist_service import get_wishlist_service as _get_ws + def _resolve_search_sources(): + """Mirror the Track Redownload modal's source list. Every + configured metadata source contributes to the parallel multi-source + search — Spotify (only when authenticated), iTunes, Deezer, plus + Discogs / Hydrabase when configured.""" + sources = [] + if spotify_client and spotify_client.is_authenticated(): + sources.append(('spotify', spotify_client)) + try: + sources.append(('itunes', _get_itunes_client())) + except Exception: + pass + try: + sources.append(('deezer', _get_deezer_client())) + except Exception: + pass + # Discogs needs an explicit token; only include when configured. + try: + _discogs_token = config_manager.get('discogs.token', '') + if _discogs_token: + sources.append(('discogs', _get_discogs_client(_discogs_token))) + except Exception: + pass + # Hydrabase only when connected (dev-mode + active client). + try: + if hydrabase_client and hydrabase_client.is_connected(): + sources.append(('hydrabase', hydrabase_client)) + except Exception: + pass + return sources + return _artists_quality.ArtistQualityDeps( spotify_client=spotify_client, matching_engine=matching_engine, @@ -9410,8 +9441,7 @@ def _build_artist_quality_deps(): get_wishlist_service=_get_ws, get_current_profile_id=get_current_profile_id, get_quality_tier_from_extension=_get_quality_tier_from_extension, - get_metadata_fallback_client=_get_metadata_fallback_client, - get_metadata_fallback_source=_get_metadata_fallback_source, + get_metadata_search_sources=_resolve_search_sources, ) @@ -11558,21 +11588,16 @@ def redownload_search_metadata(track_id): 'thumb_url': thumb_url, } - # Search all available metadata sources in parallel - from concurrent.futures import ThreadPoolExecutor, as_completed - from difflib import SequenceMatcher + # Search all available metadata sources in parallel via the + # shared module — same code path the Enhance Quality flow uses + # so both endpoints have identical scoring + per-source query + # optimization + current-match flagging. + from core.metadata.multi_source_search import ( + TrackQuery, + search_all_sources, + ) - def _score_metadata_match(result): - """Score a metadata result against the current track.""" - title_sim = SequenceMatcher(None, track_title.lower(), (result.get('name') or '').lower()).ratio() - artist_sim = SequenceMatcher(None, artist_name.lower(), (result.get('artist') or '').lower()).ratio() - dur_diff = abs((row['duration'] or 0) - (result.get('duration_ms') or 0)) - dur_score = max(0, 1 - dur_diff / 30000) if row['duration'] else 0.5 - return round((title_sim * 0.5 + artist_sim * 0.35 + dur_score * 0.15), 3) - - metadata_results = {} sources_to_search = [] - if spotify_client and spotify_client.is_authenticated(): sources_to_search.append(('spotify', spotify_client)) try: @@ -11584,65 +11609,23 @@ def redownload_search_metadata(track_id): except Exception as e: logger.debug(f"Deezer client not available for redownload search: {e}") - # Build source-optimized queries - deezer_query = f'artist:"{artist_name}" track:"{clean_title}"' - - def _search_source(source_name, client): - try: - # Deezer works best with structured artist:/track: queries - search_q = deezer_query if source_name == 'deezer' else query - logger.info(f"[Redownload] Searching {source_name} for: {search_q}") - track_objs = client.search_tracks(search_q, limit=10) - # If no results, try plain query as fallback - if not track_objs and search_q != query: - track_objs = client.search_tracks(query, limit=10) - # Last resort: title only - if not track_objs and clean_title != query: - track_objs = client.search_tracks(clean_title, limit=10) - logger.info(f"[Redownload] {source_name} returned {len(track_objs)} results") - results = [] - for t in track_objs: - r = { - 'id': str(t.id), - 'name': t.name, - 'artist': ', '.join(t.artists) if t.artists else '', - 'album': t.album or '', - 'duration_ms': t.duration_ms or 0, - 'image_url': t.image_url or '', - 'is_current_match': False, - } - # Flag current match - if source_name == 'spotify' and row['spotify_track_id'] and str(t.id) == str(row['spotify_track_id']): - r['is_current_match'] = True - elif source_name == 'deezer' and row['deezer_id'] and str(t.id) == str(row['deezer_id']): - r['is_current_match'] = True - r['match_score'] = _score_metadata_match(r) - results.append(r) - results.sort(key=lambda x: (-int(x['is_current_match']), -x['match_score'])) - return source_name, results - except Exception as e: - logger.error(f"[Redownload] Metadata search failed for {source_name}: {e}", exc_info=True) - return source_name, [] - - with ThreadPoolExecutor(max_workers=3) as pool: - futures = {pool.submit(_search_source, name, client): name for name, client in sources_to_search} - for future in as_completed(futures): - source_name, results = future.result() - metadata_results[source_name] = results - - # Find best overall match - best_match = None - for source, results in metadata_results.items(): - if results: - top = results[0] - if not best_match or top['match_score'] > best_match['score']: - best_match = {'source': source, 'index': 0, 'score': top['match_score']} + track_query = TrackQuery( + title=track_title, + artist=artist_name, + album=row['album_title'] or '', + duration_ms=row['duration'] or 0, + spotify_track_id=row['spotify_track_id'], + deezer_id=row['deezer_id'], + ) + result = search_all_sources( + track_query, sources_to_search, clean_title=clean_title, + ) return jsonify({ "success": True, "current_track": current_track, - "metadata_results": metadata_results, - "best_match": best_match, + "metadata_results": result.metadata_results, + "best_match": result.best_match, }) except Exception as e: logger.error(f"Error in redownload metadata search: {e}", exc_info=True) diff --git a/webui/static/helper.js b/webui/static/helper.js index 0907a7bd..e987ac9f 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -3432,7 +3432,7 @@ const WHATS_NEW = { '2.4.2': [ // --- post-2.4.1 dev work — entries hidden by _getLatestWhatsNewVersion until the build version bumps --- { date: 'Unreleased — 2.4.2 dev cycle' }, - { title: 'Fix: Enhance Quality Producing "Unknown" Wishlist Entries Without Spotify', desc: 'discord report: clicking enhance quality on an artist with neither spotify nor deezer connected added tracks to the wishlist as "unknown artist - unknown album - unknown track". root cause was structural: `core/artists/quality.py` had a hardcoded spotify-direct → spotify-search → itunes-fallback chain that ignored the user\'s configured primary metadata source. when spotify wasn\'t connected, every track fell through to a brittle itunes-only fallback that occasionally returned matches with empty fields (cleared the 0.7 confidence threshold but missing artist / album / title), and those empty strings propagated through the wishlist payload normalizer\'s truthy-check passthrough as "Unknown". rewrote the flow source-agnostic: it now resolves the user\'s active primary metadata source once via `_get_metadata_fallback_source()` and dispatches direct-lookup + search through whichever client (spotify / itunes / deezer / discogs / hydrabase) the user has configured. spotify-direct-lookup remains as a per-source optimization (only spotify exposes `get_track_details(id)` returning rich raw payload), but otherwise every metadata source is treated equally — discogs users get a discogs search, hydrabase users get a hydrabase search, etc. matches with empty title / album / artists are rejected before the wishlist add so users see a clear "no usable {source} match — connect another metadata source" error instead of junk entries. 6 new tests pin: empty-field rejection paths (3), primary-source dispatch (2), source-named failure reason (1).', page: 'library' }, + { title: 'Enhance Quality: Multi-Source Search Like Redownload', desc: 'discord report: enhance quality on an artist with neither spotify nor deezer connected added tracks as "unknown artist - unknown album - unknown track". enhance was running a single-source itunes fallback chain that returned junk matches with empty fields, while track redownload had been doing parallel multi-source search the whole time. extracted that search into `core/metadata/multi_source_search.py` and pointed both flows at it — enhance now searches spotify / itunes / deezer / discogs / hydrabase in parallel, picks the best match across all of them, and rejects empty-field matches before they hit the wishlist. users without spotify get the same coverage as users with it. failure message lists the sources that were searched so it\'s obvious what to connect if nothing matched.', page: 'library' }, { title: 'Internal: Media Server Engine Cin/JohnBaumb Pass', desc: 'internal — applied the same architectural cleanups the download engine PR went through to the media server engine PR before review. (1) every server client (Plex / Jellyfin / Navidrome / SoulSync) now explicitly inherits `MediaServerClient` instead of relying on structural typing — drift in any class fails at the conformance test boundary. (2) generic accessors on the engine: `configured_clients()` (replaces per-server `if X and X.is_connected(): clients[name] = X` chains in web_server.py) and `reload_config(name=None)` (generic dispatch instead of per-client reload calls). (3) singleton factory: `get_media_server_engine()` / `set_media_server_engine()` matching the metadata + download engine shape. web_server.py boots via `set_media_server_engine(...)` so factory + global handle share state. (4) ~70 direct `plex_client.X` / `jellyfin_client.X` / `navidrome_client.X` / `soulsync_library_client.X` attribute reaches in web_server.py migrated to `media_server_engine.client(\'\').X`. ~60 standalone refs (truthy checks, media_client assignments, source-name tuples) also routed through the engine. (5) the per-server `plex_client` / `jellyfin_client` / `navidrome_client` / `soulsync_library_client` globals in web_server.py are gone entirely — engine owns the client instances now, every caller reaches via `media_server_engine.client(\'\')`. four multi-client consumers (`PlaylistSyncService`, `ListeningStatsWorker`, `WebScanManager`, discovery `SyncDeps`) refactored to take the engine instead of separate per-server kwargs. (6) `TrackInfo` and `PlaylistInfo` lifted out of `core/plex_client.py` / `jellyfin_client.py` / `navidrome_client.py` (each was defining a near-identical copy) into the neutral `core/media_server/types.py` module — same lift Cin caught on the download `TrackResult`/`AlbumResult`/`DownloadStatus` situation. consumers (matching engine, sync service) get one import. zero behavior change.' }, { title: 'Internal: Media Server Engine Foundation', desc: 'internal — companion to the download engine refactor. introduces a media-server engine + plugin contract on top of the four server clients (plex / jellyfin / navidrome / soulsync standalone). pre-refactor web_server.py held four separate per-server client globals that every dispatch site reached individually. new `core/media_server/` package provides `MediaServerEngine` that owns the per-server clients + a small set of generic accessors (`client(name)`, `active_client()`, `configured_clients()`, `reload_config(name)`) so call sites use one canonical lookup pattern. plugin contract requires the four methods every server actually implements (is_connected, ensure_connection, get_all_artists, get_all_album_ids) — methods that exist on most-but-not-all servers (search_tracks, trigger_library_scan, get_library_stats, etc.) are listed in `KNOWN_PER_SERVER_METHODS` for discoverability and reached directly via `engine.client(name).` since there\'s no uniform safe-default that fits every method. honest scope: the four uniform-shape `is_connected` dispatches were lifted into `engine.is_connected()` (the one cross-server wrapper kept on the engine); the ~18 server-specific chains that do genuinely different per-server work (playlist track replace, metadata sync, scan strategies) stay explicit at the call site per the "lift what\'s truly shared" standard, but reach the per-server client through the engine. 42 tests pin: per-server observable behavior (4 server pinning files, 20 tests), engine surface + accessor contracts (15 tests), structural conformance + explicit-inheritance (9 tests). zero behavior change for users.' }, { title: 'Drop SoundCloud Preview Snippets at Search Time', desc: 'soundcloud serves a ~30s preview clip for tracks that are gated behind go+ / login (very common for major-label uploads — official content basically doesn\'t exist on soundcloud, so what shows up is bootlegs, fan uploads, type beats, and these 30s previews). yt-dlp accepts the preview as the download payload, the post-download integrity check catches the duration mismatch and quarantines the file, but the user just sees "all candidates failed" with no explanation. previews also showed up in the candidate-review modal where clicking one bypassed validation and downloaded the same broken file. now `filter_soundcloud_previews` drops these candidates at every entry point — auto-search scoring, modal-cache fallback, AND the not-found raw-results path — so previews never reach the matcher OR the user. drops candidates < 35s or below half the expected duration, gated on expected being non-trivially long (>60s) so genuine short tracks still pass. also fixed a silent regression in the hybrid-fallback path where the per-source attribute removal left `getattr(orch, \'youtube\', None)` returning None for every source — fallback never fired. now resolves through the orchestrator\'s `client(name)` accessor.', page: 'downloads' }, @@ -3775,6 +3775,17 @@ const WHATS_NEW = { // Section shape: { title, description, features: [bullet strings], // usage_note?: 'optional hint shown at the bottom' } const VERSION_MODAL_SECTIONS = [ + { + title: "Enhance Quality Now Searches Every Source", + description: "discord report — clicking enhance quality on an artist with no spotify/deezer was adding tracks as \"unknown artist - unknown album - unknown track\".", + features: [ + "• enhance was running a single-source itunes fallback chain that returned junk matches with empty fields", + "• track redownload had been doing parallel multi-source search the whole time — same question, different answer", + "• extracted that search into a shared module; both flows now hit spotify / itunes / deezer / discogs / hydrabase in parallel and pick the cross-source best match", + "• empty-field matches rejected before the wishlist add", + "• failure message lists the sources that were searched so it\'s obvious what to connect when nothing matched", + ], + }, { title: "Watchlist No Longer Re-Downloads Compilations", description: "compilation / soundtrack tracks were getting redownloaded on every watchlist scan because the album-name fuzzy check failed on naming drift between spotify and your media server.",