diff --git a/core/personalized_playlists.py b/core/personalized_playlists.py index b4c5aed7..64882b97 100644 --- a/core/personalized_playlists.py +++ b/core/personalized_playlists.py @@ -105,8 +105,235 @@ 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`). + _STANDARD_DISCOVERY_COLUMNS: Tuple[str, ...] = ( + 'spotify_track_id', + 'itunes_track_id', + 'deezer_track_id', + 'track_name', + 'artist_name', + 'album_name', + 'album_cover_url', + 'duration_ms', + 'popularity', + 'track_data_json', + 'source', + ) + + def _select_discovery_tracks( + self, + *, + source: str, + extra_where: str = "", + extra_params: tuple = (), + order_by: str = "RANDOM()", + fetch_limit: int, + extra_columns: tuple = (), + exclude_owned: bool = True, + ) -> List[Dict]: + """ + Shared selector for discovery_pool playlist methods. + + Builds and runs a SELECT against `discovery_pool` with a baked-in + ID-validity gate so callers cannot accidentally return rows with no + usable source IDs (which would fail downstream when the user clicks + download). + + The WHERE clause always includes: + 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) + + 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. + + Callers compose additional filters via `extra_where` (a SQL fragment + beginning with "AND ...") and `extra_params` (positional bindings for + any `?` placeholders inside `extra_where`). + + Diversity filtering is the caller's responsibility — apply + `_apply_diversity_filter` to the returned list if needed. + + Args: + source: discovery_pool.source value to filter on. + extra_where: optional SQL fragment appended to the WHERE clause. + Must start with "AND " if non-empty. + extra_params: positional bindings for `?` placeholders in + `extra_where`. + order_by: ORDER BY expression, used as-is (e.g. "RANDOM()", + "popularity DESC, RANDOM()"). + fetch_limit: LIMIT applied to the query. Callers that intend to + 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 + error (logged at error level). + """ + try: + 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} + FROM discovery_pool + 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 ? + """ + + params = (source,) + tuple(extra_params) + (fetch_limit,) + + with self.database._get_connection() as conn: + cursor = conn.cursor() + cursor.execute(query, params) + rows = cursor.fetchall() + + return [self._build_track_dict(row, source) for row in rows] + + except Exception as e: + logger.error(f"Error in _select_discovery_tracks (source={source}): {e}") + return [] + + def _apply_diversity_filter( + self, + tracks: List[Dict], + *, + max_per_album: int, + max_per_artist: int, + limit: int, + ) -> List[Dict]: + """ + Apply per-album / per-artist diversity caps to a track list. + + Iterates `tracks` in order, accepting each only if its album count + is still under `max_per_album` AND its artist count is still under + `max_per_artist`. Stops once `limit` tracks have been accepted. + + Returns a new list, already trimmed to at most `limit` items. + """ + tracks_by_album: Dict[str, int] = {} + tracks_by_artist: Dict[str, int] = {} + diverse_tracks: List[Dict] = [] + + for track in tracks: + album = track['album_name'] + artist = track['artist_name'] + + album_count = tracks_by_album.get(album, 0) + artist_count = tracks_by_artist.get(artist, 0) + + if album_count < max_per_album and artist_count < max_per_artist: + diverse_tracks.append(track) + tracks_by_album[album] = album_count + 1 + tracks_by_artist[artist] = artist_count + 1 + + if len(diverse_tracks) >= limit: + break + + return diverse_tracks + + def _compute_adaptive_diversity_limits( + self, + tracks: List[Dict], + *, + relaxed: bool = False, + ) -> Tuple[int, int]: + """ + Pick (max_per_album, max_per_artist) caps based on artist variety. + + Mirrors the step-functions previously inlined in the decade and + genre playlist methods. With more unique artists we can be strict; + with fewer we relax to still hit the requested track count. + + When `relaxed=True` (used for genre playlists), the moderate and + low bands use looser caps and an extra "very limited" tier kicks + in below 5 unique artists. + + Args: + tracks: candidate track list to inspect. + relaxed: True to apply the looser genre-playlist limits. + + Returns: + Tuple of (max_per_album, max_per_artist). + """ + unique_artists = len(set(t['artist_name'] for t in tracks)) + + if relaxed: + # Genre playlist tiers + if unique_artists >= 20: + return 3, 5 + if unique_artists >= 10: + return 4, 10 + if unique_artists >= 5: + return 6, 15 + return 8, 25 + + # Decade-style strict tiers + if unique_artists >= 20: + return 3, 5 + if unique_artists >= 10: + return 4, 8 + return 5, 12 + def _build_track_dict(self, row, source: str) -> Dict: - """Build a standardized track dictionary from a database row.""" + """Build a standardized track dictionary from a database row. + + If the row carries the optional `artist_genres` column (selected via + `_select_discovery_tracks(extra_columns=('artist_genres',))`), the raw + JSON string is passed through under `_artist_genres_raw` so callers + can run Python-side genre matching without re-querying the row. + """ # Convert sqlite3.Row to dict if needed (Row objects don't support .get()) if hasattr(row, 'keys'): row = dict(row) @@ -118,7 +345,7 @@ class PersonalizedPlaylistsService: except: track_data = None - return { + result = { 'track_id': row.get('spotify_track_id') or row.get('itunes_track_id') or row.get('deezer_track_id'), 'spotify_track_id': row.get('spotify_track_id'), 'itunes_track_id': row.get('itunes_track_id'), @@ -130,9 +357,19 @@ class PersonalizedPlaylistsService: 'duration_ms': row.get('duration_ms', 0), 'popularity': row.get('popularity', 0), 'track_data_json': track_data, - 'source': source + 'source': source, } + # Pass through optional extra columns under underscore-prefixed keys + # so callers that requested them via `extra_columns` can use them + # without having to re-query. + if 'artist_genres' in row: + result['_artist_genres_raw'] = row.get('artist_genres') + if 'release_date' in row: + result['_release_date'] = row.get('release_date') + + return result + @staticmethod def get_parent_genre(spotify_genre: str) -> str: """ @@ -148,57 +385,6 @@ class PersonalizedPlaylistsService: return 'Other' - # ======================================== - # LIBRARY-BASED PLAYLISTS - # ======================================== - - def get_recently_added(self, limit: int = 50) -> List[Dict]: - """ - Get recently added tracks from library. - - Returns tracks ordered by date_added DESC - - NOTE: This requires library tracks to have Spotify metadata which may not be available. - Returns empty list if schema incompatible. - """ - try: - logger.warning("Recently Added requires Spotify-linked library tracks - returning empty") - return [] - - except Exception as e: - logger.error(f"Error getting recently added tracks: {e}") - return [] - - def get_top_tracks(self, limit: int = 50) -> List[Dict]: - """ - Get user's all-time top tracks based on play count. - - NOTE: This requires library tracks to have Spotify metadata which may not be available. - Returns empty list if schema incompatible. - """ - try: - logger.warning("Top Tracks requires Spotify-linked library tracks - returning empty") - return [] - - except Exception as e: - logger.error(f"Error getting top tracks: {e}") - return [] - - def get_forgotten_favorites(self, limit: int = 50) -> List[Dict]: - """ - Get tracks you loved but haven't played recently. - - NOTE: This requires library tracks to have Spotify metadata which may not be available. - Returns empty list if schema incompatible. - """ - try: - logger.warning("Forgotten Favorites requires Spotify-linked library tracks - returning empty") - return [] - - except Exception as e: - logger.error(f"Error getting forgotten favorites: {e}") - return [] - def get_decade_playlist(self, decade: int, limit: int = 100, source: str = None) -> List[Dict]: """ Get tracks from a specific decade from discovery pool with diversity filtering. @@ -208,99 +394,46 @@ class PersonalizedPlaylistsService: limit: Maximum tracks to return source: Optional source filter ('spotify' or 'itunes'), auto-detects if not provided """ - try: - start_year = decade - end_year = decade + 9 + start_year = decade + end_year = decade + 9 + active_source = source or self._get_active_source() - # Determine active source if not specified - active_source = source or self._get_active_source() + # Over-fetch 10x for diversity filtering headroom. + all_tracks = self._select_discovery_tracks( + source=active_source, + extra_where=( + "AND release_date IS NOT NULL " + "AND CAST(SUBSTR(release_date, 1, 4) AS INTEGER) BETWEEN ? AND ?" + ), + extra_params=(start_year, end_year), + order_by="RANDOM()", + fetch_limit=limit * 10, + extra_columns=('release_date',), + ) - with self.database._get_connection() as conn: - cursor = conn.cursor() - - # Query discovery_pool - get 10x more for diversity filtering, filtered by source - cursor.execute(""" - SELECT - spotify_track_id, - itunes_track_id, - track_name, - artist_name, - album_name, - album_cover_url, - duration_ms, - popularity, - release_date, - track_data_json, - source - FROM discovery_pool - WHERE release_date IS NOT NULL - AND CAST(SUBSTR(release_date, 1, 4) AS INTEGER) BETWEEN ? AND ? - AND source = ? - AND LOWER(artist_name) NOT IN (SELECT LOWER(artist_name) FROM discovery_artist_blacklist) - ORDER BY RANDOM() - LIMIT ? - """, (start_year, end_year, active_source, limit * 10)) - - rows = cursor.fetchall() - all_tracks = [] - for row in rows: - all_tracks.append(self._build_track_dict(row, active_source)) - - if not all_tracks: - logger.warning(f"No tracks found for {decade}s") - return [] - - # Shuffle first for randomness - import random - random.shuffle(all_tracks) - - # Count unique artists to determine diversity level - unique_artists = len(set(track['artist_name'] for track in all_tracks)) - - # Adaptive diversity limits based on artist variety - if unique_artists >= 20: - # Good variety - apply diversity constraints - max_per_album = 3 - max_per_artist = 5 - elif unique_artists >= 10: - # Moderate variety - more lenient - max_per_album = 4 - max_per_artist = 8 - else: - # Low variety - very lenient to hit 50 tracks - max_per_album = 5 - max_per_artist = 12 - - logger.info(f"{decade}s has {unique_artists} unique artists - using limits: {max_per_album} per album, {max_per_artist} per artist") - - # Apply diversity constraints - tracks_by_album = {} - tracks_by_artist = {} - diverse_tracks = [] - - for track in all_tracks: - album = track['album_name'] - artist = track['artist_name'] - - # Count current tracks for this album/artist - album_count = tracks_by_album.get(album, 0) - artist_count = tracks_by_artist.get(artist, 0) - - if album_count < max_per_album and artist_count < max_per_artist: - diverse_tracks.append(track) - tracks_by_album[album] = album_count + 1 - tracks_by_artist[artist] = artist_count + 1 - - if len(diverse_tracks) >= limit: - break - - logger.info(f"Found {len(diverse_tracks)} tracks from {decade}s in discovery pool (adaptive diversity)") - return diverse_tracks[:limit] - - except Exception as e: - logger.error(f"Error getting decade playlist for {decade}s: {e}") + if not all_tracks: + logger.warning(f"No tracks found for {decade}s") return [] + random.shuffle(all_tracks) + + max_per_album, max_per_artist = self._compute_adaptive_diversity_limits(all_tracks) + unique_artists = len(set(t['artist_name'] for t in all_tracks)) + logger.info( + f"{decade}s has {unique_artists} unique artists - using limits: " + f"{max_per_album} per album, {max_per_artist} per artist" + ) + + diverse_tracks = self._apply_diversity_filter( + all_tracks, + max_per_album=max_per_album, + max_per_artist=max_per_artist, + limit=limit, + ) + + logger.info(f"Found {len(diverse_tracks)} tracks from {decade}s in discovery pool (adaptive diversity)") + return diverse_tracks + def get_available_genres(self, source: str = None) -> List[Dict]: """ Get list of consolidated parent genres with track counts from discovery pool. @@ -368,287 +501,166 @@ class PersonalizedPlaylistsService: 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 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. """ - try: - # Determine active source if not specified - active_source = source or self._get_active_source() + active_source = source or self._get_active_source() - with self.database._get_connection() as conn: - cursor = conn.cursor() + # Build keyword list: parent genre expands to all child keywords; + # specific genre uses its own name for partial matching. + if genre in self.GENRE_MAPPING: + search_keywords = [k.lower() for k in self.GENRE_MAPPING[genre]] + logger.info(f"Matching parent genre '{genre}' with {len(search_keywords)} child keywords") + else: + search_keywords = [genre.lower()] + logger.info(f"Matching specific genre '{genre}' with partial matching") - # Get all tracks with genres from discovery pool, filtered by source - cursor.execute(""" - SELECT - spotify_track_id, - itunes_track_id, - track_name, - artist_name, - album_name, - album_cover_url, - duration_ms, - popularity, - artist_genres, - track_data_json, - source - FROM discovery_pool - WHERE artist_genres IS NOT NULL - AND source = ? - AND LOWER(artist_name) NOT IN (SELECT LOWER(artist_name) FROM discovery_artist_blacklist) - """, (active_source,)) - rows = cursor.fetchall() + # 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) - # Determine if this is a parent genre or specific genre - is_parent_genre = genre in self.GENRE_MAPPING - 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=f"AND artist_genres IS NOT NULL AND ({like_clauses})", + extra_params=like_params, + order_by="RANDOM()", + fetch_limit=limit * 10, + extra_columns=('artist_genres',), + ) - if is_parent_genre: - # Use all child genre keywords for matching - search_keywords = self.GENRE_MAPPING[genre] - logger.info(f"Matching parent genre '{genre}' with {len(search_keywords)} child keywords") - else: - # Use the genre name itself for partial matching - search_keywords = [genre.lower()] - logger.info(f"Matching specific genre '{genre}' with partial matching") - - # Filter tracks that match the genre - matching_tracks = [] - - for row in rows: - try: - artist_genres_json = row['artist_genres'] - if artist_genres_json: - genres = json.loads(artist_genres_json) - - # Check if any artist genre matches any search keyword - genre_match = False - for artist_genre in genres: - artist_genre_lower = artist_genre.lower() - for keyword in search_keywords: - if keyword in artist_genre_lower: - genre_match = True - break - if genre_match: - break - - if genre_match: - matching_tracks.append(self._build_track_dict(row, active_source)) - except Exception as e: - logger.debug(f"Error parsing genres for track: {e}") - continue - - if not matching_tracks: - logger.warning(f"No tracks found for genre: {genre}") - return [] - - # Shuffle before limiting for better variety - random.shuffle(matching_tracks) - - # Limit to 10x for diversity filtering - all_tracks = matching_tracks[:limit * 10] if len(matching_tracks) > limit * 10 else matching_tracks - - if not all_tracks: - return [] - - # Apply adaptive diversity filtering (relaxed for genres) - unique_artists = len(set(track['artist_name'] for track in all_tracks)) - - if unique_artists >= 20: - max_per_album = 3 - max_per_artist = 5 - elif unique_artists >= 10: - max_per_album = 4 - max_per_artist = 10 - elif unique_artists >= 5: - max_per_album = 6 - max_per_artist = 15 - else: - # Very limited artist pool - be more lenient - max_per_album = 8 - max_per_artist = 25 - - logger.info(f"Genre '{genre}' has {unique_artists} artists, {len(all_tracks)} total tracks - limits: {max_per_album}/album, {max_per_artist}/artist") - - # Shuffle and apply diversity - random.shuffle(all_tracks) - tracks_by_album = {} - tracks_by_artist = {} - diverse_tracks = [] - - for track in all_tracks: - album = track['album_name'] - artist = track['artist_name'] - - album_count = tracks_by_album.get(album, 0) - artist_count = tracks_by_artist.get(artist, 0) - - if album_count < max_per_album and artist_count < max_per_artist: - diverse_tracks.append(track) - tracks_by_album[album] = album_count + 1 - tracks_by_artist[artist] = artist_count + 1 - - if len(diverse_tracks) >= limit: - break - - logger.info(f"Found {len(diverse_tracks)} tracks for genre '{genre}'") - return diverse_tracks[:limit] - - except Exception as e: - logger.error(f"Error getting genre playlist for {genre}: {e}") + if not all_tracks: + logger.warning(f"No tracks found for genre: {genre}") return [] + 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( + f"Genre '{genre}' has {unique_artists} artists, {len(all_tracks)} total tracks - " + f"limits: {max_per_album}/album, {max_per_artist}/artist" + ) + + diverse_tracks = self._apply_diversity_filter( + all_tracks, + max_per_album=max_per_album, + max_per_artist=max_per_artist, + limit=limit, + ) + + logger.info(f"Found {len(diverse_tracks)} tracks for genre '{genre}'") + return diverse_tracks + # ======================================== # DISCOVERY POOL PLAYLISTS # ======================================== def get_popular_picks(self, limit: int = 50) -> List[Dict]: - """Get high popularity tracks from discovery pool with diversity (max 2 tracks per album/artist)""" - # Determine active source + """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) - try: - with self.database._get_connection() as conn: - cursor = conn.cursor() + 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()" - # Get more tracks than needed to allow for filtering, filtered by source - cursor.execute(""" - SELECT - spotify_track_id, - itunes_track_id, - track_name, - artist_name, - album_name, - album_cover_url, - duration_ms, - popularity, - track_data_json, - source - FROM discovery_pool - WHERE popularity >= 60 AND source = ? - AND LOWER(artist_name) NOT IN (SELECT LOWER(artist_name) FROM discovery_artist_blacklist) - ORDER BY popularity DESC, RANDOM() - LIMIT ? - """, (active_source, limit * 3)) + # Over-fetch 3x so the diversity filter has room to spread albums/artists. + all_tracks = self._select_discovery_tracks( + source=active_source, + extra_where=extra_where, + extra_params=extra_params, + order_by=order_by, + fetch_limit=limit * 3, + ) - rows = cursor.fetchall() - all_tracks = [self._build_track_dict(row, active_source) for row in rows] + diverse_tracks = self._apply_diversity_filter( + all_tracks, + max_per_album=2, + max_per_artist=3, + limit=limit, + ) - # Apply diversity constraint: max 2 tracks per album, max 3 per artist - tracks_by_album = {} - tracks_by_artist = {} - diverse_tracks = [] - - for track in all_tracks: - album = track['album_name'] - artist = track['artist_name'] - - # Count current tracks for this album/artist - album_count = tracks_by_album.get(album, 0) - artist_count = tracks_by_artist.get(artist, 0) - - # Apply limits: max 2 per album, max 3 per artist - if album_count < 2 and artist_count < 3: - diverse_tracks.append(track) - tracks_by_album[album] = album_count + 1 - tracks_by_artist[artist] = artist_count + 1 - - if len(diverse_tracks) >= limit: - break - - logger.info(f"Popular Picks ({active_source}): Selected {len(diverse_tracks)} tracks with diversity") - return diverse_tracks[:limit] - - except Exception as e: - logger.error(f"Error getting popular picks: {e}") - return [] + logger.info(f"Popular Picks ({active_source}): selected {len(diverse_tracks)} tracks with diversity") + return diverse_tracks def get_hidden_gems(self, limit: int = 50) -> List[Dict]: - """Get low popularity (underground/indie) tracks from discovery pool""" - # Determine active source + """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() + _, hidden_max = self._get_popularity_thresholds(active_source) - try: - with self.database._get_connection() as conn: - cursor = conn.cursor() + if hidden_max is not None: + extra_where = "AND popularity < ?" + extra_params: tuple = (hidden_max,) + else: + extra_where = "" + extra_params = () - cursor.execute(""" - SELECT - spotify_track_id, - itunes_track_id, - track_name, - artist_name, - album_name, - album_cover_url, - duration_ms, - popularity, - track_data_json, - source - FROM discovery_pool - WHERE popularity < 40 AND source = ? - AND LOWER(artist_name) NOT IN (SELECT LOWER(artist_name) FROM discovery_artist_blacklist) - ORDER BY RANDOM() - LIMIT ? - """, (active_source, limit)) + # Over-fetch 3x for diversity filter headroom. + all_tracks = self._select_discovery_tracks( + source=active_source, + extra_where=extra_where, + extra_params=extra_params, + order_by="RANDOM()", + fetch_limit=limit * 3, + ) - rows = cursor.fetchall() - return [self._build_track_dict(row, active_source) for row in rows] + diverse_tracks = self._apply_diversity_filter( + all_tracks, + max_per_album=2, + max_per_artist=3, + limit=limit, + ) - except Exception as e: - logger.error(f"Error getting hidden gems: {e}") - return [] + 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. """ - # Determine active source active_source = self._get_active_source() - try: - with self.database._get_connection() as conn: - cursor = conn.cursor() + # Over-fetch 3x for diversity filter headroom. + all_tracks = self._select_discovery_tracks( + source=active_source, + order_by="RANDOM()", + fetch_limit=limit * 3, + ) - cursor.execute(""" - SELECT - spotify_track_id, - itunes_track_id, - track_name, - artist_name, - album_name, - album_cover_url, - duration_ms, - popularity, - track_data_json, - source - FROM discovery_pool - WHERE source = ? - AND LOWER(artist_name) NOT IN (SELECT LOWER(artist_name) FROM discovery_artist_blacklist) - ORDER BY RANDOM() - LIMIT ? - """, (active_source, limit)) + diverse_tracks = self._apply_diversity_filter( + all_tracks, + max_per_album=2, + max_per_artist=2, + limit=limit, + ) - rows = cursor.fetchall() - return [self._build_track_dict(row, active_source) for row in rows] - - except Exception as e: - logger.error(f"Error getting discovery shuffle: {e}") - return [] - - def get_familiar_favorites(self, limit: int = 50) -> List[Dict]: - """ - Get tracks with medium play counts (3-15 plays) - your reliable go-tos. - - NOTE: This requires library tracks to have Spotify metadata which may not be available. - Returns empty list if schema incompatible. - """ - try: - logger.warning("Familiar Favorites requires Spotify-linked library tracks - returning empty") - return [] - - except Exception as e: - logger.error(f"Error getting familiar favorites: {e}") - return [] + logger.info(f"Discovery Shuffle ({active_source}): selected {len(diverse_tracks)} tracks with diversity") + return diverse_tracks # ======================================== # DAILY MIX (HYBRID PLAYLISTS) diff --git a/tests/test_personalized_playlists_id_gate.py b/tests/test_personalized_playlists_id_gate.py new file mode 100644 index 00000000..365051d3 --- /dev/null +++ b/tests/test_personalized_playlists_id_gate.py @@ -0,0 +1,616 @@ +"""Tests for the lifted PersonalizedPlaylistsService selectors and the +mandatory ID-validity gate every section must enforce. + +Context: discover-page selection methods used to return tracks/albums +with all source IDs NULL — the UI displayed them, the user clicked +download, the download silently failed because there was nothing to +look up. This test file pins the gate at the helper level so a future +section can't accidentally bypass it. + +Coverage: +- `_select_discovery_tracks` filters out rows where every source ID is NULL +- `_select_discovery_tracks` honors source filter + blacklist filter +- `_apply_diversity_filter` caps per-album + per-artist counts +- `_compute_adaptive_diversity_limits` returns the right tier for the + unique-artist count + relaxed flag +- The 5 discovery_pool methods (decade / genre / popular_picks / + hidden_gems / discovery_shuffle) each filter NULL-id rows +""" + +from __future__ import annotations + +import sqlite3 +from contextlib import contextmanager +from unittest.mock import patch + +import pytest + +from core.personalized_playlists import PersonalizedPlaylistsService + + +# --------------------------------------------------------------------------- +# Fake DB +# --------------------------------------------------------------------------- + + +class _FakeDatabase: + """Wraps an in-memory sqlite connection so the service's + `database._get_connection()` calls work the same as in production. + + The schema mirrors the real `discovery_pool` + `tracks` + `albums` + + `artists` shape just enough for the selection methods to exercise. + """ + + def __init__(self): + self._conn = sqlite3.connect(":memory:") + self._conn.row_factory = sqlite3.Row + self._setup_schema() + + def _setup_schema(self): + cursor = self._conn.cursor() + cursor.executescript(""" + CREATE TABLE discovery_pool ( + id INTEGER PRIMARY KEY, + source TEXT NOT NULL, + spotify_track_id TEXT, + itunes_track_id TEXT, + deezer_track_id TEXT, + track_name TEXT, + artist_name TEXT, + album_name TEXT, + album_cover_url TEXT, + duration_ms INTEGER, + popularity INTEGER, + release_date TEXT, + artist_genres TEXT, + track_data_json TEXT + ); + 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() + + @contextmanager + def _get_connection(self): + # Match the production interface (`with database._get_connection() as conn`) + try: + yield self._conn + finally: + pass + + def insert_discovery_track(self, **kwargs): + cols = ", ".join(kwargs.keys()) + placeholders = ", ".join(["?"] * len(kwargs)) + self._conn.execute( + f"INSERT INTO discovery_pool ({cols}) VALUES ({placeholders})", + tuple(kwargs.values()), + ) + self._conn.commit() + + def blacklist(self, artist_name): + self._conn.execute( + "INSERT INTO discovery_artist_blacklist (artist_name) VALUES (?)", + (artist_name,), + ) + 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(): + """Service with a fresh in-memory DB. `_get_active_source` patched + to return 'spotify' so every selector targets the same source.""" + db = _FakeDatabase() + svc = PersonalizedPlaylistsService(db) + with patch.object(svc, '_get_active_source', return_value='spotify'): + yield svc, db + + +# --------------------------------------------------------------------------- +# `_select_discovery_tracks` — the helper everyone goes through +# --------------------------------------------------------------------------- + + +def test_discovery_helper_filters_null_id_rows(service): + svc, db = service + db.insert_discovery_track( + source='spotify', spotify_track_id='sp1', track_name='Has Spotify ID', + artist_name='A', album_name='A', popularity=50, + ) + db.insert_discovery_track( + source='spotify', spotify_track_id=None, itunes_track_id=None, + track_name='No IDs', artist_name='B', album_name='B', popularity=50, + ) + db.insert_discovery_track( + source='spotify', itunes_track_id='it1', track_name='Has iTunes ID', + artist_name='C', album_name='C', popularity=50, + ) + + tracks = svc._select_discovery_tracks( + source='spotify', + order_by='track_name', + fetch_limit=100, + ) + names = sorted(t['track_name'] for t in tracks) + assert names == ['Has Spotify ID', 'Has iTunes ID'] + + +def test_discovery_helper_accepts_deezer_only_id_rows(service): + """Discovery pool rows with NULL spotify + NULL itunes but a populated + deezer_track_id MUST pass the gate. Regression test — early version + of the gate only checked Spotify + iTunes, which silently filtered + out every row for Deezer-primary users (entire Time Machine / + Genre / Hidden Gems / Shuffle / Popular Picks rendered empty).""" + svc, db = service + db.insert_discovery_track( + source='deezer', deezer_track_id='dz1', + spotify_track_id=None, itunes_track_id=None, + track_name='Deezer Only', artist_name='A', album_name='X', + popularity=50, release_date='2024-01-01', + ) + 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 [t['track_name'] for t in tracks] == ['Deezer Only'] + assert tracks[0]['deezer_track_id'] == 'dz1' + + +def test_discovery_helper_filters_blacklisted_artists(service): + svc, db = service + db.insert_discovery_track( + source='spotify', spotify_track_id='sp1', track_name='Keep', + artist_name='Good Artist', album_name='X', + ) + db.insert_discovery_track( + source='spotify', spotify_track_id='sp2', track_name='Drop', + artist_name='Bad Artist', album_name='X', + ) + db.blacklist('bad artist') + + tracks = svc._select_discovery_tracks( + source='spotify', + order_by='track_name', + fetch_limit=100, + ) + assert [t['track_name'] for t in tracks] == ['Keep'] + + +def test_discovery_helper_honors_source_filter(service): + svc, db = service + db.insert_discovery_track( + source='spotify', spotify_track_id='sp1', track_name='SP', + artist_name='A', album_name='X', + ) + db.insert_discovery_track( + source='itunes', itunes_track_id='it1', track_name='IT', + artist_name='A', album_name='X', + ) + + tracks = svc._select_discovery_tracks( + source='spotify', + order_by='track_name', + fetch_limit=100, + ) + assert [t['track_name'] for t in tracks] == ['SP'] + + +def test_discovery_helper_honors_extra_where(service): + svc, db = service + db.insert_discovery_track( + source='spotify', spotify_track_id='sp1', track_name='Pop60', + artist_name='A', album_name='X', popularity=60, + ) + db.insert_discovery_track( + source='spotify', spotify_track_id='sp2', track_name='Pop20', + artist_name='A', album_name='X', popularity=20, + ) + + tracks = svc._select_discovery_tracks( + source='spotify', + extra_where='AND popularity >= 50', + order_by='popularity DESC', + fetch_limit=100, + ) + assert [t['track_name'] for t in tracks] == ['Pop60'] + + +# --------------------------------------------------------------------------- +# Diversity filter +# --------------------------------------------------------------------------- + + +def test_diversity_filter_caps_per_album(): + svc = PersonalizedPlaylistsService(_FakeDatabase()) + tracks = [ + {'track_name': f't{i}', 'artist_name': 'A', 'album_name': 'Album1'} + for i in range(10) + ] + out = svc._apply_diversity_filter( + tracks, max_per_album=3, max_per_artist=10, limit=10, + ) + assert len(out) == 3 + + +def test_diversity_filter_caps_per_artist(): + svc = PersonalizedPlaylistsService(_FakeDatabase()) + tracks = [ + {'track_name': f't{i}', 'artist_name': 'OnlyArtist', 'album_name': f'Album{i}'} + for i in range(10) + ] + out = svc._apply_diversity_filter( + tracks, max_per_album=10, max_per_artist=2, limit=10, + ) + assert len(out) == 2 + + +def test_diversity_filter_stops_at_limit(): + svc = PersonalizedPlaylistsService(_FakeDatabase()) + tracks = [ + {'track_name': f't{i}', 'artist_name': f'a{i}', 'album_name': f'b{i}'} + for i in range(20) + ] + out = svc._apply_diversity_filter( + tracks, max_per_album=10, max_per_artist=10, limit=5, + ) + assert len(out) == 5 + + +# --------------------------------------------------------------------------- +# Adaptive diversity limits +# --------------------------------------------------------------------------- + + +def test_adaptive_limits_high_variety(): + svc = PersonalizedPlaylistsService(_FakeDatabase()) + tracks = [{'artist_name': f'a{i}'} for i in range(30)] + max_album, max_artist = svc._compute_adaptive_diversity_limits(tracks) + # High variety tier — strict limits + assert (max_album, max_artist) == (3, 5) + + +def test_adaptive_limits_low_variety(): + svc = PersonalizedPlaylistsService(_FakeDatabase()) + tracks = [{'artist_name': f'a{i % 3}'} for i in range(30)] # only 3 unique artists + max_album, max_artist = svc._compute_adaptive_diversity_limits(tracks) + # Low variety tier — much more lenient (matches existing decade-style limits) + assert max_album >= 4 + assert max_artist >= 8 + + +def test_adaptive_limits_relaxed_flag_loosens_genre_tier(): + svc = PersonalizedPlaylistsService(_FakeDatabase()) + # 15 unique artists triggers the moderate tier + tracks = [{'artist_name': f'a{i}'} for i in range(15)] + strict = svc._compute_adaptive_diversity_limits(tracks, relaxed=False) + relaxed = svc._compute_adaptive_diversity_limits(tracks, relaxed=True) + assert relaxed[1] >= strict[1] + + +# --------------------------------------------------------------------------- +# Public methods enforce the gate (smoke test on each) +# --------------------------------------------------------------------------- + + +def test_get_hidden_gems_filters_null_id_rows(service): + svc, db = service + db.insert_discovery_track( + source='spotify', spotify_track_id='sp1', track_name='Gem', + artist_name='A', album_name='X', popularity=20, + ) + db.insert_discovery_track( + source='spotify', spotify_track_id=None, itunes_track_id=None, + track_name='Nogem', artist_name='A', album_name='X', popularity=20, + ) + out = svc.get_hidden_gems(limit=10) + assert [t['track_name'] for t in out] == ['Gem'] + + +def test_get_discovery_shuffle_filters_null_id_rows(service): + svc, db = service + db.insert_discovery_track( + source='spotify', spotify_track_id='sp1', track_name='Yes', + artist_name='A', album_name='X', + ) + db.insert_discovery_track( + source='spotify', spotify_track_id=None, itunes_track_id=None, + track_name='No', artist_name='A', album_name='X', + ) + out = svc.get_discovery_shuffle(limit=10) + assert [t['track_name'] for t in out] == ['Yes'] + + +def test_get_popular_picks_filters_null_id_rows(service): + svc, db = service + db.insert_discovery_track( + source='spotify', spotify_track_id='sp1', track_name='Yes', + artist_name='A', album_name='X', popularity=80, + ) + db.insert_discovery_track( + source='spotify', spotify_track_id=None, itunes_track_id=None, + track_name='No', artist_name='A', album_name='X', popularity=80, + ) + out = svc.get_popular_picks(limit=10) + assert [t['track_name'] for t in out] == ['Yes'] + + +def test_get_decade_playlist_filters_null_id_rows(service): + svc, db = service + db.insert_discovery_track( + source='spotify', spotify_track_id='sp1', track_name='Yes', + artist_name='A', album_name='X', release_date='2024-06-01', + ) + db.insert_discovery_track( + source='spotify', spotify_track_id=None, itunes_track_id=None, + track_name='No', artist_name='A', album_name='X', release_date='2024-06-01', + ) + out = svc.get_decade_playlist(2020, limit=10) + 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 == [] + diff --git a/web_server.py b/web_server.py index f9709dcc..dfde1998 100644 --- a/web_server.py +++ b/web_server.py @@ -27189,66 +27189,6 @@ def refresh_seasonal_content(): # PERSONALIZED PLAYLISTS ENDPOINTS # ======================================== -@app.route('/api/discover/personalized/recently-added', methods=['GET']) -def get_recently_added_playlist(): - """Get recently added tracks from library""" - try: - from core.personalized_playlists import get_personalized_playlists_service - - database = get_database() - service = get_personalized_playlists_service(database, spotify_client) - - tracks = service.get_recently_added(limit=50) - - return jsonify({ - "success": True, - "tracks": tracks - }) - - except Exception as e: - logger.error(f"Error getting recently added playlist: {e}") - return jsonify({"success": False, "error": str(e)}), 500 - -@app.route('/api/discover/personalized/top-tracks', methods=['GET']) -def get_top_tracks_playlist(): - """Get user's all-time top tracks""" - try: - from core.personalized_playlists import get_personalized_playlists_service - - database = get_database() - service = get_personalized_playlists_service(database, spotify_client) - - tracks = service.get_top_tracks(limit=50) - - return jsonify({ - "success": True, - "tracks": tracks - }) - - except Exception as e: - logger.error(f"Error getting top tracks playlist: {e}") - return jsonify({"success": False, "error": str(e)}), 500 - -@app.route('/api/discover/personalized/forgotten-favorites', methods=['GET']) -def get_forgotten_favorites_playlist(): - """Get forgotten favorites - tracks you loved but haven't played recently""" - try: - from core.personalized_playlists import get_personalized_playlists_service - - database = get_database() - service = get_personalized_playlists_service(database, spotify_client) - - tracks = service.get_forgotten_favorites(limit=50) - - return jsonify({ - "success": True, - "tracks": tracks - }) - - except Exception as e: - logger.error(f"Error getting forgotten favorites playlist: {e}") - return jsonify({"success": False, "error": str(e)}), 500 - @app.route('/api/discover/personalized/decade/', methods=['GET']) def get_decade_playlist(decade): """Get tracks from a specific decade""" @@ -27353,27 +27293,6 @@ def get_discovery_shuffle(): logger.error(f"Error getting discovery shuffle playlist: {e}") return jsonify({"success": False, "error": str(e)}), 500 -@app.route('/api/discover/personalized/familiar-favorites', methods=['GET']) -def get_familiar_favorites(): - """Get Familiar Favorites playlist - reliable go-to tracks""" - try: - from core.personalized_playlists import get_personalized_playlists_service - - database = get_database() - service = get_personalized_playlists_service(database, spotify_client) - - limit = int(request.args.get('limit', 50)) - tracks = service.get_familiar_favorites(limit=limit) - - return jsonify({ - "success": True, - "tracks": tracks - }) - - except Exception as e: - logger.error(f"Error getting familiar favorites playlist: {e}") - return jsonify({"success": False, "error": str(e)}), 500 - @app.route('/api/discover/artist-blacklist', methods=['GET']) def get_discovery_artist_blacklist(): """Get all blacklisted discovery artists.""" diff --git a/webui/index.html b/webui/index.html index 7b0d993a..f4159613 100644 --- a/webui/index.html +++ b/webui/index.html @@ -2985,17 +2985,6 @@ - - - - - - - - - - - -
diff --git a/webui/static/discover.js b/webui/static/discover.js index c7ba7e36..f5ead10d 100644 --- a/webui/static/discover.js +++ b/webui/static/discover.js @@ -15,14 +15,10 @@ let discoverSeasonalTracks = []; let currentSeasonKey = null; // Personalized playlists storage -let personalizedRecentlyAdded = []; -let personalizedTopTracks = []; -let personalizedForgottenFavorites = []; let personalizedPopularPicks = []; let personalizedHiddenGems = []; let personalizedDailyMixes = []; let personalizedDiscoveryShuffle = []; -let personalizedFamiliarFavorites = []; let buildPlaylistSelectedArtists = []; async function loadDiscoverPage() { @@ -35,16 +31,12 @@ async function loadDiscoverPage() { loadYourAlbums(), loadDiscoverRecentReleases(), loadSeasonalContent(), // Seasonal discovery - loadPersonalizedRecentlyAdded(), // NEW: Recently added from library // loadPersonalizedDailyMixes(), // NEW: Daily Mix playlists (HIDDEN) loadDiscoverReleaseRadar(), loadDiscoverWeekly(), loadPersonalizedPopularPicks(), // NEW: Popular picks from discovery pool loadPersonalizedHiddenGems(), // NEW: Hidden gems from discovery pool - loadPersonalizedTopTracks(), // NEW: Your top tracks - loadPersonalizedForgottenFavorites(), // NEW: Forgotten favorites loadDiscoveryShuffle(), // NEW: Discovery Shuffle - loadFamiliarFavorites(), // NEW: Familiar Favorites loadBecauseYouListenTo(), // Personalized by listening stats loadCacheUndiscoveredAlbums(), // From metadata cache loadCacheGenreNewReleases(), // From metadata cache @@ -1702,45 +1694,162 @@ function _renderSyncStatusBlock(idPrefix) { `; } -async function loadDecadeBrowserTabs() { - try { - const tabsContainer = document.getElementById('decade-tabs'); - const contentsContainer = document.getElementById('decade-tab-contents'); +// =============================== +// TABBED BROWSER HELPER +// =============================== +// +// Drives the lifecycle the decade browser ("Time Machine") and +// genre browser ("Browse by Genre") share: fetch tab list → paint +// tab strip + per-tab content shells → fetch + render content for +// the active tab → handle empty / error states. +// +// The two browsers paint slightly different markup (different CSS +// prefixes, different action buttons, different sync handlers) but +// the lifecycle is identical. Each browser registers a config; the +// helper handles the rest. +// +// Two-phase render: +// Phase 1 (loadTabs) — paint tab strip + N content shells, +// each shell containing a loading +// spinner in its playlist container, +// then trigger Phase 2 for first tab. +// Phase 2 (loadTabContent) — fetch tracks for one tab, swap the +// spinner in its playlist container +// for the rendered track list. +// +// Renderers stay per-browser because action buttons + classes +// legitimately differ. The helper owns the lifecycle, not the look. +function createTabbedBrowserSection(config) { + const cfg = Object.assign({ + // Diagnostic id used in console errors. + id: 'tabbed-browser', + // DOM IDs of the tab-strip + per-tab-contents containers. + tabsContainerId: null, + contentsContainerId: null, + // Async fn returning array of tab descriptors (e.g. decades). + fetchTabs: null, + // (tab) => string unique id for one tab (e.g. 'decade-1980'). + // Used as the prefix for that tab's content + playlist + sync IDs. + tabId: null, + // (tab) => string HTML for one tab button. Receives `(tab, isActive)`. + renderTabButton: null, + // (tab) => string HTML for one tab's content shell (action + // buttons + sync-status block + empty playlist container). + // The playlist container inside MUST have id `${tabId}-playlist` + // so the helper can fill it during Phase 2. + renderTabShell: null, + // Async fn (tab) => array of tracks for that tab. + fetchTabContent: null, + // (tracks, tab) => string HTML for the playlist container. + renderTabTracks: null, + // Copy / messages. + emptyTabsMessage: 'No content available', + emptyContentMessage: (tab) => 'No tracks found', + errorTabsMessage: 'Failed to load', + errorContentMessage: 'Failed to load tracks', + // Fired after Phase 1 paints the tab strip + shells, before + // Phase 2 is triggered for the first tab. Useful for caching + // the tab list (e.g. `availableGenres = ...`). + onTabsRendered: null, + }, config || {}); - if (!tabsContainer || !contentsContainer) return; + async function loadTabs() { + try { + const tabsContainer = document.getElementById(cfg.tabsContainerId); + const contentsContainer = document.getElementById(cfg.contentsContainerId); + if (!tabsContainer || !contentsContainer) return; - // Fetch available decades from backend - const response = await fetch('/api/discover/decades/available'); - if (!response.ok) { - throw new Error('Failed to fetch available decades'); + const tabs = await cfg.fetchTabs(); + if (!Array.isArray(tabs) || tabs.length === 0) { + tabsContainer.innerHTML = `

${cfg.emptyTabsMessage}

`; + return; + } + + let tabsHTML = ''; + let contentsHTML = ''; + tabs.forEach((tab, index) => { + const isActive = index === 0; + tabsHTML += cfg.renderTabButton(tab, isActive); + contentsHTML += cfg.renderTabShell(tab, isActive); + }); + + tabsContainer.innerHTML = tabsHTML; + contentsContainer.innerHTML = contentsHTML; + + if (typeof cfg.onTabsRendered === 'function') { + try { cfg.onTabsRendered(tabs); } + catch (err) { console.debug(`[${cfg.id}] onTabsRendered threw:`, err); } + } + + // Phase 2: kick off content load for the first tab. + await loadTabContent(tabs[0]); + } catch (error) { + console.error(`Error loading ${cfg.id} tabs:`, error); + const tabsContainer = document.getElementById(cfg.tabsContainerId); + if (tabsContainer) { + tabsContainer.innerHTML = `

${cfg.errorTabsMessage}

`; + } } + } - const data = await response.json(); - if (!data.success || !data.decades || data.decades.length === 0) { - tabsContainer.innerHTML = '

No decade content available yet. Run a watchlist scan to populate your discovery pool!

'; - return; + async function loadTabContent(tab) { + const tabId = cfg.tabId(tab); + const playlistContainer = document.getElementById(`${tabId}-playlist`); + if (!playlistContainer) return; + + try { + const tracks = await cfg.fetchTabContent(tab); + if (!Array.isArray(tracks) || tracks.length === 0) { + const msg = (typeof cfg.emptyContentMessage === 'function') + ? cfg.emptyContentMessage(tab) + : cfg.emptyContentMessage; + playlistContainer.innerHTML = `

${msg}

`; + return; + } + playlistContainer.innerHTML = cfg.renderTabTracks(tracks, tab); + } catch (error) { + console.error(`Error loading ${cfg.id} tab content:`, error); + const stillThere = document.getElementById(`${tabId}-playlist`); + if (stillThere) { + stillThere.innerHTML = `

${cfg.errorContentMessage}

`; + } } + } - // Build decade tabs - let tabsHTML = ''; - let contentsHTML = ''; + return { loadTabs, loadTabContent }; +} - data.decades.forEach((decade, index) => { - const isActive = index === 0; +// ----- Decade browser config + thin wrappers ----- + +let _decadeBrowserTabsCtrl = null; + +function _getDecadeBrowserTabsCtrl() { + if (_decadeBrowserTabsCtrl) return _decadeBrowserTabsCtrl; + _decadeBrowserTabsCtrl = createTabbedBrowserSection({ + id: 'decade-browser-tabs', + tabsContainerId: 'decade-tabs', + contentsContainerId: 'decade-tab-contents', + fetchTabs: async () => { + const response = await fetch('/api/discover/decades/available'); + if (!response.ok) throw new Error('Failed to fetch available decades'); + const data = await response.json(); + if (!data.success) return []; + return data.decades || []; + }, + tabId: (decade) => `decade-${decade.year}`, + renderTabButton: (decade, isActive) => { const icon = getDecadeIcon(decade.year); - const tabId = `decade-${decade.year}`; - - // Tab button - tabsHTML += ` + return ` `; - - // Tab content - contentsHTML += ` + }, + renderTabShell: (decade, isActive) => { + const tabId = `decade-${decade.year}`; + return `
@@ -1770,23 +1879,69 @@ async function loadDecadeBrowserTabs() {
`; - }); + }, + fetchTabContent: async (decade) => { + const response = await fetch(`/api/discover/decade/${decade.year}`); + if (!response.ok) throw new Error('Failed to fetch decade playlist'); + const data = await response.json(); + if (!data.success) return []; + // Side-effect: cache + active marker, exactly as old code did. + decadeTracksCache[decade.year] = data.tracks || []; + activeDecade = decade.year; + return data.tracks || []; + }, + renderTabTracks: (tracks) => _renderTabbedTrackList(tracks), + emptyContentMessage: (decade) => `No tracks found for the ${decade.year}s`, + errorTabsMessage: 'Failed to load decades', + errorContentMessage: 'Failed to load decade tracks', + emptyTabsMessage: 'No decade content available yet. Run a watchlist scan to populate your discovery pool!', + }); + return _decadeBrowserTabsCtrl; +} - tabsContainer.innerHTML = tabsHTML; - contentsContainer.innerHTML = contentsHTML; - - // Load first decade's tracks - if (data.decades.length > 0) { - await loadDecadeTracks(data.decades[0].year); +// Shared track-row markup for tabbed browsers. Decade + genre rows +// have the same shape — both pull from `track_data_json` first then +// fall back to top-level fields. Lifted so the helper-driven +// renderers don't each carry a copy. +function _renderTabbedTrackList(tracks) { + let html = '
'; + tracks.forEach((track, index) => { + let trackData = track; + if (track.track_data_json) { + trackData = track.track_data_json; } - } catch (error) { - console.error('Error loading decade browser tabs:', error); - const tabsContainer = document.getElementById('decade-tabs'); - if (tabsContainer) { - tabsContainer.innerHTML = '

Failed to load decades

'; - } - } + const trackName = trackData.name || trackData.track_name || track.track_name || 'Unknown Track'; + const artistName = trackData.artists?.[0]?.name || trackData.artists?.[0] || trackData.artist_name || track.artist_name || 'Unknown Artist'; + const albumName = trackData.album?.name || trackData.album_name || track.album_name || 'Unknown Album'; + const coverUrl = trackData.album?.images?.[0]?.url || track.album_cover_url || '/static/placeholder-album.png'; + const durationMs = trackData.duration_ms || track.duration_ms || 0; + + const durationMin = Math.floor(durationMs / 60000); + const durationSec = Math.floor((durationMs % 60000) / 1000); + const duration = `${durationMin}:${durationSec.toString().padStart(2, '0')}`; + + html += ` +
+
${index + 1}
+
+ ${albumName} +
+
+
${trackName}
+
${artistName}
+
+
${albumName}
+
${duration}
+
+ `; + }); + html += '
'; + return html; +} + +async function loadDecadeBrowserTabs() { + return _getDecadeBrowserTabsCtrl().loadTabs(); } function switchDecadeTab(decade) { @@ -1817,71 +1972,7 @@ function switchDecadeTab(decade) { } async function loadDecadeTracks(decade) { - try { - const playlistContainer = document.getElementById(`decade-${decade}-playlist`); - if (!playlistContainer) return; - - const response = await fetch(`/api/discover/decade/${decade}`); - if (!response.ok) { - throw new Error('Failed to fetch decade playlist'); - } - - const data = await response.json(); - if (!data.success || !data.tracks || data.tracks.length === 0) { - playlistContainer.innerHTML = '

No tracks found for the ' + decade + 's

'; - return; - } - - // Store tracks in cache - decadeTracksCache[decade] = data.tracks; - activeDecade = decade; - - // Build compact playlist HTML - let html = '
'; - data.tracks.forEach((track, index) => { - // Extract track data from track_data_json if available - let trackData = track; - if (track.track_data_json) { - trackData = track.track_data_json; - } - - // Get track properties with fallbacks - const trackName = trackData.name || trackData.track_name || track.track_name || 'Unknown Track'; - const artistName = trackData.artists?.[0]?.name || trackData.artists?.[0] || trackData.artist_name || track.artist_name || 'Unknown Artist'; - const albumName = trackData.album?.name || trackData.album_name || track.album_name || 'Unknown Album'; - const coverUrl = trackData.album?.images?.[0]?.url || track.album_cover_url || '/static/placeholder-album.png'; - const durationMs = trackData.duration_ms || track.duration_ms || 0; - - const durationMin = Math.floor(durationMs / 60000); - const durationSec = Math.floor((durationMs % 60000) / 1000); - const duration = `${durationMin}:${durationSec.toString().padStart(2, '0')}`; - - html += ` -
-
${index + 1}
-
- ${albumName} -
-
-
${trackName}
-
${artistName}
-
-
${albumName}
-
${duration}
-
- `; - }); - html += '
'; - - playlistContainer.innerHTML = html; - - } catch (error) { - console.error('Error loading decade tracks:', error); - const playlistContainer = document.getElementById(`decade-${decade}-playlist`); - if (playlistContainer) { - playlistContainer.innerHTML = '

Failed to load decade tracks

'; - } - } + return _getDecadeBrowserTabsCtrl().loadTabContent({ year: decade }); } async function startDecadeSync(decade) { @@ -2080,64 +2171,58 @@ let genreTracksCache = {}; // Store tracks for each genre let activeGenre = null; let availableGenres = []; -async function loadGenreBrowserTabs() { - try { - const tabsContainer = document.getElementById('genre-tabs'); - const contentsContainer = document.getElementById('genre-tab-contents'); +// Helper: derive the URL/DOM-safe id used for a genre tab. Both +// the tab shell and the playlist-container element are keyed on this. +function _genreTabId(genreName) { + const safe = genreName.replace(/\s+/g, '-').replace(/[^a-zA-Z0-9-]/g, ''); + return `genre-${safe}`; +} - if (!tabsContainer || !contentsContainer) return; +let _genreBrowserTabsCtrl = null; - // Fetch available genres from backend - const response = await fetch('/api/discover/genres/available'); - if (!response.ok) { - throw new Error('Failed to fetch available genres'); - } - - const data = await response.json(); - if (!data.success || !data.genres || data.genres.length === 0) { - tabsContainer.innerHTML = '

No genre content available yet. Run a watchlist scan to populate your discovery pool!

'; - return; - } - - availableGenres = data.genres; - - // Build genre tabs (limit to first 8-10 to avoid overcrowding) - const displayGenres = data.genres.slice(0, 10); - let tabsHTML = ''; - let contentsHTML = ''; - - displayGenres.forEach((genre, index) => { - const isActive = index === 0; +function _getGenreBrowserTabsCtrl() { + if (_genreBrowserTabsCtrl) return _genreBrowserTabsCtrl; + _genreBrowserTabsCtrl = createTabbedBrowserSection({ + id: 'genre-browser-tabs', + tabsContainerId: 'genre-tabs', + contentsContainerId: 'genre-tab-contents', + fetchTabs: async () => { + const response = await fetch('/api/discover/genres/available'); + if (!response.ok) throw new Error('Failed to fetch available genres'); + const data = await response.json(); + if (!data.success) return []; + // Cap at 10 to avoid overcrowding; old behavior preserved. + return (data.genres || []).slice(0, 10); + }, + onTabsRendered: (genres) => { availableGenres = genres; }, + tabId: (genre) => _genreTabId(genre.name), + renderTabButton: (genre, isActive) => { const icon = getGenreIcon(genre.name); - const genreName = genre.name; - const genreId = genreName.replace(/\s+/g, '-').replace(/[^a-zA-Z0-9-]/g, ''); - const tabId = `genre-${genreId}`; - - // Tab button - tabsHTML += ` + return ` `; - - // Tab content - contentsHTML += ` -
+ }, + renderTabShell: (genre, isActive) => { + const tabId = _genreTabId(genre.name); + return ` +
-

${capitalizeGenre(genreName)} Mix

+

${capitalizeGenre(genre.name)} Mix

${genre.track_count} tracks

- - @@ -2149,27 +2234,32 @@ async function loadGenreBrowserTabs() {
-

Loading ${capitalizeGenre(genreName)} tracks...

+

Loading ${capitalizeGenre(genre.name)} tracks...

`; - }); + }, + fetchTabContent: async (genre) => { + const response = await fetch(`/api/discover/genre/${encodeURIComponent(genre.name)}`); + if (!response.ok) throw new Error('Failed to fetch genre playlist'); + const data = await response.json(); + if (!data.success) return []; + // Side-effect: cache + active marker, exactly as old code did. + genreTracksCache[genre.name] = data.tracks || []; + activeGenre = genre.name; + return data.tracks || []; + }, + renderTabTracks: (tracks) => _renderTabbedTrackList(tracks), + emptyContentMessage: (genre) => `No tracks found for ${capitalizeGenre(genre.name)}`, + errorTabsMessage: 'Failed to load genres', + errorContentMessage: 'Failed to load genre tracks', + emptyTabsMessage: 'No genre content available yet. Run a watchlist scan to populate your discovery pool!', + }); + return _genreBrowserTabsCtrl; +} - tabsContainer.innerHTML = tabsHTML; - contentsContainer.innerHTML = contentsHTML; - - // Load first genre's tracks - if (displayGenres.length > 0) { - await loadGenreTracks(displayGenres[0].name); - } - - } catch (error) { - console.error('Error loading genre browser tabs:', error); - const tabsContainer = document.getElementById('genre-tabs'); - if (tabsContainer) { - tabsContainer.innerHTML = '

Failed to load genres

'; - } - } +async function loadGenreBrowserTabs() { + return _getGenreBrowserTabsCtrl().loadTabs(); } function switchGenreTab(genreName) { @@ -2200,73 +2290,7 @@ function switchGenreTab(genreName) { } async function loadGenreTracks(genreName) { - try { - const genreId = genreName.replace(/\s+/g, '-').replace(/[^a-zA-Z0-9-]/g, ''); - const playlistContainer = document.getElementById(`genre-${genreId}-playlist`); - if (!playlistContainer) return; - - const response = await fetch(`/api/discover/genre/${encodeURIComponent(genreName)}`); - if (!response.ok) { - throw new Error('Failed to fetch genre playlist'); - } - - const data = await response.json(); - if (!data.success || !data.tracks || data.tracks.length === 0) { - playlistContainer.innerHTML = `

No tracks found for ${capitalizeGenre(genreName)}

`; - return; - } - - // Store tracks in cache - genreTracksCache[genreName] = data.tracks; - activeGenre = genreName; - - // Build compact playlist HTML - let html = '
'; - data.tracks.forEach((track, index) => { - // Extract track data from track_data_json if available - let trackData = track; - if (track.track_data_json) { - trackData = track.track_data_json; - } - - // Get track properties with fallbacks - const trackName = trackData.name || trackData.track_name || track.track_name || 'Unknown Track'; - const artistName = trackData.artists?.[0]?.name || trackData.artists?.[0] || trackData.artist_name || track.artist_name || 'Unknown Artist'; - const albumName = trackData.album?.name || trackData.album_name || track.album_name || 'Unknown Album'; - const coverUrl = trackData.album?.images?.[0]?.url || track.album_cover_url || '/static/placeholder-album.png'; - const durationMs = trackData.duration_ms || track.duration_ms || 0; - - const durationMin = Math.floor(durationMs / 60000); - const durationSec = Math.floor((durationMs % 60000) / 1000); - const duration = `${durationMin}:${durationSec.toString().padStart(2, '0')}`; - - html += ` -
-
${index + 1}
-
- ${albumName} -
-
-
${trackName}
-
${artistName}
-
-
${albumName}
-
${duration}
-
- `; - }); - html += '
'; - - playlistContainer.innerHTML = html; - - } catch (error) { - console.error('Error loading genre tracks:', error); - const genreId = genreName.replace(/\s+/g, '-').replace(/[^a-zA-Z0-9-]/g, ''); - const playlistContainer = document.getElementById(`genre-${genreId}-playlist`); - if (playlistContainer) { - playlistContainer.innerHTML = '

Failed to load genre tracks

'; - } - } + return _getGenreBrowserTabsCtrl().loadTabContent({ name: genreName }); } async function startGenreSync(genreName) { @@ -3794,75 +3818,6 @@ async function syncSeasonalPlaylist() { // PERSONALIZED PLAYLISTS // =============================== -async function loadPersonalizedRecentlyAdded() { - try { - const container = document.getElementById('personalized-recently-added'); - if (!container) return; - - const response = await fetch('/api/discover/personalized/recently-added'); - if (!response.ok) return; - - const data = await response.json(); - if (!data.success || !data.tracks || data.tracks.length === 0) { - container.closest('.discover-section').style.display = 'none'; - return; - } - - personalizedRecentlyAdded = data.tracks; - renderCompactPlaylist(container, data.tracks); - container.closest('.discover-section').style.display = 'block'; - - } catch (error) { - console.error('Error loading recently added:', error); - } -} - -async function loadPersonalizedTopTracks() { - try { - const container = document.getElementById('personalized-top-tracks'); - if (!container) return; - - const response = await fetch('/api/discover/personalized/top-tracks'); - if (!response.ok) return; - - const data = await response.json(); - if (!data.success || !data.tracks || data.tracks.length === 0) { - container.closest('.discover-section').style.display = 'none'; - return; - } - - personalizedTopTracks = data.tracks; - renderCompactPlaylist(container, data.tracks); - container.closest('.discover-section').style.display = 'block'; - - } catch (error) { - console.error('Error loading top tracks:', error); - } -} - -async function loadPersonalizedForgottenFavorites() { - try { - const container = document.getElementById('personalized-forgotten-favorites'); - if (!container) return; - - const response = await fetch('/api/discover/personalized/forgotten-favorites'); - if (!response.ok) return; - - const data = await response.json(); - if (!data.success || !data.tracks || data.tracks.length === 0) { - container.closest('.discover-section').style.display = 'none'; - return; - } - - personalizedForgottenFavorites = data.tracks; - renderCompactPlaylist(container, data.tracks); - container.closest('.discover-section').style.display = 'block'; - - } catch (error) { - console.error('Error loading forgotten favorites:', error); - } -} - async function loadPersonalizedPopularPicks() { try { const container = document.getElementById('personalized-popular-picks'); @@ -6580,29 +6535,6 @@ async function loadDiscoveryShuffle() { } } -async function loadFamiliarFavorites() { - try { - const container = document.getElementById('personalized-familiar-favorites'); - if (!container) return; - - const response = await fetch('/api/discover/personalized/familiar-favorites?limit=50'); - if (!response.ok) return; - - const data = await response.json(); - if (!data.success || !data.tracks || data.tracks.length === 0) { - container.closest('.discover-section').style.display = 'none'; - return; - } - - personalizedFamiliarFavorites = data.tracks; - renderCompactPlaylist(container, data.tracks); - container.closest('.discover-section').style.display = 'block'; - - } catch (error) { - console.error('Error loading familiar favorites:', error); - } -} - // =============================== // BECAUSE YOU LISTEN TO // =============================== @@ -7357,14 +7289,6 @@ async function openDownloadModalForDiscoverPlaylist(playlistType, playlistName) tracks = personalizedHiddenGems; } else if (playlistType === 'discovery_shuffle') { tracks = personalizedDiscoveryShuffle; - } else if (playlistType === 'familiar_favorites') { - tracks = personalizedFamiliarFavorites; - } else if (playlistType === 'recently_added') { - tracks = personalizedRecentlyAdded; - } else if (playlistType === 'top_tracks') { - tracks = personalizedTopTracks; - } else if (playlistType === 'forgotten_favorites') { - tracks = personalizedForgottenFavorites; } else if (playlistType === 'build_playlist') { tracks = buildPlaylistTracks; } @@ -7484,8 +7408,6 @@ async function startDiscoverPlaylistSync(playlistType, playlistName) { tracks = personalizedHiddenGems; } else if (playlistType === 'discovery_shuffle') { tracks = personalizedDiscoveryShuffle; - } else if (playlistType === 'familiar_favorites') { - tracks = personalizedFamiliarFavorites; } else if (playlistType === 'build_playlist') { tracks = buildPlaylistTracks; } @@ -7616,7 +7538,7 @@ function startDiscoverSyncPolling(playlistType, virtualPlaylistId) { 'release_radar': 'Fresh Tape', 'discovery_weekly': 'The Archives', 'seasonal_playlist': 'Seasonal Mix', 'popular_picks': 'Popular Picks', 'hidden_gems': 'Hidden Gems', 'discovery_shuffle': 'Discovery Shuffle', - 'familiar_favorites': 'Familiar Favorites', 'build_playlist': 'Custom Playlist' + 'build_playlist': 'Custom Playlist' }; showToast(`${playlistNames[playlistType] || playlistType} sync complete!`, 'success'); setTimeout(() => { const sd = el(`${prefix}-sync-status`); if (sd) sd.style.display = 'none'; }, 3000); @@ -7682,7 +7604,6 @@ function startDiscoverSyncPolling(playlistType, virtualPlaylistId) { 'popular_picks': 'Popular Picks', 'hidden_gems': 'Hidden Gems', 'discovery_shuffle': 'Discovery Shuffle', - 'familiar_favorites': 'Familiar Favorites', 'build_playlist': 'Custom Playlist' }; const displayName = playlistNames[playlistType] || playlistType; @@ -8207,8 +8128,6 @@ async function rehydrateDiscoverDownloadModal(playlistId) { apiEndpoint = '/api/discover/hidden-gems'; } else if (playlistId === 'discover_discovery_shuffle') { apiEndpoint = '/api/discover/discovery-shuffle'; - } else if (playlistId === 'discover_familiar_favorites') { - apiEndpoint = '/api/discover/familiar-favorites'; } else if (playlistId === 'build_playlist_custom') { apiEndpoint = '/api/discover/build-playlist'; } else if (playlistId.startsWith('discover_lb_')) { diff --git a/webui/static/helper.js b/webui/static/helper.js index 7323a284..5bb24929 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -1056,10 +1056,6 @@ const HELPER_CONTENT = { }, // Personalized Playlists - '#personalized-recently-added': { - title: 'Recently Added', - description: 'The latest tracks added to your library. A quick way to see what\'s new in your collection.', - }, '#personalized-popular-picks': { title: 'Popular Picks', description: 'Trending tracks from your discovery pool artists. These are the most popular songs from artists similar to the ones you follow.', @@ -1071,23 +1067,11 @@ const HELPER_CONTENT = { description: 'Rare and deeper cuts from your discovery pool artists. Lower popularity tracks that you might not find on mainstream playlists.', docsId: 'disc-playlists' }, - '#personalized-top-tracks': { - title: 'Your Top 50', - description: 'Your all-time most played tracks from listening history. A snapshot of your personal favorites.', - }, - '#personalized-forgotten-favorites': { - title: 'Forgotten Favorites', - description: 'Tracks you used to play frequently but haven\'t listened to in a while. Rediscover music you loved.', - }, '#personalized-discovery-shuffle': { title: 'Discovery Shuffle', description: 'Random tracks from your entire discovery pool — different every time you load. A surprise mix for when you want something new.', docsId: 'disc-playlists' }, - '#personalized-familiar-favorites': { - title: 'Familiar Favorites', - description: 'Your reliable go-to tracks. Consistently played songs that define your taste.', - }, // Curated Playlists '#release-radar-playlist': { @@ -3432,6 +3416,8 @@ 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 < 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' }, { title: 'Internal: Migrate 7 More Discover Sections to the Controller', desc: 'follow-up to the foundation commit. migrated fresh tape, the archives, time machine intro carousel, browse by genre intro carousel, seasonal mix, your artists, and because-you-listen-to onto `createDiscoverSectionController`. each one drops its own hand-rolled try/catch + spinner injection + empty-state HTML + error swallow in favor of a config object — controller owns the lifecycle. net 76 lines smaller in discover.js even after adding the per-section render helpers. skipped two sections that don\'t fit the controller\'s single-fetch / single-render-target shape: `loadYourAlbums` (paginated grid + filters, four separate UI elements updated) and `loadSeasonalAlbums` (no fetch — receives pre-fetched data from parent). hidden / dead sections (~13 of them) untouched in this pass — separate audit commit will surface or kill them. controller extension candidates surfaced for follow-up: callable `fetchUrl` (so seasonal playlist doesn\'t need controller-recreate-on-key-change), explicit `isStale` / `onStale` hook (so your-artists doesn\'t fold stale handling into renderItems), `beforeLoad` hook (so because-you-listen-to can let the controller own the dynamic container creation), and a no-fetch `data:` mode (so render-only sections like seasonal albums can use the controller). zero behavior changes — every public load function keeps its name + signature so existing callers, refresh buttons, and dashboard wiring don\'t notice the swap.', page: 'discover' }, diff --git a/webui/static/sync-spotify.js b/webui/static/sync-spotify.js index 95e592c2..796d1ce5 100644 --- a/webui/static/sync-spotify.js +++ b/webui/static/sync-spotify.js @@ -293,8 +293,6 @@ async function rehydrateDiscoverPlaylistModal(virtualPlaylistId, playlistName, b apiEndpoint = '/api/discover/hidden-gems'; } else if (virtualPlaylistId === 'discover_discovery_shuffle') { apiEndpoint = '/api/discover/discovery-shuffle'; - } else if (virtualPlaylistId === 'discover_familiar_favorites') { - apiEndpoint = '/api/discover/familiar-favorites'; } else if (virtualPlaylistId === 'build_playlist_custom') { apiEndpoint = '/api/discover/build-playlist'; } else if (virtualPlaylistId.startsWith('discover_lb_')) {