Discover: sharpen track selection (diversity, source-aware popularity, library dedup, SQL genre)

Four selection-quality fixes on the SoulSync-made discover playlists.
None change public method signatures; all are tightenings on what's
already there.

(1) Diversity for Hidden Gems + Discovery Shuffle

Both used to be `RANDOM() LIMIT N` with no diversity. Could return
50 tracks from one artist or 20 from one album if the discovery
pool happened to be skewed. Both now over-fetch 3x and run the
existing `_apply_diversity_filter`:

- Hidden Gems: max 2 per album, 3 per artist
- Discovery Shuffle: max 2 per album, 2 per artist (tighter — shuffle
  should feel maximally varied)

(2) Source-aware popularity thresholds

`popularity >= 60` for "Popular Picks" and `popularity < 40` for
"Hidden Gems" was Spotify-shaped (0-100 scale). Deezer writes its
`rank` value into that column (often six-digit integers); iTunes
writes nothing meaningful. For Deezer-primary users:
- Popular Picks pulled essentially everything (rank >= 60 = all)
- Hidden Gems pulled essentially nothing (rank < 40 = none)

New `_get_popularity_thresholds(source)` helper returns per-source
values:

- Spotify: (60, 40) — the existing 0-100 scale
- Deezer: (500_000, 100_000) — ballpark from real rank values
- iTunes / unknown: (None, None) — skip the popularity filter
  entirely, fall back to random + diversity

`get_popular_picks` and `get_hidden_gems` now consult the helper.
When threshold is None they skip the popularity SQL filter. Diversity
+ ID gate still apply.

(3) Push genre keyword filter into SQL

`get_genre_playlist` used to fetch `limit=1_000_000` rows into Python
then run a substring keyword filter on `artist_genres`. Bad on big
discovery pools.

Now the keyword OR chain is generated as SQL placeholders:

    AND (artist_genres LIKE ? OR artist_genres LIKE ? OR ...)

Each placeholder gets `f'%{keyword.lower()}%'` via `extra_params`.
`fetch_limit` drops back to `limit * 10`. `_genre_matches` Python
helper deleted (only intra-file caller; verified via grep).

Parent-genre expansion via `GENRE_MAPPING` preserved — keywords list
feeds the LIKE chain unchanged.

(4) Filter out tracks already in library

Discovery pool can include tracks the user already owns. Hidden Gems
/ Shuffle / Popular Picks shouldn't surface those.

`_select_discovery_tracks` gained `exclude_owned: bool = True`
parameter. When True, adds a correlated NOT EXISTS subquery against
the `tracks` table covering all 3 source IDs:

    AND NOT EXISTS (
        SELECT 1 FROM tracks t WHERE
            (t.spotify_track_id IS NOT NULL AND t.spotify_track_id = discovery_pool.spotify_track_id)
         OR (t.itunes_track_id IS NOT NULL AND t.itunes_track_id = discovery_pool.itunes_track_id)
         OR (t.deezer_id IS NOT NULL AND t.deezer_id = discovery_pool.deezer_track_id)
    )

Note column-name asymmetry: tracks.deezer_id vs
discovery_pool.deezer_track_id. Inline comment marks the trap. All
5 public discovery methods automatically benefit (default True).
Seasonal Playlist doesn't go through the helper so it's unaffected
(curated content, dedup is wrong intent there).

Tests

12 new tests in `tests/test_personalized_playlists_id_gate.py` (27
total in the file):

- Hidden Gems + Discovery Shuffle apply diversity (cap proven by
  inserting 10 same-artist + same-album rows and asserting return
  count ≤ per-album cap)
- Popularity thresholds: Spotify (60, 40), Deezer larger scale,
  iTunes None / None
- Popular Picks skips threshold filter when None
- Genre playlist pushes filter to SQL (parent + child genre expansion)
- Owned-track exclusion: filtered when match, kept when no match,
  opt-out flag works
- Deezer column-name asymmetry pinned (regression footgun)

Test fixture re-added the minimal `tracks` table (4 columns: id,
spotify_track_id, itunes_track_id, deezer_id) — only what the new
NOT EXISTS subquery needs to join. Plus `insert_library_track`
helper.

Verification

- 27/27 in this test file pass (15 prior + 12 new)
- 2232/2232 full suite green
- ruff clean

LOC delta:
- core/personalized_playlists.py: 1030 → 1101 (+71)
- tests/test_personalized_playlists_id_gate.py: 352 → 616 (+264)

core/personalized_playlists.py

@@ -105,6 +105,26 @@ class PersonalizedPlaylistsService:
         from core.metadata_service import get_primary_source
         return get_primary_source()
 
+    def _get_popularity_thresholds(self, source: str) -> Tuple[Optional[int], Optional[int]]:
+        """Return (popular_min, hidden_max) thresholds for the given source.
+
+        Either value can be None to skip that filter for that source.
+
+        - Spotify: 60 / 40 (the existing 0-100 popularity scale)
+        - Deezer:  500000 / 100000 (rank values — ballpark from real data)
+        - iTunes / others: None / None (no popularity data; fall back to no
+          threshold filter, just diversity-on-random)
+
+        Sources are normalized lowercase so 'Spotify'/'spotify' both match.
+        """
+        normalized = (source or '').lower()
+        if normalized == 'spotify':
+            return 60, 40
+        if normalized == 'deezer':
+            return 500_000, 100_000
+        # iTunes, hydrabase, anything else — no usable popularity data
+        return None, None
+
     # Standard column set returned by every discovery_pool selector.
     # Callers can request additional columns via the `extra_columns` parameter
     # of `_select_discovery_tracks` (e.g. `release_date`, `artist_genres`).
@@ -131,6 +151,7 @@ class PersonalizedPlaylistsService:
         order_by: str = "RANDOM()",
         fetch_limit: int,
         extra_columns: tuple = (),
+        exclude_owned: bool = True,
     ) -> List[Dict]:
         """
         Shared selector for discovery_pool playlist methods.
@@ -146,6 +167,13 @@ class PersonalizedPlaylistsService:
             AND LOWER(artist_name) NOT IN
                 (SELECT LOWER(artist_name) FROM discovery_artist_blacklist)
 
+        When `exclude_owned=True` (default) the WHERE additionally excludes
+        any discovery_pool row whose IDs already match a row in the local
+        `tracks` table — i.e. tracks the user already has in their library.
+        Without this filter, Discovery / Hidden Gems / Popular Picks etc.
+        would happily surface tracks the user owns. Note the column-name
+        asymmetry: `tracks.deezer_id` ↔ `discovery_pool.deezer_track_id`.
+
         The ID gate is mandatory and not opt-out by design — if a future
         method needs to skip it, that's a design discussion, not a flag.
 
@@ -168,6 +196,8 @@ class PersonalizedPlaylistsService:
                 run a diversity filter should over-fetch (e.g. `limit * 3`).
             extra_columns: additional columns to SELECT beyond
                 `_STANDARD_DISCOVERY_COLUMNS` (e.g. `('release_date',)`).
+            exclude_owned: when True (default), filter out discovery rows
+                whose IDs match any row in the local `tracks` table.
 
         Returns:
             List of track dicts via `_build_track_dict`. Returns `[]` on any
@@ -177,6 +207,18 @@ class PersonalizedPlaylistsService:
             columns = self._STANDARD_DISCOVERY_COLUMNS + tuple(extra_columns)
             select_cols = ",\n                        ".join(columns)
 
+            owned_clause = ""
+            if exclude_owned:
+                # Note column-name asymmetry: discovery_pool.deezer_track_id
+                # but tracks.deezer_id. Don't refactor without checking.
+                owned_clause = """
+                  AND NOT EXISTS (
+                      SELECT 1 FROM tracks t
+                      WHERE (t.spotify_track_id IS NOT NULL AND t.spotify_track_id = discovery_pool.spotify_track_id)
+                         OR (t.itunes_track_id IS NOT NULL AND t.itunes_track_id = discovery_pool.itunes_track_id)
+                         OR (t.deezer_id IS NOT NULL AND t.deezer_id = discovery_pool.deezer_track_id)
+                  )"""
+
             query = f"""
                 SELECT
                         {select_cols}
@@ -184,6 +226,7 @@ class PersonalizedPlaylistsService:
                 WHERE source = ?
                   AND (spotify_track_id IS NOT NULL OR itunes_track_id IS NOT NULL OR deezer_track_id IS NOT NULL)
                   AND LOWER(artist_name) NOT IN (SELECT LOWER(artist_name) FROM discovery_artist_blacklist)
+                  {owned_clause}
                   {extra_where}
                 ORDER BY {order_by}
                 LIMIT ?
@@ -453,29 +496,16 @@ class PersonalizedPlaylistsService:
             logger.error(f"Error getting available genres: {e}")
             return []
 
-    def _genre_matches(self, artist_genres_json: Optional[str], search_keywords: List[str]) -> bool:
-        """Return True if any artist genre in the JSON-encoded column matches any keyword."""
-        if not artist_genres_json:
-            return False
-        try:
-            genres = json.loads(artist_genres_json)
-        except Exception:
-            return False
-        for artist_genre in genres:
-            artist_genre_lower = artist_genre.lower()
-            for keyword in search_keywords:
-                if keyword in artist_genre_lower:
-                    return True
-        return False
-
     def get_genre_playlist(self, genre: str, limit: int = 50, source: str = None) -> List[Dict]:
         """
         Get tracks from a specific genre with diversity filtering.
         Uses cached artist genres from database (populated during discovery scan).
         Supports both parent genres (e.g., "Electronic/Dance") and specific genres (e.g., "house").
 
-        The genre keyword match runs Python-side over the JSON-encoded artist_genres
-        column, so this method overfetches via the shared selector then filters.
+        The keyword match is pushed into SQL as an OR-chain of LIKE clauses
+        on `artist_genres`, so we no longer have to over-fetch the entire
+        pool to filter Python-side. SQLite's LIKE is case-insensitive for
+        ASCII, matching the previous case-insensitive Python comparison.
         """
         active_source = source or self._get_active_source()
 
@@ -488,38 +518,27 @@ class PersonalizedPlaylistsService:
             search_keywords = [genre.lower()]
             logger.info(f"Matching specific genre '{genre}' with partial matching")
 
-        # Pull every source row with non-null artist_genres through the shared
-        # selector (gets the ID gate + blacklist filter for free). Cap at a high
-        # bound — the Python keyword filter narrows the result drastically.
-        candidate_tracks = self._select_discovery_tracks(
+        # Build the SQL keyword OR-chain. One LIKE per keyword, all OR'd
+        # together inside a single parenthesized clause so the surrounding
+        # AND structure isn't broken.
+        like_clauses = " OR ".join(["artist_genres LIKE ?"] * len(search_keywords))
+        like_params = tuple(f"%{k}%" for k in search_keywords)
+
+        # Over-fetch 10x for diversity filter headroom — the SQL filter
+        # has already narrowed to genre matches, so this is bounded.
+        all_tracks = self._select_discovery_tracks(
             source=active_source,
-            extra_where="AND artist_genres IS NOT NULL",
+            extra_where=f"AND artist_genres IS NOT NULL AND ({like_clauses})",
+            extra_params=like_params,
             order_by="RANDOM()",
-            fetch_limit=1_000_000,
+            fetch_limit=limit * 10,
             extra_columns=('artist_genres',),
         )
 
-        if not candidate_tracks:
-            logger.warning(f"No tracks with genre data found for source: {active_source}")
-            return []
-
-        # `_build_track_dict` stashes the raw `artist_genres` column under
-        # `_artist_genres_raw` (since we requested it via `extra_columns`),
-        # so the keyword match can run without re-querying.
-        matching_tracks = [
-            track for track in candidate_tracks
-            if self._genre_matches(track.get('_artist_genres_raw'), search_keywords)
-        ]
-
-        if not matching_tracks:
+        if not all_tracks:
             logger.warning(f"No tracks found for genre: {genre}")
             return []
 
-        random.shuffle(matching_tracks)
-
-        # Cap candidate set at 10x limit for diversity filtering headroom.
-        all_tracks = matching_tracks[:limit * 10] if len(matching_tracks) > limit * 10 else matching_tracks
-
         max_per_album, max_per_artist = self._compute_adaptive_diversity_limits(all_tracks, relaxed=True)
         unique_artists = len(set(t['artist_name'] for t in all_tracks))
         logger.info(
@@ -527,7 +546,6 @@ class PersonalizedPlaylistsService:
             f"limits: {max_per_album}/album, {max_per_artist}/artist"
         )
 
-        random.shuffle(all_tracks)
         diverse_tracks = self._apply_diversity_filter(
             all_tracks,
             max_per_album=max_per_album,
@@ -543,14 +561,30 @@ class PersonalizedPlaylistsService:
     # ========================================
 
     def get_popular_picks(self, limit: int = 50) -> List[Dict]:
-        """Get high popularity tracks from discovery pool with diversity (max 2 per album, 3 per artist)."""
+        """Get high popularity tracks from discovery pool with diversity (max 2 per album, 3 per artist).
+
+        Popularity threshold is source-aware via `_get_popularity_thresholds`
+        — sources without a usable popularity scale (iTunes / Hydrabase)
+        skip the threshold filter and fall back to RANDOM + diversity only.
+        """
         active_source = self._get_active_source()
+        popular_min, _ = self._get_popularity_thresholds(active_source)
+
+        if popular_min is not None:
+            extra_where = "AND popularity >= ?"
+            extra_params: tuple = (popular_min,)
+            order_by = "popularity DESC, RANDOM()"
+        else:
+            extra_where = ""
+            extra_params = ()
+            order_by = "RANDOM()"
 
         # Over-fetch 3x so the diversity filter has room to spread albums/artists.
         all_tracks = self._select_discovery_tracks(
             source=active_source,
-            extra_where="AND popularity >= 60",
-            order_by="popularity DESC, RANDOM()",
+            extra_where=extra_where,
+            extra_params=extra_params,
+            order_by=order_by,
             fetch_limit=limit * 3,
         )
 
@@ -565,31 +599,68 @@ class PersonalizedPlaylistsService:
         return diverse_tracks
 
     def get_hidden_gems(self, limit: int = 50) -> List[Dict]:
-        """Get low-popularity (underground/indie) tracks from discovery pool."""
+        """Get low-popularity (underground/indie) tracks from discovery pool with diversity.
+
+        Mirrors Popular Picks' diversity caps (2 per album, 3 per artist)
+        since both are curated-feel sections. Popularity threshold is
+        source-aware — iTunes/Hydrabase fall back to RANDOM + diversity
+        only.
+        """
         active_source = self._get_active_source()
-        tracks = self._select_discovery_tracks(
+        _, hidden_max = self._get_popularity_thresholds(active_source)
+
+        if hidden_max is not None:
+            extra_where = "AND popularity < ?"
+            extra_params: tuple = (hidden_max,)
+        else:
+            extra_where = ""
+            extra_params = ()
+
+        # Over-fetch 3x for diversity filter headroom.
+        all_tracks = self._select_discovery_tracks(
             source=active_source,
-            extra_where="AND popularity < 40",
+            extra_where=extra_where,
+            extra_params=extra_params,
             order_by="RANDOM()",
-            fetch_limit=limit,
+            fetch_limit=limit * 3,
         )
-        logger.info(f"Hidden Gems ({active_source}): selected {len(tracks)} tracks")
-        return tracks
+
+        diverse_tracks = self._apply_diversity_filter(
+            all_tracks,
+            max_per_album=2,
+            max_per_artist=3,
+            limit=limit,
+        )
+
+        logger.info(f"Hidden Gems ({active_source}): selected {len(diverse_tracks)} tracks with diversity")
+        return diverse_tracks
 
     def get_discovery_shuffle(self, limit: int = 50) -> List[Dict]:
         """
         Get random tracks from discovery pool - pure exploration.
 
-        Different every time you call it!
+        Different every time you call it! Tighter diversity than Popular
+        Picks / Hidden Gems (2 per album, 2 per artist) since shuffle
+        should feel maximally varied.
         """
         active_source = self._get_active_source()
-        tracks = self._select_discovery_tracks(
+
+        # Over-fetch 3x for diversity filter headroom.
+        all_tracks = self._select_discovery_tracks(
             source=active_source,
             order_by="RANDOM()",
-            fetch_limit=limit,
+            fetch_limit=limit * 3,
+        )
+
+        diverse_tracks = self._apply_diversity_filter(
+            all_tracks,
+            max_per_album=2,
+            max_per_artist=2,
+            limit=limit,
         )
-        logger.info(f"Discovery Shuffle ({active_source}): selected {len(tracks)} tracks")
-        return tracks
+
+        logger.info(f"Discovery Shuffle ({active_source}): selected {len(diverse_tracks)} tracks with diversity")
+        return diverse_tracks
 
     # ========================================
     # DAILY MIX (HYBRID PLAYLISTS)

tests/test_personalized_playlists_id_gate.py

@@ -68,6 +68,16 @@ class _FakeDatabase:
             CREATE TABLE discovery_artist_blacklist (
                 artist_name TEXT PRIMARY KEY
             );
+            -- Minimal `tracks` table: exists so the `exclude_owned`
+            -- subquery in `_select_discovery_tracks` can join. Real
+            -- schema has many more columns; we only need the source-id
+            -- columns it actually inspects.
+            CREATE TABLE tracks (
+                id INTEGER PRIMARY KEY,
+                spotify_track_id TEXT,
+                itunes_track_id TEXT,
+                deezer_id TEXT
+            );
         """)
         self._conn.commit()
 
@@ -95,6 +105,18 @@ class _FakeDatabase:
         )
         self._conn.commit()
 
+    def insert_library_track(self, **kwargs):
+        """Insert a row into the local `tracks` table (the user's library).
+        Used to prove `exclude_owned=True` filters discovery rows whose IDs
+        match a library row."""
+        cols = ", ".join(kwargs.keys())
+        placeholders = ", ".join(["?"] * len(kwargs))
+        self._conn.execute(
+            f"INSERT INTO tracks ({cols}) VALUES ({placeholders})",
+            tuple(kwargs.values()),
+        )
+        self._conn.commit()
+
 
 @pytest.fixture
 def service():
@@ -350,3 +372,245 @@ def test_get_decade_playlist_filters_null_id_rows(service):
     assert [t['track_name'] for t in out] == ['Yes']
 
 
+# ---------------------------------------------------------------------------
+# Fix #1 — Hidden Gems + Discovery Shuffle apply diversity
+# ---------------------------------------------------------------------------
+
+
+def test_get_hidden_gems_applies_diversity(service):
+    """10 low-popularity tracks all from the same album/artist should be
+    capped at the per-album limit (2) — Hidden Gems no longer returns
+    raw RANDOM() rows."""
+    svc, db = service
+    for i in range(10):
+        db.insert_discovery_track(
+            source='spotify', spotify_track_id=f'sp{i}',
+            track_name=f't{i}', artist_name='SoloArtist',
+            album_name='OnlyAlbum', popularity=20,
+        )
+    out = svc.get_hidden_gems(limit=10)
+    # All 10 share the same album → diversity cap of 2 per album wins.
+    assert len(out) == 2
+    assert all(t['album_name'] == 'OnlyAlbum' for t in out)
+
+
+def test_get_discovery_shuffle_applies_diversity(service):
+    """Same idea for shuffle, with tighter caps (2 per artist)."""
+    svc, db = service
+    for i in range(10):
+        db.insert_discovery_track(
+            source='spotify', spotify_track_id=f'sp{i}',
+            track_name=f't{i}', artist_name='SoloArtist',
+            album_name=f'Album{i}',  # different albums so artist cap bites
+            popularity=50,
+        )
+    out = svc.get_discovery_shuffle(limit=10)
+    # Same artist → per-artist cap of 2 wins.
+    assert len(out) == 2
+    assert all(t['artist_name'] == 'SoloArtist' for t in out)
+
+
+# ---------------------------------------------------------------------------
+# Fix #2 — Source-aware popularity thresholds
+# ---------------------------------------------------------------------------
+
+
+def test_popularity_thresholds_spotify_returns_60_40():
+    svc = PersonalizedPlaylistsService(_FakeDatabase())
+    popular_min, hidden_max = svc._get_popularity_thresholds('spotify')
+    assert popular_min == 60
+    assert hidden_max == 40
+
+
+def test_popularity_thresholds_deezer_returns_higher_scale():
+    """Deezer writes `rank` (raw integer, often 100k+) into the popularity
+    column, so thresholds must be in that range — Spotify's 60/40 would
+    classify almost every Deezer track as a hidden gem."""
+    svc = PersonalizedPlaylistsService(_FakeDatabase())
+    popular_min, hidden_max = svc._get_popularity_thresholds('deezer')
+    assert popular_min is not None and popular_min >= 100_000
+    assert hidden_max is not None and hidden_max >= 50_000
+    assert popular_min > hidden_max
+
+
+def test_popularity_thresholds_itunes_skips_filter():
+    """iTunes has no usable popularity data — both thresholds should be
+    None so callers fall back to RANDOM + diversity only."""
+    svc = PersonalizedPlaylistsService(_FakeDatabase())
+    popular_min, hidden_max = svc._get_popularity_thresholds('itunes')
+    assert popular_min is None
+    assert hidden_max is None
+
+
+def test_get_popular_picks_skips_threshold_when_none():
+    """When the active source has no popularity data, Popular Picks
+    should skip the popularity filter entirely (just diversity + ID
+    gate). Insert rows with a mix of popularity values; with the iTunes
+    source they should ALL pass the popularity gate."""
+    db = _FakeDatabase()
+    svc = PersonalizedPlaylistsService(db)
+    db.insert_discovery_track(
+        source='itunes', itunes_track_id='it1', track_name='Low',
+        artist_name='A', album_name='Album1', popularity=5,
+    )
+    db.insert_discovery_track(
+        source='itunes', itunes_track_id='it2', track_name='High',
+        artist_name='B', album_name='Album2', popularity=95,
+    )
+    db.insert_discovery_track(
+        source='itunes', itunes_track_id='it3', track_name='Zero',
+        artist_name='C', album_name='Album3', popularity=0,
+    )
+    with patch.object(svc, '_get_active_source', return_value='itunes'):
+        out = svc.get_popular_picks(limit=10)
+    names = sorted(t['track_name'] for t in out)
+    assert names == ['High', 'Low', 'Zero']
+
+
+# ---------------------------------------------------------------------------
+# Fix #3 — Genre keyword filter pushed to SQL
+# ---------------------------------------------------------------------------
+
+
+def test_get_genre_playlist_pushes_filter_to_sql(service):
+    """Insert tracks with various artist_genres JSON values; only those
+    whose genres contain a rock keyword should come through. The match
+    happens in SQL (LIKE on the JSON-encoded string) rather than after
+    a million-row over-fetch."""
+    import json as _json
+    svc, db = service
+    db.insert_discovery_track(
+        source='spotify', spotify_track_id='sp1', track_name='RockSong',
+        artist_name='RockBand', album_name='Album1',
+        artist_genres=_json.dumps(['indie rock', 'alternative']),
+    )
+    db.insert_discovery_track(
+        source='spotify', spotify_track_id='sp2', track_name='JazzSong',
+        artist_name='JazzCat', album_name='Album2',
+        artist_genres=_json.dumps(['bebop', 'cool jazz']),
+    )
+    db.insert_discovery_track(
+        source='spotify', spotify_track_id='sp3', track_name='PopSong',
+        artist_name='PopStar', album_name='Album3',
+        artist_genres=_json.dumps(['k-pop']),
+    )
+    out = svc.get_genre_playlist('rock', limit=10)
+    names = [t['track_name'] for t in out]
+    # Only the indie rock track matches the literal "rock" keyword.
+    assert 'RockSong' in names
+    assert 'JazzSong' not in names
+    # k-pop doesn't contain "rock" as a substring → excluded.
+    assert 'PopSong' not in names
+
+
+def test_get_genre_playlist_handles_parent_genre(service):
+    """Parent genres in GENRE_MAPPING expand to all their child keywords;
+    a track tagged with any child genre should be included."""
+    import json as _json
+    svc, db = service
+    # 'Electronic/Dance' parent expands to keywords like 'house', 'techno',
+    # 'edm' etc. Tag tracks with various children.
+    db.insert_discovery_track(
+        source='spotify', spotify_track_id='sp1', track_name='HouseTrack',
+        artist_name='DJ1', album_name='A1',
+        artist_genres=_json.dumps(['deep house']),
+    )
+    db.insert_discovery_track(
+        source='spotify', spotify_track_id='sp2', track_name='TechnoTrack',
+        artist_name='DJ2', album_name='A2',
+        artist_genres=_json.dumps(['minimal techno']),
+    )
+    db.insert_discovery_track(
+        source='spotify', spotify_track_id='sp3', track_name='RockTrack',
+        artist_name='Band1', album_name='A3',
+        artist_genres=_json.dumps(['indie rock']),
+    )
+    out = svc.get_genre_playlist('Electronic/Dance', limit=10)
+    names = sorted(t['track_name'] for t in out)
+    assert 'HouseTrack' in names
+    assert 'TechnoTrack' in names
+    assert 'RockTrack' not in names
+
+
+# ---------------------------------------------------------------------------
+# Fix #4 — `_select_discovery_tracks` excludes already-owned tracks
+# ---------------------------------------------------------------------------
+
+
+def test_discovery_helper_excludes_owned_tracks(service):
+    """Discovery row with spotify_track_id='sp1' + library track with the
+    same spotify_track_id should be filtered out. Owned tracks shouldn't
+    surface in Hidden Gems / Shuffle / Popular Picks."""
+    svc, db = service
+    db.insert_discovery_track(
+        source='spotify', spotify_track_id='sp1', track_name='Owned',
+        artist_name='A', album_name='X', popularity=50,
+    )
+    db.insert_library_track(spotify_track_id='sp1')
+
+    tracks = svc._select_discovery_tracks(
+        source='spotify',
+        order_by='track_name',
+        fetch_limit=100,
+    )
+    assert tracks == []
+
+
+def test_discovery_helper_keeps_unowned_tracks(service):
+    """Same shape but the library row carries a different spotify_track_id
+    — discovery row should pass through."""
+    svc, db = service
+    db.insert_discovery_track(
+        source='spotify', spotify_track_id='sp1', track_name='NotOwned',
+        artist_name='A', album_name='X', popularity=50,
+    )
+    db.insert_library_track(spotify_track_id='sp_different')
+
+    tracks = svc._select_discovery_tracks(
+        source='spotify',
+        order_by='track_name',
+        fetch_limit=100,
+    )
+    assert [t['track_name'] for t in tracks] == ['NotOwned']
+
+
+def test_discovery_helper_can_disable_owned_filter(service):
+    """`exclude_owned=False` lets owned rows pass through — used by code
+    paths that legitimately want library matches (currently none, but
+    the flag should still work)."""
+    svc, db = service
+    db.insert_discovery_track(
+        source='spotify', spotify_track_id='sp1', track_name='Owned',
+        artist_name='A', album_name='X', popularity=50,
+    )
+    db.insert_library_track(spotify_track_id='sp1')
+
+    tracks = svc._select_discovery_tracks(
+        source='spotify',
+        order_by='track_name',
+        fetch_limit=100,
+        exclude_owned=False,
+    )
+    assert [t['track_name'] for t in tracks] == ['Owned']
+
+
+def test_discovery_helper_owned_filter_handles_deezer_id_asymmetry(service):
+    """Column-name asymmetry: discovery_pool.deezer_track_id vs
+    tracks.deezer_id. Pin this — easy to break in a future refactor."""
+    svc, db = service
+    db.insert_discovery_track(
+        source='deezer', deezer_track_id='dz1',
+        spotify_track_id=None, itunes_track_id=None,
+        track_name='OwnedDeezer', artist_name='A', album_name='X',
+        popularity=50,
+    )
+    db.insert_library_track(deezer_id='dz1')
+
+    with patch.object(svc, '_get_active_source', return_value='deezer'):
+        tracks = svc._select_discovery_tracks(
+            source='deezer',
+            order_by='track_name',
+            fetch_limit=100,
+        )
+    assert tracks == []
+

webui/static/helper.js

@@ -3416,6 +3416,7 @@ const WHATS_NEW = {
     '2.4.3': [
         // --- post-2.4.2 dev work — entries hidden by _getLatestWhatsNewVersion until the build version bumps ---
         { date: 'Unreleased — 2.4.3 dev cycle' },
+        { title: 'Discover: Sharper Track Selection (Diversity, Source-Aware Popularity, Library Dedup, SQL Genre Filter)', desc: 'four selection-quality fixes on the soulsync-made discover playlists. (1) hidden gems and discovery shuffle had no diversity caps — they could return 50 tracks from the same artist or 20 from one album. now both apply the existing `_apply_diversity_filter` (over-fetch 3x then enforce per-album/per-artist caps; shuffle uses tighter caps because it should feel maximally varied). (2) `popularity` thresholds were spotify-shaped (0-100 scale, popular >= 60 / hidden < 40), but deezer writes its rank value into that column (often six-digit integers) and itunes writes nothing meaningful. for deezer-primary users this meant popular picks pulled essentially everything and hidden gems pulled nothing. new `_get_popularity_thresholds(source)` returns per-source values: spotify (60, 40), deezer (500_000, 100_000) ballpark, itunes/other (None, None) which skips the popularity filter entirely and falls back to random + diversity. (3) `get_genre_playlist` used to load up to 1M discovery_pool rows into python and run a substring keyword filter on the json column. now the keyword OR chain pushes down into sql via `(artist_genres LIKE ? OR ...)` placeholders, fetch_limit drops to limit*10. parent-genre expansion via GENRE_MAPPING preserved. (4) discovery selectors now exclude tracks the user already owns — `_select_discovery_tracks` gained `exclude_owned: bool = True` (default on) which adds a `NOT EXISTS (SELECT FROM tracks WHERE source_id matches)` correlated subquery covering the spotify/itunes/deezer-id columns (with the deezer-column-name asymmetry handled inline: discovery_pool.deezer_track_id vs tracks.deezer_id). hidden gems / shuffle / popular picks / decade / genre browser all benefit automatically. 12 new tests (27 total in the file): diversity caps, source-aware threshold values, threshold-skip behavior, sql-pushed genre filter, parent-genre expansion, owned-track exclusion, opt-out flag, and the deezer-column-name asymmetry trap. 2232/2232 full suite green.', page: 'discover' },
         { title: 'Discover: Stop Showing Undownloadable Tracks (+Lift +Cleanup)', desc: 'audit found multiple discover-page sections (hidden gems / discovery shuffle / popular picks / decade browser / genre browser) had no `WHERE (spotify_track_id IS NOT NULL OR itunes_track_id IS NOT NULL OR ...)` gate on their selection sql. tracks with no source ids in the discovery pool were getting displayed, the user would click download, and the download would silently fail because there was nothing to look up. fix: lifted all five discovery_pool selection methods into shared private helpers (`_select_discovery_tracks`, `_apply_diversity_filter`, `_compute_adaptive_diversity_limits`) on `PersonalizedPlaylistsService`. mandatory id-validity gate is hard-coded into the selector — no opt-out flag, every public method inherits it for free. behavior preserved: same diversity tiers, same over-fetch multipliers, same popularity thresholds, same blacklist filter. ~314 lines of repeated select/diversity boilerplate collapsed across the 5 methods (-55% on those methods\' business logic). also deleted four sections that had been stubbed returning [] for ages (recently added / top tracks / forgotten favorites / familiar favorites) — frontend, backend endpoints, html blocks, helper docs, all gone. on the frontend, lifted the duplicated decade-browser + genre-browser tab management (~314 lines of identical fetch-tabs / render-tabstrip / fetch-content / render-tracklist / wire-sync-button code) into one shared `createTabbedBrowserSection(config)` helper. each browser is now a thin wrapper: ~3 lines per public function. 14 new tests pin the gate (every selector filters null-id rows), the diversity caps, the adaptive limit tiers, the source filter, and the blacklist filter.', page: 'discover' },
         { title: 'Internal: Discover Controller — Cin Pre-Review Polish', desc: 'tightened the controller before opening the PR. (1) dropped the magic `extractItems` defaults — controller used to auto-pull `data.items` / `data.albums` / `data.artists` / `data.tracks` / `data.results` if no extractor was provided. removed the fallback chain. each section now MUST supply its own `extractItems(data) => array` callback. cin standard: explicit > implicit; the auto-fallback could silently grab the wrong key on endpoints that return multiple arrays. validated at register-time so misuse fails immediately. all 10 existing call sites already had explicit extractors so no migration churn. (2) replaced the `renderItems` returning null convention (used by Your Albums + manualDom-style sections) with an explicit `manualDom: true` config flag. clearer intent at the call site, less likely to be confused with a renderer error. (3) added a minimal node `--test` JS test file at `tests/static/test_discover_section_controller.mjs` — 32 tests pin the lifecycle contract: config validation (every required field), happy-path fetch+render, empty/stale/error states, no-fetch `data:` mode, manualDom mode, callable `fetchUrl`, load coalescing, refresh bypass, hook error containment, error toasts. runs via `node --test tests/static/` directly, OR via the regular pytest sweep (`tests/test_discover_section_controller_js.py` shells out to node and asserts a clean exit). skipped gracefully when node isn\'t available or is &lt; 22. closes the "controller is a contract, pin it at the test boundary" gap that cin would have flagged. 2205/2205 full suite green (was 2204 + 1 new pytest wrapper); 32/32 node --test pass; ruff clean; js parses clean.', page: 'discover' },
         { title: 'Internal: Discover Cleanup Round — Toast Errors, Stale State, Skipped Sections', desc: 'follow-up to the controller migration. extended `createDiscoverSectionController` with the hooks the per-section migrations surfaced as needed: callable `fetchUrl` (resolves the seasonal-playlist recreate-on-key-change hack), no-fetch `data:` mode (lets render-only sections like seasonal albums use the controller without inventing a fake endpoint), `beforeLoad` hook (lets dynamically-inserted sections like because-you-listen-to ensure their container exists before the spinner shows), `onSuccess(data)` hook (cleaner home for sibling header / subtitle / button updates than folding them into renderItems), and an `isStale` / `onStale` / `renderStale` triple for the third render state (data is empty BUT upstream is still discovering — show updating UI + start a poller, instead of the bare empty-state copy). turned on `showErrorToast: true` for every migrated section — section load failures now surface a global toast instead of silently spinning forever or swallowing into console.debug. that\'s the JohnBaumb #369 pattern applied at the UI layer. migrated the two sections that didn\'t fit the original controller contract: `loadYourAlbums` (uses isStale/onStale for stale-fetch UI + onSuccess for subtitle/filters/download-button side-effects + renderItems returning null since it delegates to the existing grid renderer) and `loadSeasonalAlbums` (uses no-fetch data mode since the parent `loadSeasonalContent` already fetched the season payload). also lifted the duplicated decade-tab + genre-tab sync-status block (✓/⏳/✗/percentage) into a `_renderSyncStatusBlock(idPrefix)` helper — two call sites now share one implementation. listenbrainz playlists keep their own block because the semantics differ (matching progress vs download progress). audit found the 13 supposedly-dead hidden sections aren\'t dead at all — they\'re gated on user data (discovery pool, library content, metadata cache) and self-surface when their data exists. removed one orphaned `loadPersonalizedDailyMixes()` call from `blockDiscoveryArtist` — daily mixes is intentionally paused, refreshing it from there was a no-op.', page: 'discover' },