Make extract_external_ids recognize all source-tagging conventions
Smoke-testing the just-merged provenance PR against live logs revealed the new ID-match block was silently no-opping: no [ExtID Match] / [Provenance Match] log lines despite the code path being live. Tracing revealed two related gaps in extract_external_ids' source detection: 1. **Underscore-prefixed key.** Deezer / Discogs / Hydrabase clients tag normalized track dicts with ``_source`` (underscore prefix — convention used in 8+ places across core/). The extractor only looked for ``provider`` and ``source``, so Deezer-sourced tracks silently returned no IDs. 2. **No provider field at all.** Spotify and iTunes raw API responses carry ``id`` but no provider/source key of any kind. The extractor couldn't disambiguate the native ``id``, so Spotify-primary scans would have hit the same silent miss once the user switched primary sources. Two-part fix: - ``extract_external_ids`` now recognizes ``_source`` as another candidate provider field. - New optional ``source_hint`` parameter lets the caller supply the configured primary source as a fallback when the track dict has no provider field of its own. Track-side provider field still wins when present (defensive against a wrong hint). Watchlist scanner now passes ``get_primary_source()`` as the hint so both naming conventions (Deezer-style _source, Spotify-style no-tag) get handled uniformly. 6 new regression tests cover: - _source recognized for Deezer - _source recognized for Hydrabase (cross-provider mapping) - _source recognized for Discogs (no library column — verifies graceful no-crash) - source_hint disambiguates raw tracks for spotify/itunes/deezer - track-side provider takes precedence over hint - None hint defaults safely Full pytest 1630 passed; ruff clean. After this lands and the server restarts, watchlist scans should produce [ExtID Match] / [Provenance Match] log lines for tracks already on disk regardless of which metadata source the user has configured as primary.
This commit is contained in:
parent
59b8f8c199
commit
24c2d75c6d
3 changed files with 61 additions and 7 deletions
|
|
@ -67,15 +67,23 @@ def _get(track: Any, *names: str) -> Optional[str]:
|
|||
return None
|
||||
|
||||
|
||||
def extract_external_ids(track: Any) -> Dict[str, str]:
|
||||
def extract_external_ids(track: Any, source_hint: Optional[str] = None) -> Dict[str, str]:
|
||||
"""Pull every recognized external ID off a metadata-source track.
|
||||
|
||||
Handles the source-source naming drift: Spotify tracks expose ``id``
|
||||
as the Spotify track ID; Deezer tracks expose ``id`` as the Deezer
|
||||
track ID; iTunes tracks may use ``trackId`` or ``id``. The disamb-
|
||||
iguating field is ``provider`` / ``source``. Tracks coming from a
|
||||
SoulSync internal pipeline often carry every known ID set to its
|
||||
source-specific value — we just collect whatever's there.
|
||||
iguating field is ``provider`` / ``source`` / ``_source``. Tracks
|
||||
coming from a SoulSync internal pipeline often carry every known ID
|
||||
set to its source-specific value — we just collect whatever's there.
|
||||
|
||||
``source_hint`` is the caller's known answer to "where did this
|
||||
track dict come from?" — used as a fallback when the track itself
|
||||
doesn't carry a provider / source / _source field. Spotify and
|
||||
iTunes return raw API responses without provider tags, so the
|
||||
watchlist scanner passes ``get_primary_source()`` here to make sure
|
||||
a Spotify-primary scan isn't silently no-opping just because the
|
||||
raw API track has no provider key.
|
||||
|
||||
Returns a dict mapping conceptual ID name → ID value. Keys present
|
||||
in ``EXTERNAL_ID_COLUMNS``. Empty dict when no IDs are available.
|
||||
|
|
@ -106,8 +114,13 @@ def extract_external_ids(track: Any) -> Dict[str, str]:
|
|||
|
||||
# Provider field tells us which native ``id`` belongs to. Without
|
||||
# this, a Deezer track's ``id`` field would be silently ignored
|
||||
# (we wouldn't know to map it to deezer_id).
|
||||
provider = (_get(track, 'provider', 'source') or '').lower()
|
||||
# (we wouldn't know to map it to deezer_id). Convention varies by
|
||||
# client: Spotify-shaped tracks usually have no provider field,
|
||||
# Deezer / Discogs / Hydrabase clients tag tracks with ``_source``,
|
||||
# internal pipeline normalization may use ``source`` or ``provider``.
|
||||
# Fall back to the caller's source_hint when the track has no
|
||||
# provider field of its own (Spotify / iTunes raw API responses).
|
||||
provider = (_get(track, 'provider', 'source', '_source') or source_hint or '').lower()
|
||||
native_id = _get(track, 'id')
|
||||
if native_id and provider:
|
||||
provider_to_key = {
|
||||
|
|
|
|||
|
|
@ -2077,7 +2077,16 @@ class WatchlistScanner:
|
|||
find_provenance_by_external_id,
|
||||
)
|
||||
import os as _os_local
|
||||
source_ids = extract_external_ids(track)
|
||||
# Pass the configured primary source as a hint so the
|
||||
# extractor can disambiguate raw Spotify / iTunes API
|
||||
# responses that don't carry a provider / source field
|
||||
# of their own (Deezer / Discogs / Hydrabase clients
|
||||
# already tag tracks with _source).
|
||||
try:
|
||||
_source_hint = get_primary_source()
|
||||
except Exception:
|
||||
_source_hint = None
|
||||
source_ids = extract_external_ids(track, source_hint=_source_hint)
|
||||
if source_ids:
|
||||
matched = find_library_track_by_external_id(
|
||||
self.database,
|
||||
|
|
|
|||
|
|
@ -99,11 +99,43 @@ class TestExtractExternalIdsFromProviderField:
|
|||
track = {'source': 'deezer', 'id': 'dz1', 'name': 'Hello'}
|
||||
assert extract_external_ids(track) == {'deezer_id': 'dz1'}
|
||||
|
||||
def test_underscore_source_field_treated_same_as_provider(self):
|
||||
"""Deezer / Discogs / Hydrabase clients tag normalized tracks
|
||||
with ``_source`` (underscore prefix). Must be recognized as the
|
||||
provider disambiguator — otherwise watchlist scans against those
|
||||
sources silently miss the ID match and fall through to fuzzy."""
|
||||
track = {'_source': 'deezer', 'id': 'dz1', 'name': 'Hello'}
|
||||
assert extract_external_ids(track) == {'deezer_id': 'dz1'}
|
||||
|
||||
def test_underscore_source_hydrabase(self):
|
||||
track = {'_source': 'hydrabase', 'id': 'hyd-soul-1', 'name': 'Hello'}
|
||||
assert extract_external_ids(track) == {'soul_id': 'hyd-soul-1'}
|
||||
|
||||
def test_native_id_without_provider_is_ignored(self):
|
||||
"""Without a provider field we can't tell which source 'id' belongs to."""
|
||||
track = {'id': 'unknown', 'name': 'Hello'}
|
||||
assert extract_external_ids(track) == {}
|
||||
|
||||
def test_source_hint_disambiguates_when_track_has_no_provider(self):
|
||||
"""Spotify and iTunes raw API responses don't carry a provider /
|
||||
source / _source field. The watchlist scanner passes the
|
||||
configured primary source as a hint so the extractor can map
|
||||
the track's native ``id`` field correctly."""
|
||||
track = {'id': 'sp1', 'name': 'Hello'} # No provider field
|
||||
assert extract_external_ids(track, source_hint='spotify') == {'spotify_id': 'sp1'}
|
||||
assert extract_external_ids(track, source_hint='itunes') == {'itunes_id': 'sp1'}
|
||||
assert extract_external_ids(track, source_hint='deezer') == {'deezer_id': 'sp1'}
|
||||
|
||||
def test_source_hint_does_not_override_track_provider(self):
|
||||
"""If the track already carries an explicit provider, that wins
|
||||
over the hint — defensive against the hint being wrong."""
|
||||
track = {'_source': 'deezer', 'id': 'dz1', 'name': 'Hello'}
|
||||
assert extract_external_ids(track, source_hint='spotify') == {'deezer_id': 'dz1'}
|
||||
|
||||
def test_source_hint_none_is_ignored(self):
|
||||
track = {'id': 'unknown', 'name': 'Hello'}
|
||||
assert extract_external_ids(track, source_hint=None) == {}
|
||||
|
||||
|
||||
class TestExtractExternalIdsMixedAndDefensive:
|
||||
def test_track_with_multiple_provider_specific_fields(self):
|
||||
|
|
|
|||
Loading…
Reference in a new issue