diff --git a/core/tidal_client.py b/core/tidal_client.py index a983944c..a18ab04f 100644 --- a/core/tidal_client.py +++ b/core/tidal_client.py @@ -1324,13 +1324,20 @@ class TidalClient: track_ids.append(item.get("id")) if track_ids: - # Batch fetch full track details with artists and albums - try: - batch_tracks = self._get_tracks_batch(track_ids) - except Exception as e: - logger.error(f"Error fetching track details for page {page_num}: {e}") - # Continue pagination — we lose this batch but can still get remaining - batch_tracks = [] + # Batch fetch full track details with artists and albums. + # Chunk to the filter[id] page cap — Tidal returns at most + # _COLLECTION_BATCH_SIZE tracks per request, so a relationships + # page larger than the cap would be silently truncated if sent + # in one shot. Mirrors get_album_tracks. (#867) + batch_tracks = [] + for j in range(0, len(track_ids), self._COLLECTION_BATCH_SIZE): + chunk_ids = track_ids[j:j + self._COLLECTION_BATCH_SIZE] + try: + batch_tracks.extend(self._get_tracks_batch(chunk_ids)) + except Exception as e: + logger.error(f"Error fetching track details for page {page_num}: {e}") + # Lose this chunk but keep going — remaining chunks/pages can still load + continue if len(batch_tracks) < len(track_ids): logger.warning(f"Page {page_num}: requested {len(track_ids)} tracks but only {len(batch_tracks)} returned (some may be unavailable in your region)") diff --git a/tests/test_tidal_playlist_batch_chunking.py b/tests/test_tidal_playlist_batch_chunking.py new file mode 100644 index 00000000..3f097678 --- /dev/null +++ b/tests/test_tidal_playlist_batch_chunking.py @@ -0,0 +1,107 @@ +"""#867 regression: Tidal playlist track hydration must chunk to the API cap. + +Tidal's ``/tracks?filter[id]=...`` endpoint returns at most ``_COLLECTION_BATCH_SIZE`` +(20) tracks per request. ``get_playlist`` fetches a page of track-ID links and then +hydrates them via ``_get_tracks_batch``. If it sends a relationships page with more +than 20 IDs in a single hydration call, the surplus is silently dropped — a 59-track +playlist would surface as ~20. ``get_album_tracks`` already chunks; this pins the same +behavior for ``get_playlist`` so discovery sees every track. +""" + +from __future__ import annotations + +from core.tidal_client import TidalClient, Track + + +class _FakeResp: + status_code = 200 + + def raise_for_status(self): + return None + + def json(self): + return { + "data": { + "id": "PL1", + "attributes": {"name": "My Playlist", "accessType": "PRIVATE"}, + "relationships": {}, + } + } + + +class _FakeSession: + def get(self, *args, **kwargs): + return _FakeResp() + + +def _make_client(): + """Build a TidalClient without running its network/config __init__.""" + client = object.__new__(TidalClient) + client.base_url = "https://api.tidal.test" + client.session = _FakeSession() + return client + + +def test_get_playlist_chunks_oversized_relationships_page(monkeypatch): + client = _make_client() + cap = TidalClient._COLLECTION_BATCH_SIZE # 20 + n = 59 + ids = [str(i) for i in range(n)] + + monkeypatch.setattr(client, "_ensure_valid_token", lambda: True) + # One relationships page returns ALL 59 ID links at once, then no cursor. + monkeypatch.setattr( + client, + "_get_playlist_tracks_page", + lambda playlist_id, cursor=None: { + "data": [{"type": "tracks", "id": i} for i in ids], + "links": {"meta": {}}, # no nextCursor -> single page + }, + ) + + seen_chunk_sizes = [] + + def fake_batch(chunk_ids): + # Simulate the real filter[id] cap: never return more than the cap, so a + # single oversized call would lose the surplus (the bug being guarded). + seen_chunk_sizes.append(len(chunk_ids)) + capped = chunk_ids[:cap] + return [Track(id=i, name=f"t{i}", artists=["a"]) for i in capped] + + monkeypatch.setattr(client, "_get_tracks_batch", fake_batch) + + playlist = client.get_playlist("PL1") + + assert playlist is not None + # The whole point: all 59 hydrate, not just the first 20. + assert len(playlist.tracks) == n + # Every hydration call stayed within the API cap (so none truncated). + assert seen_chunk_sizes and max(seen_chunk_sizes) <= cap + + +def test_get_playlist_small_page_single_call(monkeypatch): + """A page at/under the cap still hydrates in one call (no behavior change).""" + client = _make_client() + ids = [str(i) for i in range(5)] + + monkeypatch.setattr(client, "_ensure_valid_token", lambda: True) + monkeypatch.setattr( + client, + "_get_playlist_tracks_page", + lambda playlist_id, cursor=None: { + "data": [{"type": "tracks", "id": i} for i in ids], + "links": {"meta": {}}, + }, + ) + + calls = [] + + def fake_batch(chunk_ids): + calls.append(list(chunk_ids)) + return [Track(id=i, name=f"t{i}", artists=["a"]) for i in chunk_ids] + + monkeypatch.setattr(client, "_get_tracks_batch", fake_batch) + + playlist = client.get_playlist("PL1") + assert len(playlist.tracks) == 5 + assert len(calls) == 1 # one chunk, one hydration call diff --git a/webui/static/sync-services.js b/webui/static/sync-services.js index 0005e649..24c93f56 100644 --- a/webui/static/sync-services.js +++ b/webui/static/sync-services.js @@ -10186,8 +10186,30 @@ function updateYouTubeDiscoveryModal(urlHash, status) { // Update table rows status.results.forEach(result => { - const row = document.getElementById(`discovery-row-${urlHash}-${result.index}`); - if (!row) return; + let row = document.getElementById(`discovery-row-${urlHash}-${result.index}`); + if (!row) { + // #867: the initial rows are pre-rendered from a separately-fetched + // track list (state.playlist.tracks) that can be SHORTER than the + // backend's authoritative discovery results — e.g. a Tidal playlist + // whose discovery fetched 59 tracks while the modal's own track fetch + // returned a rate-limited partial (~21). The old `if (!row) return` + // silently dropped every result past the pre-rendered rows. Create + // the missing row instead so the authoritative results drive the + // list and no discovered track disappears. The existing cell-fill + // logic below then populates it like any other row. + const trackName = result.yt_track || result.lb_track || result.track_name || '-'; + row = document.createElement('tr'); + row.id = `discovery-row-${urlHash}-${result.index}`; + row.innerHTML = + `${trackName}` + + '' + + '' + + '-' + + '-' + + '-' + + '-'; + tableBody.appendChild(row); + } const statusCell = row.querySelector('.discovery-status'); const spotifyTrackCell = row.querySelector('.spotify-track');