spotify search endpoint: plain query, not field-scoped — fixes pool-fix "no results"
The Wing It pool "Fix Match" search returned "no results" for everything (even obvious tracks). Root cause: /api/spotify/search_tracks built a Spotify field-filtered query (track:X artist:Y) and handed it to spotify_client.search_tracks, which falls back to the user's configured source when official Spotify isn't serving the request. The fallback (Deezer here) got the raw Spotify `track:…artist:…` syntax it can't parse and aborted the connection (RemoteDisconnected) — so the user's perfectly working Deezer failed ONLY on this path, on this query format. The iTunes and Deezer search endpoints already dropped field syntax for exactly this reason; the Spotify one was the lone holdout. fix: - new pure helper relevance.build_combined_search_query(track, artist, legacy) — plain, source-agnostic query; documents WHY field syntax is wrong here. the endpoint already reranks by expected title/artist, so precision is recovered without the brittle syntax. - the Spotify endpoint uses it (now consistent with iTunes/Deezer). - frontend (searchPoolFix): surface the real error (auth / 500 / upstream abort) instead of masking everything as "No results found" — which is what made this undiagnosable. 5 helper tests incl. the regression (output must contain no 'track:'/'artist:' syntax). 654 metadata/search tests green, 64 script-integrity green, ruff clean.
This commit is contained in:
parent
62aa2bef2d
commit
c593a17ac2
4 changed files with 71 additions and 13 deletions
|
|
@ -52,6 +52,27 @@ from typing import List, Optional, Sequence
|
||||||
from core.metadata.types import Track
|
from core.metadata.types import Track
|
||||||
|
|
||||||
|
|
||||||
|
def build_combined_search_query(track: str = '', artist: str = '',
|
||||||
|
legacy: str = '') -> str:
|
||||||
|
"""Combine a track + artist into a PLAIN, source-agnostic search query.
|
||||||
|
|
||||||
|
Deliberately NOT field-scoped (``track:"X" artist:"Y"``). That Spotify/Lucene
|
||||||
|
filter syntax leaks to non-Spotify sources when a search falls back: Deezer
|
||||||
|
closed the connection on it outright (``RemoteDisconnected``), and even sources
|
||||||
|
that parse it over-constrain and miss the canonical cut (the iTunes/Deezer search
|
||||||
|
endpoints already dropped it for that reason). The matching ``/search_tracks``
|
||||||
|
endpoints rerank by expected title/artist afterward, so precision is recovered
|
||||||
|
without the brittle field syntax.
|
||||||
|
|
||||||
|
Falls back to ``legacy`` when neither track nor artist is given. Returns ``''``
|
||||||
|
when there's nothing to search (caller decides how to handle empty).
|
||||||
|
"""
|
||||||
|
parts = [p.strip() for p in (track, artist) if p and p.strip()]
|
||||||
|
if parts:
|
||||||
|
return ' '.join(parts)
|
||||||
|
return (legacy or '').strip()
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# Pattern tables — public so tests can introspect, callers can extend
|
# Pattern tables — public so tests can introspect, callers can extend
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
|
||||||
|
|
@ -447,3 +447,36 @@ class TestFilterAndRerank:
|
||||||
# Karaoke pattern reduces score by 0.05x — well below 0.5
|
# Karaoke pattern reduces score by 0.05x — well below 0.5
|
||||||
assert all(t.id != 'karaoke-id' for t in result)
|
assert all(t.id != 'karaoke-id' for t in result)
|
||||||
assert any(t.id == 'real-id' for t in result)
|
assert any(t.id == 'real-id' for t in result)
|
||||||
|
|
||||||
|
|
||||||
|
# ── build_combined_search_query: plain, source-agnostic queries (pool-fix bug) ──
|
||||||
|
|
||||||
|
from core.metadata.relevance import build_combined_search_query as _bcsq
|
||||||
|
|
||||||
|
|
||||||
|
def test_combined_query_is_plain_not_field_scoped():
|
||||||
|
# THE fix: must NOT emit Spotify `track:`/`artist:` syntax — that leaked to Deezer
|
||||||
|
# (which aborted the connection) and other fallbacks that can't parse it.
|
||||||
|
q = _bcsq('Not Like Us', 'Kendrick Lamar')
|
||||||
|
assert q == 'Not Like Us Kendrick Lamar'
|
||||||
|
assert 'track:' not in q and 'artist:' not in q
|
||||||
|
|
||||||
|
|
||||||
|
def test_combined_query_track_or_artist_alone():
|
||||||
|
assert _bcsq('Not Like Us', '') == 'Not Like Us'
|
||||||
|
assert _bcsq('', 'Kendrick Lamar') == 'Kendrick Lamar'
|
||||||
|
|
||||||
|
|
||||||
|
def test_combined_query_trims_whitespace():
|
||||||
|
assert _bcsq(' Not Like Us ', ' Kendrick Lamar ') == 'Not Like Us Kendrick Lamar'
|
||||||
|
|
||||||
|
|
||||||
|
def test_combined_query_falls_back_to_legacy():
|
||||||
|
assert _bcsq('', '', 'free text search') == 'free text search'
|
||||||
|
# track/artist win over legacy when present
|
||||||
|
assert _bcsq('A', 'B', 'ignored') == 'A B'
|
||||||
|
|
||||||
|
|
||||||
|
def test_combined_query_empty_when_nothing():
|
||||||
|
assert _bcsq('', '', '') == ''
|
||||||
|
assert _bcsq(' ', '', ' ') == ''
|
||||||
|
|
|
||||||
|
|
@ -21612,17 +21612,13 @@ def search_spotify_tracks():
|
||||||
legacy_query = request.args.get('query', '').strip()
|
legacy_query = request.args.get('query', '').strip()
|
||||||
limit = int(request.args.get('limit', 20))
|
limit = int(request.args.get('limit', 20))
|
||||||
|
|
||||||
# Build Spotify field-filtered query
|
# Plain combined query — NOT field-scoped (`track:X artist:Y`). That Spotify
|
||||||
if track_q or artist_q:
|
# syntax leaks to non-Spotify sources when search falls back (Deezer aborted
|
||||||
parts = []
|
# the connection on it); the iTunes/Deezer endpoints already dropped it for the
|
||||||
if track_q:
|
# same reason, and the rerank below recovers precision. (Pool-fix "no results".)
|
||||||
parts.append(f'track:{track_q}')
|
from core.metadata.relevance import build_combined_search_query
|
||||||
if artist_q:
|
query = build_combined_search_query(track_q, artist_q, legacy_query)
|
||||||
parts.append(f'artist:{artist_q}')
|
if not query:
|
||||||
query = ' '.join(parts)
|
|
||||||
elif legacy_query:
|
|
||||||
query = legacy_query
|
|
||||||
else:
|
|
||||||
return jsonify({"error": "Query parameter is required"}), 400
|
return jsonify({"error": "Query parameter is required"}), 400
|
||||||
|
|
||||||
if use_hydrabase:
|
if use_hydrabase:
|
||||||
|
|
|
||||||
|
|
@ -1860,9 +1860,17 @@ async function searchPoolFix() {
|
||||||
if (artistVal) params.set('artist', artistVal);
|
if (artistVal) params.set('artist', artistVal);
|
||||||
params.set('limit', '20');
|
params.set('limit', '20');
|
||||||
const res = await fetch(`/api/spotify/search_tracks?${params.toString()}`);
|
const res = await fetch(`/api/spotify/search_tracks?${params.toString()}`);
|
||||||
const data = await res.json();
|
const data = await res.json().catch(() => ({}));
|
||||||
const tracks = data.tracks || [];
|
|
||||||
|
|
||||||
|
// Surface the real failure instead of masking every error (auth, 500, an
|
||||||
|
// upstream connection abort) as a bland "No results found".
|
||||||
|
if (!res.ok || data.error) {
|
||||||
|
const msg = data.error || res.statusText || `request failed (${res.status})`;
|
||||||
|
resultsContainer.innerHTML = `<div class="pool-fix-empty">Search error: ${_esc(msg)}</div>`;
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
const tracks = data.tracks || [];
|
||||||
if (tracks.length === 0) {
|
if (tracks.length === 0) {
|
||||||
resultsContainer.innerHTML = '<div class="pool-fix-empty">No results found</div>';
|
resultsContainer.innerHTML = '<div class="pool-fix-empty">No results found</div>';
|
||||||
return;
|
return;
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue