From aaf312cd34b5cf5e9e856eedb4b55f1cf4605b8a Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Mon, 18 May 2026 16:32:38 -0700 Subject: [PATCH] Honor manual library matches across source labels Manual matches can be created from sync history as mirrored while wishlist and download flows later see the same track as wishlist or a provider source. Add a shared track-level lookup that falls back from exact source/id to source_track_id and title/artist, then use it for wishlist adds, cleanup, and download analysis so mapped tracks are not re-added or redownloaded. Add coverage for mirrored-source matches being honored by wishlist cleanup and download batches, including the internal wishlist force-download path. --- core/downloads/master.py | 6 +- core/library/manual_library_match.py | 87 ++++++++++++++++++++++++ core/wishlist/processing.py | 3 +- database/music_database.py | 64 ++++++++++++++++- tests/downloads/test_downloads_master.py | 54 ++++++++++++++- tests/test_manual_library_match.py | 86 +++++++++++++++++++++-- tests/wishlist/test_processing.py | 2 +- 7 files changed, 288 insertions(+), 14 deletions(-) diff --git a/core/downloads/master.py b/core/downloads/master.py index ba1a86f8..118db159 100644 --- a/core/downloads/master.py +++ b/core/downloads/master.py @@ -382,8 +382,10 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma # Manual library matches are authoritative unless the user explicitly # requested a force re-download from the normal download modal. - _stid = track_data.get('spotify_track_id') or track_data.get('id', '') - if not ignore_manual_matches and _stid and _mlm.get_match(db, batch_profile_id, batch_source, _stid): + _stid = track_data.get('spotify_track_id') or track_data.get('source_track_id') or track_data.get('id', '') + if not ignore_manual_matches and _stid and _mlm.get_match_for_track( + db, batch_profile_id, track_data, default_source=batch_source + ): logger.info(f"[Manual Match] '{track_name}' already matched in library — skipping download") try: deps.check_and_remove_track_from_wishlist_by_metadata(track_data) diff --git a/core/library/manual_library_match.py b/core/library/manual_library_match.py index 1387c203..132ad729 100644 --- a/core/library/manual_library_match.py +++ b/core/library/manual_library_match.py @@ -42,6 +42,93 @@ def get_match( return getter(profile_id, source, source_track_id, server_source) +def _first_artist_name(track: dict[str, Any]) -> str: + artists = track.get("artists") or [] + if isinstance(artists, list) and artists: + first = artists[0] + if isinstance(first, dict): + return (first.get("name") or "").strip() + return str(first).strip() + return (track.get("artist") or track.get("artist_name") or "").strip() + + +def _track_source_candidates(track: dict[str, Any], default_source: str = "") -> list[str]: + candidates = [ + track.get("provider"), + track.get("source"), + default_source, + "spotify", + ] + out = [] + for source in candidates: + source = (source or "").strip() + if source and source not in out: + out.append(source) + return out + + +def _track_id_candidates(track: dict[str, Any]) -> list[str]: + candidates = [ + track.get("source_track_id"), + track.get("spotify_track_id"), + track.get("track_id"), + track.get("id"), + track.get("musicbrainz_recording_id"), + track.get("deezer_id") or track.get("deezer_track_id"), + track.get("itunes_track_id"), + track.get("tidal_id") or track.get("tidal_track_id"), + track.get("qobuz_id") or track.get("qobuz_track_id"), + track.get("amazon_id") or track.get("amazon_track_id"), + ] + out = [] + for value in candidates: + value = str(value).strip() if value is not None else "" + if value and value not in out: + out.append(value) + return out + + +def get_match_for_track( + db, + profile_id: int, + track: dict[str, Any], + *, + default_source: str = "", + server_source: str = "", +) -> Optional[dict]: + """Return a manual match for a wishlist/sync track. + + Exact source+ID matches are preferred, but source labels can legitimately + change between UI surfaces (for example ``mirrored`` in sync history versus + ``wishlist`` in the wishlist batch). Fall back to track ID and finally + title/artist so saved manual matches are honored consistently. + """ + if not isinstance(track, dict): + return None + + sources = _track_source_candidates(track, default_source) + track_ids = _track_id_candidates(track) + for track_id in track_ids: + for source in sources: + match = get_match(db, profile_id, source, track_id, server_source) + if match: + return match + + id_getter = getattr(db, "find_manual_library_match_by_source_track_id", None) + if id_getter is not None: + for track_id in track_ids: + match = id_getter(profile_id, track_id, server_source) + if match: + return match + + title = (track.get("name") or track.get("title") or track.get("track_name") or "").strip() + artist = _first_artist_name(track) + metadata_getter = getattr(db, "find_manual_library_match_by_metadata", None) + if metadata_getter is not None and title and artist: + return metadata_getter(profile_id, title, artist, server_source) + return None + + def delete_match(db, match_id: int, profile_id: int) -> bool: """Delete match by PK id, scoped to profile.""" return db.delete_manual_library_match(match_id, profile_id) diff --git a/core/wishlist/processing.py b/core/wishlist/processing.py index f3fe11ab..3a8cbf9d 100644 --- a/core/wishlist/processing.py +++ b/core/wishlist/processing.py @@ -256,8 +256,7 @@ def remove_tracks_already_in_library( continue # Manual match check — skip fuzzy search if user already linked this track. - _track_source = track.get('provider') or 'spotify' - if _mlm.get_match(music_database, profile_id, _track_source, spotify_track_id): + if _mlm.get_match_for_track(music_database, profile_id, track, default_source='wishlist'): try: removed = wishlist_service.mark_track_download_result(spotify_track_id, success=True) if removed: diff --git a/database/music_database.py b/database/music_database.py index 69efceea..c59b9b3a 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -4219,6 +4219,64 @@ class MusicDatabase: logger.error(f"get_manual_library_match error: {e}") return None + def find_manual_library_match_by_source_track_id(self, profile_id: int, + source_track_id: str, + server_source: str = '') -> Optional[Dict[str, Any]]: + """Return a manual match for this source track ID across source labels. + + The UI may save a match from sync history as ``mirrored`` while the + wishlist/download flow later sees the same track under ``wishlist`` or + the provider name. The source remains useful metadata, but the stored + track ID is the stable identity we need to honor. + """ + if not source_track_id: + return None + try: + with self._get_connection() as conn: + cursor = conn.cursor() + cursor.execute(""" + SELECT * FROM manual_library_track_matches + WHERE profile_id = ? + AND source_track_id = ? + AND (server_source = ? OR server_source = '') + ORDER BY + CASE WHEN server_source = ? THEN 0 ELSE 1 END, + updated_at DESC + LIMIT 1 + """, (profile_id, source_track_id, server_source or '', server_source or '')) + row = cursor.fetchone() + return dict(row) if row else None + except Exception as e: + logger.error(f"find_manual_library_match_by_source_track_id error: {e}") + return None + + def find_manual_library_match_by_metadata(self, profile_id: int, + source_title: str, + source_artist: str, + server_source: str = '') -> Optional[Dict[str, Any]]: + """Return a manual match by title/artist when provider IDs differ.""" + if not source_title or not source_artist: + return None + try: + with self._get_connection() as conn: + cursor = conn.cursor() + cursor.execute(""" + SELECT * FROM manual_library_track_matches + WHERE profile_id = ? + AND source_title = ? COLLATE NOCASE + AND source_artist = ? COLLATE NOCASE + AND (server_source = ? OR server_source = '') + ORDER BY + CASE WHEN server_source = ? THEN 0 ELSE 1 END, + updated_at DESC + LIMIT 1 + """, (profile_id, source_title, source_artist, server_source or '', server_source or '')) + row = cursor.fetchone() + return dict(row) if row else None + except Exception as e: + logger.error(f"find_manual_library_match_by_metadata error: {e}") + return None + def delete_manual_library_match(self, match_id: int, profile_id: int) -> bool: """Delete match by PK id, scoped to profile_id.""" try: @@ -7315,12 +7373,12 @@ class MusicDatabase: logger.error("Cannot add track to wishlist: missing track ID") return False - track_source = spotify_track_data.get('provider') or spotify_track_data.get('source') or 'spotify' - if self.get_manual_library_match(profile_id, track_source, track_id): + from core.library import manual_library_match as _mlm + if _mlm.get_match_for_track(self, profile_id, spotify_track_data): logger.info( "Skipping wishlist add for manually matched track: '%s' (%s:%s)", spotify_track_data.get('name', 'Unknown Track'), - track_source, + spotify_track_data.get('provider') or spotify_track_data.get('source') or 'unknown', track_id, ) return True diff --git a/tests/downloads/test_downloads_master.py b/tests/downloads/test_downloads_master.py index f5d29df1..80daa759 100644 --- a/tests/downloads/test_downloads_master.py +++ b/tests/downloads/test_downloads_master.py @@ -43,6 +43,7 @@ class _FakeDB: self.album_confidence = album_confidence self.sync_history_calls = [] self.track_results_calls = [] + self.manual_matches = [] def check_track_exists(self, title, artist, confidence_threshold=0.7, server_source=None, album=None): key = (title.lower().strip(), artist.lower().strip()) @@ -71,6 +72,23 @@ class _FakeDB: def update_sync_history_track_results(self, batch_id, results_json): self.track_results_calls.append((batch_id, results_json)) + def get_manual_library_match(self, profile_id, source, source_track_id, server_source=''): + for match in self.manual_matches: + if ( + match["profile_id"] == profile_id + and match["source"] == source + and match["source_track_id"] == source_track_id + and match.get("server_source", "") == server_source + ): + return match + return None + + def find_manual_library_match_by_source_track_id(self, profile_id, source_track_id, server_source=''): + for match in self.manual_matches: + if match["profile_id"] == profile_id and match["source_track_id"] == source_track_id: + return match + return None + class _DBTrack: def __init__(self, title): @@ -301,7 +319,7 @@ def test_manual_match_overrides_internal_force_download(monkeypatch): db = _FakeDB() monkeypatch.setattr('database.music_database.MusicDatabase', lambda: db) monkeypatch.setattr( - 'core.library.manual_library_match.get_match', + 'core.library.manual_library_match.get_match_for_track', lambda *_args, **_kwargs: {'id': 1, 'library_track_id': 42}, ) @@ -324,6 +342,38 @@ def test_manual_match_overrides_internal_force_download(monkeypatch): assert removed == ['T1'] +def test_manual_match_saved_under_mirrored_source_overrides_wishlist_batch(monkeypatch): + """Wishlist batches honor matches saved from mirrored sync history.""" + db = _FakeDB() + db.manual_matches.append({ + "id": 1, + "profile_id": 1, + "source": "mirrored", + "source_track_id": "track-abc", + "server_source": "", + "library_track_id": 42, + }) + monkeypatch.setattr('database.music_database.MusicDatabase', lambda: db) + + removed = [] + _seed_batch( + 'B2m', + force_download_all=True, + ignore_manual_matches=False, + profile_id=1, + batch_source='wishlist', + ) + deps = _build_deps(wishlist_remove=lambda td: removed.append(td.get('name'))) + tracks = [{'id': 'track-abc', 'name': 'Coffee Break', 'artists': [{'name': 'Zeds Dead'}], 'provider': 'wishlist'}] + + mw.run_full_missing_tracks_process('B2m', 'wishlist', tracks, deps) + + assert download_batches['B2m']['queue'] == [] + assert download_batches['B2m']['analysis_results'][0]['found'] is True + assert download_batches['B2m']['analysis_results'][0]['match_reason'] == 'manual_library_match' + assert removed == ['Coffee Break'] + + def test_explicit_force_download_ignores_manual_match(monkeypatch): """User-facing Force Download All can intentionally bypass manual matches.""" db = _FakeDB() @@ -331,7 +381,7 @@ def test_explicit_force_download_ignores_manual_match(monkeypatch): calls = [] monkeypatch.setattr( - 'core.library.manual_library_match.get_match', + 'core.library.manual_library_match.get_match_for_track', lambda *_args, **_kwargs: calls.append(True) or {'id': 1, 'library_track_id': 42}, ) diff --git a/tests/test_manual_library_match.py b/tests/test_manual_library_match.py index 4a87c236..7804eb25 100644 --- a/tests/test_manual_library_match.py +++ b/tests/test_manual_library_match.py @@ -79,6 +79,58 @@ def test_get_returns_none_when_absent(db): assert db.get_manual_library_match(1, "spotify", "nonexistent") is None +def test_get_match_for_track_falls_back_across_source_labels(db): + db.save_manual_library_match( + 1, + "mirrored", + "track-abc", + 42, + source_title="Coffee Break", + source_artist="Zeds Dead", + ) + + row = mlm.get_match_for_track( + db, + 1, + { + "id": "track-abc", + "name": "Coffee Break", + "artists": [{"name": "Zeds Dead"}], + "provider": "wishlist", + }, + default_source="wishlist", + ) + + assert row is not None + assert row["library_track_id"] == 42 + + +def test_get_match_for_track_falls_back_to_source_title_artist(db): + db.save_manual_library_match( + 1, + "mirrored", + "old-id", + 42, + source_title="Coffee Break", + source_artist="Zeds Dead", + ) + + row = mlm.get_match_for_track( + db, + 1, + { + "id": "new-id", + "name": "Coffee Break", + "artists": [{"name": "Zeds Dead"}], + "provider": "musicbrainz", + }, + default_source="wishlist", + ) + + assert row is not None + assert row["library_track_id"] == 42 + + def test_add_to_wishlist_skips_manual_matched_track(db): db.save_manual_library_match(1, "spotify", "track-abc", 42) @@ -98,6 +150,32 @@ def test_add_to_wishlist_skips_manual_matched_track(db): assert db.get_wishlist_tracks(profile_id=1) == [] +def test_add_to_wishlist_skips_manual_match_saved_from_mirrored_source(db): + db.save_manual_library_match( + 1, + "mirrored", + "track-abc", + 42, + source_title="Coffee Break", + source_artist="Zeds Dead", + ) + + ok = db.add_to_wishlist( + track_data={ + "id": "track-abc", + "name": "Coffee Break", + "artists": [{"name": "Zeds Dead"}], + "album": {"name": "Coffee Break"}, + "provider": "wishlist", + }, + failure_reason="Download failed", + profile_id=1, + ) + + assert ok is True + assert db.get_wishlist_tracks(profile_id=1) == [] + + def test_get_match_returns_none_when_db_lacks_manual_match_method(): class _MinimalDB: pass @@ -167,7 +245,7 @@ def test_wishlist_skips_manual_matched_track(): mock_music_db = MagicMock() mock_music_db.check_track_exists = MagicMock() - with patch("core.library.manual_library_match.get_match", return_value={"id": 1, "library_track_id": 42}): + with patch("core.library.manual_library_match.get_match_for_track", return_value={"id": 1, "library_track_id": 42}): from core.wishlist.processing import remove_tracks_already_in_library removed = remove_tracks_already_in_library( mock_wishlist_svc, @@ -199,7 +277,7 @@ def test_wishlist_falls_through_when_no_match(): mock_music_db = MagicMock() mock_music_db.check_track_exists.return_value = (None, 0.0) - with patch("core.library.manual_library_match.get_match", return_value=None): + with patch("core.library.manual_library_match.get_match_for_track", return_value=None): from core.wishlist.processing import remove_tracks_already_in_library removed = remove_tracks_already_in_library( mock_wishlist_svc, @@ -273,7 +351,7 @@ def test_master_analysis_marks_found(): mock_deps.start_next_batch_of_downloads = MagicMock() mock_deps.reset_wishlist_auto_processing = MagicMock() - with patch("core.library.manual_library_match.get_match", return_value={"id": 1, "library_track_id": 42}), \ + with patch("core.library.manual_library_match.get_match_for_track", return_value={"id": 1, "library_track_id": 42}), \ patch("database.music_database.MusicDatabase", return_value=mock_db): run_full_missing_tracks_process(batch_id, "playlist-1", [track_data], mock_deps) @@ -345,7 +423,7 @@ def test_master_analysis_manual_match_wins_over_internal_force_download(): mock_deps.start_next_batch_of_downloads = MagicMock() mock_deps.reset_wishlist_auto_processing = MagicMock() - with patch("core.library.manual_library_match.get_match", return_value={"id": 1, "library_track_id": 42}), \ + with patch("core.library.manual_library_match.get_match_for_track", return_value={"id": 1, "library_track_id": 42}), \ patch("database.music_database.MusicDatabase", return_value=mock_db): run_full_missing_tracks_process(batch_id, "playlist-1", [track_data], mock_deps) diff --git a/tests/wishlist/test_processing.py b/tests/wishlist/test_processing.py index 34d2b99a..709b3cc6 100644 --- a/tests/wishlist/test_processing.py +++ b/tests/wishlist/test_processing.py @@ -240,7 +240,7 @@ def test_automatic_wishlist_cleanup_after_db_update_removes_manual_matches(monke ) music_db = _CleanupMusicDatabase() monkeypatch.setattr( - "core.library.manual_library_match.get_match", + "core.library.manual_library_match.get_match_for_track", lambda *_args, **_kwargs: {"id": 1, "library_track_id": 42}, )