Apply artist-id name-guard to audiodb/qobuz/tidal workers too
The blind 'correct the parent artist's source id from an album/track match' logic was copy-pasted into four enrichment workers; the Deezer fix only covered one. AudioDB, Qobuz, and Tidal had the identical bug and would corrupt their own id columns (and re-corrupt after any cleanup). All three now gate the correction on a name match between the result's artist and the parent artist (audiodb reads result['strArtist']; qobuz/tidal thread the result artist name in from their callers, as Deezer does). Regression tests cover mismatch-skips and match-corrects for each.
This commit is contained in:
parent
8ce26d19fa
commit
85549197e6
4 changed files with 162 additions and 9 deletions
|
|
@ -273,8 +273,13 @@ class AudioDBWorker:
|
||||||
|
|
||||||
def _verify_artist_id(self, item: Dict[str, Any], result: Dict[str, Any]) -> bool:
|
def _verify_artist_id(self, item: Dict[str, Any], result: Dict[str, Any]) -> bool:
|
||||||
"""Verify that the result's artist ID matches the parent artist's stored AudioDB ID.
|
"""Verify that the result's artist ID matches the parent artist's stored AudioDB ID.
|
||||||
|
|
||||||
If mismatched, the album/track search is more specific (uses artist+title),
|
If mismatched, the album/track search is more specific (uses artist+title),
|
||||||
so we trust it and correct the parent artist's audiodb_id."""
|
so we trust it and correct the parent artist's audiodb_id — BUT only when
|
||||||
|
the result's artist *name* matches our parent artist. Without that guard,
|
||||||
|
a collaboration/compilation (a track our library credits to one artist
|
||||||
|
that lives on another artist's album) would stamp the wrong AudioDB id
|
||||||
|
onto our artist. See the Deezer fix for the full write-up."""
|
||||||
parent_audiodb_id = item.get('artist_audiodb_id')
|
parent_audiodb_id = item.get('artist_audiodb_id')
|
||||||
if not parent_audiodb_id:
|
if not parent_audiodb_id:
|
||||||
return True
|
return True
|
||||||
|
|
@ -284,6 +289,18 @@ class AudioDBWorker:
|
||||||
return True
|
return True
|
||||||
|
|
||||||
if str(result_artist_id) != str(parent_audiodb_id):
|
if str(result_artist_id) != str(parent_audiodb_id):
|
||||||
|
parent_name = item.get('artist') or ''
|
||||||
|
result_artist_name = result.get('strArtist') or ''
|
||||||
|
if (result_artist_name and parent_name
|
||||||
|
and not self._name_matches(parent_name, result_artist_name)):
|
||||||
|
logger.info(
|
||||||
|
f"Skipping artist-ID correction from {item['type']} "
|
||||||
|
f"'{item['name']}': result artist '{result_artist_name}' "
|
||||||
|
f"≠ parent '{parent_name}' (collab/compilation, not a "
|
||||||
|
f"correction)"
|
||||||
|
)
|
||||||
|
return True
|
||||||
|
|
||||||
logger.info(
|
logger.info(
|
||||||
f"Artist ID correction from {item['type']} '{item['name']}': "
|
f"Artist ID correction from {item['type']} '{item['name']}': "
|
||||||
f"updating parent artist AudioDB ID from {parent_audiodb_id} to {result_artist_id}"
|
f"updating parent artist AudioDB ID from {parent_audiodb_id} to {result_artist_id}"
|
||||||
|
|
|
||||||
|
|
@ -301,13 +301,29 @@ class QobuzWorker:
|
||||||
logger.debug(f"Name similarity: '{query_name}' vs '{result_name}' = {similarity:.2f}")
|
logger.debug(f"Name similarity: '{query_name}' vs '{result_name}' = {similarity:.2f}")
|
||||||
return similarity >= self.name_similarity_threshold
|
return similarity >= self.name_similarity_threshold
|
||||||
|
|
||||||
def _verify_artist_id(self, item: Dict[str, Any], result_artist_id) -> bool:
|
def _verify_artist_id(self, item: Dict[str, Any], result_artist_id,
|
||||||
"""Verify/correct parent artist's Qobuz ID based on album/track match"""
|
result_artist_name: Optional[str] = None) -> bool:
|
||||||
|
"""Verify/correct parent artist's Qobuz ID based on album/track match.
|
||||||
|
|
||||||
|
Only corrects when the result's artist *name* matches our parent artist —
|
||||||
|
otherwise a collaboration/compilation would stamp the wrong Qobuz id onto
|
||||||
|
our artist. See the Deezer fix for the full write-up."""
|
||||||
parent_qobuz_id = item.get('artist_qobuz_id')
|
parent_qobuz_id = item.get('artist_qobuz_id')
|
||||||
if not parent_qobuz_id or not result_artist_id:
|
if not parent_qobuz_id or not result_artist_id:
|
||||||
return True
|
return True
|
||||||
|
|
||||||
if str(result_artist_id) != str(parent_qobuz_id):
|
if str(result_artist_id) != str(parent_qobuz_id):
|
||||||
|
parent_name = item.get('artist') or ''
|
||||||
|
if (result_artist_name and parent_name
|
||||||
|
and not self._name_matches(parent_name, result_artist_name)):
|
||||||
|
logger.info(
|
||||||
|
f"Skipping artist-ID correction from {item['type']} "
|
||||||
|
f"'{item['name']}': result artist '{result_artist_name}' "
|
||||||
|
f"≠ parent '{parent_name}' (collab/compilation, not a "
|
||||||
|
f"correction)"
|
||||||
|
)
|
||||||
|
return True
|
||||||
|
|
||||||
logger.info(
|
logger.info(
|
||||||
f"Artist ID correction from {item['type']} '{item['name']}': "
|
f"Artist ID correction from {item['type']} '{item['name']}': "
|
||||||
f"updating parent artist Qobuz ID from {parent_qobuz_id} to {result_artist_id}"
|
f"updating parent artist Qobuz ID from {parent_qobuz_id} to {result_artist_id}"
|
||||||
|
|
@ -467,7 +483,8 @@ class QobuzWorker:
|
||||||
# Verify artist ID
|
# Verify artist ID
|
||||||
result_artist = result.get('artist', {})
|
result_artist = result.get('artist', {})
|
||||||
result_artist_id = result_artist.get('id') if result_artist else None
|
result_artist_id = result_artist.get('id') if result_artist else None
|
||||||
self._verify_artist_id(item, result_artist_id)
|
result_artist_name = result_artist.get('name') if result_artist else None
|
||||||
|
self._verify_artist_id(item, result_artist_id, result_artist_name)
|
||||||
|
|
||||||
# Fetch full album details
|
# Fetch full album details
|
||||||
qobuz_album_id = result.get('id')
|
qobuz_album_id = result.get('id')
|
||||||
|
|
@ -528,7 +545,8 @@ class QobuzWorker:
|
||||||
# Verify artist ID
|
# Verify artist ID
|
||||||
result_artist = result.get('artist', result.get('performer', {}))
|
result_artist = result.get('artist', result.get('performer', {}))
|
||||||
result_artist_id = result_artist.get('id') if result_artist else None
|
result_artist_id = result_artist.get('id') if result_artist else None
|
||||||
self._verify_artist_id(item, result_artist_id)
|
result_artist_name = result_artist.get('name') if result_artist else None
|
||||||
|
self._verify_artist_id(item, result_artist_id, result_artist_name)
|
||||||
|
|
||||||
# Fetch full track details
|
# Fetch full track details
|
||||||
qobuz_track_id = result.get('id')
|
qobuz_track_id = result.get('id')
|
||||||
|
|
|
||||||
|
|
@ -313,13 +313,29 @@ class TidalWorker:
|
||||||
logger.debug(f"Name similarity: '{query_name}' vs '{result_name}' = {similarity:.2f}")
|
logger.debug(f"Name similarity: '{query_name}' vs '{result_name}' = {similarity:.2f}")
|
||||||
return similarity >= self.name_similarity_threshold
|
return similarity >= self.name_similarity_threshold
|
||||||
|
|
||||||
def _verify_artist_id(self, item: Dict[str, Any], result_artist_id) -> bool:
|
def _verify_artist_id(self, item: Dict[str, Any], result_artist_id,
|
||||||
"""Verify/correct parent artist's Tidal ID based on album/track match"""
|
result_artist_name: Optional[str] = None) -> bool:
|
||||||
|
"""Verify/correct parent artist's Tidal ID based on album/track match.
|
||||||
|
|
||||||
|
Only corrects when the result's artist *name* matches our parent artist —
|
||||||
|
otherwise a collaboration/compilation would stamp the wrong Tidal id onto
|
||||||
|
our artist. See the Deezer fix for the full write-up."""
|
||||||
parent_tidal_id = item.get('artist_tidal_id')
|
parent_tidal_id = item.get('artist_tidal_id')
|
||||||
if not parent_tidal_id or not result_artist_id:
|
if not parent_tidal_id or not result_artist_id:
|
||||||
return True
|
return True
|
||||||
|
|
||||||
if str(result_artist_id) != str(parent_tidal_id):
|
if str(result_artist_id) != str(parent_tidal_id):
|
||||||
|
parent_name = item.get('artist') or ''
|
||||||
|
if (result_artist_name and parent_name
|
||||||
|
and not self._name_matches(parent_name, result_artist_name)):
|
||||||
|
logger.info(
|
||||||
|
f"Skipping artist-ID correction from {item['type']} "
|
||||||
|
f"'{item['name']}': result artist '{result_artist_name}' "
|
||||||
|
f"≠ parent '{parent_name}' (collab/compilation, not a "
|
||||||
|
f"correction)"
|
||||||
|
)
|
||||||
|
return True
|
||||||
|
|
||||||
logger.info(
|
logger.info(
|
||||||
f"Artist ID correction from {item['type']} '{item['name']}': "
|
f"Artist ID correction from {item['type']} '{item['name']}': "
|
||||||
f"updating parent artist Tidal ID from {parent_tidal_id} to {result_artist_id}"
|
f"updating parent artist Tidal ID from {parent_tidal_id} to {result_artist_id}"
|
||||||
|
|
@ -479,7 +495,8 @@ class TidalWorker:
|
||||||
# Verify artist ID
|
# Verify artist ID
|
||||||
result_artist = result.get('artist', {})
|
result_artist = result.get('artist', {})
|
||||||
result_artist_id = result_artist.get('id') if result_artist else None
|
result_artist_id = result_artist.get('id') if result_artist else None
|
||||||
self._verify_artist_id(item, result_artist_id)
|
result_artist_name = result_artist.get('name') if result_artist else None
|
||||||
|
self._verify_artist_id(item, result_artist_id, result_artist_name)
|
||||||
|
|
||||||
# Fetch full album details
|
# Fetch full album details
|
||||||
tidal_album_id = result.get('id')
|
tidal_album_id = result.get('id')
|
||||||
|
|
@ -533,7 +550,8 @@ class TidalWorker:
|
||||||
# Verify artist ID
|
# Verify artist ID
|
||||||
result_artist = result.get('artist', {})
|
result_artist = result.get('artist', {})
|
||||||
result_artist_id = result_artist.get('id') if result_artist else None
|
result_artist_id = result_artist.get('id') if result_artist else None
|
||||||
self._verify_artist_id(item, result_artist_id)
|
result_artist_name = result_artist.get('name') if result_artist else None
|
||||||
|
self._verify_artist_id(item, result_artist_id, result_artist_name)
|
||||||
|
|
||||||
# Fetch full track details
|
# Fetch full track details
|
||||||
tidal_track_id = result.get('id')
|
tidal_track_id = result.get('id')
|
||||||
|
|
|
||||||
100
tests/test_enrichment_artist_id_guard.py
Normal file
100
tests/test_enrichment_artist_id_guard.py
Normal file
|
|
@ -0,0 +1,100 @@
|
||||||
|
"""Regression: the album/track-driven 'artist id correction' in every
|
||||||
|
enrichment worker that has it must NOT overwrite an artist's source id unless
|
||||||
|
the result's artist name actually matches.
|
||||||
|
|
||||||
|
This is the same Kendrick/Jorja bug fixed in the Deezer worker (see
|
||||||
|
tests/test_deezer_worker_artist_id_guard.py), proven here to be closed in the
|
||||||
|
three other workers that copy-pasted the pattern: AudioDB, Qobuz, Tidal.
|
||||||
|
|
||||||
|
A track our library credits to Jorja Smith that lives on Kendrick's curated
|
||||||
|
'Black Panther' album resolves to that album, whose primary artist is Kendrick.
|
||||||
|
Without a name check, each worker would 'correct' Jorja's source id to
|
||||||
|
Kendrick's — corrupting it (and sharing one id across unrelated artists).
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from core.audiodb_worker import AudioDBWorker
|
||||||
|
from core.qobuz_worker import QobuzWorker
|
||||||
|
from core.tidal_worker import TidalWorker
|
||||||
|
|
||||||
|
|
||||||
|
def _stub(cls, correct_attr):
|
||||||
|
"""Build a bare worker instance (no __init__/clients) wired to record
|
||||||
|
corrections instead of writing to a db."""
|
||||||
|
w = cls.__new__(cls)
|
||||||
|
w.name_similarity_threshold = 0.80
|
||||||
|
w._corrections = []
|
||||||
|
setattr(w, correct_attr, lambda item, cid: w._corrections.append((item['id'], cid)))
|
||||||
|
return w
|
||||||
|
|
||||||
|
|
||||||
|
def _item(artist_name, parent_id, id_key):
|
||||||
|
return {'type': 'track', 'id': 1, 'name': 'Some Track',
|
||||||
|
'artist': artist_name, id_key: parent_id}
|
||||||
|
|
||||||
|
|
||||||
|
# --------------------------------------------------------------------------
|
||||||
|
# AudioDB — _verify_artist_id(item, result_dict); name is result['strArtist'].
|
||||||
|
# --------------------------------------------------------------------------
|
||||||
|
|
||||||
|
def test_audiodb_skips_correction_on_name_mismatch():
|
||||||
|
w = _stub(AudioDBWorker, '_correct_artist_audiodb_id')
|
||||||
|
item = _item('Jorja Smith', '111', 'artist_audiodb_id')
|
||||||
|
w._verify_artist_id(item, {'idArtist': '999', 'strArtist': 'Kendrick Lamar'})
|
||||||
|
assert w._corrections == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_audiodb_corrects_on_name_match():
|
||||||
|
w = _stub(AudioDBWorker, '_correct_artist_audiodb_id')
|
||||||
|
item = _item('Kendrick Lamar', '111', 'artist_audiodb_id')
|
||||||
|
w._verify_artist_id(item, {'idArtist': '999', 'strArtist': 'Kendrick Lamar'})
|
||||||
|
assert w._corrections == [(1, '999')]
|
||||||
|
|
||||||
|
|
||||||
|
# --------------------------------------------------------------------------
|
||||||
|
# Qobuz — _verify_artist_id(item, result_artist_id, result_artist_name).
|
||||||
|
# --------------------------------------------------------------------------
|
||||||
|
|
||||||
|
def test_qobuz_skips_correction_on_name_mismatch():
|
||||||
|
w = _stub(QobuzWorker, '_correct_artist_qobuz_id')
|
||||||
|
item = _item('Jorja Smith', '111', 'artist_qobuz_id')
|
||||||
|
w._verify_artist_id(item, '999', 'Kendrick Lamar')
|
||||||
|
assert w._corrections == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_qobuz_corrects_on_name_match():
|
||||||
|
w = _stub(QobuzWorker, '_correct_artist_qobuz_id')
|
||||||
|
item = _item('Kendrick Lamar', '111', 'artist_qobuz_id')
|
||||||
|
w._verify_artist_id(item, '999', 'Kendrick Lamar')
|
||||||
|
assert w._corrections == [(1, '999')]
|
||||||
|
|
||||||
|
|
||||||
|
# --------------------------------------------------------------------------
|
||||||
|
# Tidal — _verify_artist_id(item, result_artist_id, result_artist_name).
|
||||||
|
# --------------------------------------------------------------------------
|
||||||
|
|
||||||
|
def test_tidal_skips_correction_on_name_mismatch():
|
||||||
|
w = _stub(TidalWorker, '_correct_artist_tidal_id')
|
||||||
|
item = _item('Jorja Smith', '111', 'artist_tidal_id')
|
||||||
|
w._verify_artist_id(item, '999', 'Kendrick Lamar')
|
||||||
|
assert w._corrections == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_tidal_corrects_on_name_match():
|
||||||
|
w = _stub(TidalWorker, '_correct_artist_tidal_id')
|
||||||
|
item = _item('Kendrick Lamar', '111', 'artist_tidal_id')
|
||||||
|
w._verify_artist_id(item, '999', 'Kendrick Lamar')
|
||||||
|
assert w._corrections == [(1, '999')]
|
||||||
|
|
||||||
|
|
||||||
|
# --------------------------------------------------------------------------
|
||||||
|
# Shared: a missing result name preserves the old "trust the search" behavior
|
||||||
|
# (only the workers that pass an id+name — qobuz/tidal — exercise this path).
|
||||||
|
# --------------------------------------------------------------------------
|
||||||
|
|
||||||
|
def test_qobuz_missing_result_name_preserves_old_behavior():
|
||||||
|
w = _stub(QobuzWorker, '_correct_artist_qobuz_id')
|
||||||
|
item = _item('Kendrick Lamar', '111', 'artist_qobuz_id')
|
||||||
|
w._verify_artist_id(item, '999', None)
|
||||||
|
assert w._corrections == [(1, '999')]
|
||||||
Loading…
Reference in a new issue