From 9bb73c4cb4744014c0811a5ab14c4d15458cc804 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sun, 28 Jun 2026 23:10:11 -0700 Subject: [PATCH] fix(#945): Spotify backfill must disable cross-service search fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Double-checking the backfill logic found a real correctness bug. Spotify search_tracks defaults to allow_fallback=True, so when Spotify is rate-limited or in free mode it returns iTunes/Deezer tracks whose .id is an iTunes/Deezer id, NOT a Spotify id. The backfill took that .id as a Spotify track id and would push wrong/garbage tracks into the exported Spotify playlist. The unit tests used fake Track objects with hand-set ids, so they could never surface this cross-service contamination. Fix: the Spotify backfill search now passes allow_fallback=False — real Spotify hits or nothing (an unmatched track is left out, never replaced by a non-Spotify id). Deezer is unaffected: its search fallback is query-only and stays within Deezer, so its ids are always Deezer ids. Regression test asserts the Spotify backfill search is invoked with allow_fallback=False. 8 orchestration tests green, ruff clean. --- tests/test_service_playlist_export.py | 18 ++++++++++++++++++ web_server.py | 15 ++++++++++++--- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/tests/test_service_playlist_export.py b/tests/test_service_playlist_export.py index f2571aa2..e9bc7b8d 100644 --- a/tests/test_service_playlist_export.py +++ b/tests/test_service_playlist_export.py @@ -110,3 +110,21 @@ def test_backfill_on_wires_search_fn(monkeypatch): _FakeClient({'success': True, 'playlist_id': 'p', 'added': 1}), _fake_resolver(['sx'], seen)) assert seen['search_id_fn'] == 'SEARCH_FN' + + +def test_spotify_backfill_search_disables_cross_service_fallback(monkeypatch): + """REGRESSION: Spotify's search_tracks falls back to iTunes/Deezer (non-Spotify ids) under + rate-limit/free. The backfill MUST disable that or it pushes wrong ids into the Spotify + playlist. Assert the search is invoked with allow_fallback=False.""" + seen = {} + + class _FakeSpotify: + def search_tracks(self, q, limit=10, allow_fallback=True): + seen['allow_fallback'] = allow_fallback + return [] + + monkeypatch.setattr(ws, 'get_spotify_client', lambda: _FakeSpotify()) + fn = ws._build_service_search_id_fn('spotify') + assert fn is not None + fn('Kendrick Lamar', 'Not Like Us') # drives the search + assert seen['allow_fallback'] is False diff --git a/web_server.py b/web_server.py index 49825799..fb59a315 100644 --- a/web_server.py +++ b/web_server.py @@ -27749,12 +27749,21 @@ def _build_service_search_id_fn(service): try: if service == 'spotify': client = get_spotify_client() + if not client: + return None + # allow_fallback=False is CRITICAL: Spotify's search falls back to iTunes/Deezer + # when rate-limited/free, returning tracks whose .id is NOT a Spotify id — backfill + # would then push wrong ids into the Spotify playlist. Disable it: real Spotify hits + # or nothing. + search_fn = lambda q: client.search_tracks(q, limit=8, allow_fallback=False) # noqa: E731 else: from core.metadata.registry import get_deezer_client client = get_deezer_client() - if not client: - return None - search_fn = lambda q: client.search_tracks(q, limit=8) # noqa: E731 + if not client: + return None + # Deezer's search stays within Deezer (query-only fallback), so its .id is always a + # Deezer id — no cross-service guard needed. + search_fn = lambda q: client.search_tracks(q, limit=8) # noqa: E731 return lambda artist, title: search_service_track_id(artist, title, search_fn=search_fn) except Exception: return None