From 843de8a45ef3fe150e7e816fd9f200ab281ada18 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Wed, 3 Jun 2026 16:26:29 -0700 Subject: [PATCH] Similar Artists worker: guarantee every stored similar has a source id Verified against live data: 1312/1313 stored similars carry a metadata source id, but 1 slipped through name-only (a match on a source with no id column, e.g. discogs). Enforce the standard: process_artist now SKIPS any similar whose match doesn't map to a storable id column (spotify/itunes/deezer/musicbrainz) instead of writing a useless name-only row. Regression test covers discogs-match + no-id cases. Now 100% of newly-stored similars are actionable. --- core/similar_artists_worker.py | 7 +++++++ tests/test_similar_artists_worker.py | 16 ++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/core/similar_artists_worker.py b/core/similar_artists_worker.py index 283e05a5..282ae360 100644 --- a/core/similar_artists_worker.py +++ b/core/similar_artists_worker.py @@ -108,6 +108,13 @@ def process_artist( if not name: continue kwargs = map_payload_to_store_kwargs(s) + if not kwargs: + # The match resolved to a source with no id column in similar_artists + # (e.g. discogs). Storing it name-only would be useless — you can't + # navigate/explore/download it. Enforce the standard: every stored + # similar carries a metadata source id, or we skip it. + logger.debug("Skipping similar '%s' (matched %s — no storable source id)", name, s.get('source')) + continue try: ok = store_similar( source_artist_id=source_artist_id, diff --git a/tests/test_similar_artists_worker.py b/tests/test_similar_artists_worker.py index 364f0f14..3f496041 100644 --- a/tests/test_similar_artists_worker.py +++ b/tests/test_similar_artists_worker.py @@ -115,6 +115,22 @@ def test_process_artist_error_when_fetch_raises_carries_detail(): assert status == 'error' and count == 0 and 'musicmap down' in detail +def test_process_artist_skips_similars_without_a_storable_source_id(): + # Every stored similar MUST carry a metadata source id (spotify/itunes/deezer/ + # musicbrainz) — otherwise it's not actionable. A match on a source with no id + # column (e.g. discogs) is skipped, never stored name-only. + store, calls = _capture_store() + payload = {'success': True, 'similar_artists': [ + {'name': 'KeepMe', 'id': 'sp1', 'source': 'spotify'}, + {'name': 'DropMe', 'id': 'dg1', 'source': 'discogs'}, # no id column → must skip + {'name': 'NoId', 'source': 'spotify'}, # no id at all → must skip + ]} + status, count, _ = w.process_artist('S', 'A', lambda n, l: payload, store) + assert count == 1 and len(calls) == 1 + assert calls[0]['similar_artist_name'] == 'KeepMe' + assert calls[0]['similar_artist_spotify_id'] == 'sp1' + + def test_process_artist_skips_unstorable_but_counts_real(): # A store() that fails for one row shouldn't abort the rest. calls = []