Fix: "Discovery state not found" when fixing a match after restart/import (#843)
The discovery FIX → Confirm flow 404'd with "Discovery state not found" whenever the in-memory discovery state was gone — a server restart, or an imported playlist that wasn't discovered in THIS process — even though the card is still shown from persisted data (the reporter's log shows "Returning 0 stored ... playlists for hydration", i.e. the in-memory states were empty). The thing that actually makes a manual fix STICK is writing it to the discovery cache (save_discovery_cache_match), keyed by the original track's name + artist — which doesn't need the in-memory state at all. But the endpoint 404'd on the missing state before reaching that write, so the fix was dead after a restart. - update_discovery_match (core/discovery/endpoints.py) now only does the in-memory result update when the state exists; the durable discovery-cache write always runs, falling back to client-provided original_name/original_artist when there's no in-memory state. With neither a state nor originals it still 404s (unchanged). - The FIX confirm (wishlist-tools.js) now sends original_name/original_artist (from the source track it already has) so the backend can key the cache. Covers all sources that share the helper (tidal/deezer/qobuz/spotify-public). Tests: no-state-but-originals saves the cache + returns success; no-state-no- originals still 404s; existing with-state path unchanged. 73 discovery tests pass.
This commit is contained in:
BoulderBadgeDad
parent
a15fe15842
commit
0afa3c9705
3 changed files with 86 additions and 28 deletions
|
|
@ -579,45 +579,62 @@ def update_discovery_match(
|
|||
return {'error': 'Missing required fields'}, 400
|
||||
|
||||
state = states.get(identifier)
|
||||
if not state:
|
||||
return {'error': 'Discovery state not found'}, 404
|
||||
result = None
|
||||
if state:
|
||||
if track_index >= len(state['discovery_results']):
|
||||
return {'error': 'Invalid track index'}, 400
|
||||
|
||||
if track_index >= len(state['discovery_results']):
|
||||
return {'error': 'Invalid track index'}, 400
|
||||
result = state['discovery_results'][track_index]
|
||||
old_status = result.get('status')
|
||||
|
||||
result = state['discovery_results'][track_index]
|
||||
old_status = result.get('status')
|
||||
result['status'] = 'Found'
|
||||
result['status_class'] = 'found'
|
||||
result['spotify_track'] = spotify_track['name']
|
||||
result['spotify_artist'] = join_artist_names(spotify_track['artists']) if isinstance(spotify_track['artists'], list) else extract_artist_name(spotify_track['artists'])
|
||||
result['spotify_album'] = spotify_track['album']
|
||||
result['spotify_id'] = spotify_track['id']
|
||||
|
||||
result['status'] = 'Found'
|
||||
result['status_class'] = 'found'
|
||||
result['spotify_track'] = spotify_track['name']
|
||||
result['spotify_artist'] = join_artist_names(spotify_track['artists']) if isinstance(spotify_track['artists'], list) else extract_artist_name(spotify_track['artists'])
|
||||
result['spotify_album'] = spotify_track['album']
|
||||
result['spotify_id'] = spotify_track['id']
|
||||
duration_ms = spotify_track.get('duration_ms', 0)
|
||||
if duration_ms:
|
||||
minutes = duration_ms // 60000
|
||||
seconds = (duration_ms % 60000) // 1000
|
||||
result['duration'] = f"{minutes}:{seconds:02d}"
|
||||
else:
|
||||
result['duration'] = '0:00'
|
||||
|
||||
duration_ms = spotify_track.get('duration_ms', 0)
|
||||
if duration_ms:
|
||||
minutes = duration_ms // 60000
|
||||
seconds = (duration_ms % 60000) // 1000
|
||||
result['duration'] = f"{minutes}:{seconds:02d}"
|
||||
else:
|
||||
result['duration'] = '0:00'
|
||||
result['spotify_data'] = build_fix_modal_spotify_data(spotify_track)
|
||||
result['wing_it_fallback'] = False
|
||||
result['manual_match'] = True
|
||||
|
||||
result['spotify_data'] = build_fix_modal_spotify_data(spotify_track)
|
||||
result['wing_it_fallback'] = False
|
||||
result['manual_match'] = True
|
||||
if old_status != 'found' and old_status != 'Found':
|
||||
state['spotify_matches'] = state.get('spotify_matches', 0) + 1
|
||||
|
||||
if old_status != 'found' and old_status != 'Found':
|
||||
state['spotify_matches'] = state.get('spotify_matches', 0) + 1
|
||||
logger.info(f"Manual match updated: {source_log_label} - {identifier} - track {track_index}")
|
||||
logger.info(f" → {result['spotify_artist']} - {result['spotify_track']}")
|
||||
|
||||
logger.info(f"Manual match updated: {source_log_label} - {identifier} - track {track_index}")
|
||||
logger.info(f" → {result['spotify_artist']} - {result['spotify_track']}")
|
||||
|
||||
try:
|
||||
original_track = result.get(original_track_key, {})
|
||||
original_name = original_track.get('name', spotify_track['name'])
|
||||
original_artist = original_artist_getter(original_track)
|
||||
else:
|
||||
# #843: the in-memory discovery state can be gone — a server restart,
|
||||
# or an imported playlist that wasn't discovered in THIS process —
|
||||
# while the card is still shown from persisted data. The DURABLE part
|
||||
# of a manual fix (writing the match to the discovery cache so future
|
||||
# syncs resolve it) doesn't need the in-memory state, only the original
|
||||
# track's name + artist, which the client now sends. Fall back to those
|
||||
# instead of 404ing the fix into uselessness.
|
||||
original_name = (data.get('original_name') or '').strip()
|
||||
original_artist = (data.get('original_artist') or '').strip()
|
||||
if not original_name and not original_artist:
|
||||
return {'error': 'Discovery state not found'}, 404
|
||||
if not original_name:
|
||||
original_name = spotify_track['name']
|
||||
logger.info(
|
||||
f"Manual match (no in-memory state) → discovery cache: "
|
||||
f"{source_log_label} - {identifier} - '{original_name}' by '{original_artist}'"
|
||||
)
|
||||
|
||||
try:
|
||||
cache_key = get_discovery_cache_key(original_name, original_artist)
|
||||
artists_list = spotify_track['artists']
|
||||
if isinstance(artists_list, list):
|
||||
|
|
|
|||
|
|
@ -955,3 +955,39 @@ def test_snapshot_exception_returns_500():
|
|||
raise RuntimeError('db down')
|
||||
body, code = save_bubble_snapshot(lambda: {'bubbles': [1]}, **_snap_kwargs(_BoomDB()))
|
||||
assert code == 500 and body == {'success': False, 'error': 'db down'}
|
||||
|
||||
|
||||
def test_update_match_no_state_but_originals_saves_cache():
|
||||
"""#843: the in-memory discovery state can be gone (server restart, or an
|
||||
imported playlist not discovered this run) while the card is still shown from
|
||||
persisted data. A manual fix must still save the match to the discovery cache
|
||||
from the client-provided originals instead of 404ing."""
|
||||
from core.discovery.endpoints import update_discovery_match
|
||||
db = _FakeCacheDB()
|
||||
gj, kw, _ = _update_kwargs(cache_db=db, json_data={
|
||||
'identifier': 'gone-from-memory', 'track_index': 0,
|
||||
'original_name': 'Acid Dream', 'original_artist': 'Cherrymoon Traxx',
|
||||
'spotify_track': {
|
||||
'id': 'sp1', 'name': 'Acid Dream (The Prophet remix)',
|
||||
'artists': ['Cherry Moon Trax'], 'album': 'X', 'duration_ms': 1000,
|
||||
},
|
||||
})
|
||||
body, code = update_discovery_match({}, gj, **kw) # empty in-memory states
|
||||
|
||||
assert code == 200 and body['success'] is True
|
||||
assert body['result'] is None # nothing to update in memory
|
||||
# the durable cache match WAS written, keyed by the original track
|
||||
assert len(db.saved) == 1
|
||||
saved = db.saved[0]
|
||||
assert saved[0] == 'acid dream' # cache_key from get_discovery_cache_key
|
||||
assert saved[5] == 'Acid Dream' and saved[6] == 'Cherrymoon Traxx' # original name/artist
|
||||
|
||||
|
||||
def test_update_match_no_state_and_no_originals_still_404():
|
||||
"""Without an in-memory state AND without originals to key the cache, there's
|
||||
genuinely nothing to do — keep the 404 (the existing safety)."""
|
||||
from core.discovery.endpoints import update_discovery_match
|
||||
gj, kw, _ = _update_kwargs(json_data={
|
||||
'identifier': 'gone', 'track_index': 0, 'spotify_track': {'id': 'x'}})
|
||||
body, code = update_discovery_match({}, gj, **kw)
|
||||
assert code == 404 and body == {'error': 'Discovery state not found'}
|
||||
|
|
|
|||
|
|
@ -410,6 +410,11 @@ async function selectDiscoveryFixTrack(track) {
|
|||
const requestBody = {
|
||||
identifier: backendIdentifier,
|
||||
track_index: trackIndex,
|
||||
// #843: send the original (source) track so the backend can still save
|
||||
// the match to the discovery cache when its in-memory discovery state
|
||||
// is gone (server restart / imported playlist not discovered this run).
|
||||
original_name: currentDiscoveryFix.sourceTrack || '',
|
||||
original_artist: currentDiscoveryFix.sourceArtist || '',
|
||||
spotify_track: {
|
||||
id: track.id,
|
||||
name: track.name,
|
||||
|
|
|
|||
Loading…
Reference in a new issue