From 4f24c2af6d7369dd2d487e58f691f34ad8a86842 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 12 Jun 2026 20:14:55 -0700 Subject: [PATCH] #863: derive YouTube track artist from music fields / -Topic channel / 'Artist - Title' instead of the playlist owner --- core/youtube_track_meta.py | 91 +++++++++++++++++++++++++++++++ tests/test_youtube_track_meta.py | 94 ++++++++++++++++++++++++++++++++ web_server.py | 26 ++++++--- 3 files changed, 204 insertions(+), 7 deletions(-) create mode 100644 core/youtube_track_meta.py create mode 100644 tests/test_youtube_track_meta.py diff --git a/core/youtube_track_meta.py b/core/youtube_track_meta.py new file mode 100644 index 00000000..c1742beb --- /dev/null +++ b/core/youtube_track_meta.py @@ -0,0 +1,91 @@ +"""Derive a track's artist + title from a yt-dlp playlist entry. + +Flat playlist extraction (used to dodge YouTube rate limits) gives sparse +per-entry data: often just ``title``, ``id``, ``duration``, and an +``uploader``/``channel`` that — for a playlist like "Likes" — is the PLAYLIST +OWNER, not the track artist. GitHub #863: every track came out as the owner +("Wing It"), or "Unknown Artist" when ``uploader`` was absent, because the +parser used ``entry['uploader']`` as the artist. + +The artist is usually recoverable from one of, in priority order: + +1. yt-dlp music-metadata fields (``artists`` / ``artist`` / ``creator``), + populated for YouTube Music tracks. +2. An auto-generated ``" - Topic"`` channel name. +3. The classic ``" - "`` form embedded in the video title. + +This module is the single, pure place that decides which signal wins, so the +precedence is unit-testable instead of buried in the web_server endpoint. It +deliberately does NOT fall back to the channel/uploader as the artist — on a +playlist that's the owner, and mislabelling every track is worse than an honest +"Unknown Artist" (which downstream MusicBrainz discovery can still try to fix). +""" + +from __future__ import annotations + +import re +from typing import Any, Mapping, Tuple + +# Trailing "- Topic" on an auto-generated YouTube Music channel. +_TOPIC_RE = re.compile(r'\s*-\s*topic\s*$', re.IGNORECASE) + +# "Artist - Title": a hyphen/en-dash/em-dash flanked by spaces, both sides +# non-empty. Splits on the FIRST such separator so "A - B (C Remix)" → ("A", +# "B (C Remix)"). Spaces around the dash are required so hyphenated names like +# "Jean-Michel Jarre" aren't split. +_TITLE_SPLIT_RE = re.compile(r'^\s*(?P<artist>.+?)\s+[-–—]\s+(?P<title>.+?)\s*$') + + +def _first_music_field(entry: Mapping[str, Any]) -> str: + """First non-empty value from yt-dlp's music-metadata fields.""" + artists = entry.get('artists') + if isinstance(artists, (list, tuple)): + for a in artists: + s = str(a or '').strip() + if s: + return s + for key in ('artist', 'creator'): + v = entry.get(key) + if isinstance(v, str) and v.strip(): + return v.strip() + return '' + + +def derive_artist_and_title(entry: Mapping[str, Any]) -> Tuple[str, str]: + """Return ``(artist, title)`` from a yt-dlp (flat) playlist entry. + + ``artist`` is ``''`` when no reliable signal exists — the caller defaults + that to "Unknown Artist" rather than using the playlist owner's channel + (#863). ``title`` is the raw video title, except when an "Artist - Title" + split provided the artist, in which case it's the right-hand side. + """ + if not isinstance(entry, Mapping): + return '', 'Unknown Track' + + title = str(entry.get('title') or '').strip() or 'Unknown Track' + + # 1. Music-metadata fields (YouTube Music). + field_artist = _first_music_field(entry) + if field_artist: + return field_artist, title + + # 2. "<Artist> - Topic" auto-channel — the channel name IS the artist. + channel = str(entry.get('uploader') or entry.get('channel') or '').strip() + if _TOPIC_RE.search(channel): + stripped = _TOPIC_RE.sub('', channel).strip() + if stripped: + return stripped, title + + # 3. "<Artist> - <Title>" embedded in the title. + m = _TITLE_SPLIT_RE.match(title) + if m: + artist = m.group('artist').strip() + rest = m.group('title').strip() + if artist and rest: + return artist, rest + + # 4. No reliable artist signal — caller defaults to "Unknown Artist". + return '', title + + +__all__ = ['derive_artist_and_title'] diff --git a/tests/test_youtube_track_meta.py b/tests/test_youtube_track_meta.py new file mode 100644 index 00000000..82f9c83d --- /dev/null +++ b/tests/test_youtube_track_meta.py @@ -0,0 +1,94 @@ +"""Seam tests for YouTube playlist artist/title derivation (GitHub #863). + +Flat playlist extraction gives sparse entries; the parser used to take the +artist straight from `uploader`, which on a playlist is the OWNER — so every +track came out as "Wing It" / "Unknown Artist". `derive_artist_and_title` picks +the best available signal instead. These pin the precedence + the +"never use the playlist owner" guarantee. +""" + +from __future__ import annotations + +from core.youtube_track_meta import derive_artist_and_title + + +def test_music_artists_field_wins(): + artist, title = derive_artist_and_title( + {'title': 'Forgiven', 'artists': ['Within Temptation'], 'uploader': 'Wing It'}) + assert artist == 'Within Temptation' + assert title == 'Forgiven' + + +def test_artist_field_used_when_no_artists_list(): + artist, title = derive_artist_and_title( + {'title': 'Alive', 'artist': 'Empire of the Sun', 'uploader': 'Wing It'}) + assert artist == 'Empire of the Sun' + assert title == 'Alive' + + +def test_topic_channel_is_the_artist(): + artist, title = derive_artist_and_title( + {'title': 'Revolte', 'uploader': 'Paul Kalkbrenner - Topic'}) + assert artist == 'Paul Kalkbrenner' + assert title == 'Revolte' + + +def test_artist_title_split_from_title(): + # The exact #863 log case — title carries "Artist - Track", uploader is the + # playlist owner. + artist, title = derive_artist_and_title( + {'title': 'Paul Kalkbrenner - Revolte (Original Mix) [Bpitch]', 'uploader': 'Wing It'}) + assert artist == 'Paul Kalkbrenner' + # Splits on the FIRST separator; the remainder keeps the qualifiers for the + # title cleaner to strip downstream. + assert title == 'Revolte (Original Mix) [Bpitch]' + + +def test_no_signal_returns_empty_artist_not_playlist_owner(): + # The unrecoverable case: plain title, uploader is the owner. Must NOT label + # the track with the owner's channel (#863). + artist, title = derive_artist_and_title( + {'title': 'Forgiven', 'uploader': 'Wing It'}) + assert artist == '' + assert title == 'Forgiven' + + +def test_hyphenated_name_without_spaces_not_split(): + # "Jean-Michel Jarre" has no spaced dash → not an Artist-Title split. + artist, title = derive_artist_and_title({'title': 'Jean-Michel Jarre', 'uploader': 'Wing It'}) + assert artist == '' + assert title == 'Jean-Michel Jarre' + + +def test_en_dash_separator_splits(): + artist, title = derive_artist_and_title({'title': 'Koven – Worlds Apart'}) # en dash + assert artist == 'Koven' + assert title == 'Worlds Apart' + + +def test_topic_beats_title_split_but_cleaner_handles_prefix(): + # Topic channel present AND title repeats "Artist - Title": topic wins for the + # artist; the full title is returned for the downstream cleaner to de-prefix. + artist, title = derive_artist_and_title( + {'title': 'Paul Kalkbrenner - Revolte', 'uploader': 'Paul Kalkbrenner - Topic'}) + assert artist == 'Paul Kalkbrenner' + assert title == 'Paul Kalkbrenner - Revolte' + + +def test_missing_title_is_safe(): + artist, title = derive_artist_and_title({'uploader': 'Wing It'}) + assert artist == '' + assert title == 'Unknown Track' + + +def test_bad_input_is_safe(): + assert derive_artist_and_title(None) == ('', 'Unknown Track') + assert derive_artist_and_title("not a dict") == ('', 'Unknown Track') + + +def test_empty_artists_list_falls_through(): + # An empty/blank artists list must not win — fall through to the title split. + artist, title = derive_artist_and_title( + {'title': 'Koven - Worlds Apart', 'artists': ['', None]}) + assert artist == 'Koven' + assert title == 'Worlds Apart' diff --git a/web_server.py b/web_server.py index 3d1c010d..a06d436a 100644 --- a/web_server.py +++ b/web_server.py @@ -14742,6 +14742,7 @@ def parse_youtube_playlist(url): Uses flat playlist extraction to avoid rate limits and get all tracks Returns a list of track dictionaries compatible with our Track structure """ + from core.youtube_track_meta import derive_artist_and_title try: # Configure yt-dlp options for flat playlist extraction (avoids rate limits) ydl_opts = { @@ -14774,14 +14775,25 @@ def parse_youtube_playlist(url): # Extract basic information from flat extraction raw_title = entry.get('title', 'Unknown Track') - raw_uploader = entry.get('uploader', 'Unknown Artist') + raw_uploader = entry.get('uploader') or entry.get('channel') or '' duration = entry.get('duration', 0) video_id = entry.get('id', '') - - # Clean the track title and artist using our cleaning functions - cleaned_artist = clean_youtube_artist(raw_uploader) - cleaned_title = clean_youtube_track_title(raw_title, cleaned_artist) - + + # Derive the artist from the best available signal — music + # fields, a "- Topic" channel, or an "Artist - Title" split — + # instead of blindly using `uploader`, which on a playlist is the + # OWNER, not the track artist (#863: every track became "Wing It" + # / "Unknown Artist"). Returns ('' , title) when nothing reliable. + derived_artist, derived_title = derive_artist_and_title(entry) + + # Clean the track title and artist using our cleaning functions. + if derived_artist: + cleaned_artist = clean_youtube_artist(derived_artist) + cleaned_title = clean_youtube_track_title(derived_title, cleaned_artist) + else: + cleaned_artist = 'Unknown Artist' + cleaned_title = clean_youtube_track_title(derived_title, None) + # Create track object matching GUI structure track_data = { 'id': video_id, @@ -14789,7 +14801,7 @@ def parse_youtube_playlist(url): 'artists': [cleaned_artist], 'duration_ms': duration * 1000 if duration else 0, 'raw_title': raw_title, # Keep original for reference - 'raw_artist': raw_uploader, # Keep original for reference + 'raw_artist': derived_artist or raw_uploader, # Keep original for reference 'url': f"https://www.youtube.com/watch?v={video_id}" }