Bulk watchlist add: fall back through every source ID, not just active

The /api/library/watchlist-all-unwatched endpoint required the
user's currently active metadata source's ID column on each library
artist. A Spotify-primary user with library artists only matched
against iTunes or Deezer saw them silently skipped — surfacing on
Discord as "Library and Watchlist not syncing correctly". The per-
artist Enhanced View sync sometimes "fixed" them because it triggered
metadata enrichment that occasionally populated the missing Spotify
ID, but couldn't help artists Spotify simply doesn't carry.

Extracts the picker as a standalone helper so it can be tested
directly:

  core/watchlist/source_picker.py:pick_artist_id_for_watchlist

Picks the active source first when available, then falls back through
spotify -> itunes -> deezer -> discogs in registration order. Empty
strings count as missing. Numeric IDs are coerced to str so SQLite's
TEXT columns store them in the same form library code reads back.
Returns (None, None) only when the artist has zero source IDs — the
only legitimate skip reason now.

Adds 10 regression tests covering active-source priority for each
supported primary, fallback ordering through every secondary, the
zero-IDs base case, unrecognized active source (e.g. hydrabase still
falls through), empty-string handling, and numeric coercion.

core/watchlist/source_picker.py

@@ -0,0 +1,67 @@
+"""Active-source-aware artist ID picker for bulk watchlist add.
+
+The bulk "Add unwatched library artists to watchlist" endpoint used
+to drop artists silently whenever they didn't carry an ID for the
+user's currently active metadata source. A Spotify-primary user with
+library artists matched only against iTunes/Deezer would see them
+counted as ``skipped_no_id`` and never make it onto the watchlist —
+surfacing on Discord as "Library and Watchlist not syncing
+correctly". The per-artist Enhanced View sync sometimes "fixed" them
+because it re-ran enrichment that occasionally populated the missing
+ID, but that workaround couldn't help artists Spotify simply doesn't
+have.
+
+This helper picks the active source's ID first, then falls back
+through every other supported source so an artist makes it onto the
+watchlist as long as ANY metadata source can identify them.
+"""
+
+from typing import Any, Dict, Optional, Tuple
+
+
+# (source_name, library-artist-row column). Order is also the
+# fallback priority — Spotify > iTunes > Deezer > Discogs by default
+# coverage. The active source moves to the front of the queue inside
+# pick_artist_id_for_watchlist.
+SOURCE_ID_COLUMNS = (
+    ('spotify', 'spotify_artist_id'),
+    ('itunes', 'itunes_artist_id'),
+    ('deezer', 'deezer_id'),
+    ('discogs', 'discogs_id'),
+)
+
+
+def pick_artist_id_for_watchlist(
+    artist: Dict[str, Any],
+    active_source: Optional[str],
+) -> Tuple[Optional[str], Optional[str]]:
+    """Pick a (source-id, source-name) pair for adding ``artist`` to
+    the watchlist.
+
+    Tries ``active_source`` first when it appears in
+    ``SOURCE_ID_COLUMNS``, then falls back through every other source
+    in registration order. Empty strings count as missing. Returns
+    ``(None, None)`` only when the artist truly has no usable source
+    ID — that's the only legitimate skip reason for the bulk-add
+    flow.
+
+    The returned ID is always coerced to ``str`` because watchlist
+    columns are TEXT and SQLite will happily store the original int
+    type otherwise (which then breaks ID-based equality checks
+    between watchlist and library code paths).
+    """
+    preferred = next(
+        ((src, col) for src, col in SOURCE_ID_COLUMNS if src == active_source),
+        None,
+    )
+    ordered = [preferred] if preferred else []
+    ordered.extend(
+        (src, col) for src, col in SOURCE_ID_COLUMNS if (src, col) != preferred
+    )
+    for src, col in ordered:
+        if not src:
+            continue
+        value = artist.get(col)
+        if value:
+            return str(value), src
+    return None, None

tests/test_watchlist_bulk_add.py

@@ -0,0 +1,130 @@
+"""Regression tests for the bulk "Add unwatched library artists to
+watchlist" endpoint.
+
+Discord report: bulk add silently skipped library artists that didn't
+have an ID for the user's currently active metadata source. A
+Spotify-primary user with library artists matched only against iTunes
+or Deezer would see them counted as ``skipped_no_id`` and never make
+it onto the watchlist — the user perceived this as "Library and
+Watchlist not syncing correctly".
+
+These tests pin the new behaviour: try the active source first, then
+fall back to any other source ID the artist carries. Drop only when
+the artist has zero source IDs.
+"""
+
+from core.watchlist.source_picker import pick_artist_id_for_watchlist
+
+
+def _make_picker(active_source):
+    """Tiny adapter so test bodies stay readable as ``pick(artist)``."""
+    return lambda artist: pick_artist_id_for_watchlist(artist, active_source)
+
+
+# ---------------------------------------------------------------------------
+# Happy paths
+# ---------------------------------------------------------------------------
+
+
+def test_active_source_id_takes_priority_when_present() -> None:
+    """When the artist has the active source's ID, that one wins —
+    other sources don't override it."""
+    pick = _make_picker('spotify')
+    artist = {
+        'spotify_artist_id': 'sp-123',
+        'itunes_artist_id': 'it-456',
+        'deezer_id': 'dz-789',
+    }
+    assert pick(artist) == ('sp-123', 'spotify')
+
+
+def test_falls_back_to_itunes_when_active_spotify_missing() -> None:
+    """Spotify-primary user with an iTunes-only library artist must
+    still get the artist on the watchlist instead of being silently
+    skipped (the Discord-reported regression)."""
+    pick = _make_picker('spotify')
+    artist = {
+        'itunes_artist_id': 'it-456',
+        'deezer_id': 'dz-789',
+    }
+    assert pick(artist) == ('it-456', 'itunes')
+
+
+def test_falls_back_to_deezer_when_active_and_itunes_missing() -> None:
+    """Order matters — iTunes is preferred over Deezer when both
+    fallbacks exist, matching the real-world catalogue coverage
+    ranking the picker uses."""
+    pick = _make_picker('spotify')
+    artist = {
+        'deezer_id': 'dz-789',
+    }
+    assert pick(artist) == ('dz-789', 'deezer')
+
+
+def test_falls_back_to_discogs_as_last_resort() -> None:
+    pick = _make_picker('spotify')
+    artist = {
+        'discogs_id': 'dg-999',
+    }
+    assert pick(artist) == ('dg-999', 'discogs')
+
+
+def test_returns_none_when_artist_has_zero_source_ids() -> None:
+    """Drop only when the artist has no source IDs at all — that's
+    the only legitimate skip reason now."""
+    pick = _make_picker('spotify')
+    assert pick({'name': 'Some Artist'}) == (None, None)
+
+
+def test_active_source_itunes_picks_itunes_first() -> None:
+    """Active source ordering must work for non-Spotify primary too."""
+    pick = _make_picker('itunes')
+    artist = {
+        'spotify_artist_id': 'sp-123',
+        'itunes_artist_id': 'it-456',
+    }
+    assert pick(artist) == ('it-456', 'itunes')
+
+
+def test_active_source_deezer_picks_deezer_first() -> None:
+    pick = _make_picker('deezer')
+    artist = {
+        'spotify_artist_id': 'sp-123',
+        'deezer_id': 'dz-789',
+    }
+    assert pick(artist) == ('dz-789', 'deezer')
+
+
+def test_unrecognized_active_source_still_falls_back() -> None:
+    """If active_source is something the picker doesn't know (e.g.
+    'hydrabase'), still try every known source — better to add the
+    artist with whatever ID exists than reject silently."""
+    pick = _make_picker('hydrabase')
+    artist = {
+        'spotify_artist_id': 'sp-123',
+    }
+    # First fallback is Spotify per source_id_columns order
+    assert pick(artist) == ('sp-123', 'spotify')
+
+
+def test_empty_string_id_does_not_count_as_present() -> None:
+    """SQL NULL surfaces as None; defensive check that empty string
+    also falls through to the next source."""
+    pick = _make_picker('spotify')
+    artist = {
+        'spotify_artist_id': '',
+        'itunes_artist_id': 'it-456',
+    }
+    assert pick(artist) == ('it-456', 'itunes')
+
+
+def test_numeric_id_is_coerced_to_string() -> None:
+    """Some sources return numeric IDs from SQLite; the watchlist DB
+    stores them as TEXT, so the picker must coerce to string before
+    add_artist_to_watchlist sees them."""
+    pick = _make_picker('itunes')
+    artist = {'itunes_artist_id': 12345}
+    artist_id, src = pick(artist)
+    assert isinstance(artist_id, str)
+    assert artist_id == '12345'
+    assert src == 'itunes'

web_server.py

@@ -24834,15 +24834,16 @@ def watchlist_all_unwatched_library_artists():
         skipped_no_id = 0
         skipped_already = 0
 
+        # Try the active source's ID first, fall back through every other
+        # supported source. Pre-fix this loop required the active source's
+        # ID and silently dropped library artists that only had iTunes,
+        # Deezer, or Discogs IDs — surfaced as "Library and Watchlist not
+        # syncing correctly" on Discord because the bulk add reported
+        # "Added X" with no breakdown of why others were rejected.
+        from core.watchlist.source_picker import pick_artist_id_for_watchlist
+
         for artist in unwatched_artists:
-            # Use only the active source's ID — matches frontend modal filtering
-            artist_id = None
-            if active_source == 'spotify':
-                artist_id = artist.get('spotify_artist_id')
-            elif active_source == 'itunes':
-                artist_id = artist.get('itunes_artist_id')
-            elif active_source == 'deezer':
-                artist_id = artist.get('deezer_id')
+            artist_id, picked_source = pick_artist_id_for_watchlist(artist, active_source)
 
             if not artist_id:
                 skipped_no_id += 1
@@ -24857,8 +24858,7 @@ def watchlist_all_unwatched_library_artists():
                 skipped_already += 1
                 continue
 
-            src = active_source if active_source in ('spotify', 'itunes', 'deezer') else _get_metadata_fallback_source()
-            success = database.add_artist_to_watchlist(artist_id, artist_name, profile_id=get_current_profile_id(), source=src)
+            success = database.add_artist_to_watchlist(artist_id, artist_name, profile_id=get_current_profile_id(), source=picked_source)
             if success:
                 added += 1
                 # Use library thumb_url if available (no HTTP calls needed)