From 24c2d75c6d9830a6b7a44d4cc2065f0932fef0c0 Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Sat, 2 May 2026 18:20:32 -0700 Subject: [PATCH] Make extract_external_ids recognize all source-tagging conventions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- core/library/track_identity.py | 25 ++++++++++++++++------ core/watchlist_scanner.py | 11 +++++++++- tests/test_library_track_identity.py | 32 ++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 7 deletions(-) diff --git a/core/library/track_identity.py b/core/library/track_identity.py index 584fba45..3d2ff982 100644 --- a/core/library/track_identity.py +++ b/core/library/track_identity.py @@ -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 = { diff --git a/core/watchlist_scanner.py b/core/watchlist_scanner.py index 50bda80b..e7528697 100644 --- a/core/watchlist_scanner.py +++ b/core/watchlist_scanner.py @@ -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, diff --git a/tests/test_library_track_identity.py b/tests/test_library_track_identity.py index a7e41a7d..b1e24923 100644 --- a/tests/test_library_track_identity.py +++ b/tests/test_library_track_identity.py @@ -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):