danvics/soulsync
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1
soulsync/tests/artists/test_quality.py
Broque Thomas 4a27f3c245 Source-agnostic Enhance Quality flow + reject empty matches 
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 an iTunes-only fallback that
occasionally returned matches with empty fields (cleared the 0.7
confidence threshold but missing artist / album / title). Those
empty strings propagated through the wishlist payload normalizer's
truthy-check passthrough at core/wishlist/payloads.py:77-80 and the
UI rendered them as "Unknown" defaults.

Rewrote the flow source-agnostic:

- ArtistQualityDeps gains get_metadata_fallback_source. Flow resolves
  the user's active primary source once up front.
- New _build_payload_from_track helper produces the Spotify-shaped
  wishlist payload from any source's Track object — single place
  that knows how to construct it (replaces the duplicate construction
  in the Spotify-search and iTunes-fallback paths).
- New _search_match helper does generic confidence-scored search
  against any client implementing search_tracks(query, limit). Same
  0.7 threshold, same album-bonus weighting as before.
- New _has_complete_metadata validator rejects matches with empty
  title / album / artists before they reach the wishlist.
- _spotify_direct_lookup kept as a Spotify-only optimization (only
  Spotify exposes get_track_details(id) returning rich raw payload);
  other sources fall through to search.
- Failure reason now names the active source: "No usable {source}
  match — connect another metadata source for better coverage".

Result: Discogs users get a Discogs search. Hydrabase users get a
Hydrabase search. iTunes users get an iTunes search with empty-field
rejection. Spotify keeps its direct-lookup fast path.

6 new tests pin the architectural change:
- Primary-source dispatch routes to the configured client (Discogs,
  not Spotify) when Spotify isn't primary
- Spotify direct-lookup is gated on Spotify being the active primary
  (skipped when Discogs is configured even if track has spotify_track_id)
- Empty title / album / artists fields all reject the match
- Failure reason names the active source
2026-05-06 10:13:26 -07:00

506 lines
18 KiB
Python
Raw Blame History

"""Tests for core/artists/quality.py — artist quality enhancement helper."""
from __future__ import annotations
from dataclasses import dataclass
import pytest
from core.artists import quality as aq
# ---------------------------------------------------------------------------
# Fakes
# ---------------------------------------------------------------------------
@dataclass
class _SpotifyTrack:
id: str = 'sp-1'
name: str = 'Found'
artists: list = None
album: str = 'Album'
duration_ms: int = 200000
image_url: str = ''
popularity: int = 50
preview_url: str = ''
external_urls: dict = None
album_type: str = 'album'
release_date: str = '2024-01-01'
def __post_init__(self):
if self.artists is None:
self.artists = ['Artist Name']
if self.external_urls is None:
self.external_urls = {}
class _FakeSpotify:
def __init__(self, track_details=None, search_results=None, album=None):
self._track_details = track_details
self._search_results = search_results or []
self._album = album
self.search_calls = []
def get_track_details(self, track_id):
return self._track_details
def get_album(self, album_id):
return self._album
def search_tracks(self, query, limit=5):
self.search_calls.append((query, limit))
return self._search_results
class _FakeMatchingEngine:
def generate_download_queries(self, track):
return [f"{track.artists[0]} {track.name}"]
def normalize_string(self, s):
return (s or '').lower().strip()
def similarity_score(self, a, b):
if a == b:
return 1.0
if not a or not b:
return 0.0
return 0.95 if a in b or b in a else 0.0
class _FakeWishlist:
def __init__(self):
self.added = []
def add_spotify_track_to_wishlist(self, **kwargs):
self.added.append(kwargs)
return True
class _FakeDatabase:
def __init__(self, artist_detail=None):
self._artist_detail = artist_detail or {'success': False}
def get_artist_full_detail(self, artist_id):
return self._artist_detail
def _build_deps(
*,
spotify=None,
matching_engine=None,
artist_detail=None,
wishlist=None,
fallback_client=None,
fallback_source=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
deps = aq.ArtistQualityDeps(
spotify_client=spotify,
matching_engine=matching_engine or _FakeMatchingEngine(),
get_database=lambda: _FakeDatabase(artist_detail=artist_detail),
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,
)
return deps
def _artist_with_track(*, track_id='t1', file_path='/file.mp3', spotify_tid=None):
return {
'success': True,
'artist': {'name': 'Artist Name'},
'albums': [{
'id': 'a1',
'title': 'Album X',
'tracks': [{
'id': track_id,
'title': 'Track One',
'file_path': file_path,
'spotify_track_id': spotify_tid,
'track_number': 1,
'duration': 180000,
'bitrate': 320,
}],
}],
}
# ---------------------------------------------------------------------------
# Input validation
# ---------------------------------------------------------------------------
def test_no_track_ids_returns_400():
deps = _build_deps()
payload, status = aq.enhance_artist_quality('artist-1', [], deps)
assert status == 400
assert payload == {"success": False, "error": "No track IDs provided"}
def test_artist_not_found_returns_404():
deps = _build_deps(artist_detail={'success': False})
payload, status = aq.enhance_artist_quality('artist-x', ['t1'], deps)
assert status == 404
assert payload == {"success": False, "error": "Artist not found"}
# ---------------------------------------------------------------------------
# Spotify direct lookup (priority 1)
# ---------------------------------------------------------------------------
def test_spotify_direct_lookup_via_track_id_uses_raw_data():
"""Track has spotify_track_id → get_track_details, raw_data fed to wishlist."""
raw = {'id': 'sp-stored', 'name': 'Track One', 'artists': [{'name': 'Artist Name'}],
'album': {'name': 'Album X', 'images': [{'url': 'http://i'}]}, 'duration_ms': 180000}
spotify = _FakeSpotify(track_details={'raw_data': raw})
wishlist = _FakeWishlist()
deps = _build_deps(
spotify=spotify,
artist_detail=_artist_with_track(spotify_tid='sp-stored'),
wishlist=wishlist,
)
payload, status = aq.enhance_artist_quality('artist-1', ['t1'], deps)
assert status == 200
assert payload['enhanced_count'] == 1
assert wishlist.added[0]['spotify_track_data'] == raw
def test_spotify_direct_lookup_enhanced_format_rebuilds_payload():
"""Track details without raw_data → rebuild payload with album images via get_album."""
enhanced = {'name': 'Track One', 'artists': ['Artist Name'],
'album': {'id': 'alb-id', 'name': 'Album X'},
'duration_ms': 180000, 'track_number': 1, 'disc_number': 1}
full_album = {'images': [{'url': 'http://art'}]}
spotify = _FakeSpotify(track_details=enhanced, album=full_album)
wishlist = _FakeWishlist()
deps = _build_deps(
spotify=spotify,
artist_detail=_artist_with_track(spotify_tid='sp-stored'),
wishlist=wishlist,
)
payload, _ = aq.enhance_artist_quality('artist-1', ['t1'], deps)
assert payload['enhanced_count'] == 1
md = wishlist.added[0]['spotify_track_data']
assert md['album']['images'] == [{'url': 'http://art'}]
# ---------------------------------------------------------------------------
# Spotify search fallback (priority 2)
# ---------------------------------------------------------------------------
def test_spotify_search_fallback_when_no_stored_id():
"""No spotify_track_id → search via matching_engine, pick best match."""
track = _SpotifyTrack(name='Track One', artists=['Artist Name'])
raw = {'id': 'sp-search', 'name': 'Track One', 'artists': [{'name': 'Artist Name'}],
'album': {'name': 'Album X'}}
spotify = _FakeSpotify(track_details={'raw_data': raw}, search_results=[track])
wishlist = _FakeWishlist()
deps = _build_deps(
spotify=spotify,
artist_detail=_artist_with_track(spotify_tid=None),
wishlist=wishlist,
)
payload, _ = aq.enhance_artist_quality('artist-1', ['t1'], deps)
assert payload['enhanced_count'] == 1
assert wishlist.added[0]['spotify_track_data'] == raw
# ---------------------------------------------------------------------------
# Fallback source (iTunes/Deezer)
# ---------------------------------------------------------------------------
def test_fallback_source_when_spotify_none():
"""Spotify client None → iTunes/fallback search runs."""
fallback_track = _SpotifyTrack(id='it-1', name='Track One', artists=['Artist Name'],
image_url='http://it')
fallback_track.track_number = 1
fallback_track.disc_number = 1
fallback = type('FB', (), {
'search_tracks': lambda self, q, limit=5: [fallback_track],
})()
wishlist = _FakeWishlist()
deps = _build_deps(
spotify=None,
artist_detail=_artist_with_track(),
wishlist=wishlist,
fallback_client=fallback,
)
payload, _ = aq.enhance_artist_quality('artist-1', ['t1'], deps)
assert payload['enhanced_count'] == 1
md = wishlist.added[0]['spotify_track_data']
assert md['id'] == 'it-1'
assert md['album']['images'] == [{'url': 'http://it', 'height': 600, 'width': 600}]
def test_dispatches_through_primary_source_not_spotify_specific():
"""Architectural pin: when the user's primary metadata source is
Discogs (or any non-Spotify source), the enhance flow searches
THAT source's client, not Spotify. Pre-fix the flow had a
hardcoded Spotify-direct → Spotify-search → iTunes-fallback chain
that ignored the user's actual configured primary source.
"""
discogs_track = _SpotifyTrack(id='dc-1', name='Track One',
artists=['Artist Name'])
discogs_track.track_number = 1
discogs_track.disc_number = 1
discogs_track.album = 'Album X'
discogs_calls = []
class _DiscogsStub:
def search_tracks(self, q, limit=5):
discogs_calls.append(q)
return [discogs_track]
wishlist = _FakeWishlist()
deps = _build_deps(
spotify=None,
artist_detail=_artist_with_track(),
wishlist=wishlist,
fallback_client=_DiscogsStub(),
fallback_source='discogs',
)
payload, _ = aq.enhance_artist_quality('artist-1', ['t1'], deps)
# Discogs (the configured primary) was the one searched; match queued.
assert discogs_calls, "Primary source (Discogs) was not searched"
assert payload['enhanced_count'] == 1
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.
"""
spotify_calls = []
class _SpotifyStub:
def get_track_details(self, tid):
spotify_calls.append(('details', tid))
return None
def search_tracks(self, q, limit=5):
spotify_calls.append(('search', q))
return []
discogs_track = _SpotifyTrack(id='dc-1', name='Track One',
artists=['Artist Name'])
discogs_track.track_number = 1
discogs_track.disc_number = 1
discogs_track.album = 'Album X'
class _DiscogsStub:
def search_tracks(self, q, limit=5):
return [discogs_track]
wishlist = _FakeWishlist()
deps = _build_deps(
spotify=_SpotifyStub(),
artist_detail=_artist_with_track(spotify_tid='sp-stored'),
wishlist=wishlist,
fallback_client=_DiscogsStub(),
fallback_source='discogs',
)
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.
assert payload['enhanced_count'] == 1
# ---------------------------------------------------------------------------
# Failure modes
# ---------------------------------------------------------------------------
def test_track_not_in_artist_detail_marked_failed():
"""Track ID provided but missing from artist's albums → failed_tracks entry."""
deps = _build_deps(artist_detail=_artist_with_track(track_id='t1'))
payload, _ = aq.enhance_artist_quality('artist-1', ['t99'], deps)
assert payload['enhanced_count'] == 0
assert payload['failed_count'] == 1
assert payload['failed_tracks'][0]['reason'] == 'Track not found'
def test_track_with_no_file_path_marked_failed():
"""Track has no file_path → failed reason 'No file path'."""
detail = _artist_with_track()
detail['albums'][0]['tracks'][0]['file_path'] = None
deps = _build_deps(artist_detail=detail)
payload, _ = aq.enhance_artist_quality('artist-1', ['t1'], deps)
assert payload['failed_count'] == 1
assert payload['failed_tracks'][0]['reason'] == 'No file path'
def test_no_match_anywhere_marked_failed():
"""No primary-source match → failed reason names the source so the
user knows which one returned nothing."""
spotify = _FakeSpotify(track_details=None, search_results=[])
deps = _build_deps(
spotify=spotify,
artist_detail=_artist_with_track(),
)
payload, _ = aq.enhance_artist_quality('artist-1', ['t1'], deps)
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
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
case: user with no Spotify / Deezer saw enhance silently produce
'unknown artist - unknown album - unknown track' wishlist entries
instead of a clear failure."""
fallback = type('FB', (), {
'search_tracks': lambda self, q, limit=5: [],
})()
deps = _build_deps(
spotify=None,
artist_detail=_artist_with_track(),
fallback_client=fallback,
fallback_source='itunes',
)
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
def test_fallback_match_with_empty_artist_rejected():
"""Per the user-reported bug: an iTunes match that clears the 0.7
confidence threshold but has empty/missing artists is rejected
instead of producing a wishlist entry with empty artist field
(which the wishlist payload normalizer happily accepts and the
UI then displays as 'unknown artist')."""
fallback_track = _SpotifyTrack(id='it-empty', name='Track One',
artists=[], # empty artists list
image_url='')
fallback_track.track_number = 1
fallback_track.disc_number = 1
fallback_track.album = 'Album X'
fallback = type('FB', (), {
'search_tracks': lambda self, q, limit=5: [fallback_track],
})()
wishlist = _FakeWishlist()
deps = _build_deps(
spotify=None,
artist_detail=_artist_with_track(),
wishlist=wishlist,
fallback_client=fallback,
)
payload, _ = aq.enhance_artist_quality('artist-1', ['t1'], deps)
# No wishlist entry with empty fields — match was rejected.
assert payload['enhanced_count'] == 0
assert payload['failed_count'] == 1
assert wishlist.added == []
def test_fallback_match_with_empty_album_rejected():
"""Empty album field on iTunes match → reject (was producing
'unknown album' wishlist entries)."""
fallback_track = _SpotifyTrack(id='it-no-album', name='Track One',
artists=['Artist Name'])
fallback_track.track_number = 1
fallback_track.disc_number = 1
fallback_track.album = '' # empty
fallback = type('FB', (), {
'search_tracks': lambda self, q, limit=5: [fallback_track],
})()
wishlist = _FakeWishlist()
deps = _build_deps(
spotify=None,
artist_detail=_artist_with_track(),
wishlist=wishlist,
fallback_client=fallback,
)
payload, _ = aq.enhance_artist_quality('artist-1', ['t1'], deps)
assert payload['enhanced_count'] == 0
assert payload['failed_count'] == 1
assert wishlist.added == []
def test_fallback_match_with_empty_name_rejected():
"""Empty title on iTunes match → reject."""
fallback_track = _SpotifyTrack(id='it-no-name', name='',
artists=['Artist Name'])
fallback_track.track_number = 1
fallback_track.disc_number = 1
fallback_track.album = 'Album X'
fallback = type('FB', (), {
'search_tracks': lambda self, q, limit=5: [fallback_track],
})()
wishlist = _FakeWishlist()
deps = _build_deps(
spotify=None,
artist_detail=_artist_with_track(),
wishlist=wishlist,
fallback_client=fallback,
)
payload, _ = aq.enhance_artist_quality('artist-1', ['t1'], deps)
assert payload['enhanced_count'] == 0
assert payload['failed_count'] == 1
assert wishlist.added == []
# ---------------------------------------------------------------------------
# Wishlist source_context payload
# ---------------------------------------------------------------------------
def test_wishlist_source_context_carries_quality_metadata():
"""source_context includes original_file_path, format tier, bitrate, artist_name."""
raw = {'id': 'sp-1', 'name': 'Track One', 'artists': [{'name': 'Artist Name'}],
'album': {'name': 'Album X'}}
spotify = _FakeSpotify(track_details={'raw_data': raw})
wishlist = _FakeWishlist()
deps = _build_deps(
spotify=spotify,
artist_detail=_artist_with_track(spotify_tid='sp-1'),
wishlist=wishlist,
quality_tier=('mp3_192', 4),
)
aq.enhance_artist_quality('artist-1', ['t1'], deps)
ctx = wishlist.added[0]['source_context']
assert ctx['enhance'] is True
assert ctx['original_file_path'] == '/file.mp3'
assert ctx['original_format'] == 'mp3_192'
assert ctx['original_bitrate'] == 320
assert ctx['original_tier'] == 4
assert ctx['artist_name'] == 'Artist Name'
assert wishlist.added[0]['source_type'] == 'enhance'
Reference in a new issue View git blame Copy permalink