Merge pull request #557 from Nezreka/fix/multi-artist-tag-settings

Fix/multi artist tag settings

core/metadata/enrichment.py

@@ -110,9 +110,21 @@ def enhance_file_metadata(file_path: str, context: dict, artist: dict, album_inf
                 if metadata.get("title"):
                     audio_file.tags.add(symbols.TIT2(encoding=3, text=[metadata["title"]]))
                 if metadata.get("artist"):
+                    # TPE1 = display artist string (already joined by
+                    # source.py with the configured separator, or
+                    # primary-only when feat_in_title is on).
                     audio_file.tags.add(symbols.TPE1(encoding=3, text=[metadata["artist"]]))
+                    # When write_multi_artist is on, ALSO write the
+                    # multi-value list to a TXXX:Artists frame (Picard
+                    # convention). Keeps TPE1 as the display string AND
+                    # exposes the per-artist list for media servers
+                    # that read ARTISTS. Pre-fix this path overwrote
+                    # TPE1 with the list, which clobbered the
+                    # configured separator + feat_in_title semantics.
                     if write_multi and len(artists_list) > 1:
-                        audio_file.tags.add(symbols.TPE1(encoding=3, text=artists_list))
+                        audio_file.tags.add(
+                            symbols.TXXX(encoding=3, desc='Artists', text=list(artists_list))
+                        )
                 if metadata.get("album_artist"):
                     audio_file.tags.add(symbols.TPE2(encoding=3, text=[metadata["album_artist"]]))
                 if metadata.get("album"):

core/metadata/source.py

@@ -917,8 +917,84 @@ def extract_source_metadata(context: dict, artist: dict, album_info: dict) -> di
                 all_artists.append(artist_item)
             else:
                 all_artists.append(str(artist_item))
-        metadata["artist"] = ", ".join(all_artists)
-        logger.info("Metadata: Using all artists: '%s'", metadata["artist"])
+
+        # Deezer upgrade path: Deezer's `/search` endpoint only returns
+        # the primary artist for each track. The full contributors
+        # array (feat., remix collaborators, producers credited as
+        # artists) lives on `/track/<id>` and gets parsed by
+        # `_build_enhanced_track`. Without this upgrade Deezer-sourced
+        # tracks never get multi-artist tags even with the right
+        # settings on. One extra API call per Deezer-sourced track,
+        # only when the search response had a single artist (so it's
+        # a no-op when search already returned multiple).
+        if (source == "deezer" and len(all_artists) == 1
+                and source_ids.get("track_id")):
+            try:
+                from core.metadata import get_deezer_client
+                deezer = get_deezer_client()
+                if deezer:
+                    full = deezer.get_track_details(str(source_ids["track_id"]))
+                    if full and isinstance(full.get("artists"), list) and len(full["artists"]) > 1:
+                        upgraded = [a for a in full["artists"] if a]
+                        if upgraded:
+                            logger.info(
+                                "Metadata: Deezer contributors upgrade — search returned "
+                                "%d artist, /track/<id> returned %d (%s)",
+                                len(all_artists), len(upgraded), upgraded,
+                            )
+                            all_artists = upgraded
+            except Exception as e:
+                logger.debug("Deezer contributors upgrade failed: %s", e)
+
+        # Store the multi-artist list so the enrichment writer can emit
+        # proper multi-value ARTIST tags (TPE1 multi-value for ID3,
+        # "artists" key for Vorbis) when `write_multi_artist` is on.
+        # Without this assignment the field was always empty and the
+        # multi-artist write path silently no-op'd.
+        metadata["_artists_list"] = list(all_artists)
+
+        # `feat_in_title` (when true): pull featured artists out of the
+        # ARTIST tag entirely and append "(feat. X, Y)" to the title.
+        # Matches Picard / Beets convention and lets media servers
+        # group by primary artist instead of treating "A, B & C" as a
+        # distinct artist string.
+        # `artist_separator`: when feat_in_title is off (or there's
+        # only one artist) and write_multi_artist is on, this is the
+        # delimiter used to join all artists into the single ARTIST
+        # string. Picard defaults to "; " — we default to ", " to
+        # preserve historical behavior for users who haven't touched
+        # the setting.
+        feat_in_title = cfg.get("metadata_enhancement.tags.feat_in_title", False)
+        artist_separator = cfg.get("metadata_enhancement.tags.artist_separator", ", ")
+
+        if feat_in_title and len(all_artists) > 1:
+            metadata["artist"] = all_artists[0]
+            featured = all_artists[1:]
+            existing_title = metadata.get("title", "") or ""
+            # Don't double-append if the title already carries the
+            # featured artists. Source titles vary: "(feat. X)",
+            # "(featuring X)", "(ft. X)", "ft. X" (no parens), "[feat X]"
+            # (no period, brackets), etc. Word-boundary regex catches
+            # `feat`, `feat.`, `featuring`, `ft`, `ft.` regardless of
+            # surrounding punctuation. Case-insensitive.
+            import re as _feat_re
+            already_has_feat = bool(_feat_re.search(
+                r'\b(?:feat|feat\.|featuring|ft|ft\.)\b',
+                existing_title,
+                _feat_re.IGNORECASE,
+            ))
+            if existing_title and not already_has_feat:
+                metadata["title"] = f"{existing_title} (feat. {', '.join(featured)})"
+            logger.info(
+                "Metadata: feat_in_title — primary='%s', featured=%s, title='%s'",
+                metadata["artist"], featured, metadata["title"],
+            )
+        else:
+            metadata["artist"] = artist_separator.join(all_artists)
+            logger.info(
+                "Metadata: Using all artists joined with %r: '%s'",
+                artist_separator, metadata["artist"],
+            )
     else:
         metadata["artist"] = artist_dict.get("name", "") or get_import_clean_artist(context)
         logger.info("Metadata: Using primary artist: '%s'", metadata["artist"])

tests/metadata/test_multi_artist_tag_settings.py

@@ -0,0 +1,470 @@
+"""Pin multi-artist tag-write settings (issue: 'Multi artists settings not working').
+
+Three settings under `metadata_enhancement.tags`:
+  - `write_multi_artist` (bool) — write a separate multi-value tag
+    listing every artist (TXXX:Artists for ID3, "artists" key for
+    Vorbis). Picard convention.
+  - `artist_separator` (string, default ", ") — delimiter used to
+    join multiple artists into the single ARTIST/TPE1 string.
+  - `feat_in_title` (bool) — when true, ARTIST/TPE1 carries ONLY
+    the primary artist; featured artists get pulled out and
+    appended to the title as " (feat. X, Y)".
+
+Reporter (Netti93): all three were partially or completely
+unimplemented.
+  - Bug 1: `_artists_list` field read by enrichment.py was never
+    populated by source.py → multi-value writes silently no-op'd.
+  - Bug 2: `artist_separator` referenced in UI but ZERO Python code
+    read it → always hardcoded ", ".
+  - Bug 3: `feat_in_title` referenced in UI but ZERO Python code
+    read it → no implementation at all.
+
+These tests pin the fixed `extract_source_metadata` behavior:
+  - `_artists_list` populated whenever search response has multiple artists
+  - `artist_separator` config drives the join character for ARTIST string
+  - `feat_in_title` pulls featured artists into title, leaves only
+    primary in ARTIST string
+  - Title-already-has-feat case isn't double-appended
+  - Single-artist case unaffected by either setting
+"""
+
+from __future__ import annotations
+
+from types import SimpleNamespace
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+
+def _make_cfg(overrides=None):
+    """Stub config_manager. Defaults match the unset-config case
+    so each test can selectively override."""
+    overrides = overrides or {}
+    defaults = {
+        "metadata_enhancement.enabled": True,
+        "metadata_enhancement.tags.write_multi_artist": False,
+        "metadata_enhancement.tags.feat_in_title": False,
+        "metadata_enhancement.tags.artist_separator": ", ",
+    }
+    full = {**defaults, **overrides}
+
+    cfg = MagicMock()
+    cfg.get.side_effect = lambda key, default=None: full.get(key, default)
+    return cfg
+
+
+def _build_context(artists_list):
+    """Minimal context dict matching what extract_source_metadata reads."""
+    return {
+        "original_search_result": {
+            "title": "Sample Track",
+            "artists": [{"name": a} for a in artists_list],
+        },
+        "source": "spotify",
+    }
+
+
+def _call_extract(artists_list, cfg_overrides=None):
+    """Helper: patches config + calls extract_source_metadata, returns the
+    metadata dict. Avoids the broader source-specific embedding loop by
+    only using fields the multi-artist branch touches."""
+    from core.metadata import source as src_module
+
+    context = _build_context(artists_list)
+    artist_dict = {"name": artists_list[0] if artists_list else ""}
+    album_info = {"album_name": "Sample Album"}
+
+    with patch.object(src_module, "get_config_manager", return_value=_make_cfg(cfg_overrides)):
+        return src_module.extract_source_metadata(context, artist_dict, album_info)
+
+
+# ---------------------------------------------------------------------------
+# Bug 1: `_artists_list` populated
+# ---------------------------------------------------------------------------
+
+
+class TestArtistsListPopulated:
+    def test_multiple_artists_populate_list(self):
+        """Reporter's first bug — `_artists_list` field was always
+        empty. Verify it now contains every artist from the search
+        response."""
+        meta = _call_extract(["Eminem", "Dr. Dre", "50 Cent"])
+        assert meta.get("_artists_list") == ["Eminem", "Dr. Dre", "50 Cent"]
+
+    def test_single_artist_still_populates_list(self):
+        """Edge: even single-artist case populates the list (length 1).
+        Avoids special-casing downstream — `len(_artists_list) > 1`
+        check in enrichment.py is the gate."""
+        meta = _call_extract(["Solo Artist"])
+        assert meta.get("_artists_list") == ["Solo Artist"]
+
+    def test_no_artists_falls_through(self):
+        """When search response has no artists list, falls through to
+        the single-artist branch — no `_artists_list` written."""
+        from core.metadata import source as src_module
+
+        context = {
+            "original_search_result": {"title": "T", "artists": None},
+            "source": "spotify",
+        }
+        with patch.object(src_module, "get_config_manager", return_value=_make_cfg()):
+            meta = src_module.extract_source_metadata(context, {"name": "X"}, {})
+        assert "_artists_list" not in meta or meta.get("_artists_list") in (None, [])
+
+
+# ---------------------------------------------------------------------------
+# Bug 2: artist_separator drives ARTIST string
+# ---------------------------------------------------------------------------
+
+
+class TestArtistSeparator:
+    def test_default_separator_is_comma_space(self):
+        """Default preserves historical behavior — joining with ', '
+        so users who haven't set the config see no behavior change."""
+        meta = _call_extract(["A", "B", "C"])
+        assert meta["artist"] == "A, B, C"
+
+    def test_semicolon_separator(self):
+        """Reporter's exact case: artist_separator=';'. Picard convention."""
+        meta = _call_extract(["A", "B", "C"], cfg_overrides={
+            "metadata_enhancement.tags.artist_separator": ";",
+        })
+        assert meta["artist"] == "A;B;C"
+
+    def test_separator_with_space(self):
+        """Many users prefer '; ' (semi + space). Whatever string
+        the user puts in the config gets used verbatim — no trimming."""
+        meta = _call_extract(["A", "B"], cfg_overrides={
+            "metadata_enhancement.tags.artist_separator": "; ",
+        })
+        assert meta["artist"] == "A; B"
+
+    def test_separator_unused_for_single_artist(self):
+        """Single-artist case: separator irrelevant, ARTIST is just
+        the one name. No spurious trailing/leading separator."""
+        meta = _call_extract(["Solo"], cfg_overrides={
+            "metadata_enhancement.tags.artist_separator": ";",
+        })
+        assert meta["artist"] == "Solo"
+
+
+# ---------------------------------------------------------------------------
+# Bug 3: feat_in_title — pull featured into title
+# ---------------------------------------------------------------------------
+
+
+class TestFeatInTitle:
+    def test_feat_in_title_pulls_featured_to_title(self):
+        """Reporter's third bug. With feat_in_title=true, ARTIST holds
+        only primary; title gets " (feat. ...)" appended for
+        all-but-first."""
+        meta = _call_extract(["Eminem", "Dr. Dre", "50 Cent"], cfg_overrides={
+            "metadata_enhancement.tags.feat_in_title": True,
+        })
+        assert meta["artist"] == "Eminem"
+        assert "(feat. Dr. Dre, 50 Cent)" in meta["title"]
+
+    def test_feat_in_title_off_uses_separator(self):
+        """When feat_in_title is off (default), all artists join the
+        ARTIST string per `artist_separator`. Title stays unchanged."""
+        meta = _call_extract(["A", "B", "C"], cfg_overrides={
+            "metadata_enhancement.tags.feat_in_title": False,
+            "metadata_enhancement.tags.artist_separator": " & ",
+        })
+        assert meta["artist"] == "A & B & C"
+        assert "feat" not in meta["title"].lower()
+
+    def test_feat_in_title_skips_when_only_one_artist(self):
+        """Single-artist case: feat_in_title is a no-op. ARTIST = the
+        single name, title untouched."""
+        meta = _call_extract(["Solo"], cfg_overrides={
+            "metadata_enhancement.tags.feat_in_title": True,
+        })
+        assert meta["artist"] == "Solo"
+        assert "feat" not in meta["title"].lower()
+
+    def test_feat_in_title_no_double_append_when_title_already_has_feat(self):
+        """Defensive: if the source title already includes 'feat.' or
+        '(ft.', don't append again. Common on remixes / collabs where
+        the platform stores the featured artist in the track name."""
+        from core.metadata import source as src_module
+
+        context = {
+            "original_search_result": {
+                "title": "Track (feat. Already Listed)",
+                "artists": [{"name": "Primary"}, {"name": "Featured"}],
+            },
+            "source": "spotify",
+        }
+        cfg_overrides = {"metadata_enhancement.tags.feat_in_title": True}
+        with patch.object(src_module, "get_config_manager", return_value=_make_cfg(cfg_overrides)):
+            meta = src_module.extract_source_metadata(context, {"name": "Primary"}, {})
+
+        # Primary still pulled out of ARTIST...
+        assert meta["artist"] == "Primary"
+        # ...but title NOT double-appended (would be "(feat. X) (feat. Y)")
+        assert meta["title"].count("feat.") == 1
+
+    @pytest.mark.parametrize("source_title", [
+        "Track (feat. X)",        # standard parens + period
+        "Track (Feat. X)",        # capitalized
+        "Track (FEAT X)",         # all caps, no period
+        "Track (feat X)",         # no period, parens
+        "Track (Featuring X)",    # full word
+        "Track [feat. X]",        # square brackets
+        "Track ft. X",            # ft + period, no parens/brackets
+        "Track (ft X)",           # ft no period, parens
+        "Track FT. X",            # FT all caps
+    ])
+    def test_double_append_guard_recognizes_feat_variants(self, source_title):
+        """Defensive: source platforms (spotify / tidal / deezer) use
+        wildly different title conventions for featured artists. Guard
+        must recognize all of them so we never double-append."""
+        from core.metadata import source as src_module
+
+        context = {
+            "original_search_result": {
+                "title": source_title,
+                "artists": [{"name": "Primary"}, {"name": "Featured"}],
+            },
+            "source": "spotify",
+        }
+        cfg_overrides = {"metadata_enhancement.tags.feat_in_title": True}
+        with patch.object(src_module, "get_config_manager", return_value=_make_cfg(cfg_overrides)):
+            meta = src_module.extract_source_metadata(context, {"name": "Primary"}, {})
+
+        # Title left unchanged — no double-append for any variant
+        assert meta["title"] == source_title, (
+            f"Variant {source_title!r} got double-appended → {meta['title']!r}"
+        )
+
+    def test_double_append_guard_does_NOT_falsely_match_substrings(self):
+        """Sanity: word-boundary regex must NOT match 'ft' or 'feat'
+        as part of bigger words like 'aftermath', 'shaft', 'feature'.
+        Otherwise titles containing those words would skip the
+        legitimate (feat. X) append."""
+        from core.metadata import source as src_module
+
+        context = {
+            "original_search_result": {
+                "title": "Aftermath",  # contains 'ft' as substring
+                "artists": [{"name": "Primary"}, {"name": "Featured"}],
+            },
+            "source": "spotify",
+        }
+        cfg_overrides = {"metadata_enhancement.tags.feat_in_title": True}
+        with patch.object(src_module, "get_config_manager", return_value=_make_cfg(cfg_overrides)):
+            meta = src_module.extract_source_metadata(context, {"name": "Primary"}, {})
+
+        # Should APPEND because 'ft' inside 'Aftermath' isn't a
+        # standalone "ft" feature marker
+        assert "(feat. Featured)" in meta["title"]
+
+
+# ---------------------------------------------------------------------------
+# Integration — settings combine correctly
+# ---------------------------------------------------------------------------
+
+
+class TestSettingsCombination:
+    def test_feat_in_title_overrides_separator_for_artist_string(self):
+        """When BOTH settings are on, feat_in_title wins for the
+        ARTIST string (primary only). Separator is irrelevant in
+        that branch but `_artists_list` still carries every artist
+        for the multi-value tag write."""
+        meta = _call_extract(["A", "B", "C"], cfg_overrides={
+            "metadata_enhancement.tags.feat_in_title": True,
+            "metadata_enhancement.tags.artist_separator": ";",
+        })
+        assert meta["artist"] == "A"
+        assert "(feat. B, C)" in meta["title"]
+        # Multi-value list still complete — write_multi_artist would
+        # use this regardless of feat_in_title.
+        assert meta["_artists_list"] == ["A", "B", "C"]
+
+    def test_all_three_off_default_behavior_preserved(self):
+        """Sanity: unset config → joined ARTIST, no title change,
+        list still populated. Picks up no behavior change for users
+        who haven't touched the settings."""
+        meta = _call_extract(["A", "B"])
+        assert meta["artist"] == "A, B"
+        assert meta["title"] == "Sample Track"
+        assert meta["_artists_list"] == ["A", "B"]
+
+
+# ---------------------------------------------------------------------------
+# Deezer-specific: upgrade single-artist search results via /track/<id>
+# ---------------------------------------------------------------------------
+#
+# Deezer's `/search` endpoint only returns the primary artist for each
+# track. The full contributors array (feat., remix collaborators,
+# producers credited as artists) lives on `/track/<id>`. Reporter said
+# their Retag flow worked because it called the per-track endpoint, but
+# the initial enrichment used search-result data and missed the
+# contributors. The fix: when source==deezer AND search returned only
+# one artist AND a track_id is available, fetch the full track details
+# and upgrade the artists list.
+
+
+class TestDeezerContributorsUpgrade:
+    def test_upgrades_when_deezer_search_returns_single_artist(self):
+        """Reporter's exact case: Deezer track with multiple
+        contributors, search returns just the primary, /track/<id>
+        returns all 3. Upgrade path fetches the full set."""
+        from core.metadata import source as src_module
+
+        context = {
+            "original_search_result": {
+                "title": "Collab Track",
+                "artists": [{"name": "Primary"}],  # Only one — search-response shape
+            },
+            "source": "deezer",
+        }
+        # track_id resolved via original_search_result.id by get_import_source_ids
+        context["original_search_result"]["id"] = "12345"
+
+        fake_deezer = SimpleNamespace(get_track_details=MagicMock(return_value={
+            "id": "12345",
+            "name": "Collab Track",
+            "artists": ["Primary", "Featured1", "Featured2"],  # Full contributors
+        }))
+
+        cfg_overrides = {"metadata_enhancement.tags.artist_separator": "; "}
+
+        with patch.object(src_module, "get_config_manager", return_value=_make_cfg(cfg_overrides)), \
+             patch("core.metadata.get_deezer_client", return_value=fake_deezer):
+            meta = src_module.extract_source_metadata(context, {"name": "Primary"}, {})
+
+        # Upgraded list reaches the multi-value tag
+        assert meta["_artists_list"] == ["Primary", "Featured1", "Featured2"]
+        # And the joined ARTIST string respects the separator
+        assert meta["artist"] == "Primary; Featured1; Featured2"
+        fake_deezer.get_track_details.assert_called_once_with("12345")
+
+    def test_no_upgrade_when_search_already_returned_multiple(self):
+        """When search already has multiple artists, skip the upgrade —
+        no extra API call needed."""
+        from core.metadata import source as src_module
+
+        context = {
+            "original_search_result": {
+                "title": "T",
+                "artists": [{"name": "A"}, {"name": "B"}],  # Already multi
+            },
+            "source": "deezer",
+        }
+        # track_id resolved via original_search_result.id by get_import_source_ids
+        context["original_search_result"]["id"] = "12345"
+
+        fake_deezer = SimpleNamespace(get_track_details=MagicMock())
+
+        with patch.object(src_module, "get_config_manager", return_value=_make_cfg()), \
+             patch("core.metadata.get_deezer_client", return_value=fake_deezer):
+            meta = src_module.extract_source_metadata(context, {"name": "A"}, {})
+
+        assert meta["_artists_list"] == ["A", "B"]
+        # No upgrade call — search already had what we needed
+        fake_deezer.get_track_details.assert_not_called()
+
+    def test_no_upgrade_for_non_deezer_sources(self):
+        """Spotify/iTunes/Tidal already return multi-artist in search,
+        so the Deezer-specific upgrade path must NOT fire for them.
+        Otherwise we'd be making redundant API calls."""
+        from core.metadata import source as src_module
+
+        context = {
+            "original_search_result": {
+                "title": "T",
+                "artists": [{"name": "A"}],
+            },
+            "source": "spotify",
+            "source_track_id": "12345",
+        }
+
+        fake_deezer = SimpleNamespace(get_track_details=MagicMock())
+
+        with patch.object(src_module, "get_config_manager", return_value=_make_cfg()), \
+             patch("core.metadata.get_deezer_client", return_value=fake_deezer):
+            meta = src_module.extract_source_metadata(context, {"name": "A"}, {})
+
+        # Single artist preserved, no Deezer upgrade attempted
+        assert meta["_artists_list"] == ["A"]
+        fake_deezer.get_track_details.assert_not_called()
+
+    def test_upgrade_failure_falls_through_to_search_result(self):
+        """Defensive: if /track/<id> fails (network error, deezer
+        client unavailable), fall through to the search-result list.
+        Don't lose the single-artist data we already had."""
+        from core.metadata import source as src_module
+
+        context = {
+            "original_search_result": {
+                "title": "T",
+                "artists": [{"name": "Primary"}],
+            },
+            "source": "deezer",
+        }
+        # track_id resolved via original_search_result.id by get_import_source_ids
+        context["original_search_result"]["id"] = "12345"
+
+        fake_deezer = SimpleNamespace(get_track_details=MagicMock(
+            side_effect=RuntimeError("network down"),
+        ))
+
+        with patch.object(src_module, "get_config_manager", return_value=_make_cfg()), \
+             patch("core.metadata.get_deezer_client", return_value=fake_deezer):
+            meta = src_module.extract_source_metadata(context, {"name": "Primary"}, {})
+
+        # Search-result list preserved
+        assert meta["_artists_list"] == ["Primary"]
+        assert meta["artist"] == "Primary"
+
+    def test_upgrade_returns_same_count_no_change(self):
+        """Edge: /track/<id> returns the same single artist (track
+        genuinely has one artist on Deezer too). Should preserve the
+        list without false-positive upgrade."""
+        from core.metadata import source as src_module
+
+        context = {
+            "original_search_result": {
+                "title": "T",
+                "artists": [{"name": "Solo"}],
+            },
+            "source": "deezer",
+        }
+        # track_id resolved via original_search_result.id by get_import_source_ids
+        context["original_search_result"]["id"] = "12345"
+
+        fake_deezer = SimpleNamespace(get_track_details=MagicMock(return_value={
+            "id": "12345",
+            "artists": ["Solo"],  # Same single artist confirmed
+        }))
+
+        with patch.object(src_module, "get_config_manager", return_value=_make_cfg()), \
+             patch("core.metadata.get_deezer_client", return_value=fake_deezer):
+            meta = src_module.extract_source_metadata(context, {"name": "Solo"}, {})
+
+        assert meta["_artists_list"] == ["Solo"]
+
+    def test_no_upgrade_when_no_track_id(self):
+        """Edge: source==deezer but no track_id. Can't call
+        /track/<id> without an id. Don't attempt the upgrade."""
+        from core.metadata import source as src_module
+
+        context = {
+            "original_search_result": {
+                "title": "T",
+                "artists": [{"name": "Primary"}],
+            },
+            "source": "deezer",
+            "source_track_id": "",  # Missing
+        }
+
+        fake_deezer = SimpleNamespace(get_track_details=MagicMock())
+
+        with patch.object(src_module, "get_config_manager", return_value=_make_cfg()), \
+             patch("core.metadata.get_deezer_client", return_value=fake_deezer):
+            meta = src_module.extract_source_metadata(context, {"name": "Primary"}, {})
+
+        assert meta["_artists_list"] == ["Primary"]
+        fake_deezer.get_track_details.assert_not_called()

webui/static/helper.js

@@ -3416,6 +3416,7 @@ const WHATS_NEW = {
     '2.5.1': [
         // --- post-release patch work on the 2.5.1 line — entries hidden by _getLatestWhatsNewVersion until the build version bumps ---
         { date: 'Unreleased — 2.5.1 patch work' },
+        { title: 'Multi-Artist Tag Settings Now Actually Work (artist_separator + feat_in_title + write_multi_artist)', desc: 'three settings on settings → metadata → tags were partially or completely unimplemented. (1) `write_multi_artist` only worked because of a never-populated `_artists_list` field — `core/metadata/source.py` built `metadata["artist"]` as a hardcoded ", "-joined string but never assigned `metadata["_artists_list"]`, so `core/metadata/enrichment.py:114` always saw an empty list and silently no-op\'d the multi-value tag write. (2) `artist_separator` (default ", ") was referenced in the UI + settings.js save path but ZERO python code read the value — every multi-artist track ended up with hardcoded ", " regardless of what the user picked. (3) `feat_in_title` (when true: pull featured artists into the title as " (feat. X, Y)" and leave only primary in the ARTIST tag — picard convention) had no implementation at all. fix in source.py: populate `_artists_list` from the search response\'s artists array, then build the ARTIST string per the user\'s settings — primary-only when feat_in_title is on (with featured names appended to title; double-append guarded for source titles that already include "feat."), else joined with the configured separator. fix in enrichment.py id3 path: writing TPE1 twice (single-string then list) was overwriting the configured separator. now keeps TPE1 as the display string and writes a separate `TXXX:Artists` frame for the multi-value list (picard convention). vorbis path was already correct (separate "artist" + "artists" keys). deezer-specific upgrade path: deezer\'s `/search` endpoint only returns the primary artist — full contributors live on `/track/<id>`. when source==deezer AND the search response had a single artist AND a track_id is available, enrichment now fetches the per-track endpoint and upgrades the artists list before tag-write. one extra API call per affected deezer track (skipped when search already returned multiple). spotify, tidal, itunes search responses already include all artists so they\'re unaffected. 29 new tests pin: `_artists_list` populated for multi/single/no-artist cases, separator drives ARTIST string (default + custom), single-artist case unaffected by either setting, feat_in_title pulls featured to title + leaves primary in ARTIST, feat_in_title no-op for single artist, double-append guard recognizes 9 source-title variants ("(feat. X)", "(Feat. X)", "(FEAT X)", "(feat X)", "(Featuring X)", "[feat. X]", "ft. X", "(ft X)", "FT. X"), word-boundary regex doesn\'t false-match substrings ("Aftermath" still gets the append), combined-settings precedence (feat_in_title wins over separator for ARTIST string but `_artists_list` carries everyone for the multi-value tag), deezer upgrade fires only when search returned single artist + track_id available, no upgrade for non-deezer sources, upgrade failure falls through to search-result list, no false-positive when /track/<id> confirms single artist.', page: 'settings' },
         { title: 'AudioDB Enrichment: Track Worker No Longer Stuck In Infinite Retry Loop', desc: 'github issue #553: audiodb track enrichment "stuck" — constant requests, no progress, only error log was a 10s read-timeout from `lookup_track_by_id` repeating against the same track. trace: when an entity already has `audiodb_id` populated (from manual match or earlier scan) but `audiodb_match_status` is NULL, the worker tries a direct ID lookup. if it fails (returns None on timeout — audiodb\'s `track.php` endpoint is slow, 10s timeouts common), the prior code logged "preserving manual match" and returned WITHOUT marking status. row stayed NULL → queue picked it up next tick → tried direct lookup → timed out → returned → infinite loop. fix: (1) when direct lookup fails (None or exception), mark `audiodb_match_status="error"` so the queue\'s NULL-status filter stops re-picking the row on every tick. preserves the existing `audiodb_id` (no fallback to name-search guess that would overwrite a manual match). (2) extended the retry-after-cutoff queue priorities (4/5/6) to include `\'error\'` rows alongside `\'not_found\'` — same `retry_days=30` window. transient audiodb outages still recover automatically; permanently-broken IDs eventually get re-attempted once a month. only triggered for entities in the inconsistent state of `audiodb_id` set + `match_status` NULL — happy path and already-matched/already-not-found rows unchanged. 5 new tests pin: lookup-returns-none marks error (no infinite loop), lookup-raises-exception marks error, lookup-success preserves happy path, error-row-past-cutoff gets re-picked, error-row-within-cutoff stays skipped.', page: 'tools' },
         { title: 'Docker: Container No Longer Restart-Loops On Bind-Mounted Staging Folder', desc: 'after pulling latest, the container refused to start. logs showed `mkdir: cannot create directory \'/app/Staging\': Permission denied`. cause traced back to the 2026-05-08 image-bloat fix (commit 70e1750) which changed the Dockerfile from `chown -R /app` to a scoped chown on specific subdirs (the recursive chown was duplicating the whole /app tree into a new layer and ballooning image size). side effect: `/app` itself went from soulsync:soulsync to root:root (Docker WORKDIR default), AND `/app/Staging` was left out of both the Dockerfile mkdir + chown list and only created at runtime by the entrypoint script. on rootless Docker / Podman where in-container "root" maps to a host UID, the entrypoint mkdir on `/app/Staging` could fail with EACCES depending on the bind-mount path\'s host ownership — `set -e` then aborted the script and the container restart-looped. fix: (1) Dockerfile now pre-bakes `/app/Staging` into the image alongside the other runtime mount points (mkdir + scoped chown) so the entrypoint mkdir is a guaranteed no-op even when bind-mount perms are weird. (2) entrypoint mkdir + chown both have `|| true` now so any future bind-mount permission quirk surfaces as a log line, not a restart loop. (3) new writability audit at the end of entrypoint setup — `gosu soulsync test -w` on every bind-mountable dir, logs a loud warning with the exact `chown` command to run on the host if perms mismatch the configured PUID/PGID. catches the underlying bind-mount perm issue that the restart-loop fix would otherwise mask (container starts, but auto-import / downloads write into unwritable dirs and fail silently). zero behavior change for users whose containers were already starting fine; defensive against the rootless/podman config that broke after the image-bloat refactor.', page: 'tools' },
         { title: 'Your Albums: Download Missing Now Opens Selectable Modal + Tidal Resolution', desc: 'two-part fix to the your albums "download missing" flow on discover. (1) replaced the broken per-album direct-download loop with a selectable-grid modal mirroring the library page\'s download discography flow. clicking the download button now opens a checkbox grid showing every missing album (cover, title, artist, year, track count, source) with select all / deselect all controls. user picks what they actually want, hits "add to wishlist", each album\'s tracks get resolved + queued through the existing wishlist auto-download processor. matches the discography flow\'s per-album ndjson progress stream so users see ✓/✗ per album as it processes. previous loop fired direct downloads via `openDownloadMissingModalForYouTube` which the user reported as silently failing — "queuing 2/2" toast with no actual transfer activity. wishlist is the right destination for batch missing-album adds since it already handles retry, source fallback, dedup, and rate limiting. (2) added tidal source resolution. backend `/api/discover/album/<source>/<album_id>` got a new `tidal` source branch that calls a NEW `tidal_client.get_album_tracks(album_id)` method — two-phase fetch (cursor-walk `/v2/albums/<id>/relationships/items?include=items` for track refs + position metadata, batch-hydrate via existing `_get_tracks_batch` for artist/album names). track refs carry `meta.trackNumber` + `meta.volumeNumber` so multi-disc compilations render in album order. inline `?include=coverArt` lookup pulls the album cover too. single-album click flow (`openYourAlbumDownload`) gets `tidal_album_id` added to `trySources`. virtual-id generation includes tidal_album_id for stable identifiers. backend reuses the existing `/api/artist/<id>/download-discography` endpoint — its url artist_id param is functionally unused (per-album payload carries everything), so the modal posts with placeholder `your-albums` and gets multi-artist resolution for free. 10 new tests pin the tidal album-tracks method: single-page walk + hydration, multi-page cursor chain, multi-disc sort order, limit short-circuit, no-token short-circuit, http error returns empty, 429 propagates to rate_limited decorator, forward-compat type filter, partial-batch failure containment, empty-album short-circuit.', page: 'discover' },