diff --git a/core/artist_source_lookup.py b/core/artist_source_lookup.py index 60fff775..e43274de 100644 --- a/core/artist_source_lookup.py +++ b/core/artist_source_lookup.py @@ -53,7 +53,11 @@ def find_library_artist_for_source( Lookup order: 1. Direct match on the source-specific ID column (server-agnostic — any - library record with the right external ID is a hit). + library record with the right external ID is a hit). If that id is + stamped on MORE than one library artist, the mapping is corrupt / + ambiguous (e.g. an enrichment bug wrote one Deezer id onto several + artists) — we refuse to guess and fall through, so the caller can + show the source artist directly instead of an arbitrary wrong one. 2. Case-insensitive name match within ``active_server`` (defaults to the active media server when not provided), so we don't jump the user across server contexts on a name collision. @@ -67,13 +71,23 @@ def find_library_artist_for_source( try: with database._get_connection() as conn: cursor = conn.cursor() + # LIMIT 2 so we can tell a unique match from an ambiguous one. cursor.execute( - f"SELECT id, name FROM artists WHERE {column} = ? LIMIT 1", + f"SELECT id FROM artists WHERE {column} = ? LIMIT 2", (str(source_artist_id),), ) - row = cursor.fetchone() - if row: - return row[0] + rows = cursor.fetchall() + if len(rows) == 1: + return rows[0][0] + if len(rows) > 1: + # Same source id on multiple artists — corrupt mapping. Don't + # upgrade on the id; fall through to the name match (and, if + # that misses, let the caller render the source artist). + logger.warning( + f"Source id {source}:{source_artist_id} maps to " + f"{len(rows)}+ library artists — ambiguous, skipping " + f"id-based library upgrade" + ) if artist_name and active_server: cursor.execute( diff --git a/tests/metadata/test_artist_source_lookup.py b/tests/metadata/test_artist_source_lookup.py index 32c03cb6..949d42c5 100644 --- a/tests/metadata/test_artist_source_lookup.py +++ b/tests/metadata/test_artist_source_lookup.py @@ -164,6 +164,45 @@ class TestFindLibraryArtistForSource: ) assert result is None + def test_ambiguous_source_id_skips_id_upgrade(self, db): + """Regression for the Kendrick/Jorja bug: when one Deezer id is + stamped on several library artists (enrichment corruption), the id + match is ambiguous and must NOT pick an arbitrary row — it returns + None so the caller falls back to showing the source artist.""" + _insert_artist(db, artist_id="pk-kendrick", name="Kendrick Lamar", + deezer_id="525046", server_source="plex") + _insert_artist(db, artist_id="pk-jorja", name="Jorja Smith", + deezer_id="525046", server_source="plex") + _insert_artist(db, artist_id="pk-vince", name="Vince Staples", + deezer_id="525046", server_source="plex") + + # No name hint (the URL-driven path) → no id guess, no name fallback. + assert find_library_artist_for_source( + db, "deezer", "525046", active_server="plex" + ) is None + + def test_ambiguous_source_id_still_allows_name_fallback(self, db): + """An ambiguous id shouldn't block a correct name match when the + caller does have the name.""" + _insert_artist(db, artist_id="pk-kendrick", name="Kendrick Lamar", + deezer_id="525046", server_source="plex") + _insert_artist(db, artist_id="pk-jorja", name="Jorja Smith", + deezer_id="525046", server_source="plex") + + result = find_library_artist_for_source( + db, "deezer", "525046", artist_name="Kendrick Lamar", + active_server="plex", + ) + assert result == "pk-kendrick" + + def test_unique_source_id_still_matches(self, db): + """Positive control: a non-duplicated id still upgrades as before.""" + _insert_artist(db, artist_id="pk-solo", name="Solo Artist", + deezer_id="999999", server_source="plex") + assert find_library_artist_for_source( + db, "deezer", "999999" + ) == "pk-solo" + def test_id_match_wins_over_name_match(self, db): """If both a source-id match and a name match exist, the id match should take priority — it's the more reliable signal."""