diff --git a/core/wishlist/album_grouping.py b/core/wishlist/album_grouping.py new file mode 100644 index 00000000..148ea263 --- /dev/null +++ b/core/wishlist/album_grouping.py @@ -0,0 +1,201 @@ +"""Wishlist album grouping for the per-album bundle dispatch. + +When the auto-wishlist cycle is ``'albums'`` the user expects each +album with missing tracks to fire ONE album-bundle search instead +of one per-track search per missing track. Track lists in the +wishlist may span multiple albums in one cycle, so we group them +upfront + emit one sub-batch per album. + +Pure function — no IO, no runtime-state dependency — so it can be +unit-tested without standing up the wishlist runner. +""" + +from __future__ import annotations + +import json +from dataclasses import dataclass, field +from typing import Any, Dict, List, Optional + + +def _extract_track_data(track: Dict[str, Any]) -> Dict[str, Any]: + """Mirror of ``classification._extract_track_data``: unwrap nested + Spotify payloads regardless of which key the wishlist row chose + to stash them under.""" + for key in ("track_data", "spotify_data", "metadata", "track"): + data = track.get(key) + if isinstance(data, str): + try: + data = json.loads(data) + except Exception: + data = {} + if isinstance(data, dict) and data: + nested = ( + data.get("track_data") + or data.get("spotify_data") + or data.get("metadata") + or data.get("track") + ) + if isinstance(nested, str): + try: + nested = json.loads(nested) + except Exception: + nested = {} + if isinstance(nested, dict) and nested: + return nested + return data + return {} + + +def _album_key(spotify_data: Dict[str, Any]) -> Optional[str]: + """Derive a stable grouping key from a track's Spotify metadata. + + Prefers album id (canonical). Falls back to a name-normalized + key when the album row has no id (older wishlist rows can be + missing it). Returns ``None`` when no album information is + available at all — those tracks can't participate in an + album-bundle search and stay on the residual per-track flow. + """ + album = spotify_data.get('album') or {} + if not isinstance(album, dict): + return None + album_id = album.get('id') + if isinstance(album_id, str) and album_id.strip(): + return album_id.strip() + name = album.get('name') + if isinstance(name, str) and name.strip(): + return f"_name_{name.strip().lower()}" + return None + + +def _artist_name_from_track(spotify_data: Dict[str, Any], track: Dict[str, Any]) -> str: + """Pick a primary artist name from the track's metadata. + + Album-bundle search needs an artist string. Prefer the first + Spotify artist (most accurate), fall back to ``track_info['artist']`` + or ``track['artist_name']`` from the wishlist row, then to empty + string (caller will skip the bundle). + """ + artists = spotify_data.get('artists') or [] + if isinstance(artists, list) and artists: + first = artists[0] + if isinstance(first, dict): + name = first.get('name') + if isinstance(name, str) and name.strip(): + return name.strip() + elif isinstance(first, str) and first.strip(): + return first.strip() + for key in ('artist_name', 'artist'): + val = track.get(key) + if isinstance(val, str) and val.strip(): + return val.strip() + return '' + + +@dataclass +class WishlistAlbumGroup: + """One album's worth of wishlist tracks ready for a sub-batch.""" + + album_key: str + album_context: Dict[str, Any] + artist_context: Dict[str, Any] + tracks: List[Dict[str, Any]] = field(default_factory=list) + + +@dataclass +class WishlistGroupingResult: + """Aggregated grouping output. + + - ``album_groups``: one entry per resolvable album. Each carries + enough context to be submitted as an album-bundle batch. + - ``residual_tracks``: tracks that couldn't be grouped (no + album metadata + no artist). They fall through to the normal + per-track flow. + """ + + album_groups: List[WishlistAlbumGroup] = field(default_factory=list) + residual_tracks: List[Dict[str, Any]] = field(default_factory=list) + + +def group_wishlist_tracks_by_album( + tracks: List[Dict[str, Any]], + *, + min_tracks_per_album: int = 1, +) -> WishlistGroupingResult: + """Group wishlist tracks by their owning album. + + ``min_tracks_per_album`` controls the threshold for promoting an + album to its own sub-batch. Default ``1`` means even a single + missing track gets the album-bundle treatment (which is what the + user wants for releases where they only need one track from the + album). Set higher to require multiple missing tracks before + engaging the bundle search. + """ + result = WishlistGroupingResult() + if not tracks: + return result + + # First pass: bucket by album key. + buckets: Dict[str, WishlistAlbumGroup] = {} + unbucketable: List[Dict[str, Any]] = [] + + for track in tracks: + spotify_data = _extract_track_data(track) + key = _album_key(spotify_data) + if key is None: + unbucketable.append(track) + continue + + artist_name = _artist_name_from_track(spotify_data, track) + if not artist_name: + unbucketable.append(track) + continue + + album = spotify_data.get('album') or {} + if not isinstance(album, dict): + album = {} + album_name = album.get('name', '') + if not (isinstance(album_name, str) and album_name.strip()): + unbucketable.append(track) + continue + + group = buckets.get(key) + if group is None: + album_context = { + 'id': album.get('id') or key, + 'name': album_name.strip(), + 'release_date': album.get('release_date', ''), + 'total_tracks': album.get('total_tracks', 0), + 'album_type': album.get('album_type', 'album'), + 'images': album.get('images', []), + 'artists': album.get('artists', []), + } + artist_context = { + 'id': 'wishlist', + 'name': artist_name, + 'genres': [], + } + group = WishlistAlbumGroup( + album_key=key, + album_context=album_context, + artist_context=artist_context, + ) + buckets[key] = group + group.tracks.append(track) + + # Second pass: promote groups meeting the threshold; demote + # smaller groups to residual. + for group in buckets.values(): + if len(group.tracks) >= min_tracks_per_album: + result.album_groups.append(group) + else: + result.residual_tracks.extend(group.tracks) + + result.residual_tracks.extend(unbucketable) + return result + + +__all__ = [ + 'group_wishlist_tracks_by_album', + 'WishlistAlbumGroup', + 'WishlistGroupingResult', +] diff --git a/core/wishlist/processing.py b/core/wishlist/processing.py index c3de917e..9a794098 100644 --- a/core/wishlist/processing.py +++ b/core/wishlist/processing.py @@ -639,45 +639,115 @@ def process_wishlist_automatically(runtime: WishlistAutoProcessingRuntime, autom for i, track in enumerate(wishlist_tracks): track['_original_index'] = i - # Create batch for automatic processing - batch_id = str(uuid.uuid4()) - playlist_name = f"Wishlist (Auto - {current_cycle.capitalize()})" + # When the cycle is 'albums', try to split the wishlist + # into per-album sub-batches so each album fires ONE + # album-bundle search (slskd / torrent / usenet) instead + # of N per-track searches. Residual tracks (no resolvable + # album metadata) fall through to a normal per-track + # batch. Singles cycle keeps its original single-batch + # shape — Spotify already classifies them away from + # albums. + _submitted_batches: list[str] = [] + if current_cycle == 'albums': + from core.wishlist.album_grouping import group_wishlist_tracks_by_album + grouping = group_wishlist_tracks_by_album(wishlist_tracks) + else: + grouping = None - # Create task queue - convert wishlist tracks to expected format - with runtime.tasks_lock: - runtime.download_batches[batch_id] = { - 'phase': 'analysis', - 'playlist_id': playlist_id, - 'playlist_name': playlist_name, - 'queue': [], - 'active_count': 0, - 'max_concurrent': runtime.get_batch_max_concurrent(), # Wishlist always does single-track downloads, not folder grabs - 'queue_index': 0, - 'analysis_total': len(wishlist_tracks), - 'analysis_processed': 0, - 'analysis_results': [], - # Track state management (replicating sync.py) - 'permanently_failed_tracks': [], - 'cancelled_tracks': set(), - # Wishlist tracks are already known-missing — skip the expensive library check - 'force_download_all': True, - # Mark as auto-initiated - 'auto_initiated': True, - 'auto_processing_timestamp': runtime.current_time_fn(), - # Store current cycle for toggling after completion - 'current_cycle': current_cycle, - # Profile context for failed track wishlist re-adds (auto = profile 1 default) - 'profile_id': runtime.profile_id, - } + if grouping and grouping.album_groups: + for album_idx, group in enumerate(grouping.album_groups): + album_batch_id = str(uuid.uuid4()) + album_batch_name = ( + f"Wishlist (Auto - Album: {group.album_context.get('name', 'Unknown')})" + ) + with runtime.tasks_lock: + runtime.download_batches[album_batch_id] = { + 'phase': 'analysis', + 'playlist_id': playlist_id, + 'playlist_name': album_batch_name, + 'queue': [], + 'active_count': 0, + 'max_concurrent': runtime.get_batch_max_concurrent(), + 'queue_index': 0, + 'analysis_total': len(group.tracks), + 'analysis_processed': 0, + 'analysis_results': [], + 'permanently_failed_tracks': [], + 'cancelled_tracks': set(), + 'force_download_all': True, + 'auto_initiated': True, + 'auto_processing_timestamp': runtime.current_time_fn(), + 'current_cycle': current_cycle, + 'profile_id': runtime.profile_id, + # Album-bundle dispatch gate reads these + # three. With them set, the master worker + # routes through slskd / torrent / usenet + # album-bundle search instead of per-track. + 'is_album_download': True, + 'album_context': group.album_context, + 'artist_context': group.artist_context, + } + logger.info( + f"[Auto-Wishlist] Album sub-batch {album_idx + 1}/{len(grouping.album_groups)}: " + f"'{group.album_context.get('name')}' by '{group.artist_context.get('name')}' " + f"({len(group.tracks)} tracks) → {album_batch_id}" + ) + _submitted_batches.append(album_batch_id) + runtime.missing_download_executor.submit( + runtime.run_full_missing_tracks_process, + album_batch_id, playlist_id, group.tracks, + ) - logger.info(f"Starting automatic wishlist batch {batch_id} with {len(wishlist_tracks)} tracks") - runtime.update_automation_progress(automation_id, progress=50, phase=f'Downloading {len(wishlist_tracks)} tracks', - log_line=f'Started batch: {len(wishlist_tracks)} {current_cycle}', log_type='success') + # Residual tracks (no album group could be formed, OR + # singles cycle): one classic per-track batch as before. + residual_tracks = ( + grouping.residual_tracks if grouping is not None else wishlist_tracks + ) + if residual_tracks: + batch_id = str(uuid.uuid4()) + playlist_name = f"Wishlist (Auto - {current_cycle.capitalize()})" + with runtime.tasks_lock: + runtime.download_batches[batch_id] = { + 'phase': 'analysis', + 'playlist_id': playlist_id, + 'playlist_name': playlist_name, + 'queue': [], + 'active_count': 0, + 'max_concurrent': runtime.get_batch_max_concurrent(), + 'queue_index': 0, + 'analysis_total': len(residual_tracks), + 'analysis_processed': 0, + 'analysis_results': [], + 'permanently_failed_tracks': [], + 'cancelled_tracks': set(), + 'force_download_all': True, + 'auto_initiated': True, + 'auto_processing_timestamp': runtime.current_time_fn(), + 'current_cycle': current_cycle, + 'profile_id': runtime.profile_id, + } + _submitted_batches.append(batch_id) + runtime.missing_download_executor.submit( + runtime.run_full_missing_tracks_process, + batch_id, playlist_id, residual_tracks, + ) + logger.info( + f"Starting wishlist residual batch {batch_id} with {len(residual_tracks)} tracks " + f"({'singles' if current_cycle == 'singles' else 'unbucketed albums'})" + ) - # Submit the wishlist processing job using existing infrastructure - runtime.missing_download_executor.submit(runtime.run_full_missing_tracks_process, batch_id, playlist_id, wishlist_tracks) - - # Don't mark auto_processing as False here - let completion handler do it + _summary_parts: list[str] = [] + if grouping and grouping.album_groups: + _summary_parts.append(f"{len(grouping.album_groups)} album batch(es)") + if residual_tracks: + _summary_parts.append(f"{len(residual_tracks)} per-track") + _summary_text = ', '.join(_summary_parts) or 'no batches' + runtime.update_automation_progress( + automation_id, progress=50, + phase=f'Downloading {len(wishlist_tracks)} tracks', + log_line=f'Started: {_summary_text} for cycle {current_cycle}', + log_type='success', + ) except Exception as e: logger.error(f"Error in automatic wishlist processing: {e}") diff --git a/tests/wishlist/test_album_grouping.py b/tests/wishlist/test_album_grouping.py new file mode 100644 index 00000000..9fc3264a --- /dev/null +++ b/tests/wishlist/test_album_grouping.py @@ -0,0 +1,159 @@ +"""Tests for the wishlist-cycle album grouping helper that drives +the per-album bundle dispatch. + +Pins the bucketing contract so future changes to the dispatch flow +don't silently regress the user-visible behavior: wishlist 'albums' +cycle should emit one album-bundle search per missing album, not +one per missing track. +""" + +from __future__ import annotations + +from core.wishlist.album_grouping import ( + WishlistAlbumGroup, + WishlistGroupingResult, + group_wishlist_tracks_by_album, +) + + +def _wt(track_name, artist, album_id, album_name, **extra): + """Build a wishlist row in the shape the wishlist service returns.""" + return { + 'track_name': track_name, + 'artist_name': artist, + 'spotify_data': { + 'name': track_name, + 'artists': [{'name': artist}], + 'album': { + 'id': album_id, + 'name': album_name, + **extra, + }, + }, + } + + +def test_empty_input_returns_empty_result(): + res = group_wishlist_tracks_by_album([]) + assert res.album_groups == [] + assert res.residual_tracks == [] + + +def test_single_album_groups_all_tracks_together(): + tracks = [ + _wt('Dragon Soul', 'Ryoto', 'alb1', 'Cha-La Head-Cha-La'), + _wt('Cha-La Head-Cha-La', 'Ryoto', 'alb1', 'Cha-La Head-Cha-La'), + _wt('Zenkai Power', 'Ryoto', 'alb1', 'Cha-La Head-Cha-La'), + ] + res = group_wishlist_tracks_by_album(tracks) + assert len(res.album_groups) == 1 + g = res.album_groups[0] + assert g.album_key == 'alb1' + assert g.album_context['name'] == 'Cha-La Head-Cha-La' + assert g.artist_context['name'] == 'Ryoto' + assert len(g.tracks) == 3 + + +def test_multiple_albums_emit_separate_groups(): + tracks = [ + _wt('Song A', 'Artist 1', 'alb1', 'Album 1'), + _wt('Song B', 'Artist 1', 'alb1', 'Album 1'), + _wt('Song C', 'Artist 2', 'alb2', 'Album 2'), + ] + res = group_wishlist_tracks_by_album(tracks) + assert len(res.album_groups) == 2 + keys = {g.album_key for g in res.album_groups} + assert keys == {'alb1', 'alb2'} + for g in res.album_groups: + if g.album_key == 'alb1': + assert len(g.tracks) == 2 + else: + assert len(g.tracks) == 1 + + +def test_missing_album_metadata_falls_through_to_residual(): + tracks = [ + # No spotify_data.album at all + {'track_name': 'Orphan', 'artist_name': 'X', 'spotify_data': {'artists': [{'name': 'X'}]}}, + # Empty album dict + {'track_name': 'Empty Album', 'artist_name': 'X', 'spotify_data': {'album': {}, 'artists': [{'name': 'X'}]}}, + ] + res = group_wishlist_tracks_by_album(tracks) + assert res.album_groups == [] + assert len(res.residual_tracks) == 2 + + +def test_missing_artist_demotes_to_residual(): + """Album-bundle search needs an artist; if we can't recover one, + skip the bundle path and let the track go through per-track.""" + tracks = [{ + 'track_name': 'Song', + 'spotify_data': { + 'artists': [], + 'album': {'id': 'a', 'name': 'Album'}, + }, + }] + res = group_wishlist_tracks_by_album(tracks) + assert res.album_groups == [] + assert res.residual_tracks == tracks + + +def test_min_tracks_threshold_demotes_solos(): + """When ``min_tracks_per_album=2``, single-track albums fall to + residual so the user doesn't fire a bundle search for a 1-track + rip when per-track would do.""" + tracks = [ + _wt('Solo Track', 'Artist 1', 'alb1', 'Album 1'), + _wt('Song A', 'Artist 2', 'alb2', 'Album 2'), + _wt('Song B', 'Artist 2', 'alb2', 'Album 2'), + ] + res = group_wishlist_tracks_by_album(tracks, min_tracks_per_album=2) + assert len(res.album_groups) == 1 + assert res.album_groups[0].album_key == 'alb2' + assert len(res.residual_tracks) == 1 + assert res.residual_tracks[0]['track_name'] == 'Solo Track' + + +def test_default_threshold_promotes_solo_albums(): + """Default ``min_tracks_per_album=1`` — even one missing track + triggers the album-bundle path. Matches the user's stated + preference (don't gate on track count).""" + tracks = [_wt('Solo', 'Artist 1', 'alb1', 'Album 1')] + res = group_wishlist_tracks_by_album(tracks) + assert len(res.album_groups) == 1 + assert res.residual_tracks == [] + + +def test_album_without_id_uses_name_normalized_key(): + """Some older wishlist rows are missing the album id. Group by + a name-normalized key so they still bucket together.""" + tracks = [ + _wt('S1', 'Artist', None, 'Same Album'), + _wt('S2', 'Artist', None, 'Same Album'), + ] + # First track has explicit id=None which is filtered; the fallback + # is ``_name_``. Build manually so the + # helper sees no id at all. + for t in tracks: + del t['spotify_data']['album']['id'] + res = group_wishlist_tracks_by_album(tracks) + assert len(res.album_groups) == 1 + assert res.album_groups[0].album_key == '_name_same album' + assert len(res.album_groups[0].tracks) == 2 + + +def test_nested_track_data_payloads_normalized(): + """The wishlist service sometimes nests spotify_data under + track_data (JSON-string in DB → re-parsed). Ensure the grouper + digs through the same shapes ``classify_wishlist_track`` does.""" + tracks = [{ + 'track_data': { + 'spotify_data': { + 'artists': [{'name': 'Artist'}], + 'album': {'id': 'a', 'name': 'Album'}, + }, + }, + }] + res = group_wishlist_tracks_by_album(tracks) + assert len(res.album_groups) == 1 + assert res.album_groups[0].album_key == 'a' diff --git a/tests/wishlist/test_automation.py b/tests/wishlist/test_automation.py index fa5fd11d..af9255da 100644 --- a/tests/wishlist/test_automation.py +++ b/tests/wishlist/test_automation.py @@ -233,7 +233,104 @@ def test_process_wishlist_automatically_creates_batch_for_matching_tracks(): assert batch["analysis_total"] == 1 assert any(kwargs.get("progress") == 50 for _args, kwargs in progress_calls) assert guard_events == ["enter", "exit"] - assert any("Starting automatic wishlist batch" in msg for msg in logger.info_messages) + # Track has no album id/name → falls to residual batch path + assert any("Starting wishlist residual batch" in msg for msg in logger.info_messages) + + +def test_wishlist_albums_cycle_splits_into_per_album_batches(): + """Multi-album wishlist run: each album emits its own sub-batch + with ``is_album_download=True`` + populated album/artist context. + Pinned so the album-bundle dispatch gate (which keys on those + fields) engages per album instead of falling through to per-track + on a single mixed batch.""" + batch_map = {} + runtime, _service, _profiles_db, music_db, executor, _logger, _progress, _guards = _build_runtime( + tracks=[ + { + "name": "Song A1", + "artists": [{"name": "Artist 1"}], + "spotify_data": { + "album": {"id": "alb1", "name": "Album One", "album_type": "album"}, + "artists": [{"name": "Artist 1"}], + }, + }, + { + "name": "Song A2", + "artists": [{"name": "Artist 1"}], + "spotify_data": { + "album": {"id": "alb1", "name": "Album One", "album_type": "album"}, + "artists": [{"name": "Artist 1"}], + }, + }, + { + "name": "Song B1", + "artists": [{"name": "Artist 2"}], + "spotify_data": { + "album": {"id": "alb2", "name": "Album Two", "album_type": "album"}, + "artists": [{"name": "Artist 2"}], + }, + }, + ], + cycle_value="albums", + count=3, + batch_map=batch_map, + ) + + process_wishlist_automatically(runtime, automation_id="auto-multi-album") + + # Two album groups → two sub-batches submitted (no residual). + assert len(executor.submissions) == 2 + assert len(batch_map) == 2 + + # Each sub-batch must carry album-bundle dispatch context. + for batch in batch_map.values(): + assert batch.get("is_album_download") is True + assert batch.get("album_context", {}).get("name") in {"Album One", "Album Two"} + assert batch.get("artist_context", {}).get("name") in {"Artist 1", "Artist 2"} + + submitted_track_lists = [submitted_args[2] for _fn, submitted_args, _kw in executor.submissions] + track_counts = sorted(len(tracks) for tracks in submitted_track_lists) + assert track_counts == [1, 2] + + +def test_wishlist_albums_cycle_residual_for_orphan_tracks(): + """Tracks without resolvable album metadata fall to the classic + per-track residual batch (no ``is_album_download`` flag), while + sibling tracks with valid album info still get their own + album-bundle sub-batch.""" + batch_map = {} + runtime, _service, _profiles_db, music_db, executor, _logger, _progress, _guards = _build_runtime( + tracks=[ + { + "name": "Real Album Track", + "artists": [{"name": "Artist 1"}], + "spotify_data": { + "album": {"id": "alb1", "name": "Album One", "album_type": "album"}, + "artists": [{"name": "Artist 1"}], + }, + }, + { + # No album id, no album name — orphan + "name": "Orphan", + "artists": [{"name": "X"}], + "spotify_data": {"album": {"album_type": "album"}, "artists": [{"name": "X"}]}, + }, + ], + cycle_value="albums", + count=2, + batch_map=batch_map, + ) + + process_wishlist_automatically(runtime, automation_id="auto-mixed") + + assert len(executor.submissions) == 2 # 1 album batch + 1 residual + + album_batches = [b for b in batch_map.values() if b.get("is_album_download")] + residual_batches = [b for b in batch_map.values() if not b.get("is_album_download")] + assert len(album_batches) == 1 + assert len(residual_batches) == 1 + assert album_batches[0]["album_context"]["name"] == "Album One" + assert residual_batches[0]["analysis_total"] == 1 def test_process_wishlist_automatically_returns_early_when_already_processing(): diff --git a/webui/static/helper.js b/webui/static/helper.js index ce7a9108..d173c2ba 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -3422,6 +3422,7 @@ const WHATS_NEW = { { title: 'Last.fm Radio Sync tab', desc: 'sibling to the ListenBrainz tab — lists your generated Last.fm Radio playlists alongside the rest of the Sync sources. same discovery → mirror flow under the hood, just a different entry point. new Last.fm radios are still generated from the Discover page by picking a seed track; this tab is for syncing existing ones. mirrors auto-trim when Last.fm Radio cache rotates so old radios don\'t pile up.', page: 'sync' }, { title: 'SoulSync Discovery Sync tab', desc: 'last of the unified-tab trio. surfaces your personalized SoulSync Discovery playlists (decade mixes, hidden gems, popular picks, daily mixes, discovery shuffle, etc.) on the Sync page. clicking a card regenerates the playlist + mirrors it under a stable synthetic id, so the same mirror updates in place every Auto-Sync refresh. tracks come out already matched against Spotify / iTunes / Deezer so there\'s no discovery hop — straight to download / sync.', page: 'sync' }, { title: 'Fix: album-bundle downloads all landing as track 1', desc: 'soulseek album-bundle downloads (and any other untagged release-staging path) were importing every track with track_number=1. the staging-file reader was using the auto-import\'s filename extractor that defaults to 1 when no NN- prefix is present in the filename — so for albums like Ryoto\'s "Cha-La Head-Cha-La" where slskd hands you bare titles, every file got "track 1" stamped on it. now the staging path uses a strict extractor that returns 0 when it can\'t see an explicit prefix, so the downstream resolver correctly falls through to the authoritative Spotify metadata and the right track numbers land in the library.' }, + { title: 'Wishlist albums-cycle: one album-bundle search per album', desc: 'auto-wishlist runs in two cycles — albums + singles. previously the albums cycle dumped every missing track from every album into one big batch and ran a separate per-track Soulseek / Prowlarr search for each (~50 searches for a typical scan). now the albums cycle splits the wishlist into per-album sub-batches at submission time, and each one engages the existing slskd / torrent / usenet album-bundle release-first flow with that album\'s context. tracks without resolvable album metadata stay on the classic per-track residual batch so nothing falls off. singles cycle is unchanged.' }, ], '2.6.2': [ { date: 'May 24, 2026 — 2.6.2 release' },