#909: backfill the YT-artist column from a confident match instead of 'Unknown Artist'
YouTube's flat playlist extraction returns ONLY the title (verified: no artist/channel/uploader field at all), so a track starts as 'Unknown Artist' and only gains a name if per-video recovery succeeds. When recovery comes up empty (no cookies / age-gated / bot-checked) but the track still matched confidently, the worker threw the match's artist away and left the column 'Unknown Artist' — the #909 symptom. Now the displayed yt_artist falls back to the matched artist when it's still Unknown. Display-only: the match itself, track['artists'], cache, and download flow are untouched, so a real recovered name always wins and an unmatched/error row honestly stays Unknown. Extracted resolve_display_artist as a pure, tested seam; applied in the cache-hit and fresh-match result paths (the error path has no match to draw from).
This commit is contained in:
parent
e301877e64
commit
9f5bc0de89
2 changed files with 56 additions and 5 deletions
|
|
@ -32,6 +32,26 @@ from typing import Any, Callable
|
|||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
_UNKNOWN_ARTIST = 'Unknown Artist'
|
||||
|
||||
|
||||
def resolve_display_artist(yt_artist: str, matched_artist: str) -> str:
|
||||
"""The artist to show in the 'YT Artist' column (#909).
|
||||
|
||||
YouTube's flat playlist data carries no artist, so a track starts as
|
||||
"Unknown Artist" and only gains a real name if per-video recovery succeeds.
|
||||
When recovery comes up empty but the track still matched confidently, show
|
||||
the matched artist instead of a misleading "Unknown Artist". Returns the
|
||||
original ``yt_artist`` whenever it's already a real name (recovery worked) or
|
||||
when there's no matched artist to fall back to — purely a display choice, the
|
||||
match itself is unaffected.
|
||||
"""
|
||||
current = (yt_artist or '').strip()
|
||||
if current and current != _UNKNOWN_ARTIST:
|
||||
return current # recovery already gave a real name — keep it
|
||||
fallback = (matched_artist or '').strip()
|
||||
return fallback or _UNKNOWN_ARTIST # backfill from the match, else honest Unknown
|
||||
|
||||
|
||||
@dataclass
|
||||
class YoutubeDiscoveryDeps:
|
||||
|
|
@ -131,14 +151,15 @@ def run_youtube_discovery_worker(url_hash, deps: YoutubeDiscoveryDeps):
|
|||
cached_match = cache_db.get_discovery_cache_match(cache_key[0], cache_key[1], discovery_source)
|
||||
if cached_match and deps.validate_discovery_cache_artist(cleaned_artist, cached_match):
|
||||
logger.debug(f"CACHE HIT [{i+1}/{len(tracks)}]: {cleaned_artist} - {cleaned_title}")
|
||||
_match_artist = deps.extract_artist_name(cached_match.get('artists', [''])[0]) if cached_match.get('artists') else ''
|
||||
result = {
|
||||
'index': i,
|
||||
'yt_track': cleaned_title,
|
||||
'yt_artist': cleaned_artist,
|
||||
'yt_artist': resolve_display_artist(cleaned_artist, _match_artist),
|
||||
'status': 'Found',
|
||||
'status_class': 'found',
|
||||
'spotify_track': cached_match.get('name', ''),
|
||||
'spotify_artist': deps.extract_artist_name(cached_match.get('artists', [''])[0]) if cached_match.get('artists') else '',
|
||||
'spotify_artist': _match_artist,
|
||||
'spotify_album': cached_match.get('album', {}).get('name', '') if isinstance(cached_match.get('album'), dict) else cached_match.get('album', ''),
|
||||
'duration': f"{int(track['duration_ms']) // 60000}:{(int(track['duration_ms']) % 60000) // 1000:02d}" if track['duration_ms'] else '0:00',
|
||||
'discovery_source': discovery_source,
|
||||
|
|
@ -265,15 +286,17 @@ def run_youtube_discovery_worker(url_hash, deps: YoutubeDiscoveryDeps):
|
|||
best_confidence = confidence
|
||||
logger.info(f"Strategy 4 YouTube match (extended): {match.artists[0]} - {match.name} (confidence: {confidence:.3f})")
|
||||
|
||||
# Create result entry
|
||||
# Create result entry. yt_artist falls back to the matched artist when
|
||||
# YouTube/recovery left it "Unknown Artist" but we matched confidently (#909).
|
||||
_match_artist = deps.extract_artist_name(matched_track.artists[0]) if matched_track else ''
|
||||
result = {
|
||||
'index': i,
|
||||
'yt_track': cleaned_title,
|
||||
'yt_artist': cleaned_artist,
|
||||
'yt_artist': resolve_display_artist(cleaned_artist, _match_artist),
|
||||
'status': 'Found' if matched_track else 'Not Found',
|
||||
'status_class': 'found' if matched_track else 'not-found',
|
||||
'spotify_track': matched_track.name if matched_track else '',
|
||||
'spotify_artist': deps.extract_artist_name(matched_track.artists[0]) if matched_track else '',
|
||||
'spotify_artist': _match_artist,
|
||||
'spotify_album': matched_track.album if matched_track else '',
|
||||
'duration': f"{int(track['duration_ms']) // 60000}:{(int(track['duration_ms']) % 60000) // 1000:02d}" if track['duration_ms'] else '0:00',
|
||||
'discovery_source': discovery_source,
|
||||
|
|
|
|||
|
|
@ -406,3 +406,31 @@ def test_results_sorted_by_index():
|
|||
|
||||
indices = [r['index'] for r in states['h11']['discovery_results']]
|
||||
assert indices == sorted(indices)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# resolve_display_artist (#909) — YT-artist column backfill
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_display_artist_keeps_recovered_name():
|
||||
# Recovery already produced a real artist → never overwritten by the match.
|
||||
assert dy.resolve_display_artist('Goo Goo Dolls', 'Spotify Goo') == 'Goo Goo Dolls'
|
||||
|
||||
|
||||
def test_display_artist_backfills_from_match_when_unknown():
|
||||
# YouTube/recovery gave nothing, but we matched → show the matched artist.
|
||||
assert dy.resolve_display_artist('Unknown Artist', 'Don McLean') == 'Don McLean'
|
||||
assert dy.resolve_display_artist('', 'Don McLean') == 'Don McLean'
|
||||
assert dy.resolve_display_artist(None, 'Don McLean') == 'Don McLean'
|
||||
|
||||
|
||||
def test_display_artist_stays_unknown_with_no_match():
|
||||
# No recovery AND no match → honest "Unknown Artist" (an unmatched/error row).
|
||||
assert dy.resolve_display_artist('Unknown Artist', '') == 'Unknown Artist'
|
||||
assert dy.resolve_display_artist('Unknown Artist', None) == 'Unknown Artist'
|
||||
assert dy.resolve_display_artist('', '') == 'Unknown Artist'
|
||||
|
||||
|
||||
def test_display_artist_trims_whitespace():
|
||||
assert dy.resolve_display_artist(' ', 'Matched') == 'Matched'
|
||||
assert dy.resolve_display_artist('Real Artist ', '') == 'Real Artist'
|
||||
|
|
|
|||
Loading…
Reference in a new issue