#867: Tidal playlist discovery shows all tracks (was capped to ~21)
Two issues in the same path: 1. The shared discovery modal pre-renders one row per track from a separately-fetched frontend track list, then the poll dropped any backend result without a pre-rendered row (if (!row) return). When the frontend's track fetch came back rate-limited/partial (~21) while discovery's own fetch got all 59, the surplus results vanished. Now the modal CREATES a row for any result lacking one, so authoritative backend results drive the list (fixes all sources sharing the modal). 2. get_playlist hydrated a whole relationships page in one _get_tracks_batch call, but Tidal caps filter[id] at 20/request, silently truncating larger pages. Chunk to the cap like get_album_tracks already does. Seam + regression tests (tests/test_tidal_playlist_batch_chunking.py).
This commit is contained in:
parent
c7ca657d56
commit
846a9c75a0
3 changed files with 145 additions and 9 deletions
|
|
@ -1324,13 +1324,20 @@ class TidalClient:
|
||||||
track_ids.append(item.get("id"))
|
track_ids.append(item.get("id"))
|
||||||
|
|
||||||
if track_ids:
|
if track_ids:
|
||||||
# Batch fetch full track details with artists and albums
|
# Batch fetch full track details with artists and albums.
|
||||||
try:
|
# Chunk to the filter[id] page cap — Tidal returns at most
|
||||||
batch_tracks = self._get_tracks_batch(track_ids)
|
# _COLLECTION_BATCH_SIZE tracks per request, so a relationships
|
||||||
except Exception as e:
|
# page larger than the cap would be silently truncated if sent
|
||||||
logger.error(f"Error fetching track details for page {page_num}: {e}")
|
# in one shot. Mirrors get_album_tracks. (#867)
|
||||||
# Continue pagination — we lose this batch but can still get remaining
|
batch_tracks = []
|
||||||
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):
|
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)")
|
logger.warning(f"Page {page_num}: requested {len(track_ids)} tracks but only {len(batch_tracks)} returned (some may be unavailable in your region)")
|
||||||
|
|
|
||||||
107
tests/test_tidal_playlist_batch_chunking.py
Normal file
107
tests/test_tidal_playlist_batch_chunking.py
Normal file
|
|
@ -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
|
||||||
|
|
@ -10186,8 +10186,30 @@ function updateYouTubeDiscoveryModal(urlHash, status) {
|
||||||
|
|
||||||
// Update table rows
|
// Update table rows
|
||||||
status.results.forEach(result => {
|
status.results.forEach(result => {
|
||||||
const row = document.getElementById(`discovery-row-${urlHash}-${result.index}`);
|
let row = document.getElementById(`discovery-row-${urlHash}-${result.index}`);
|
||||||
if (!row) return;
|
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 =
|
||||||
|
`<td class="yt-track">${trackName}</td>` +
|
||||||
|
'<td class="yt-artist"></td>' +
|
||||||
|
'<td class="discovery-status"></td>' +
|
||||||
|
'<td class="spotify-track">-</td>' +
|
||||||
|
'<td class="spotify-artist">-</td>' +
|
||||||
|
'<td class="spotify-album">-</td>' +
|
||||||
|
'<td class="discovery-actions">-</td>';
|
||||||
|
tableBody.appendChild(row);
|
||||||
|
}
|
||||||
|
|
||||||
const statusCell = row.querySelector('.discovery-status');
|
const statusCell = row.querySelector('.discovery-status');
|
||||||
const spotifyTrackCell = row.querySelector('.spotify-track');
|
const spotifyTrackCell = row.querySelector('.spotify-track');
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue