# Background User reported (#534) that the import-modal "Search for Match" dialog returned irrelevant results when Deezer was the metadata source. Searching `Dirty White Boy` + `Foreigner` returned 5+ karaoke / "originally performed by" / "in the style of" / "re-recorded" / tribute-band results ranked above the actual Foreigner studio cut from Head Games. User had to scroll past the junk every time, or fall back to iTunes search which is much slower. # Root cause — two layers 1. **Endpoint joined `track + artist` into free-text query.** `/api/deezer/search_tracks` was passing `q=Dirty White Boy Foreigner` to Deezer's `/search/track` API. Deezer fuzzy-matches that string across title / lyrics / artist / album / contributors and orders by global popularity — anything that appears across many compilations outranks the canonical recording. 2. **No local rerank.** None of the search-modal endpoints applied any post-filtering. Deezer's API order shipped straight to the user. # Fix — same architectural shape Cin would build ## Layer 1: field-scoped query at the client boundary `core/deezer_client.py::search_tracks()` now accepts optional `track`, `artist`, `album` kwargs. When provided, builds Deezer's advanced search syntax: `q=track:"X" artist:"Y" album:"Z"`. Massive relevance improvement because each term matches the right field instead of fuzzy-matching everywhere. Backward compat preserved: legacy free-text `query=` callers still work unchanged. Field-scoped path takes precedence when both are provided. Empty input fast-fails without an API call. Embedded double-quotes stripped (Deezer's syntax has no escape mechanism). ## Layer 2: provider-neutral relevance reranker New `core/metadata/relevance.py` module — pure-function rerank over the canonical `Track` dataclass. Composable scoring: - **Cover/karaoke patterns** (multiplier 0.05, effectively buries): matches "karaoke", "originally performed by", "in the style of", "made famous by", "tribute", "vocal version", "backing track", "cover version", "re-recorded", "cover by", etc. across title, album, AND artist fields. Catches the screenshot's exact junk: artist credits like "Pop Music Workshop" / "The Karaoke Channel" / "Foreigner Tribute Band". - **Variant tags** (multiplier 0.4): live / acoustic / demo / instrumental / remix / radio edit / club mix etc. — softer penalty since the user MAY want them. Skipped entirely when the expected_title contains the same tag (so searching "Track (Live)" still ranks Live versions first). - **Exact artist boost** (multiplier 1.5): primary artist exactly matches expected_artist after normalisation. Single strongest signal for "this is the canonical recording". - **Title + artist similarity** via SequenceMatcher (parentheticals + punctuation stripped before comparison). - **Album-type weighting**: album=1.0 > single/ep=0.85 > compilation=0.7. Compilations are more likely tribute / karaoke repackages. Each component is a standalone function so tests pin them individually without standing up the full pipeline. ## Wired at three search-modal endpoints - `/api/deezer/search_tracks` — uses both layers (field-scoped query + rerank). - `/api/itunes/search_tracks` — uses rerank only (iTunes API has no advanced-syntax search, but karaoke / cover variants still leak through and need the local penalty). - `/api/spotify/search_tracks` — already builds field-scoped `track:X artist:Y` query; rerank added as the consistency safety net so all three sources behave the same from the user's perspective. Other Deezer call sites (matching engine, watchlist scanner, auto-import single-track ID) deliberately not touched in this PR — they have their own elaborate scoring pipelines tuned to their specific contexts and aren't surfacing the user-reported issue. Per Cin: "don't refactor beyond what the task requires." # Tests 71 new tests across 3 files: - `tests/metadata/test_relevance.py` (50 tests) — every scoring component pinned individually + the issue #534 screenshot reproduced as a regression test (real Foreigner cut wins after rerank, karaoke variants drop to bottom). - `tests/metadata/test_deezer_search_query.py` (14 tests) — advanced-syntax query construction, field-scoped wiring at the client boundary, free-text path unchanged, kwargs win when ambiguous, limit clamping, cache key consistency. - `tests/imports/test_search_match_endpoints.py` (7 tests) — end-to-end through Flask test client: Deezer endpoint passes kwargs not joined query; karaoke buried at bottom for all three sources; legacy query param still works without rerank. # Verification - 2441 full suite passes (+71 from baseline 2370) - 0 failures (the prior watchdog flake fix held) - Ruff clean across all changed files - JS parses clean (`node -c webui/static/helper.js`) # Architectural standards followed - **Logic at the right boundary.** Query construction lives in the client (every caller benefits from one change). Rerank lives in a neutral module (`core/metadata/relevance.py`) over the canonical `Track` dataclass — works for any source, not Deezer- specific. - **Explicit > implicit.** Every scoring rule has its own named function. Pattern tables are module-level constants tests can introspect. - **Scope discipline.** Audited every Deezer search call site; fixed the user-reported one + the consistent siblings. Did NOT speculatively normalise every Deezer call across the codebase. - **Backward compat.** Free-text `query=` callers untouched. Kwargs added to existing client method signature with safe defaults. - **Tests pin contract at correct boundary.** Pure-function rerank tests don't mock anything; client-query tests stub at `_api_get`; endpoint tests run through the real Flask app.
194 lines
7.3 KiB
Python
194 lines
7.3 KiB
Python
"""Pin Deezer search query construction.
|
|
|
|
Issue #534 — Deezer's free-text search returns karaoke / cover /
|
|
"originally performed by" variants ranked above the canonical
|
|
recording. Switching to Deezer's advanced search syntax
|
|
(`track:"X" artist:"Y"`) tightens the API's relevance ranking
|
|
dramatically by matching each term against the right field instead
|
|
of fuzzy-matching across title / lyrics / artist / album.
|
|
|
|
These tests pin the query construction at the client boundary so
|
|
the wire-shape contract is obvious from the tests alone (no need
|
|
to read the client source to know what query string the API
|
|
receives).
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
from unittest.mock import MagicMock, patch
|
|
|
|
import pytest
|
|
|
|
from core.deezer_client import DeezerClient
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# _build_advanced_query — pure helper, no API calls
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
class TestBuildAdvancedQuery:
|
|
def test_track_and_artist_quoted(self):
|
|
q = DeezerClient._build_advanced_query(
|
|
track='Dirty White Boy', artist='Foreigner',
|
|
)
|
|
assert q == 'track:"Dirty White Boy" artist:"Foreigner"'
|
|
|
|
def test_track_only(self):
|
|
q = DeezerClient._build_advanced_query(track='Dirty White Boy')
|
|
assert q == 'track:"Dirty White Boy"'
|
|
|
|
def test_artist_only(self):
|
|
q = DeezerClient._build_advanced_query(artist='Foreigner')
|
|
assert q == 'artist:"Foreigner"'
|
|
|
|
def test_all_three_fields(self):
|
|
q = DeezerClient._build_advanced_query(
|
|
track='Head Games', artist='Foreigner', album='Head Games',
|
|
)
|
|
assert q == 'track:"Head Games" artist:"Foreigner" album:"Head Games"'
|
|
|
|
def test_empty_inputs_produce_empty_query(self):
|
|
assert DeezerClient._build_advanced_query() == ''
|
|
|
|
def test_embedded_quotes_stripped(self):
|
|
"""Deezer's syntax has no escape mechanism for embedded
|
|
double-quotes. Strip them to keep the query well-formed.
|
|
Rare in practice but a search for `O"Hara` would otherwise
|
|
produce a malformed `track:"O"Hara"` that breaks parsing."""
|
|
q = DeezerClient._build_advanced_query(track='O"Hara')
|
|
assert q == 'track:"OHara"'
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# search_tracks — verify the right query string reaches the API
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
class TestSearchTracksQueryWiring:
|
|
def _client(self):
|
|
c = DeezerClient.__new__(DeezerClient)
|
|
# Stub state needed by _api_get's downstream methods
|
|
c._api_get = MagicMock(return_value={'data': []})
|
|
return c
|
|
|
|
def _stub_cache(self, monkeypatch):
|
|
"""Stub the metadata cache so it doesn't return stale data
|
|
from a prior test run AND so we can verify the cache key
|
|
the search uses."""
|
|
cache = MagicMock()
|
|
cache.get_search_results.return_value = None
|
|
monkeypatch.setattr('core.deezer_client.get_metadata_cache', lambda: cache)
|
|
return cache
|
|
|
|
def test_field_scoped_kwargs_use_advanced_syntax(self, monkeypatch):
|
|
"""Headline assertion of issue #534's fix. When callers pass
|
|
track + artist as kwargs, the actual API call must use
|
|
Deezer's advanced syntax — NOT the joined free-text form."""
|
|
self._stub_cache(monkeypatch)
|
|
c = self._client()
|
|
|
|
c.search_tracks(track='Dirty White Boy', artist='Foreigner')
|
|
|
|
c._api_get.assert_called_once()
|
|
params = c._api_get.call_args.args[1]
|
|
assert params['q'] == 'track:"Dirty White Boy" artist:"Foreigner"', (
|
|
f"Expected advanced-syntax query string, got {params['q']!r}"
|
|
)
|
|
|
|
def test_free_text_query_path_unchanged(self, monkeypatch):
|
|
"""Backward compat: legacy callers passing a single free-text
|
|
query string still work, no advanced syntax applied."""
|
|
self._stub_cache(monkeypatch)
|
|
c = self._client()
|
|
|
|
c.search_tracks('Foreigner Dirty White Boy')
|
|
|
|
params = c._api_get.call_args.args[1]
|
|
assert params['q'] == 'Foreigner Dirty White Boy', (
|
|
"Free-text caller must pass through unchanged"
|
|
)
|
|
|
|
def test_field_kwargs_take_precedence_over_query_param(self, monkeypatch):
|
|
"""When BOTH query and field kwargs are provided, field
|
|
kwargs win (they're authoritative). Avoids ambiguity at the
|
|
endpoint layer where someone might forget to drop the legacy
|
|
query when adding field params."""
|
|
self._stub_cache(monkeypatch)
|
|
c = self._client()
|
|
|
|
c.search_tracks(query='ignored free text',
|
|
track='Dirty White Boy', artist='Foreigner')
|
|
|
|
params = c._api_get.call_args.args[1]
|
|
assert 'track:' in params['q']
|
|
assert 'ignored' not in params['q']
|
|
|
|
def test_no_query_or_kwargs_returns_empty_without_api_call(self, monkeypatch):
|
|
"""Defensive: empty input shouldn't fire a wasted API call.
|
|
Returns empty list immediately."""
|
|
self._stub_cache(monkeypatch)
|
|
c = self._client()
|
|
|
|
result = c.search_tracks()
|
|
assert result == []
|
|
c._api_get.assert_not_called()
|
|
|
|
def test_album_only_kwarg_works(self, monkeypatch):
|
|
"""album-only field-scoped search — useful for callers who
|
|
know the album exactly but not the track or artist."""
|
|
self._stub_cache(monkeypatch)
|
|
c = self._client()
|
|
|
|
c.search_tracks(album='Head Games')
|
|
|
|
params = c._api_get.call_args.args[1]
|
|
assert params['q'] == 'album:"Head Games"'
|
|
|
|
def test_limit_parameter_passed_through(self, monkeypatch):
|
|
self._stub_cache(monkeypatch)
|
|
c = self._client()
|
|
|
|
c.search_tracks(track='X', artist='Y', limit=50)
|
|
|
|
params = c._api_get.call_args.args[1]
|
|
assert params['limit'] == 50
|
|
|
|
def test_limit_clamped_to_100(self, monkeypatch):
|
|
"""Deezer's max page size is 100. Higher requests must get
|
|
clamped on our side rather than forwarded as-is (which would
|
|
either error or get silently truncated by the API)."""
|
|
self._stub_cache(monkeypatch)
|
|
c = self._client()
|
|
|
|
c.search_tracks(track='X', limit=500)
|
|
|
|
params = c._api_get.call_args.args[1]
|
|
assert params['limit'] == 100
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Cache key consistency — both call modes share the cache via the
|
|
# constructed query string
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
class TestSearchTracksCacheKey:
|
|
def test_field_scoped_call_uses_advanced_query_as_cache_key(self, monkeypatch):
|
|
"""Cache key is the constructed query string, NOT the raw
|
|
kwargs. Means the same advanced query hits the cache no
|
|
matter how it's reconstructed by future call sites."""
|
|
cache = MagicMock()
|
|
cache.get_search_results.return_value = None
|
|
monkeypatch.setattr('core.deezer_client.get_metadata_cache', lambda: cache)
|
|
|
|
c = DeezerClient.__new__(DeezerClient)
|
|
c._api_get = MagicMock(return_value={'data': []})
|
|
|
|
c.search_tracks(track='Dirty White Boy', artist='Foreigner', limit=20)
|
|
|
|
cache.get_search_results.assert_called_once_with(
|
|
'deezer', 'track',
|
|
'track:"Dirty White Boy" artist:"Foreigner"',
|
|
20,
|
|
)
|