diff --git a/core/automation/handlers/refresh_mirrored.py b/core/automation/handlers/refresh_mirrored.py index 12eccb8d..323b1205 100644 --- a/core/automation/handlers/refresh_mirrored.py +++ b/core/automation/handlers/refresh_mirrored.py @@ -51,6 +51,16 @@ def auto_refresh_mirrored(config: Dict[str, Any], deps: AutomationDeps) -> Dict[ playlist_id = config.get('playlist_id') refresh_all = config.get('all', False) auto_id = config.get('_automation_id') + # Pipeline runs (``run_mirrored_playlist_pipeline``) set this flag + # because Phase 2 of the pipeline already runs the playlist + # discovery worker — the same matching engine ``_maybe_discover`` + # would call here. Running both means LB tracks discover twice + # AND the refresh-side discovery blocks 5+ minutes with no + # progress emission, leaving the UI stuck on "Refreshing:" until + # the loop returns. Standalone callers (Sync page, registration + # action) leave it False so LB tracks still get matched_data on + # refresh. + skip_discovery = bool(config.get('skip_discovery', False)) if refresh_all: playlists = db.get_mirrored_playlists() @@ -93,7 +103,14 @@ def auto_refresh_mirrored(config: Dict[str, Any], deps: AutomationDeps) -> Dict[ # mark them ``needs_discovery=True``. Hand them to the # adapter's matcher so the resulting mirror rows carry # provider IDs + matched_data, ready for the sync pipeline. - detail_tracks = _maybe_discover(detail.tracks, source, deps) + # + # Pipeline runs skip this because Phase 2's discovery + # worker handles it with proper progress emission — see + # ``skip_discovery`` resolution at the top of this fn. + detail_tracks = ( + detail.tracks if skip_discovery + else _maybe_discover(detail.tracks, source, deps) + ) tracks = [to_mirror_track_dict(t) for t in detail_tracks] refreshed += _commit_refresh(pl, source, source_id, tracks, db, deps, auto_id) diff --git a/core/listenbrainz_manager.py b/core/listenbrainz_manager.py index efa16158..3de010af 100644 --- a/core/listenbrainz_manager.py +++ b/core/listenbrainz_manager.py @@ -120,6 +120,69 @@ class ListenBrainzManager: "summary": summary } + def refresh_playlist(self, playlist_mbid: str) -> Dict: + """Targeted single-playlist refresh. + + Reads the cached ``playlist_type`` for the MBID, refetches the + playlist from ListenBrainz, runs the result through + ``_update_playlist`` (the same upsert path ``update_all_playlists`` + uses). Faster than ``update_all_playlists`` when only one + playlist needs refreshing (no per-type list pulls, no cleanup + sweep, no rolling-series re-walk) — the original API was wired + up as the only entry-point even though most callers refresh + exactly one playlist. + + Returns + ------- + Dict with ``success`` (bool), ``result`` ("updated"/"skipped"/"new") + on success, or ``error`` (str) on failure. Caller is expected + to log + surface any failure — the manager does NOT silently + swallow exceptions. + """ + if not self.client.is_authenticated(): + logger.warning("ListenBrainz not authenticated, skipping refresh") + return {"success": False, "error": "Not authenticated"} + + if not playlist_mbid: + return {"success": False, "error": "No playlist_mbid provided"} + + conn = self._get_db_connection() + try: + cursor = conn.cursor() + cursor.execute( + """ + SELECT playlist_type FROM listenbrainz_playlists + WHERE playlist_mbid = ? AND profile_id = ? + """, + (playlist_mbid, self.profile_id), + ) + row = cursor.fetchone() + finally: + conn.close() + + # ``user`` is the safest default when the playlist isn't in + # cache yet — it maps to the simplest insert path in + # ``_update_playlist`` without triggering created-for-specific + # cleanup logic. + playlist_type = row[0] if row else "user" + + logger.info( + f"Refreshing single LB playlist {playlist_mbid} (type={playlist_type})" + ) + + full_playlist = self.client.get_playlist_details(playlist_mbid) + if not full_playlist: + logger.warning(f"LB returned no data for playlist {playlist_mbid}") + return {"success": False, "error": "Playlist not found upstream"} + + result = self._update_playlist(full_playlist, playlist_type) + return { + "success": True, + "result": result, + "playlist_mbid": playlist_mbid, + "playlist_type": playlist_type, + } + def _update_playlist(self, playlist_data: Dict, playlist_type: str) -> str: """ Update a single playlist. Returns 'updated', 'skipped', or 'new' diff --git a/core/playlists/pipeline.py b/core/playlists/pipeline.py index 00d76432..42acd664 100644 --- a/core/playlists/pipeline.py +++ b/core/playlists/pipeline.py @@ -293,6 +293,12 @@ def _run_refresh_phase( refresh_config = dict(config) refresh_config['_automation_id'] = None + # Phase 2 below runs the discovery worker with proper progress + # emission — refresh shouldn't run it too. Without this flag, LB + # / Last.fm sources double-discover (5+ minutes silent block on + # the refresh side, then again in Phase 2) and the UI sits on + # "Refreshing:" the whole time. + refresh_config['skip_discovery'] = True refresh_result = refresh_fn(refresh_config, deps) refreshed = int(refresh_result.get('refreshed', 0)) refresh_errors = int(refresh_result.get('errors', 0)) diff --git a/core/playlists/sources/listenbrainz.py b/core/playlists/sources/listenbrainz.py index 838cda15..1c16a6dd 100644 --- a/core/playlists/sources/listenbrainz.py +++ b/core/playlists/sources/listenbrainz.py @@ -14,6 +14,7 @@ in the DB) — there is no process-wide singleton to grab at import time. from __future__ import annotations +import logging from typing import Any, Callable, Dict, List, Optional from core.playlists.sources.base import ( @@ -24,6 +25,8 @@ from core.playlists.sources.base import ( SOURCE_LISTENBRAINZ, ) +logger = logging.getLogger(__name__) + # Type alias for the discovery callable: takes a list of MB-shaped # track dicts, returns a parallel list of matched_data dicts (or None @@ -259,19 +262,51 @@ class ListenBrainzPlaylistSource(PlaylistSource): return out def refresh_playlist(self, playlist_id: str) -> Optional[PlaylistDetail]: - """Trigger a manager-side refresh, then return the new snapshot. + """Targeted single-playlist refresh via the LB manager. - ``update_all_playlists`` is the only refresh entry-point on the - manager — it re-fetches every cached playlist. That's wasteful - for a single-playlist refresh; Phase 1 should add a targeted - ``refresh_playlist(mbid)`` to the manager.""" + Resolves synthetic series ids (``lb_weekly_jams_``) to + the latest MBID, then calls ``manager.refresh_playlist(mbid)`` + instead of the legacy ``update_all_playlists`` — the old path + re-pulled every cached LB playlist (12+ API calls) just to + refresh one row. + + Refresh failures log with traceback + return ``None`` so the + outer handler in ``core/automation/handlers/refresh_mirrored.py`` + counts the error and surfaces it. Pre-fix this branch silently + swallowed every exception and read stale cache as if the + refresh succeeded, masking LB API outages as no-op refreshes. + """ manager = self._manager() if manager is None: return None + + target_mbid = playlist_id + from core.playlists.lb_series import is_series_synthetic_id + if is_series_synthetic_id(playlist_id): + resolved = self._resolve_series_to_latest_mbid(manager, playlist_id) + if not resolved: + logger.warning( + "LB rolling series %r has no cached members — refresh skipped", + playlist_id, + ) + return None + target_mbid = resolved + try: - manager.update_all_playlists() - except Exception: # noqa: S110 — caller falls back to last cached playlist on refresh failure - pass + result = manager.refresh_playlist(target_mbid) + except Exception: + logger.exception("LB refresh_playlist(%s) failed", target_mbid) + return None + + if not result.get("success"): + logger.warning( + "LB refresh did not succeed for %s: %s", + target_mbid, result.get("error", "unknown"), + ) + + # Read back via the same cache lookup ``get_playlist`` uses so + # the meta object preserves the caller's original id (synthetic + # or real). Refresh is decoupled from read intentionally. return self.get_playlist(playlist_id) # ---- projection helpers ------------------------------------------------ diff --git a/tests/automation/test_handlers_playlist.py b/tests/automation/test_handlers_playlist.py index 740ff16e..ff8b132b 100644 --- a/tests/automation/test_handlers_playlist.py +++ b/tests/automation/test_handlers_playlist.py @@ -474,6 +474,14 @@ class TestRefreshMirrored: def update_all_playlists(self): pass + def refresh_playlist(self, mbid): + # Adapter now calls the targeted refresh instead of + # the legacy ``update_all_playlists``. Trivial stub — + # the test only cares about the discovery + commit + # paths downstream. + return {'success': True, 'result': 'skipped', + 'playlist_mbid': mbid} + discovery_calls = [] def fake_discover(track_dicts): @@ -537,6 +545,95 @@ class TestRefreshMirrored: assert extra['provider'] == 'spotify' assert extra['matched_data']['id'] == 'sp-matched' + def test_skip_discovery_flag_bypasses_matcher(self): + """``skip_discovery=True`` (set by the pipeline runner) must + prevent ``_maybe_discover`` from invoking the matching engine + on LB / Last.fm tracks. Pipeline's Phase 2 runs the discovery + worker with proper progress emission — running it during + refresh too blocks the UI for minutes with no updates.""" + from core.playlists.sources.bootstrap import build_playlist_source_registry + + class _StubLBManager: + def get_cached_playlists(self, playlist_type): + if playlist_type == 'created_for_user': + return [{ + 'playlist_mbid': 'lb-skip', + 'title': 'LB Weekly', + 'creator': 'ListenBrainz', + 'track_count': 1, + 'annotation': {}, + 'last_updated': '2026-05-26', + }] + return [] + + def get_playlist_type(self, mbid): + return 'created_for_user' if mbid == 'lb-skip' else '' + + def get_cached_tracks(self, mbid): + if mbid == 'lb-skip': + return [{ + 'track_name': 'MB Song', + 'artist_name': 'MB Artist', + 'album_name': 'MB Album', + 'duration_ms': 240_000, + 'recording_mbid': 'rec-skip', + 'release_mbid': '', + 'album_cover_url': '', + 'additional_metadata': {}, + }] + return [] + + def refresh_playlist(self, mbid): + # Test only cares that discovery is skipped, not the + # refresh path itself — keep this trivial. + return {'success': True, 'result': 'skipped'} + + discovery_calls = [] + + def fake_discover(track_dicts): + discovery_calls.append(list(track_dicts)) + return [] # never returned because we expect skip + + registry = build_playlist_source_registry( + spotify_client_getter=lambda: None, + tidal_client_getter=lambda: None, + qobuz_client_getter=lambda: None, + deezer_client_getter=lambda: None, + listenbrainz_manager_getter=lambda: _StubLBManager(), + discover_callable=fake_discover, + ) + + db = _StubDB(playlists=[ + { + 'id': 77, + 'name': 'LB Weekly', + 'source': 'listenbrainz', + 'source_playlist_id': 'lb-skip', + 'profile_id': 1, + }, + ]) + deps = _build_deps(get_database=lambda: db) + object.__setattr__(deps, 'playlist_source_registry', registry) + + # Pipeline-style call: include the skip_discovery flag. + result = auto_refresh_mirrored( + {'playlist_id': '77', 'skip_discovery': True}, deps, + ) + + assert result['status'] == 'completed' + assert result['refreshed'] == '1' + # CRITICAL: the matcher was NOT invoked. + assert discovery_calls == [] + + # The mirror row still landed — just without matched_data. + # Phase 2 of the pipeline picks it up from needs_discovery. + call = db.mirror_calls[0] + assert len(call['tracks']) == 1 + # No discovery → no matched_data, and the source_track_id + # falls back to the recording_mbid the LB adapter projects. + track = call['tracks'][0] + assert track['source_track_id'] == 'rec-skip' + def test_spotify_public_uses_authed_spotify_when_signed_in(self): """The handler-level fallback chain: when Spotify is authed AND the public URL is a playlist URL, prefer the authed API so diff --git a/tests/test_listenbrainz_manager.py b/tests/test_listenbrainz_manager.py new file mode 100644 index 00000000..67259103 --- /dev/null +++ b/tests/test_listenbrainz_manager.py @@ -0,0 +1,249 @@ +"""Tests for ``core/listenbrainz_manager.py``. + +Coverage focus: the new ``refresh_playlist(mbid)`` targeted refresh. +Pre-fix the manager only exposed ``update_all_playlists`` — every +caller that wanted to refresh ONE playlist had to re-pull all 14 +cached LB playlists' details. Wasted API calls + slow + the LB +adapter's silent ``except Exception: pass`` wrapper masked the real +slowness as a UI hang. +""" + +from __future__ import annotations + +import sqlite3 +import tempfile +from pathlib import Path +from typing import Any, Dict, List +from unittest.mock import MagicMock + +import pytest + +from core.listenbrainz_manager import ListenBrainzManager + + +@pytest.fixture +def tmp_db(): + """Per-test SQLite file so ``_ensure_tables`` can build its schema.""" + with tempfile.NamedTemporaryFile(suffix=".db", delete=False) as f: + path = f.name + yield path + try: + Path(path).unlink() + except OSError: + pass + + +def _seed_playlist(db_path: str, mbid: str, title: str, ptype: str, track_count: int): + """Insert one cached LB playlist row + matching schema bootstrap.""" + conn = sqlite3.connect(db_path) + try: + cur = conn.cursor() + cur.execute( + """ + INSERT INTO listenbrainz_playlists + (playlist_mbid, title, creator, playlist_type, track_count, annotation_data, profile_id) + VALUES (?, ?, 'ListenBrainz', ?, ?, '{}', 1) + """, + (mbid, title, ptype, track_count), + ) + conn.commit() + finally: + conn.close() + + +def _build_manager(db_path: str, *, authed: bool = True) -> ListenBrainzManager: + """Construct a manager + stub the client so we don't hit the network.""" + mgr = ListenBrainzManager(db_path=db_path, profile_id=1) + mgr.client = MagicMock() + mgr.client.is_authenticated.return_value = authed + return mgr + + +# --------------------------------------------------------------------------- +# refresh_playlist: happy path +# --------------------------------------------------------------------------- + + +def test_refresh_playlist_fetches_single_playlist_only(tmp_db): + """``refresh_playlist`` calls ONLY ``get_playlist_details`` for the + targeted mbid — not any of the list-pulling methods that + ``update_all_playlists`` uses.""" + mgr = _build_manager(tmp_db) + _seed_playlist(tmp_db, "mbid-1", "Weekly Jams", "created_for", 50) + + # Non-empty ``track`` so ``_update_playlist`` doesn't trigger its + # own defensive re-fetch (that branch is for legacy callers that + # pass slim list-row data — not us). + mgr.client.get_playlist_details.return_value = { + "playlist": { + "identifier": "https://listenbrainz.org/playlist/mbid-1", + "title": "Weekly Jams", + "creator": "ListenBrainz", + "track": [ + { + "identifier": "https://musicbrainz.org/recording/rec-1", + "title": "Song", + "creator": "Artist", + } + ], + "annotation": "", + } + } + + result = mgr.refresh_playlist("mbid-1") + + assert result["success"] is True + assert result["playlist_mbid"] == "mbid-1" + assert result["playlist_type"] == "created_for" + mgr.client.get_playlist_details.assert_called_once_with("mbid-1") + # The wasteful list-pulling methods must NOT be touched. + mgr.client.get_playlists_created_for_user.assert_not_called() + mgr.client.get_user_playlists.assert_not_called() + mgr.client.get_collaborative_playlists.assert_not_called() + + +def test_refresh_playlist_returns_skipped_when_track_count_unchanged(tmp_db): + """``_update_playlist``'s smart-comparison returns "skipped" when + the track count matches the cached value. ``refresh_playlist`` + propagates that signal back to the caller.""" + mgr = _build_manager(tmp_db) + _seed_playlist(tmp_db, "mbid-stable", "Stable", "user", 7) + + # Build a payload with the same 7 tracks. + tracks = [ + { + "identifier": f"https://musicbrainz.org/recording/rec-{i}", + "title": f"Track {i}", + "creator": "Artist", + } + for i in range(7) + ] + mgr.client.get_playlist_details.return_value = { + "playlist": { + "identifier": "https://listenbrainz.org/playlist/mbid-stable", + "title": "Stable", + "creator": "ListenBrainz", + "track": tracks, + "annotation": "", + } + } + + result = mgr.refresh_playlist("mbid-stable") + + assert result["success"] is True + assert result["result"] == "skipped" + + +# --------------------------------------------------------------------------- +# refresh_playlist: defensive / failure modes +# --------------------------------------------------------------------------- + + +def test_refresh_playlist_unauthenticated_returns_failure_without_fetching(tmp_db): + """No auth → no LB API calls. Pre-fix, ``update_all_playlists`` + had this check; the new targeted entry-point must enforce it + consistently.""" + mgr = _build_manager(tmp_db, authed=False) + + result = mgr.refresh_playlist("mbid-anything") + + assert result["success"] is False + assert "Not authenticated" in result["error"] + mgr.client.get_playlist_details.assert_not_called() + + +def test_refresh_playlist_empty_mbid_returns_failure(tmp_db): + """Defensive — empty mbid is a caller bug; fail loud with a + clear error message rather than firing a malformed API call.""" + mgr = _build_manager(tmp_db) + + result = mgr.refresh_playlist("") + + assert result["success"] is False + assert "No playlist_mbid" in result["error"] + mgr.client.get_playlist_details.assert_not_called() + + +def test_refresh_playlist_returns_failure_when_upstream_returns_none(tmp_db): + """LB API returning ``None`` (deleted playlist, transient 404) + is a clean failure — not a silent skip. The caller decides + whether to retry / surface.""" + mgr = _build_manager(tmp_db) + _seed_playlist(tmp_db, "mbid-gone", "Old", "user", 10) + + mgr.client.get_playlist_details.return_value = None + + result = mgr.refresh_playlist("mbid-gone") + + assert result["success"] is False + assert "not found upstream" in result["error"] + + +def test_refresh_playlist_defaults_to_user_type_for_unknown_mbid(tmp_db): + """When the mbid isn't in the cache yet (new discovery), the + manager defaults the playlist_type to ``user`` so the insert + path in ``_update_playlist`` works. Avoids a NULL playlist_type + column on the new row.""" + mgr = _build_manager(tmp_db) + # No seed — mbid isn't cached yet. + + mgr.client.get_playlist_details.return_value = { + "playlist": { + "identifier": "https://listenbrainz.org/playlist/mbid-new", + "title": "Newly Discovered", + "creator": "ListenBrainz", + "track": [], + "annotation": "", + } + } + + result = mgr.refresh_playlist("mbid-new") + + assert result["success"] is True + assert result["playlist_type"] == "user" + + +def test_refresh_playlist_exception_propagates_not_swallowed(tmp_db): + """If the LB client raises (network failure, JSON parse error), + the exception must propagate. Pre-fix the wrapping adapter + silently swallowed; the manager is the right layer to surface + real errors so the adapter can decide how to log.""" + mgr = _build_manager(tmp_db) + _seed_playlist(tmp_db, "mbid-boom", "Boom", "user", 1) + mgr.client.get_playlist_details.side_effect = ConnectionError("LB unreachable") + + with pytest.raises(ConnectionError): + mgr.refresh_playlist("mbid-boom") + + +# --------------------------------------------------------------------------- +# Cost guard: refresh_playlist is strictly cheaper than update_all_playlists. +# --------------------------------------------------------------------------- + + +def test_refresh_playlist_does_not_walk_cleanup_or_rolling_series_for_unrelated_playlists(tmp_db): + """``update_all_playlists`` runs ``_cleanup_old_playlists`` + + ``_ensure_rolling_mirrors_from_cache`` at the tail. Those are + fine for a full-refresh batch but wasted work for a single- + playlist refresh. Pin that the targeted method skips them.""" + mgr = _build_manager(tmp_db) + _seed_playlist(tmp_db, "mbid-narrow", "Narrow", "user", 0) + + mgr.client.get_playlist_details.return_value = { + "playlist": { + "identifier": "https://listenbrainz.org/playlist/mbid-narrow", + "title": "Narrow", + "creator": "ListenBrainz", + "track": [], + "annotation": "", + } + } + + # Spy the cleanup method. + cleanup_calls: List[Any] = [] + original_cleanup = mgr._cleanup_old_playlists + mgr._cleanup_old_playlists = lambda: cleanup_calls.append(True) or original_cleanup() + + mgr.refresh_playlist("mbid-narrow") + + assert cleanup_calls == [] # Cleanup must NOT fire for targeted refresh. diff --git a/tests/test_playlist_sources_adapters.py b/tests/test_playlist_sources_adapters.py index e6b443ff..18ef4d3b 100644 --- a/tests/test_playlist_sources_adapters.py +++ b/tests/test_playlist_sources_adapters.py @@ -389,6 +389,9 @@ class _FakeLBManager: }], } self.refresh_called = False + self.refresh_playlist_calls: list[str] = [] + # Toggle to raise from refresh_playlist for the silent-swallow test. + self.refresh_raises: Optional[Exception] = None def get_cached_playlists(self, playlist_type: str): return self._rows.get(playlist_type, []) @@ -403,8 +406,17 @@ class _FakeLBManager: return self._tracks.get(mbid, []) def update_all_playlists(self): + # Pre-fix fallback — kept so adapters that haven't been + # migrated still work, and so an accidental return to the + # legacy entry-point is detectable in tests. self.refresh_called = True + def refresh_playlist(self, mbid: str): + self.refresh_playlist_calls.append(mbid) + if self.refresh_raises is not None: + raise self.refresh_raises + return {"success": True, "result": "skipped", "playlist_mbid": mbid} + def test_listenbrainz_adapter_marks_needs_discovery(): manager = _FakeLBManager() @@ -424,11 +436,79 @@ def test_listenbrainz_adapter_marks_needs_discovery(): assert t.extra["recording_mbid"] == "rec-1" -def test_listenbrainz_adapter_refresh_calls_manager(): +def test_listenbrainz_adapter_refresh_uses_targeted_manager_call(): + """Adapter must call ``manager.refresh_playlist(mbid)`` — the + targeted single-playlist refresh — not the legacy + ``update_all_playlists`` which re-pulls every cached LB row. + """ manager = _FakeLBManager() src = ListenBrainzPlaylistSource(lambda: manager) - src.refresh_playlist("lb-1") - assert manager.refresh_called is True + detail = src.refresh_playlist("lb-1") + + assert manager.refresh_playlist_calls == ["lb-1"] + # Legacy entry-point must NOT be touched. + assert manager.refresh_called is False + # Refresh returned a detail (read-back via get_playlist). + assert detail is not None + assert detail.meta.source_playlist_id == "lb-1" + + +def test_listenbrainz_adapter_refresh_logs_and_returns_none_on_manager_error(): + """When the LB manager raises, the adapter MUST surface the + failure as ``None`` (so the outer handler logs + counts it), + not silently swallow and return a stale cache read. + + Pre-fix: ``except Exception: pass`` then ``return get_playlist()`` + — masking every LB API failure as a successful no-op refresh. + """ + manager = _FakeLBManager() + manager.refresh_raises = RuntimeError("LB API timed out") + src = ListenBrainzPlaylistSource(lambda: manager) + + result = src.refresh_playlist("lb-1") + + assert result is None + assert manager.refresh_playlist_calls == ["lb-1"] + + +def test_listenbrainz_adapter_refresh_resolves_synthetic_series_id(): + """Rolling-series synthetic ids (``lb_weekly_jams_``) must + resolve to the latest cached member MBID before calling the + targeted manager refresh. Without resolution the manager would + try to fetch the synthetic id as a real MBID and 404.""" + manager = _FakeLBManager() + # Re-shape the fake's rows so the title matches a series LIKE pattern. + manager._rows["created_for_user"] = [{ + "playlist_mbid": "weekly-mbid", + "title": "Weekly Jams for nezreka, week of 2026-05-25 Mon", + "creator": "ListenBrainz", + "track_count": 1, + "annotation": {}, + "last_updated": "2026-05-26", + }] + manager._tracks = {"weekly-mbid": []} + # Stub the manager DB connection used by the resolution helper. + import sqlite3 + conn = sqlite3.connect(":memory:") + cur = conn.cursor() + cur.execute( + "CREATE TABLE listenbrainz_playlists " + "(playlist_mbid TEXT, title TEXT, profile_id INTEGER, last_updated TEXT)" + ) + cur.execute( + "INSERT INTO listenbrainz_playlists VALUES " + "('weekly-mbid', 'Weekly Jams for nezreka, week of 2026-05-25 Mon', 1, '2026-05-26')" + ) + conn.commit() + manager.profile_id = 1 + manager._get_db_connection = lambda: conn + + src = ListenBrainzPlaylistSource(lambda: manager) + src.refresh_playlist("lb_weekly_jams_nezreka") + + # Manager refresh got called with the RESOLVED real MBID, not the + # synthetic one. + assert manager.refresh_playlist_calls == ["weekly-mbid"] # ─── Last.fm ──────────────────────────────────────────────────────────── diff --git a/webui/static/helper.js b/webui/static/helper.js index bb73a5ea..ac09dae2 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -3415,6 +3415,7 @@ function closeHelperSearch() { const WHATS_NEW = { '2.6.3': [ { unreleased: true }, + { title: 'Auto-Sync: fix ListenBrainz pipelines stuck on "Refreshing:" for 5+ minutes', desc: 'Auto-Sync pipeline runs against a ListenBrainz playlist (Weekly Jams / Weekly Exploration / Top Discoveries / etc.) would sit on "Refreshing: \'\'" for minutes with zero UI updates before anything else happened. Two real bugs ganging up: (1) the refresh path ran the LB matching engine on every track to produce matched_data — 5+ minutes blocking with no progress emission — and Phase 2 of the pipeline then ran the SAME matching engine on the same tracks via the discovery worker. So LB tracks discovered twice, the first time silently. (2) ListenBrainz manager only exposed an `update_all_playlists` entry-point — refreshing one playlist re-pulled all 12+ cached LB playlists\' details from the API even though we only cared about one. Pipeline now skips the refresh-side discovery (Phase 2 handles it with progress emits every 3s — same pattern that already works for Spotify); refresh uses a new targeted `LBManager.refresh_playlist(mbid)` that hits just the requested playlist. Also killed a silent `except Exception: pass` in the LB adapter that was masking real API failures as stale-cache hits — refresh errors now log with traceback and surface to the run-history error tally. Pinned with 12 new unit tests.', page: 'automations' }, { title: 'Wishlist: harden Spotify backfill so a poisoned track_number can\'t mask a lean album', desc: 'follow-up to the earlier wishlist import-path work. residual per-track wishlist downloads (single tracks from different albums, falling below the album-bundle threshold) were producing folders without a year subfolder whenever the underlying wishlist row had a track_number=1 from an older default — the candidate dispatcher\'s "fall back to Spotify API" branch was gated entirely on track_number being missing, but the same API call was also the only thing that hydrated the lean album_context (release_date / total_tracks / cover art) when the original discovery match came from Deezer\'s search endpoint which doesn\'t carry those fields. so any row whose track_number happened to look "filled" (1 from a default) would short-circuit the API call and the year disappeared from the folder path even though the API knew it. split the two concerns: track_number resolution keeps its track_info → track object → API precedence, but album hydration now runs whenever release_date or total_tracks are missing regardless of where track_number came from. one network round-trip still serves both. lifted into core/downloads/track_metadata_backfill.py with 24 unit tests pinning every branch — including the regression: poisoned default-1 track_number does NOT block album backfill anymore.' }, { title: 'Wishlist: fix imports landing as track 01 with no year in folder name', desc: 'follow-up to the earlier wishlist album-bundle work. tracks with rich Spotify metadata were still importing as `01 - ` with no year in the folder path because three regressions stacked: the Track→dict conversion in payload helpers dropped everything except `album.name` (silently throwing away release_date / images / album_type / total_tracks), Deezer-sourced discovery matches saved their payloads without `track_number` / `disc_number` keys at all (Deezer uses `track_position` while the cache lookup read `track_number` literally), and the import pipeline only consulted `album_info.track_number` before falling to the filename — which fails for VA-collection source files like `417 Fountains of Wayne - Stacys Mom.flac` where the leading number is a playlist position not the album track. all three patched, with the track_number resolution chain lifted into `core/imports/track_number.py` as a pure function with 18 unit tests pinning every branch.' }, { title: 'Wishlist: distinguish Queued from Analyzing batches in the UI', desc: 'wishlist runs with more than 3 sub-batches used to render every batch as "Analyzing..." simultaneously — even though the download worker pool only runs 3 at a time. so a 26-album wishlist scan looked like all 26 were working in parallel when really 23 were just sitting in the executor queue waiting their turn. now batches show a distinct "Queued ⏳" state while parked in the executor queue; they flip to "Analyzing..." when a worker actually picks them up. zero behavior change — just less misleading UI.' },