From 560156abeef3d2be0145b2d3ae6ee022cc434a59 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 29 May 2026 09:30:38 -0700 Subject: [PATCH] Fix: import overwrites album-artist tag to "Unknown Artist" (#735) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported by CubeComming: importing media keeps the track artist correct (e.g. Billie Eilish) but changes the album-artist tag ("Albuminterpret") to "Unknown Artist", breaking grouping in the media server. Cause: in extract_source_metadata (core/metadata/source.py), album_artist is seeded from the resolved track artist, then overridden by the album CONTEXT's first artist. When the album lookup comes back unresolved, that first artist is the literal "Unknown Artist" placeholder — which is truthy, so it clobbered the real artist. Fix: treat "Unknown Artist" (and empty) as a non-value — only let the album context override the album_artist when it names a real artist. A genuine album artist (e.g. "Various Artists") still overrides as before. Tests: tests/metadata/test_album_artist_unknown.py — placeholder doesn't clobber, real album artist still used, no-album-context falls back to track artist, empty doesn't clobber. (Pre-existing test_album_mbid_cache.py failures are an unrelated sandbox DB disk-I/O issue.) --- core/metadata/source.py | 18 +++-- tests/metadata/test_album_artist_unknown.py | 73 +++++++++++++++++++++ 2 files changed, 86 insertions(+), 5 deletions(-) create mode 100644 tests/metadata/test_album_artist_unknown.py diff --git a/core/metadata/source.py b/core/metadata/source.py index 65ac780c..fcd69569 100644 --- a/core/metadata/source.py +++ b/core/metadata/source.py @@ -1032,11 +1032,19 @@ def extract_source_metadata(context: dict, artist: dict, album_info: dict) -> di album_artists = album_ctx.get("artists", []) if album_artists: first_album_artist = album_artists[0] - if isinstance(first_album_artist, dict) and first_album_artist.get("name"): - raw_album_artist = first_album_artist["name"] - elif isinstance(first_album_artist, str) and first_album_artist: - raw_album_artist = first_album_artist - album_artists_for_collab = album_artists + if isinstance(first_album_artist, dict): + candidate = first_album_artist.get("name", "") + elif isinstance(first_album_artist, str): + candidate = first_album_artist + else: + candidate = "" + # An unresolved "Unknown Artist" placeholder from the album context + # must NOT clobber the real track artist already in raw_album_artist + # (bug #735: album-artist tag overwritten to "Unknown Artist" on + # import). Only override when the album context names a real artist. + if candidate and candidate != "Unknown Artist": + raw_album_artist = candidate + album_artists_for_collab = album_artists collab_mode = cfg.get("file_organization.collab_artist_mode", "first") if collab_mode == "first" and raw_album_artist: diff --git a/tests/metadata/test_album_artist_unknown.py b/tests/metadata/test_album_artist_unknown.py new file mode 100644 index 00000000..f8b96d93 --- /dev/null +++ b/tests/metadata/test_album_artist_unknown.py @@ -0,0 +1,73 @@ +"""Regression for #735 (CubeComming): importing media overwrites the +album-artist tag ('Albuminterpret') to 'Unknown Artist'. + +When the metadata source resolves the track artist correctly (e.g. Billie +Eilish) but the album CONTEXT comes back with an unresolved 'Unknown Artist' +placeholder, extract_source_metadata used to take album_ctx['artists'][0]['name'] +unconditionally — clobbering the real album artist. The fix: an 'Unknown Artist' +placeholder must not override a real artist. +""" + +from __future__ import annotations + +from unittest.mock import MagicMock, patch + + +def _cfg(): + cfg = MagicMock() + cfg.get.side_effect = lambda key, default=None: { + "metadata_enhancement.enabled": True, + "metadata_enhancement.tags.write_multi_artist": False, + "metadata_enhancement.tags.feat_in_title": False, + "metadata_enhancement.tags.artist_separator": ", ", + "file_organization.collab_artist_mode": "first", + }.get(key, default) + return cfg + + +def _extract(context, artist_dict, album_info=None): + from core.metadata import source as src + with patch.object(src, "get_config_manager", return_value=_cfg()): + return src.extract_source_metadata(context, artist_dict, album_info or {}) + + +def test_unknown_album_artist_placeholder_does_not_clobber_real_artist(): + # Track resolves to a real artist; album context is an unresolved placeholder. + context = { + "original_search_result": {"title": "Therefore I Am", "artists": [{"name": "Billie Eilish"}]}, + "album": {"artists": [{"name": "Unknown Artist"}]}, + "source": "spotify", + } + md = _extract(context, {"name": "Billie Eilish"}) + assert md["artist"] == "Billie Eilish" + assert md["album_artist"] == "Billie Eilish" # NOT "Unknown Artist" + + +def test_real_album_artist_still_overrides(): + # A genuine album artist (not the placeholder) should still be used. + context = { + "original_search_result": {"title": "Song", "artists": [{"name": "Some Singer"}]}, + "album": {"artists": [{"name": "Various Artists"}]}, + "source": "spotify", + } + md = _extract(context, {"name": "Some Singer"}) + assert md["album_artist"] == "Various Artists" + + +def test_no_album_context_falls_back_to_track_artist(): + context = { + "original_search_result": {"title": "Song", "artists": [{"name": "Solo Act"}]}, + "source": "spotify", + } + md = _extract(context, {"name": "Solo Act"}) + assert md["album_artist"] == "Solo Act" + + +def test_empty_album_artist_does_not_clobber(): + context = { + "original_search_result": {"title": "Song", "artists": [{"name": "Real Name"}]}, + "album": {"artists": [{"name": ""}]}, + "source": "spotify", + } + md = _extract(context, {"name": "Real Name"}) + assert md["album_artist"] == "Real Name"