diff --git a/core/wishlist/processing.py b/core/wishlist/processing.py index a54ebe45..e774da4d 100644 --- a/core/wishlist/processing.py +++ b/core/wishlist/processing.py @@ -383,19 +383,12 @@ def _prepare_and_run_manual_wishlist_batch( if duplicates_removed > 0: logger.warning(f"[Manual-Wishlist] Removed {duplicates_removed} duplicate tracks") - logger.info("[Manual-Wishlist] Checking wishlist against library for already-owned tracks...") - cleanup_removed = remove_tracks_already_in_library( - wishlist_service, - SimpleNamespace(get_all_profiles=lambda: [{"id": manual_profile_id}]), - db, - runtime.active_server, - logger=logger, - skip_track_fn=lambda track: track.get('source_type') == 'enhance', - log_prefix="[Manual-Wishlist]", - ) - - if cleanup_removed > 0: - logger.info(f"[Manual-Wishlist] Cleaned up {cleanup_removed} already-owned tracks from wishlist") + # NOTE: We deliberately do NOT call remove_tracks_already_in_library here. + # Wishlist tracks are already known-missing (force_download_all=True is set on + # the batch). The library check duplicates the work the master worker would + # skip, and on large wishlists costs ~1s per track in serial DB lookups. + # The standalone /api/wishlist/cleanup endpoint still runs that pass when + # users explicitly ask for maintenance. raw_wishlist_tracks = wishlist_service.get_wishlist_tracks_for_download(profile_id=manual_profile_id) if not raw_wishlist_tracks: @@ -554,27 +547,19 @@ def process_wishlist_automatically(runtime: WishlistAutoProcessingRuntime, autom if duplicates_removed > 0: logger.warning(f"[Auto-Wishlist] Removed {duplicates_removed} duplicate tracks from profile {profile['id']}") - # CLEANUP: Remove tracks from wishlist that already exist in library - # This prevents wasting bandwidth on tracks we already have - logger.debug("[Auto-Wishlist] Checking wishlist against library for already-owned tracks...") - active_server = runtime.get_active_server() - cleanup_removed = remove_tracks_already_in_library( - wishlist_service, - database, - music_database, - active_server, - logger=logger, - ) + # NOTE: We deliberately do NOT call remove_tracks_already_in_library here. + # The batch sets force_download_all=True (see comment a few lines below), + # so wishlist tracks are treated as known-missing and the master worker + # skips per-track library lookups. Doing the same expensive scan here + # before submitting the batch defeats that optimization and adds + # ~1s per track in serial DB queries. The standalone + # /api/wishlist/cleanup endpoint still exposes that pass for users + # who want explicit maintenance. + runtime.update_automation_progress(automation_id, progress=25, phase='Preparing wishlist', + log_line='Skipped library scan — wishlist tracks treated as known-missing', + log_type='info') - if cleanup_removed > 0: - logger.info(f"[Auto-Wishlist] Cleaned up {cleanup_removed} already-owned tracks from wishlist") - runtime.update_automation_progress(automation_id, progress=25, phase='Cleaned up duplicates', - log_line=f'Removed {cleanup_removed} already-owned tracks', log_type='success') - else: - runtime.update_automation_progress(automation_id, progress=25, phase='Cleanup done', - log_line='No duplicates or already-owned tracks found', log_type='skip') - - # Get wishlist tracks for processing (after cleanup) - combine all profiles + # Get wishlist tracks for processing - combine all profiles raw_wishlist_tracks = [] for profile in all_profiles: raw_wishlist_tracks.extend(wishlist_service.get_wishlist_tracks_for_download(profile_id=profile['id'])) diff --git a/tests/wishlist/test_manual_download.py b/tests/wishlist/test_manual_download.py index 5a676f4e..b7a6b35d 100644 --- a/tests/wishlist/test_manual_download.py +++ b/tests/wishlist/test_manual_download.py @@ -184,8 +184,16 @@ def test_start_manual_wishlist_download_batch_filters_track_ids_and_starts_batch assert any("Filtered to 1 specific tracks by ID" in msg for msg in logger.info_messages) -def test_start_manual_wishlist_download_batch_skips_enhance_tracks_during_cleanup(): - runtime, service, _db, executor, logger, activity_calls, batch_map, master_calls = _build_runtime( +def test_start_manual_wishlist_download_batch_does_not_run_library_cleanup(): + """Manual flow does NOT scan the library for already-owned tracks. + + The batch sets force_download_all=True so owned tracks get downloaded + anyway. Running remove_tracks_already_in_library here would just add a + serial DB query per track (~30s on a 24-track wishlist) and contradict + force_download_all. The standalone /api/wishlist/cleanup endpoint + still exposes that pass for users who want explicit maintenance. + """ + runtime, service, db, executor, _logger, activity_calls, batch_map, master_calls = _build_runtime( tracks=[ { "id": "enhance-1", @@ -209,30 +217,25 @@ def test_start_manual_wishlist_download_batch_skips_enhance_tracks_during_cleanu assert status == 200 assert payload["success"] is True - # Run the bg job — cleanup happens here, not in the request handler. _run_submitted_bg_job(executor) - assert service.removed_ids == {"owned-1"} + # Owned-track removal pass does NOT run — wishlist still has the owned track. + assert service.removed_ids == set() + # The library check is skipped entirely — no per-track DB lookups. + assert db.track_checks == [] + + # All tracks are submitted to the master worker — including the "owned" one. assert len(master_calls) == 1 master_args, _ = master_calls[0] - assert [track["id"] for track in master_args[2]] == ["enhance-1"] - assert batch_map[payload["batch_id"]]["analysis_total"] == 1 - assert activity_calls == [("", "Wishlist Download Started", "1 tracks", "Now")] - assert any("Cleaned up 1 already-owned tracks" in msg for msg in logger.info_messages) + assert [track["id"] for track in master_args[2]] == ["enhance-1", "owned-1"] + assert batch_map[payload["batch_id"]]["analysis_total"] == 2 + assert activity_calls == [("", "Wishlist Download Started", "2 tracks", "Now")] -def test_bg_job_marks_batch_complete_when_wishlist_empty_after_cleanup(): - """If cleanup empties the wishlist, the bg job marks the batch complete (not error 400).""" +def test_bg_job_marks_batch_complete_when_wishlist_genuinely_empty(): + """If the wishlist is empty before the manual click, the bg job marks the batch complete.""" runtime, _service, _db, executor, _logger, _activity, batch_map, master_calls = _build_runtime( - tracks=[ - { - "id": "owned-1", - "name": "Owned Song", - "artists": [{"name": "Artist B"}], - "album": {"name": "Owned Album", "album_type": "album"}, - }, - ], - owned_matches={("Owned Song", "Artist B")}, + tracks=[], ) payload, status = processing.start_manual_wishlist_download_batch(runtime) @@ -240,7 +243,7 @@ def test_bg_job_marks_batch_complete_when_wishlist_empty_after_cleanup(): _run_submitted_bg_job(executor) - # Cleanup removed the only track. Master worker never called. Batch marked complete. + # No tracks → master worker never called, batch marked complete with explanatory error. assert master_calls == [] assert batch_map[payload["batch_id"]]["phase"] == "complete" assert batch_map[payload["batch_id"]]["error"] == "No tracks in wishlist"