From d0966ec26292d3493912f6371fae02cee9eea047 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Wed, 24 Jun 2026 16:41:40 -0700 Subject: [PATCH] Sync wishlist re-add: build the IDENTICAL payload the auto-add uses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause (the real one): the auto-add passes original_tracks_map[id] — tracks_json run through a specific normalization (album->dict with images/album_type/total_tracks/ release_date, artists->dicts). My re-add hand-rolled a different shape, so the stored spotify_data didn't match and the wishlist's nebula (which reads spotify_data.album. images[0].url) had no cover, plus album/single classification could differ. Fix: extract that normalization into one shared build_original_tracks_map() and use it in BOTH the live sync (core.discovery.sync) and the re-add. The re-add now resolves the track by source_track_id through the same map — byte-identical payload. Verified on a real sync row: re-add payload == live-sync payload, album.images present. (The shared normalizer is also copy-safe, fixing a latent tracks_json mutation in the old inline version.) Fallback (track absent from tracks_json) rebuilds through the same normalizer with the cover seeded from the row's image_url. 10 tests incl. a direct parity assertion. --- core/discovery/sync.py | 34 ++------ core/sync/wishlist_readd.py | 129 +++++++++++++++++------------- tests/test_sync_wishlist_readd.py | 108 +++++++++++++------------ 3 files changed, 132 insertions(+), 139 deletions(-) diff --git a/core/discovery/sync.py b/core/discovery/sync.py index cee281d6..ee6da03e 100644 --- a/core/discovery/sync.py +++ b/core/discovery/sync.py @@ -307,35 +307,11 @@ def run_sync_task( # This avoids needing to re-fetch it from Spotify logger.info("Converting JSON tracks to SpotifyTrack objects...") - # Store original track data with full album objects (for wishlist with cover art) - # Normalize formats for wishlist: album must be dict {'name': ...}, artists must be [{'name': ...}] - # Important: copy data — don't mutate tracks_json since SpotifyTrack expects List[str] artists - original_tracks_map = {} - for t in tracks_json: - track_id = t.get('id', '') - if track_id: - normalized = dict(t) - # Normalize album to dict format, preserving images and metadata - raw_album = normalized.get('album', '') - if isinstance(raw_album, str): - normalized['album'] = { - 'name': raw_album or normalized.get('name', 'Unknown Album'), - 'images': [], 'album_type': 'single', 'total_tracks': 1, 'release_date': '' - } - elif not isinstance(raw_album, dict): - normalized['album'] = { - 'name': str(raw_album) if raw_album else normalized.get('name', 'Unknown Album'), - 'images': [], 'album_type': 'single', 'total_tracks': 1, 'release_date': '' - } - else: - # Dict — ensure required keys exist - raw_album.setdefault('name', 'Unknown Album') - raw_album.setdefault('images', []) - # Normalize artists to list of dicts - raw_artists = normalized.get('artists', []) - if raw_artists and isinstance(raw_artists[0], str): - normalized['artists'] = [{'name': a} for a in raw_artists] - original_tracks_map[track_id] = normalized + # Store original track data with full album objects (for wishlist with cover art). + # Shared with the sync-detail "re-add to wishlist" action so both build the + # IDENTICAL payload (album→dict + images, artists→dicts). Copy-safe. + from core.sync.wishlist_readd import build_original_tracks_map + original_tracks_map = build_original_tracks_map(tracks_json) tracks = [] for i, t in enumerate(tracks_json): diff --git a/core/sync/wishlist_readd.py b/core/sync/wishlist_readd.py index 80cf0b81..6658352e 100644 --- a/core/sync/wishlist_readd.py +++ b/core/sync/wishlist_readd.py @@ -1,7 +1,9 @@ -"""Rebuild the track data the sync used to auto-wishlist an unmatched track, so the -sync-detail modal can re-add it with the EXACT same context (source_type='playlist' -+ the playlist's name/id). Pure — no I/O; the web route supplies the parsed sync -entry and calls the wishlist service. +"""Build the wishlist-add payload for a synced track — the SINGLE source of truth +shared by the live sync (core.discovery.sync) and the sync-detail "re-add to +wishlist" action, so a re-add is byte-for-byte the same payload the auto-add used. + +Pure — no I/O. The web route supplies the parsed sync entry and calls the wishlist +service; the live sync calls build_original_tracks_map directly. """ from __future__ import annotations @@ -9,75 +11,88 @@ from __future__ import annotations from typing import Any, Dict, List, Optional +def normalize_wishlist_track(track: Dict[str, Any]) -> Dict[str, Any]: + """Normalize ONE tracks_json track into the wishlist-add shape: album coerced to + a dict (preserving images + album_type/total_tracks/release_date), artists to a + list of dicts. Copy-safe — never mutates the input.""" + normalized = dict(track) + raw_album = normalized.get('album', '') + if isinstance(raw_album, dict): + album = dict(raw_album) + album.setdefault('name', 'Unknown Album') + album.setdefault('images', []) + normalized['album'] = album + else: + name = raw_album if isinstance(raw_album, str) else (str(raw_album) if raw_album else '') + normalized['album'] = { + 'name': name or normalized.get('name', 'Unknown Album'), + 'images': [], 'album_type': 'single', 'total_tracks': 1, 'release_date': '', + } + raw_artists = normalized.get('artists', []) + if raw_artists and isinstance(raw_artists[0], str): + normalized['artists'] = [{'name': a} for a in raw_artists] + return normalized + + +def build_original_tracks_map(tracks_json: Optional[List[Dict[str, Any]]]) -> Dict[str, Dict[str, Any]]: + """``{track_id: normalized_track}`` for a sync's tracks_json — the full-fidelity + map the live sync builds before auto-wishlisting unmatched tracks. One source of + truth so the re-add matches the auto-add exactly.""" + out: Dict[str, Dict[str, Any]] = {} + for t in tracks_json or []: + if not isinstance(t, dict): + continue + track_id = t.get('id', '') + if track_id: + out[str(track_id)] = normalize_wishlist_track(t) + return out + + def reconstruct_sync_track_data( track_results: Optional[List[Dict[str, Any]]], tracks: Optional[List[Dict[str, Any]]], track_index: int, ) -> Optional[Dict[str, Any]]: - """Return the ``spotify_track_data`` dict to re-add a synced track to the wishlist. + """Return the wishlist-add ``spotify_track_data`` for re-adding a synced track. - Prefers the FULL original track from the cached playlist tracks (``tracks``, i.e. - tracks_json) — matched by the track_result's ``source_track_id``, then by index — - because that carries the full album object + images the original auto-add used. - Falls back to a minimal dict rebuilt from the track_result's own fields. + Resolves the track the SAME way the auto-add did: the normalized tracks_json + entry (``build_original_tracks_map``), looked up by the track_result's + ``source_track_id`` — so the payload (full album object + images + album_type + + total_tracks + artists-as-dicts) is identical to the original auto-add. - Returns None when the index is out of range, the row isn't a 'wishlist' - (unmatched, auto-added) track, or there's no id to key on — so a caller can't - re-wishlist a matched/downloaded track or an unidentifiable one. + Only 'wishlist' rows are eligible. Falls back to a normalized rebuild from the + track_result's own fields (with the album cover from its image_url) when the + cached track is missing. None when ineligible or unidentifiable. """ if not track_results or track_index < 0 or track_index >= len(track_results): return None tr = track_results[track_index] or {} - # Only rows the sync actually sent to the wishlist are re-addable. if tr.get('download_status') != 'wishlist': return None sid = str(tr.get('source_track_id') or '') - tracks = tracks or [] - # Base: the FULL original track (best fidelity — full album object, ids, source, - # artists) by index if its id matches, else by id search. Copied so we never - # mutate the caller's tracks list. - base: Optional[Dict[str, Any]] = None - if 0 <= track_index < len(tracks): - cand = tracks[track_index] - if isinstance(cand, dict) and (not sid or str(cand.get('id') or '') == sid): - base = dict(cand) - if base is None and sid: - match = next( - (t for t in tracks if isinstance(t, dict) and str(t.get('id') or '') == sid), - None, - ) - if match is not None: - base = dict(match) + # Primary: the exact normalized track the auto-add used. + if sid: + payload = build_original_tracks_map(tracks).get(sid) + if payload: + return payload - # No full track in the cache — rebuild a minimal dict from the track_result. - if not (isinstance(base, dict) and base.get('id')): - if not sid: - return None - base = { - 'id': sid, - 'name': tr.get('name') or '', - 'artists': [{'name': tr.get('artist') or ''}], - 'album': {'name': tr.get('album') or ''}, - 'duration_ms': tr.get('duration_ms') or 0, - } - - # Ensure the cover carries through. The wishlist display reads - # spotify_data.album.images; tracks_json is sometimes stored WITHOUT images, so - # backfill from the track_result's image_url (the album art the sync extracted) - # whenever the album has none. This is what makes the re-add reach full parity - # with the original auto-add's appearance. - album = base.get('album') - if not isinstance(album, dict): - album = {'name': base.get('name', '')} - else: - album = dict(album) - img = tr.get('image_url') - if img and not album.get('images'): - album['images'] = [{'url': img}] - base['album'] = album - return base + # Fallback: rebuild from the track_result fields, through the SAME normalizer so + # the shape matches, and seed the album cover from the row's image_url. + if not sid: + return None + album: Dict[str, Any] = {'name': tr.get('album') or '', 'album_type': 'single', + 'total_tracks': 1, 'release_date': ''} + if tr.get('image_url'): + album['images'] = [{'url': tr['image_url']}] + return normalize_wishlist_track({ + 'id': sid, + 'name': tr.get('name') or '', + 'artists': [{'name': tr.get('artist') or ''}], + 'album': album, + 'duration_ms': tr.get('duration_ms') or 0, + }) -__all__ = ["reconstruct_sync_track_data"] +__all__ = ["normalize_wishlist_track", "build_original_tracks_map", "reconstruct_sync_track_data"] diff --git a/tests/test_sync_wishlist_readd.py b/tests/test_sync_wishlist_readd.py index 130a866a..b794ed10 100644 --- a/tests/test_sync_wishlist_readd.py +++ b/tests/test_sync_wishlist_readd.py @@ -1,13 +1,17 @@ -"""Re-add a synced unmatched track to the wishlist with the original context. +"""Re-add a synced unmatched track to the wishlist with the EXACT auto-add payload. -reconstruct_sync_track_data rebuilds the spotify_track_data the sync used. It must -prefer the full cached track (with album images), fall back to the track_result -fields, and refuse anything that wasn't a 'wishlist' row. +The live sync and the sync-detail re-add must build the identical wishlist payload +(via build_original_tracks_map), so a re-added track is indistinguishable from the +original auto-add — including its album cover, album_type, and artist shape. """ from __future__ import annotations -from core.sync.wishlist_readd import reconstruct_sync_track_data +from core.sync.wishlist_readd import ( + build_original_tracks_map, + normalize_wishlist_track, + reconstruct_sync_track_data, +) def _tr(index, sid, status='wishlist', **kw): @@ -18,75 +22,69 @@ def _tr(index, sid, status='wishlist', **kw): def _full(sid, name="Song", with_images=True): - album = {"name": "Album"} + album = {"name": "Album", "album_type": "album", "total_tracks": 12} if with_images: - album["images"] = [{"url": "http://cdn/cover.jpg"}] + album["images"] = [{"url": "http://cdn/cover.jpg", "height": 640, "width": 640}] return {"id": sid, "name": name, "artists": [{"name": "Artist"}], "album": album, "duration_ms": 200000, "popularity": 50} -def test_prefers_full_original_track_by_index(): +# ── normalize_wishlist_track ────────────────────────────────────────────────── + +def test_normalize_string_album_to_dict(): + out = normalize_wishlist_track({"id": "a", "name": "T", "album": "My Single", "artists": ["X"]}) + assert out["album"] == {"name": "My Single", "images": [], "album_type": "single", + "total_tracks": 1, "release_date": ""} + assert out["artists"] == [{"name": "X"}] # strings -> dicts + + +def test_normalize_dict_album_preserves_images_and_type(): + out = normalize_wishlist_track(_full("a")) + assert out["album"]["images"][0]["url"] == "http://cdn/cover.jpg" + assert out["album"]["album_type"] == "album" + assert out["album"]["total_tracks"] == 12 + + +def test_normalize_is_copy_safe(): + src = {"id": "a", "album": {"name": "A"}, "artists": ["X"]} + normalize_wishlist_track(src) + assert "images" not in src["album"] # source untouched + assert src["artists"] == ["X"] + + +# ── reconstruct: parity with the live auto-add ──────────────────────────────── + +def test_payload_is_identical_to_live_sync_map(): + # THE PARITY GUARANTEE: re-add payload == what build_original_tracks_map (used by + # the live sync) produces for the same track. trs = [_tr(0, "sp_a"), _tr(1, "sp_b")] tracks = [_full("sp_a"), _full("sp_b", name="Other")] out = reconstruct_sync_track_data(trs, tracks, 1) - assert out["id"] == "sp_b" # the full original track - assert out["album"]["images"][0]["url"] == "http://cdn/cover.jpg" # its own art kept - assert out is not tracks[1] # a copy, not the source entry + assert out == build_original_tracks_map(tracks)["sp_b"] + assert out["album"]["images"][0]["url"] == "http://cdn/cover.jpg" # cover carries through -def test_backfills_album_image_when_full_track_has_none(): - # THE BUG: tracks_json stored without album.images, but the track_result has the - # cover. The re-add must carry the image through (parity with the auto-add). - trs = [_tr(0, "sp_a", image_url="http://img/cover250.jpg")] - tracks = [_full("sp_a", with_images=False)] # full data but NO images - out = reconstruct_sync_track_data(trs, tracks, 0) - assert out["id"] == "sp_a" - assert out["album"]["images"] == [{"url": "http://img/cover250.jpg"}] - - -def test_full_track_with_images_is_not_overwritten(): - # When the cached track already has real album art, keep it (don't downgrade to - # the 250px track_result thumb). - trs = [_tr(0, "sp_a", image_url="http://img/thumb.jpg")] - tracks = [_full("sp_a", with_images=True)] - out = reconstruct_sync_track_data(trs, tracks, 0) - assert out["album"]["images"][0]["url"] == "http://cdn/cover.jpg" # original kept - - -def test_does_not_mutate_source_tracks_list(): - trs = [_tr(0, "sp_a", image_url="http://img/x.jpg")] - tracks = [_full("sp_a", with_images=False)] - reconstruct_sync_track_data(trs, tracks, 0) - assert "images" not in tracks[0]["album"] # source untouched - - -def test_matches_by_id_when_index_misaligns(): - # tracks_json in a different order than track_results -> match by source_track_id. +def test_resolves_by_source_track_id_not_position(): trs = [_tr(0, "sp_a"), _tr(1, "sp_b")] - tracks = [_full("sp_b"), _full("sp_a")] # reversed - out = reconstruct_sync_track_data(trs, tracks, 0) - assert out["id"] == "sp_a" # found by id, not by position + tracks = [_full("sp_b"), _full("sp_a")] # tracks_json reversed + out = reconstruct_sync_track_data(trs, tracks, 0) # row 0 -> sp_a + assert out["id"] == "sp_a" -def test_falls_back_to_track_result_fields_when_no_full_track(): +def test_fallback_rebuilds_with_cover_when_track_missing(): + # Track not in tracks_json -> rebuild from the row, seed cover from image_url, + # run through the same normalizer (album dict + artists dicts). trs = [_tr(0, "sp_x", name="Real Love Baby", artist="Father John Misty", album="Real Love Baby", image_url="http://img/x.jpg", duration_ms=188000)] - out = reconstruct_sync_track_data(trs, [], 0) # no tracks_json + out = reconstruct_sync_track_data(trs, [], 0) assert out["id"] == "sp_x" assert out["name"] == "Real Love Baby" assert out["artists"] == [{"name": "Father John Misty"}] - assert out["album"]["name"] == "Real Love Baby" assert out["album"]["images"] == [{"url": "http://img/x.jpg"}] - assert out["duration_ms"] == 188000 - - -def test_fallback_without_image_omits_images(): - out = reconstruct_sync_track_data([_tr(0, "sp_x", album="A")], [], 0) - assert "images" not in out["album"] + assert out["album"]["name"] == "Real Love Baby" def test_refuses_non_wishlist_row(): - # A matched/downloaded track must not be re-wishlistable. trs = [_tr(0, "sp_a", status="completed")] assert reconstruct_sync_track_data(trs, [_full("sp_a")], 0) is None @@ -99,5 +97,9 @@ def test_refuses_out_of_range_or_empty(): def test_refuses_when_no_id_and_no_full_track(): - # A wishlist row with no source_track_id and no cached track is unidentifiable. assert reconstruct_sync_track_data([_tr(0, "")], [], 0) is None + + +def test_build_map_skips_idless_and_non_dicts(): + m = build_original_tracks_map([_full("a"), {"name": "no id"}, "garbage", None]) + assert set(m.keys()) == {"a"}