From 482d5fbc794eafbb9ccd6f8a0f74432a84802265 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sun, 31 May 2026 23:00:24 -0700 Subject: [PATCH] Fix: Spotify sync crash 'unexpected keyword argument candidate_pool' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When no media server is connected, discovery/sync patches sync_service's matcher with a database-only implementation. sync_service calls it as _find_track_in_media_server(track, candidate_pool=...) (a per-artist candidate cache), but the database-only override took only (spotify_track) — so every sync raised 'database_only_find_track() got an unexpected keyword argument candidate_pool' and aborted. Lift the override from a nested closure to a module-level _database_only_find_track(spotify_track, candidate_pool=None) so it (a) accepts the kwarg for interface parity with the real matcher and (b) is importable and unit-testable. The DB-only path queries the library directly via check_track_exists, so it accepts but doesn't need the candidate cache. Also dropped the dead original_find_track local. Tests: signature includes candidate_pool; called with candidate_pool={} returns (None, 0.0) on no match; returns the match when the DB has it. --- core/discovery/sync.py | 170 +++++++++--------- .../test_sync_database_only_matcher.py | 51 ++++++ 2 files changed, 138 insertions(+), 83 deletions(-) create mode 100644 tests/discovery/test_sync_database_only_matcher.py diff --git a/core/discovery/sync.py b/core/discovery/sync.py index a77b5d3e..eb338edc 100644 --- a/core/discovery/sync.py +++ b/core/discovery/sync.py @@ -49,6 +49,90 @@ class SyncDeps: sync_lock: Any # threading.Lock +async def _database_only_find_track(spotify_track, candidate_pool=None): + """Database-only track matcher used when no media server is connected. + + Patched onto sync_service._find_track_in_media_server. Accepts + ``candidate_pool`` for interface parity with the real matcher (sync_service + calls it with candidate_pool=...); the DB path queries the library directly + via check_track_exists, so it doesn't need the per-artist candidate cache — + but it MUST accept the kwarg or sync raises "unexpected keyword argument + 'candidate_pool'". Module-level (not a nested closure) so it's importable + and unit-tested. + """ + logger.info(f"Database-only search for: '{spotify_track.name}' by {spotify_track.artists}") + try: + from database.music_database import MusicDatabase + from config.settings import config_manager + + db = MusicDatabase() + active_server = config_manager.get_active_media_server() + original_title = spotify_track.name + spotify_id = getattr(spotify_track, 'id', '') or '' + + # --- Sync match cache fast-path --- + if spotify_id: + try: + cached = db.read_sync_match_cache(spotify_id, active_server) + if cached: + db_track_check = db.get_track_by_id(cached['server_track_id']) + if db_track_check: + class DatabaseTrackCached: + def __init__(self, db_t): + self.ratingKey = db_t.id + self.title = db_t.title + self.id = db_t.id + logger.debug(f"Sync cache hit: '{original_title}' → server track {cached['server_track_id']}") + return DatabaseTrackCached(db_track_check), cached['confidence'] + logger.warning(f"Sync cache stale for '{original_title}' — track gone") + except Exception as e: + logger.debug("sync match cache fast-path failed: %s", e) + # --- End cache fast-path --- + + # Try each artist + for artist in spotify_track.artists: + if isinstance(artist, str): + artist_name = artist + elif isinstance(artist, dict) and 'name' in artist: + artist_name = artist['name'] + else: + artist_name = str(artist) + + db_track, confidence = db.check_track_exists( + original_title, artist_name, + confidence_threshold=0.80, + server_source=active_server + ) + + if db_track and confidence >= 0.80: + logger.info(f"Database match: '{db_track.title}' (confidence: {confidence:.2f})") + if spotify_id: + try: + from core.matching_engine import MusicMatchingEngine + me = MusicMatchingEngine() + db.save_sync_match_cache( + spotify_id, me.clean_title(original_title), me.clean_artist(artist_name), + active_server, db_track.id, db_track.title, confidence + ) + except Exception as e: + logger.debug("save sync match cache failed: %s", e) + + class DatabaseTrackMock: + def __init__(self, db_track): + self.ratingKey = db_track.id + self.title = db_track.title + self.id = db_track.id + + return DatabaseTrackMock(db_track), confidence + + logger.warning(f"No database match found for: '{original_title}'") + return None, 0.0 + + except Exception as e: + logger.error(f"Database search error: {e}") + return None, 0.0 + + def run_sync_task(playlist_id, playlist_name, tracks_json, automation_id=None, profile_id=1, playlist_image_url='', deps: SyncDeps = None, sync_mode: str = 'replace'): """The actual sync function that runs in the background thread.""" sync_states = deps.sync_states @@ -260,89 +344,9 @@ def run_sync_task(playlist_id, playlist_name, tracks_json, automation_id=None, p if media_client is None or not media_client.is_connected(): logger.info("Media client not connected - patching sync service for database-only matching") - # Store original method - original_find_track = sync_service._find_track_in_media_server - - # Create database-only replacement method - async def database_only_find_track(spotify_track): - logger.info(f"Database-only search for: '{spotify_track.name}' by {spotify_track.artists}") - try: - from database.music_database import MusicDatabase - from config.settings import config_manager - - db = MusicDatabase() - active_server = config_manager.get_active_media_server() - original_title = spotify_track.name - spotify_id = getattr(spotify_track, 'id', '') or '' - - # --- Sync match cache fast-path --- - if spotify_id: - try: - cached = db.read_sync_match_cache(spotify_id, active_server) - if cached: - db_track_check = db.get_track_by_id(cached['server_track_id']) - if db_track_check: - class DatabaseTrackCached: - def __init__(self, db_t): - self.ratingKey = db_t.id - self.title = db_t.title - self.id = db_t.id - logger.debug(f"Sync cache hit: '{original_title}' → server track {cached['server_track_id']}") - return DatabaseTrackCached(db_track_check), cached['confidence'] - logger.warning(f"Sync cache stale for '{original_title}' — track gone") - except Exception as e: - logger.debug("sync match cache fast-path failed: %s", e) - # --- End cache fast-path --- - - # Try each artist (same logic as original) - for artist in spotify_track.artists: - # Extract artist name from both string and dict formats - if isinstance(artist, str): - artist_name = artist - elif isinstance(artist, dict) and 'name' in artist: - artist_name = artist['name'] - else: - artist_name = str(artist) - - db_track, confidence = db.check_track_exists( - original_title, artist_name, - confidence_threshold=0.80, - server_source=active_server - ) - - if db_track and confidence >= 0.80: - logger.info(f"Database match: '{db_track.title}' (confidence: {confidence:.2f})") - - # Save to sync match cache - if spotify_id: - try: - from core.matching_engine import MusicMatchingEngine - me = MusicMatchingEngine() - db.save_sync_match_cache( - spotify_id, me.clean_title(original_title), me.clean_artist(artist_name), - active_server, db_track.id, db_track.title, confidence - ) - except Exception as e: - logger.debug("save sync match cache failed: %s", e) - - # Create mock track object for playlist creation - class DatabaseTrackMock: - def __init__(self, db_track): - self.ratingKey = db_track.id - self.title = db_track.title - self.id = db_track.id - - return DatabaseTrackMock(db_track), confidence - - logger.warning(f"No database match found for: '{original_title}'") - return None, 0.0 - - except Exception as e: - logger.error(f"Database search error: {e}") - return None, 0.0 - - # Patch the method - sync_service._find_track_in_media_server = database_only_find_track + # Patch the matcher to the module-level database-only implementation + # (importable + unit-tested; accepts candidate_pool for parity). + sync_service._find_track_in_media_server = _database_only_find_track logger.info("Patched sync service to use database-only matching") sync_start_time = time.time() diff --git a/tests/discovery/test_sync_database_only_matcher.py b/tests/discovery/test_sync_database_only_matcher.py new file mode 100644 index 00000000..b28b63d2 --- /dev/null +++ b/tests/discovery/test_sync_database_only_matcher.py @@ -0,0 +1,51 @@ +"""Regression: the database-only sync matcher must accept candidate_pool. + +sync_service calls _find_track_in_media_server(track, candidate_pool=...). When +no media server is connected, discovery/sync patches in a database-only matcher. +That override dropped the candidate_pool kwarg, so every Spotify sync failed with +"unexpected keyword argument 'candidate_pool'". These pin the contract. +""" + +from __future__ import annotations + +import asyncio +import inspect +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +import core.discovery.sync as sync_mod + + +def test_matcher_signature_accepts_candidate_pool(): + sig = inspect.signature(sync_mod._database_only_find_track) + assert 'candidate_pool' in sig.parameters + + +def _run(track, **kw): + fake_db = MagicMock() + fake_db.read_sync_match_cache.return_value = None + fake_db.check_track_exists.return_value = (None, 0.0) + fake_cm = MagicMock() + fake_cm.get_active_media_server.return_value = "plex" + with patch("database.music_database.MusicDatabase", return_value=fake_db), \ + patch("config.settings.config_manager", fake_cm): + return asyncio.run(sync_mod._database_only_find_track(track, **kw)) + + +def test_called_with_candidate_pool_no_match(): + track = SimpleNamespace(name="Song", artists=["Artist"], id="sp1") + # The exact call sync_service makes — must not raise TypeError. + assert _run(track, candidate_pool={}) == (None, 0.0) + + +def test_returns_match_when_db_has_it(): + track = SimpleNamespace(name="HUMBLE.", artists=["Kendrick Lamar"], id="sp2") + fake_db = MagicMock() + fake_db.read_sync_match_cache.return_value = None + fake_db.check_track_exists.return_value = (SimpleNamespace(id="t1", title="HUMBLE."), 0.95) + fake_cm = MagicMock() + fake_cm.get_active_media_server.return_value = "plex" + with patch("database.music_database.MusicDatabase", return_value=fake_db), \ + patch("config.settings.config_manager", fake_cm): + match, conf = asyncio.run(sync_mod._database_only_find_track(track, candidate_pool={})) + assert conf == 0.95 and match.id == "t1"