Merge pull request #539 from Nezreka/fix/deezer-search-relevance-issue-534
Fix/deezer search relevance issue 534
This commit is contained in:
BoulderBadgeDad
GitHub
commit
69c35a57b5
7 changed files with 1429 additions and 16 deletions
|
|
@ -298,10 +298,78 @@ class DeezerClient:
|
|||
# can serve as a drop-in fallback metadata source in SpotifyClient.
|
||||
|
||||
@rate_limited
|
||||
def search_tracks(self, query: str, limit: int = 20) -> List[Track]:
|
||||
"""Search for tracks — returns Track dataclass list (metadata source interface)"""
|
||||
def search_tracks(
|
||||
self,
|
||||
query: str = '',
|
||||
limit: int = 20,
|
||||
*,
|
||||
track: Optional[str] = None,
|
||||
artist: Optional[str] = None,
|
||||
album: Optional[str] = None,
|
||||
) -> List[Track]:
|
||||
"""Search for tracks — returns Track dataclass list (metadata source interface).
|
||||
|
||||
Two call modes:
|
||||
|
||||
1. **Free-text** (`query='Foreigner Dirty White Boy'`) — legacy
|
||||
shape, passes the string straight to Deezer's `q` param.
|
||||
Same behaviour as before, kept for backward compat.
|
||||
|
||||
2. **Field-scoped** (`track='Dirty White Boy', artist='Foreigner'`) —
|
||||
builds Deezer's advanced search syntax (`track:"X" artist:"Y"`).
|
||||
Massively tighter relevance than the free-text path because
|
||||
the API matches each term in the right field instead of
|
||||
anywhere across title / lyrics / artist / album / contributors.
|
||||
Without this, the Deezer ranking buries the canonical track
|
||||
under karaoke / cover / "originally performed by" variants
|
||||
— see issue #534.
|
||||
|
||||
Field-scoped form is used whenever ``track`` or ``artist`` is
|
||||
provided. ``query`` is ignored in that case (the field params
|
||||
are authoritative). When both are missing, falls through to
|
||||
``query``. The cache key is the constructed query string in
|
||||
either case so the two paths share entries naturally.
|
||||
"""
|
||||
# Build the actual API query — advanced syntax when callers pass
|
||||
# field hints, raw query otherwise.
|
||||
used_advanced = bool(track or artist or album)
|
||||
if used_advanced:
|
||||
api_query = self._build_advanced_query(track=track, artist=artist, album=album)
|
||||
else:
|
||||
api_query = query
|
||||
|
||||
if not api_query:
|
||||
return []
|
||||
|
||||
tracks = self._search_tracks_with_query(api_query, limit)
|
||||
|
||||
# Safety net: Deezer's advanced syntax is `artist:"X"`-style
|
||||
# substring match, but in practice it's brittle on artist name
|
||||
# variants ("Foreigner [US]", "The Foreigner", etc.) and on
|
||||
# tracks indexed under non-canonical title spellings. When the
|
||||
# advanced query returns nothing, fall back to a free-text join
|
||||
# so the user sees the prior (less-relevant but non-empty) result
|
||||
# set rather than "No matches" — same behaviour as pre-fix for
|
||||
# this edge case. Caller-side rerank still tightens the result.
|
||||
if not tracks and used_advanced:
|
||||
fallback_parts = [p for p in (track, artist, album) if p]
|
||||
fallback_query = ' '.join(fallback_parts)
|
||||
if fallback_query and fallback_query != api_query:
|
||||
logger.debug(
|
||||
"[Deezer] Advanced query returned 0 results, falling back "
|
||||
"to free-text: %r → %r", api_query, fallback_query,
|
||||
)
|
||||
tracks = self._search_tracks_with_query(fallback_query, limit)
|
||||
|
||||
return tracks
|
||||
|
||||
def _search_tracks_with_query(self, api_query: str, limit: int) -> List[Track]:
|
||||
"""Cache-aware single API call. Pulled out so the
|
||||
``search_tracks`` orchestration can call this twice (advanced
|
||||
query → free-text fallback) without duplicating the cache +
|
||||
parse + store dance."""
|
||||
cache = get_metadata_cache()
|
||||
cached_results = cache.get_search_results('deezer', 'track', query, limit)
|
||||
cached_results = cache.get_search_results('deezer', 'track', api_query, limit)
|
||||
if cached_results is not None:
|
||||
tracks = []
|
||||
for raw in cached_results:
|
||||
|
|
@ -312,25 +380,54 @@ class DeezerClient:
|
|||
if tracks:
|
||||
return tracks
|
||||
|
||||
data = self._api_get('search/track', {'q': query, 'limit': min(limit, 100)})
|
||||
data = self._api_get('search/track', {'q': api_query, 'limit': min(limit, 100)})
|
||||
if not data or 'data' not in data:
|
||||
return []
|
||||
|
||||
tracks = []
|
||||
raw_items = []
|
||||
for track_data in data['data']:
|
||||
track = Track.from_deezer_track(track_data)
|
||||
tracks.append(track)
|
||||
track_obj = Track.from_deezer_track(track_data)
|
||||
tracks.append(track_obj)
|
||||
raw_items.append(track_data)
|
||||
|
||||
entries = [(str(td.get('id', '')), td) for td in raw_items if td.get('id')]
|
||||
if entries:
|
||||
cache.store_entities_bulk('deezer', 'track', entries)
|
||||
cache.store_search_results('deezer', 'track', query, limit,
|
||||
cache.store_search_results('deezer', 'track', api_query, limit,
|
||||
[str(td.get('id', '')) for td in raw_items if td.get('id')])
|
||||
|
||||
return tracks
|
||||
|
||||
@staticmethod
|
||||
def _build_advanced_query(
|
||||
*,
|
||||
track: Optional[str] = None,
|
||||
artist: Optional[str] = None,
|
||||
album: Optional[str] = None,
|
||||
) -> str:
|
||||
"""Compose Deezer's advanced search syntax from field hints.
|
||||
|
||||
Per Deezer's docs:
|
||||
https://developers.deezer.com/api/search
|
||||
|
||||
q=track:"X" artist:"Y" album:"Z"
|
||||
|
||||
Quotes around each value preserve multi-word phrases. Empty
|
||||
fields are skipped. Embedded double-quotes get stripped (no
|
||||
escape mechanism in Deezer's syntax) — rare in practice, but
|
||||
a search for `O"Hara` would otherwise produce a malformed
|
||||
query.
|
||||
"""
|
||||
parts = []
|
||||
if track:
|
||||
parts.append(f'track:"{track.replace(chr(34), "")}"')
|
||||
if artist:
|
||||
parts.append(f'artist:"{artist.replace(chr(34), "")}"')
|
||||
if album:
|
||||
parts.append(f'album:"{album.replace(chr(34), "")}"')
|
||||
return ' '.join(parts)
|
||||
|
||||
@rate_limited
|
||||
def search_artists(self, query: str, limit: int = 20) -> List[Artist]:
|
||||
"""Search for artists — returns Artist dataclass list (metadata source interface)"""
|
||||
|
|
|
|||
343
core/metadata/relevance.py
Normal file
343
core/metadata/relevance.py
Normal file
|
|
@ -0,0 +1,343 @@
|
|||
"""Local relevance re-ranking for metadata-source search results.
|
||||
|
||||
Background
|
||||
----------
|
||||
|
||||
Some metadata sources (Deezer notably) return search results in a
|
||||
relevance order that puts karaoke covers, "originally performed by",
|
||||
re-recorded versions, tribute compilations, and Vocal/Backing-Track
|
||||
variants ABOVE the actual studio recording the user is looking for.
|
||||
Their global popularity ordering means anything that appears across
|
||||
many compilations outranks the canonical track. Issue #534 is the
|
||||
canonical example: searching `Dirty White Boy` + `Foreigner` returned
|
||||
five karaoke / cover variants before the real Foreigner studio cut.
|
||||
|
||||
This module is a provider-neutral helper. Given a list of typed
|
||||
``Track`` results plus an expected title + artist, it re-ranks by
|
||||
local heuristics that the source's own ranking ignores:
|
||||
|
||||
- Hard penalty for known cover/karaoke/tribute patterns (title OR
|
||||
album OR artist field). These rarely belong in import / match
|
||||
results when the user typed the original artist.
|
||||
- Soft penalty for variant types (Live, Acoustic, Remix, Demo,
|
||||
Instrumental) UNLESS the user's expected title also contains the
|
||||
variant tag (so "Track (Live)" search matches Live recordings).
|
||||
- Boost for exact artist match — the strongest signal that this is
|
||||
the canonical recording.
|
||||
- Title similarity via SequenceMatcher on normalised strings (drop
|
||||
parentheticals + punctuation before comparison).
|
||||
- Album-type weight: album > compilation > single (compilations are
|
||||
more likely to be tributes / "best of" repackages).
|
||||
|
||||
Pure-function design over the canonical ``Track`` dataclass —
|
||||
no Deezer-specific assumptions, applies to iTunes / Spotify /
|
||||
Hydrabase results equally well. Each scoring component is its own
|
||||
small function so tests can pin them independently.
|
||||
|
||||
Usage
|
||||
-----
|
||||
|
||||
>>> from core.metadata.relevance import rerank_tracks
|
||||
>>> tracks = client.search_tracks(query)
|
||||
>>> ranked = rerank_tracks(tracks, expected_title='Dirty White Boy', expected_artist='Foreigner')
|
||||
>>> # ranked[0] is now the most relevant; karaoke variants drop to bottom
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from difflib import SequenceMatcher
|
||||
from typing import List, Optional, Sequence
|
||||
|
||||
from core.metadata.types import Track
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Pattern tables — public so tests can introspect, callers can extend
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
# Title / album / artist substrings that strongly indicate a cover,
|
||||
# karaoke, tribute, or "originally performed by" compilation. Multiplier
|
||||
# applied to the final score when matched. 0.05 effectively buries these
|
||||
# unless nothing else matches.
|
||||
COVER_KARAOKE_PATTERNS = (
|
||||
'karaoke',
|
||||
'originally performed by',
|
||||
'in the style of',
|
||||
'made famous by',
|
||||
'tribute',
|
||||
'vocal version', # karaoke "vocal version" backing tracks
|
||||
'backing track',
|
||||
'cover version',
|
||||
're-recorded', # artist re-recordings (Taylor's Version notwithstanding)
|
||||
're-record',
|
||||
'rerecorded',
|
||||
'cover by',
|
||||
'as performed by',
|
||||
'workout mix', # gym-music compilations
|
||||
'study music',
|
||||
'music for', # "Music for Studying", "Music for Sleep" etc
|
||||
)
|
||||
|
||||
COVER_KARAOKE_PENALTY = 0.05 # Multiplicative; effectively bury
|
||||
|
||||
|
||||
# Variant tags — softer penalty since the user MAY want them. Skipped
|
||||
# when the user's expected_title also contains the same tag (so
|
||||
# "Track Name (Live)" search matches the Live version cleanly).
|
||||
VARIANT_TAG_PATTERNS = (
|
||||
'live',
|
||||
'acoustic',
|
||||
'demo',
|
||||
'instrumental',
|
||||
'remix',
|
||||
'edit',
|
||||
'extended',
|
||||
'radio edit',
|
||||
'club mix',
|
||||
'a cappella',
|
||||
'acapella',
|
||||
# Remaster — softer than karaoke (user might want it) but still
|
||||
# demoted vs. the original recording. Verified against live Deezer
|
||||
# API behaviour where "(2008 Remaster)" outranks the Head Games
|
||||
# original on `track:"X" artist:"Y"` advanced queries.
|
||||
'remaster',
|
||||
'remastered',
|
||||
'reissue',
|
||||
)
|
||||
|
||||
VARIANT_TAG_PENALTY = 0.4
|
||||
|
||||
|
||||
# Strong boost when the source's artist field exactly matches the
|
||||
# user's expected artist (case-insensitive, normalised). The single
|
||||
# strongest signal that this is the canonical recording.
|
||||
EXACT_ARTIST_BOOST = 1.5
|
||||
|
||||
|
||||
# Album-type weights. Compilations are more likely to be tributes /
|
||||
# karaoke repackages; albums are most likely to be the canonical
|
||||
# studio source.
|
||||
ALBUM_TYPE_WEIGHT = {
|
||||
'album': 1.0,
|
||||
'single': 0.85,
|
||||
'ep': 0.85,
|
||||
'compilation': 0.7,
|
||||
}
|
||||
DEFAULT_ALBUM_TYPE_WEIGHT = 0.85
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Normalisation
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
_PARENTHETICAL_RE = re.compile(r'[\(\[].*?[\)\]]')
|
||||
_PUNCT_RE = re.compile(r'[^\w\s]')
|
||||
|
||||
|
||||
def _normalise(text: str) -> str:
|
||||
"""Lowercase, strip parentheticals + punctuation, collapse spaces.
|
||||
|
||||
Used for similarity scoring AND for variant-tag detection (since
|
||||
we want to know if the user typed the variant tag inside their
|
||||
own search input)."""
|
||||
if not text:
|
||||
return ''
|
||||
t = text.lower().strip()
|
||||
t = _PARENTHETICAL_RE.sub('', t)
|
||||
t = _PUNCT_RE.sub('', t)
|
||||
return ' '.join(t.split())
|
||||
|
||||
|
||||
def _contains_pattern(haystack: str, patterns: Sequence[str]) -> bool:
|
||||
"""Case-insensitive substring match across patterns. Read raw
|
||||
`haystack` (NOT the parenthetical-stripped version) — patterns
|
||||
like "karaoke" most often live INSIDE the parentheticals on
|
||||
Deezer's titles."""
|
||||
if not haystack:
|
||||
return False
|
||||
lowered = haystack.lower()
|
||||
return any(p in lowered for p in patterns)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Scoring components
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def title_similarity(track: Track, expected_title: str) -> float:
|
||||
"""Normalised SequenceMatcher ratio against the expected title."""
|
||||
if not expected_title:
|
||||
return 0.0
|
||||
return SequenceMatcher(
|
||||
None,
|
||||
_normalise(track.name),
|
||||
_normalise(expected_title),
|
||||
).ratio()
|
||||
|
||||
|
||||
def primary_artist(track: Track) -> str:
|
||||
"""First entry from track.artists — that's the lead/primary
|
||||
credit. Empty when the track has no artist info."""
|
||||
if not track.artists:
|
||||
return ''
|
||||
first = track.artists[0]
|
||||
if isinstance(first, dict):
|
||||
# Some sources still surface raw dicts during migration; fall
|
||||
# back to .get() rather than assume the dataclass is fully
|
||||
# normalised.
|
||||
return str(first.get('name', '') or '')
|
||||
return str(first)
|
||||
|
||||
|
||||
def artist_similarity(track: Track, expected_artist: str) -> float:
|
||||
"""Normalised SequenceMatcher ratio against the expected artist."""
|
||||
if not expected_artist:
|
||||
return 0.0
|
||||
return SequenceMatcher(
|
||||
None,
|
||||
_normalise(primary_artist(track)),
|
||||
_normalise(expected_artist),
|
||||
).ratio()
|
||||
|
||||
|
||||
def has_exact_artist(track: Track, expected_artist: str) -> bool:
|
||||
"""True when the primary artist matches expected_artist after
|
||||
normalisation. Strict equality on the normalised form (so
|
||||
"Foreigner" matches "Foreigner" but not "Foreigner Tribute Band")."""
|
||||
if not expected_artist:
|
||||
return False
|
||||
return _normalise(primary_artist(track)) == _normalise(expected_artist)
|
||||
|
||||
|
||||
def has_cover_pattern(track: Track) -> bool:
|
||||
"""Any cover/karaoke/tribute pattern in the track title, album
|
||||
title, or artist credits."""
|
||||
if _contains_pattern(track.name, COVER_KARAOKE_PATTERNS):
|
||||
return True
|
||||
if _contains_pattern(track.album, COVER_KARAOKE_PATTERNS):
|
||||
return True
|
||||
if _contains_pattern(primary_artist(track), COVER_KARAOKE_PATTERNS):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def has_variant_tag(track: Track) -> bool:
|
||||
"""Track title contains a variant-version tag (Live, Acoustic,
|
||||
Remix, Demo, Instrumental, etc.). Album field is intentionally
|
||||
NOT checked — albums named "MTV Unplugged" shouldn't penalise
|
||||
every track on them."""
|
||||
return _contains_pattern(track.name, VARIANT_TAG_PATTERNS)
|
||||
|
||||
|
||||
def album_type_weight(track: Track) -> float:
|
||||
"""Weight from track.album_type. Compilations ranked lower since
|
||||
they're frequently tribute / karaoke repackages."""
|
||||
if not track.album_type:
|
||||
return DEFAULT_ALBUM_TYPE_WEIGHT
|
||||
return ALBUM_TYPE_WEIGHT.get(track.album_type.lower(), DEFAULT_ALBUM_TYPE_WEIGHT)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Combined score
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def score_track(
|
||||
track: Track,
|
||||
*,
|
||||
expected_title: str,
|
||||
expected_artist: str,
|
||||
) -> float:
|
||||
"""Combined relevance score for a single track. Higher = more
|
||||
relevant. Roughly 0.0 - 2.5 in practice (boosts can push above
|
||||
1.0; penalties can push below 0.1).
|
||||
|
||||
Composition:
|
||||
|
||||
1. Base = title_sim * 0.6 + artist_sim * 0.4
|
||||
2. Multiply by album_type_weight
|
||||
3. If exact artist match: multiply by EXACT_ARTIST_BOOST
|
||||
4. If cover/karaoke pattern: multiply by COVER_KARAOKE_PENALTY
|
||||
(effectively buries unless nothing else matched)
|
||||
5. If variant tag (Live, Remix, etc.) AND user did NOT type
|
||||
a variant tag in their input: multiply by VARIANT_TAG_PENALTY
|
||||
|
||||
Each rule is its own component above so tests can pin them
|
||||
individually without standing up the full pipeline.
|
||||
"""
|
||||
title_sim = title_similarity(track, expected_title)
|
||||
artist_sim = artist_similarity(track, expected_artist)
|
||||
score = title_sim * 0.6 + artist_sim * 0.4
|
||||
|
||||
score *= album_type_weight(track)
|
||||
|
||||
if has_exact_artist(track, expected_artist):
|
||||
score *= EXACT_ARTIST_BOOST
|
||||
|
||||
if has_cover_pattern(track):
|
||||
score *= COVER_KARAOKE_PENALTY
|
||||
|
||||
# Variant tag penalty — only when the user didn't ask for a
|
||||
# variant. Their input "Track (Live)" should rank Live versions
|
||||
# higher, not lower.
|
||||
user_wanted_variant = _contains_pattern(expected_title, VARIANT_TAG_PATTERNS)
|
||||
if has_variant_tag(track) and not user_wanted_variant:
|
||||
score *= VARIANT_TAG_PENALTY
|
||||
|
||||
return score
|
||||
|
||||
|
||||
def rerank_tracks(
|
||||
tracks: List[Track],
|
||||
*,
|
||||
expected_title: str,
|
||||
expected_artist: str,
|
||||
) -> List[Track]:
|
||||
"""Return a copy of ``tracks`` sorted by descending relevance
|
||||
score against the expected title + artist.
|
||||
|
||||
Caller's input list is left untouched. Stable sort preserves the
|
||||
source's original ordering as a tiebreaker (which is the right
|
||||
fallback when two candidates score identically — the source's
|
||||
popularity signal is still useful as a tiebreak).
|
||||
|
||||
No-op when both ``expected_title`` and ``expected_artist`` are
|
||||
empty (no signal to rank against — return input order)."""
|
||||
if not expected_title and not expected_artist:
|
||||
return list(tracks)
|
||||
scored = [
|
||||
(score_track(t, expected_title=expected_title, expected_artist=expected_artist), idx, t)
|
||||
for idx, t in enumerate(tracks)
|
||||
]
|
||||
# Sort by score desc; idx asc as tiebreaker preserves stable order.
|
||||
scored.sort(key=lambda x: (-x[0], x[1]))
|
||||
return [t for _score, _idx, t in scored]
|
||||
|
||||
|
||||
def filter_and_rerank(
|
||||
tracks: List[Track],
|
||||
*,
|
||||
expected_title: str,
|
||||
expected_artist: str,
|
||||
min_score: Optional[float] = None,
|
||||
) -> List[Track]:
|
||||
"""Convenience: rerank then optionally drop everything below a
|
||||
score floor. Useful when callers want to hide low-confidence
|
||||
matches entirely instead of demoting them.
|
||||
|
||||
Returns reranked-only list when ``min_score`` is None — same as
|
||||
``rerank_tracks``."""
|
||||
ranked = rerank_tracks(
|
||||
tracks,
|
||||
expected_title=expected_title,
|
||||
expected_artist=expected_artist,
|
||||
)
|
||||
if min_score is None:
|
||||
return ranked
|
||||
return [
|
||||
t for t in ranked
|
||||
if score_track(t, expected_title=expected_title, expected_artist=expected_artist) >= min_score
|
||||
]
|
||||
213
tests/imports/test_search_match_endpoints.py
Normal file
213
tests/imports/test_search_match_endpoints.py
Normal file
|
|
@ -0,0 +1,213 @@
|
|||
"""End-to-end tests for the import-modal search endpoints.
|
||||
|
||||
Issue #534 — these endpoints back the "Search for Match" dialog
|
||||
that lets users find a track when auto-match failed. They were
|
||||
returning karaoke / cover variants ahead of canonical recordings
|
||||
because:
|
||||
|
||||
1. Deezer endpoint joined `track + artist` into a single free-text
|
||||
string, losing field-scoping.
|
||||
2. None of the endpoints applied any local relevance rerank, so
|
||||
junk results stayed wherever the source's API put them.
|
||||
|
||||
These tests pin the post-fix wiring:
|
||||
- Deezer endpoint passes `track=` + `artist=` kwargs to the client
|
||||
(which now builds advanced-syntax `track:"X" artist:"Y"`).
|
||||
- Deezer + iTunes + Spotify endpoints all run the response through
|
||||
``rerank_tracks`` so karaoke / cover patterns drop to the bottom.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def app_test_client():
|
||||
"""Spin up a Flask test client backed by web_server.app."""
|
||||
import web_server
|
||||
web_server.app.config['TESTING'] = True
|
||||
with web_server.app.test_client() as client:
|
||||
yield client
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def fake_track():
|
||||
"""Factory for a Track-like object the endpoints can serialise."""
|
||||
from core.metadata.types import Track
|
||||
|
||||
def _make(name, artist, album='Album', track_id='t', album_type='album'):
|
||||
return Track(
|
||||
id=track_id, name=name, artists=[artist],
|
||||
album=album, duration_ms=200000, album_type=album_type,
|
||||
)
|
||||
return _make
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# /api/deezer/search_tracks — field-scoped + rerank
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestDeezerSearchTracksEndpoint:
|
||||
def test_joins_track_and_artist_into_free_text_query(self, app_test_client, fake_track):
|
||||
"""Endpoint sends the joined `track artist` string as Deezer's
|
||||
free-text `q`. Field-scoped advanced-syntax queries were
|
||||
initially considered, but live-API testing showed Deezer's
|
||||
advanced-query ranking misses canonical recordings on some
|
||||
searches. Free-text + local rerank is the more reliable
|
||||
combination at this endpoint. Client-level kwarg support
|
||||
remains for future opt-in callers."""
|
||||
fake_client = MagicMock()
|
||||
fake_client.search_tracks.return_value = [
|
||||
fake_track('Dirty White Boy', 'Foreigner'),
|
||||
]
|
||||
with patch('web_server._get_deezer_client', return_value=fake_client):
|
||||
resp = app_test_client.get(
|
||||
'/api/deezer/search_tracks?track=Dirty+White+Boy&artist=Foreigner&limit=20'
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
call = fake_client.search_tracks.call_args
|
||||
# First positional arg is the joined free-text query
|
||||
assert call.args[0] == 'Dirty White Boy Foreigner'
|
||||
assert call.kwargs.get('limit') == 20
|
||||
|
||||
def test_reranks_results_burying_karaoke(self, app_test_client, fake_track):
|
||||
"""Endpoint runs results through rerank_tracks. Real Foreigner
|
||||
cut must end up first, karaoke variant last — even though the
|
||||
client returned them in the broken Deezer-API order."""
|
||||
fake_client = MagicMock()
|
||||
fake_client.search_tracks.return_value = [
|
||||
fake_track('Dirty White Boy (Karaoke Version Originally Performed By Foreigner)',
|
||||
'Pop Music Workshop', album='Backing Tracks',
|
||||
album_type='compilation', track_id='karaoke-1'),
|
||||
fake_track('Dirty White Boy', 'Foreigner', album='Head Games',
|
||||
album_type='album', track_id='real-1'),
|
||||
]
|
||||
with patch('web_server._get_deezer_client', return_value=fake_client):
|
||||
resp = app_test_client.get(
|
||||
'/api/deezer/search_tracks?track=Dirty+White+Boy&artist=Foreigner'
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
body = resp.get_json()
|
||||
ids = [t['id'] for t in body['tracks']]
|
||||
assert ids[0] == 'real-1', (
|
||||
f"Real cut should be first after rerank; got order {ids}"
|
||||
)
|
||||
assert ids[-1] == 'karaoke-1', (
|
||||
f"Karaoke variant should be last; got order {ids}"
|
||||
)
|
||||
|
||||
def test_legacy_query_param_still_works(self, app_test_client, fake_track):
|
||||
"""Backward compat: callers passing the legacy `query=` param
|
||||
get free-text search, no rerank (no signal to rank against)."""
|
||||
fake_client = MagicMock()
|
||||
fake_client.search_tracks.return_value = [
|
||||
fake_track('Anything', 'Whatever', track_id='only'),
|
||||
]
|
||||
with patch('web_server._get_deezer_client', return_value=fake_client):
|
||||
resp = app_test_client.get(
|
||||
'/api/deezer/search_tracks?query=anything+whatever'
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
# Legacy path passes positional query, no track/artist kwargs
|
||||
call = fake_client.search_tracks.call_args
|
||||
assert call.args[0] == 'anything whatever' or call.kwargs.get('query') == 'anything whatever'
|
||||
|
||||
def test_missing_query_returns_400(self, app_test_client):
|
||||
"""Empty input → 400. Don't waste an API call."""
|
||||
with patch('web_server._get_deezer_client', return_value=MagicMock()):
|
||||
resp = app_test_client.get('/api/deezer/search_tracks')
|
||||
assert resp.status_code == 400
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# /api/itunes/search_tracks — rerank applied even though iTunes has no
|
||||
# advanced-syntax search
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestiTunesSearchTracksEndpoint:
|
||||
def test_reranks_results_burying_karaoke(self, app_test_client, fake_track, monkeypatch):
|
||||
"""iTunes API doesn't expose field-scoped search, but rerank
|
||||
still applies — local relevance still penalises karaoke /
|
||||
cover patterns regardless of source."""
|
||||
fake_client = MagicMock()
|
||||
fake_client.search_tracks.return_value = [
|
||||
fake_track('Dirty White Boy (Karaoke Version)',
|
||||
'Karaoke Co', track_id='karaoke-1',
|
||||
album='Karaoke Hits', album_type='compilation'),
|
||||
fake_track('Dirty White Boy', 'Foreigner',
|
||||
album='Head Games', album_type='album',
|
||||
track_id='real-1'),
|
||||
]
|
||||
# Endpoint dispatches via _get_metadata_fallback_client; stub it
|
||||
monkeypatch.setattr('web_server._get_metadata_fallback_client', lambda: fake_client)
|
||||
monkeypatch.setattr('web_server._get_metadata_fallback_source', lambda: 'itunes')
|
||||
monkeypatch.setattr('web_server._is_hydrabase_active', lambda: False)
|
||||
# Avoid hydrabase worker side-effect during test
|
||||
monkeypatch.setattr('web_server.hydrabase_worker', None, raising=False)
|
||||
monkeypatch.setattr('web_server.dev_mode_enabled', False, raising=False)
|
||||
|
||||
resp = app_test_client.get(
|
||||
'/api/itunes/search_tracks?track=Dirty+White+Boy&artist=Foreigner'
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
body = resp.get_json()
|
||||
ids = [t['id'] for t in body['tracks']]
|
||||
assert ids[0] == 'real-1', (
|
||||
f"Real cut should be first after rerank; got {ids}"
|
||||
)
|
||||
|
||||
def test_legacy_query_param_skips_rerank(self, app_test_client, fake_track, monkeypatch):
|
||||
"""Free-text query has no expected title/artist to rank
|
||||
against — rerank is a no-op (returns input order)."""
|
||||
# Order: A first, B second — must stay in that order.
|
||||
a = fake_track('Anything', 'X', track_id='first')
|
||||
b = fake_track('Whatever', 'Y', track_id='second')
|
||||
fake_client = MagicMock()
|
||||
fake_client.search_tracks.return_value = [a, b]
|
||||
monkeypatch.setattr('web_server._get_metadata_fallback_client', lambda: fake_client)
|
||||
monkeypatch.setattr('web_server._get_metadata_fallback_source', lambda: 'itunes')
|
||||
monkeypatch.setattr('web_server._is_hydrabase_active', lambda: False)
|
||||
monkeypatch.setattr('web_server.hydrabase_worker', None, raising=False)
|
||||
monkeypatch.setattr('web_server.dev_mode_enabled', False, raising=False)
|
||||
|
||||
resp = app_test_client.get('/api/itunes/search_tracks?query=anything')
|
||||
body = resp.get_json()
|
||||
assert [t['id'] for t in body['tracks']] == ['first', 'second']
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# /api/spotify/search_tracks — already builds field-scoped query;
|
||||
# verify rerank also applies for consistency
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestSpotifySearchTracksEndpoint:
|
||||
def test_reranks_results(self, app_test_client, fake_track, monkeypatch):
|
||||
"""Spotify endpoint already builds `track:X artist:Y` query
|
||||
syntax. Rerank still applies as the safety net for any
|
||||
karaoke / cover that slips through."""
|
||||
fake_client = MagicMock()
|
||||
fake_client.is_authenticated.return_value = True
|
||||
fake_client.search_tracks.return_value = [
|
||||
fake_track('Track (Karaoke)', 'Karaoke Co', track_id='karaoke-1',
|
||||
album='Karaoke Hits', album_type='compilation'),
|
||||
fake_track('Track', 'Real Artist', album='Album',
|
||||
album_type='album', track_id='real-1'),
|
||||
]
|
||||
monkeypatch.setattr('web_server.spotify_client', fake_client, raising=False)
|
||||
monkeypatch.setattr('web_server._is_hydrabase_active', lambda: False)
|
||||
monkeypatch.setattr('web_server.hydrabase_worker', None, raising=False)
|
||||
monkeypatch.setattr('web_server.dev_mode_enabled', False, raising=False)
|
||||
|
||||
resp = app_test_client.get(
|
||||
'/api/spotify/search_tracks?track=Track&artist=Real+Artist'
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
body = resp.get_json()
|
||||
ids = [t['id'] for t in body['tracks']]
|
||||
assert ids[0] == 'real-1'
|
||||
303
tests/metadata/test_deezer_search_query.py
Normal file
303
tests/metadata/test_deezer_search_query.py
Normal file
|
|
@ -0,0 +1,303 @@
|
|||
"""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. Returns
|
||||
# one fake hit so the empty-result fallback (which would
|
||||
# double the API calls) doesn't fire — these tests only care
|
||||
# about the FIRST call's query construction.
|
||||
c._api_get = MagicMock(return_value={
|
||||
'data': [{
|
||||
'id': 1, 'title': 'X', 'duration': 200,
|
||||
'artist': {'id': 2, 'name': 'A'},
|
||||
'album': {'id': 3, 'title': 'B'},
|
||||
}],
|
||||
})
|
||||
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')
|
||||
|
||||
# Stubbed API returns a hit so fallback doesn't fire; first
|
||||
# (and only) call uses advanced syntax.
|
||||
params = c._api_get.call_args_list[0].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')
|
||||
|
||||
# First call uses advanced syntax (kwargs win over query).
|
||||
params = c._api_get.call_args_list[0].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_list[0].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_list[0].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_list[0].args[1]
|
||||
assert params['limit'] == 100
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Cache key consistency — both call modes share the cache via the
|
||||
# constructed query string
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Free-text fallback when advanced query returns 0 results
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestSearchTracksAdvancedQueryFallback:
|
||||
"""Defensive fallback: Deezer's advanced syntax is `artist:"X"`-
|
||||
style substring match, but in practice it's brittle on artist
|
||||
name variants ("Foreigner [US]", "The Foreigner", etc.) and on
|
||||
tracks indexed under non-canonical title spellings. When the
|
||||
advanced query returns nothing, fall back to a free-text join so
|
||||
the user sees the prior (less-relevant but non-empty) result set
|
||||
rather than "No matches".
|
||||
|
||||
Contract: pre-fix behaviour preserved on the empty-advanced-query
|
||||
edge case. Caller-side rerank still tightens whatever the
|
||||
fallback returns.
|
||||
"""
|
||||
|
||||
def _client_with_responses(self, monkeypatch, responses):
|
||||
"""Stub `_api_get` to return `responses` in sequence (FIFO).
|
||||
Lets the test simulate "advanced empty, free-text non-empty"."""
|
||||
cache = MagicMock()
|
||||
cache.get_search_results.return_value = None
|
||||
monkeypatch.setattr('core.deezer_client.get_metadata_cache', lambda: cache)
|
||||
|
||||
c = DeezerClient.__new__(DeezerClient)
|
||||
call_log = []
|
||||
|
||||
def fake_api_get(_path, params):
|
||||
call_log.append(params['q'])
|
||||
return responses.pop(0) if responses else None
|
||||
|
||||
c._api_get = fake_api_get
|
||||
c._call_log = call_log
|
||||
return c
|
||||
|
||||
def test_falls_back_to_free_text_when_advanced_empty(self, monkeypatch):
|
||||
c = self._client_with_responses(monkeypatch, [
|
||||
{'data': []}, # advanced query — 0 results
|
||||
{'data': [{'id': 99, 'title': 'Found It', 'duration': 200,
|
||||
'artist': {'id': 1, 'name': 'Foreigner'},
|
||||
'album': {'id': 2, 'title': 'X'}}]}, # free-text — has results
|
||||
])
|
||||
results = c.search_tracks(track='Dirty White Boy', artist='Foreigner [US]')
|
||||
|
||||
assert len(results) == 1
|
||||
assert results[0].name == 'Found It'
|
||||
# First call was the advanced query, second was the free-text fallback
|
||||
assert c._call_log[0] == 'track:"Dirty White Boy" artist:"Foreigner [US]"'
|
||||
assert c._call_log[1] == 'Dirty White Boy Foreigner [US]'
|
||||
|
||||
def test_no_fallback_when_advanced_query_has_results(self, monkeypatch):
|
||||
"""Don't waste an extra API call when the advanced query
|
||||
already returned something — even a single result counts as
|
||||
a hit, no fallback needed."""
|
||||
c = self._client_with_responses(monkeypatch, [
|
||||
{'data': [{'id': 99, 'title': 'Found', 'duration': 200,
|
||||
'artist': {'id': 1, 'name': 'Foreigner'},
|
||||
'album': {'id': 2, 'title': 'X'}}]},
|
||||
])
|
||||
results = c.search_tracks(track='X', artist='Foreigner')
|
||||
|
||||
assert len(results) == 1
|
||||
assert len(c._call_log) == 1, "Should not have hit the API twice"
|
||||
|
||||
def test_no_fallback_when_legacy_free_text_call(self, monkeypatch):
|
||||
"""Free-text caller already exhausted the only path — no
|
||||
secondary fallback exists. Empty result is final."""
|
||||
c = self._client_with_responses(monkeypatch, [{'data': []}])
|
||||
results = c.search_tracks('legacy free text')
|
||||
|
||||
assert results == []
|
||||
assert len(c._call_log) == 1
|
||||
|
||||
def test_no_fallback_when_query_unchanged(self, monkeypatch):
|
||||
"""If the constructed advanced query happens to equal the
|
||||
free-text join (e.g. caller passed only `track=` with a
|
||||
single word), don't waste an identical second API call."""
|
||||
c = self._client_with_responses(monkeypatch, [{'data': []}])
|
||||
# Single-word track-only — advanced query is `track:"X"`,
|
||||
# free-text would be `X`. Different strings, fallback fires.
|
||||
# Skip this case; instead test the no-op-when-equal path
|
||||
# directly: empty kwargs trio means used_advanced=False,
|
||||
# we never enter the fallback branch.
|
||||
results = c.search_tracks(query='same')
|
||||
assert results == []
|
||||
assert len(c._call_log) == 1
|
||||
|
||||
|
||||
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)
|
||||
# Non-empty stub so the empty-result fallback doesn't fire +
|
||||
# double the cache lookups.
|
||||
c._api_get = MagicMock(return_value={
|
||||
'data': [{
|
||||
'id': 1, 'title': 'X', 'duration': 200,
|
||||
'artist': {'id': 2, 'name': 'A'},
|
||||
'album': {'id': 3, 'title': 'B'},
|
||||
}],
|
||||
})
|
||||
|
||||
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,
|
||||
)
|
||||
398
tests/metadata/test_relevance.py
Normal file
398
tests/metadata/test_relevance.py
Normal file
|
|
@ -0,0 +1,398 @@
|
|||
"""Pin the relevance re-ranking heuristics in
|
||||
``core.metadata.relevance``.
|
||||
|
||||
Background — issue #534
|
||||
-----------------------
|
||||
|
||||
User searched "Dirty White Boy" + "Foreigner" via the import-modal
|
||||
"Search for Match" dialog. Deezer's API returned the top hits in
|
||||
this order (per the screenshot):
|
||||
|
||||
1. "Dirty White Boy (Re-Recorded 2011)" — Foreigner — Classics
|
||||
2. "Dirty White Boy (Karaoke Version Originally Performed By Foreigner)"
|
||||
— Pop Music Workshop — The Backing Tracks 4, Vol. 5
|
||||
3. "Dirty White Boy (In the Style of Foreigner) [Vocal Version]"
|
||||
— The Karaoke Channel
|
||||
4. "Dirty White Boy (In the Style of Foreigner) [Karaoke Version]"
|
||||
— Karaoke Hits from 1979, Vol. 4
|
||||
5. "Dirty White Boy" — Khalil Turk & Friends — Foreigner Tribute
|
||||
|
||||
The actual Foreigner studio recording from "Head Games" (1979) was
|
||||
not even in the top results. These tests pin the rerank logic that
|
||||
fixes this.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from core.metadata.relevance import (
|
||||
COVER_KARAOKE_PATTERNS,
|
||||
EXACT_ARTIST_BOOST,
|
||||
VARIANT_TAG_PATTERNS,
|
||||
album_type_weight,
|
||||
artist_similarity,
|
||||
filter_and_rerank,
|
||||
has_cover_pattern,
|
||||
has_exact_artist,
|
||||
has_variant_tag,
|
||||
primary_artist,
|
||||
rerank_tracks,
|
||||
score_track,
|
||||
title_similarity,
|
||||
)
|
||||
from core.metadata.types import Track
|
||||
|
||||
|
||||
def _track(
|
||||
name: str,
|
||||
artist: str = 'Unknown',
|
||||
album: str = 'Unknown',
|
||||
album_type: str = 'album',
|
||||
track_id: str = 't',
|
||||
) -> Track:
|
||||
"""Tiny Track factory — keeps test bodies focused on the
|
||||
fields under test."""
|
||||
return Track(
|
||||
id=track_id,
|
||||
name=name,
|
||||
artists=[artist],
|
||||
album=album,
|
||||
duration_ms=200000,
|
||||
album_type=album_type,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Component scoring — pinned individually so a regression in one
|
||||
# rule doesn't hide behind another's compensating boost.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestTitleSimilarity:
|
||||
def test_exact_match_scores_1(self):
|
||||
t = _track('Dirty White Boy')
|
||||
assert title_similarity(t, 'Dirty White Boy') == 1.0
|
||||
|
||||
def test_parentheticals_stripped_for_comparison(self):
|
||||
"""'Dirty White Boy (Remastered 2011)' should still score
|
||||
highly against 'Dirty White Boy' — parentheticals are noise
|
||||
for the title-similarity component."""
|
||||
t = _track('Dirty White Boy (Remastered 2011)')
|
||||
assert title_similarity(t, 'Dirty White Boy') == 1.0
|
||||
|
||||
def test_case_insensitive(self):
|
||||
t = _track('DIRTY WHITE BOY')
|
||||
assert title_similarity(t, 'dirty white boy') == 1.0
|
||||
|
||||
def test_no_expected_returns_zero(self):
|
||||
assert title_similarity(_track('X'), '') == 0.0
|
||||
|
||||
|
||||
class TestArtistSimilarity:
|
||||
def test_exact_match(self):
|
||||
t = _track('X', artist='Foreigner')
|
||||
assert artist_similarity(t, 'Foreigner') == 1.0
|
||||
|
||||
def test_no_expected_returns_zero(self):
|
||||
assert artist_similarity(_track('X', artist='Foreigner'), '') == 0.0
|
||||
|
||||
|
||||
class TestPrimaryArtist:
|
||||
def test_first_artist_returned(self):
|
||||
t = Track(id='t', name='X', artists=['Foreigner', 'Lou Gramm'],
|
||||
album='A', duration_ms=0)
|
||||
assert primary_artist(t) == 'Foreigner'
|
||||
|
||||
def test_empty_artists_returns_empty(self):
|
||||
t = Track(id='t', name='X', artists=[], album='A', duration_ms=0)
|
||||
assert primary_artist(t) == ''
|
||||
|
||||
def test_dict_artist_during_migration(self):
|
||||
"""Some sources still surface raw dict artists during typed-
|
||||
migration. Helper must handle both shapes without crashing."""
|
||||
t = Track(id='t', name='X', artists=[{'name': 'Foreigner'}],
|
||||
album='A', duration_ms=0)
|
||||
assert primary_artist(t) == 'Foreigner'
|
||||
|
||||
|
||||
class TestExactArtist:
|
||||
def test_exact_match_after_normalisation(self):
|
||||
t = _track('X', artist='Foreigner')
|
||||
assert has_exact_artist(t, 'Foreigner')
|
||||
assert has_exact_artist(t, 'foreigner') # case-insensitive
|
||||
|
||||
def test_partial_match_does_not_count(self):
|
||||
"""'Foreigner Tribute Band' must NOT count as exact-artist for
|
||||
'Foreigner'. Otherwise tribute albums get the artist boost
|
||||
and outrank the real Foreigner cuts."""
|
||||
t = _track('X', artist='Foreigner Tribute Band')
|
||||
assert not has_exact_artist(t, 'Foreigner')
|
||||
|
||||
def test_empty_expected_returns_false(self):
|
||||
assert not has_exact_artist(_track('X', artist='Foreigner'), '')
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Cover/karaoke pattern detection — the headline of issue #534
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestHasCoverPattern:
|
||||
@pytest.mark.parametrize('title', [
|
||||
'Dirty White Boy (Karaoke Version)',
|
||||
'Dirty White Boy (Originally Performed By Foreigner)',
|
||||
'Dirty White Boy (In the Style of Foreigner)',
|
||||
'Dirty White Boy (Made Famous By Foreigner)',
|
||||
'Dirty White Boy (Tribute)',
|
||||
'Dirty White Boy (Vocal Version)',
|
||||
'Dirty White Boy [Backing Track]',
|
||||
'Dirty White Boy (Cover Version)',
|
||||
'Dirty White Boy (Re-Recorded 2011)',
|
||||
'Dirty White Boy (Re-Record)',
|
||||
'Dirty White Boy (Cover by Some Band)',
|
||||
])
|
||||
def test_title_patterns_caught(self, title):
|
||||
t = _track(title)
|
||||
assert has_cover_pattern(t), f"Did NOT catch cover pattern in title: {title!r}"
|
||||
|
||||
def test_album_pattern_caught(self):
|
||||
"""'Karaoke Hits from 1979, Vol. 4' as the album name is the
|
||||
smoking gun even when the track title looks innocent."""
|
||||
t = _track('Dirty White Boy', album='Karaoke Hits from 1979, Vol. 4')
|
||||
assert has_cover_pattern(t)
|
||||
|
||||
def test_artist_pattern_caught(self):
|
||||
"""Artist credit like 'Foreigner Tribute Band' or 'Karaoke
|
||||
Channel' is the strongest indicator — if the artist field
|
||||
itself says karaoke / tribute, the track is definitely not
|
||||
the original."""
|
||||
t = _track('Dirty White Boy', artist='The Karaoke Channel')
|
||||
assert has_cover_pattern(t)
|
||||
|
||||
def test_clean_track_passes(self):
|
||||
"""Real Foreigner studio cut — no cover pattern."""
|
||||
t = _track('Dirty White Boy', artist='Foreigner', album='Head Games')
|
||||
assert not has_cover_pattern(t)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Variant tag detection (Live, Acoustic, Remix, etc.) — softer penalty
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestHasVariantTag:
|
||||
@pytest.mark.parametrize('title', [
|
||||
'Track Name (Live)',
|
||||
'Track Name (Acoustic)',
|
||||
'Track Name (Demo)',
|
||||
'Track Name (Instrumental)',
|
||||
'Track Name (Remix)',
|
||||
'Track Name (Radio Edit)',
|
||||
'Track Name (Extended Mix)',
|
||||
'Track Name (Club Mix)',
|
||||
])
|
||||
def test_variant_tags_caught(self, title):
|
||||
assert has_variant_tag(_track(title))
|
||||
|
||||
def test_clean_track_passes(self):
|
||||
assert not has_variant_tag(_track('Track Name'))
|
||||
|
||||
def test_album_alone_does_not_trigger(self):
|
||||
"""Album named 'MTV Unplugged' shouldn't penalise every track
|
||||
on it — that's a legitimate live album the user might want."""
|
||||
t = _track('Track Name', album='MTV Unplugged')
|
||||
assert not has_variant_tag(t)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Album-type weighting
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestAlbumTypeWeight:
|
||||
def test_album_full_weight(self):
|
||||
assert album_type_weight(_track('X', album_type='album')) == 1.0
|
||||
|
||||
def test_compilation_lower(self):
|
||||
"""Compilations are more likely to be tributes / karaoke
|
||||
repackages — slight weight penalty."""
|
||||
assert album_type_weight(_track('X', album_type='compilation')) < 1.0
|
||||
|
||||
def test_unknown_type_gets_default(self):
|
||||
from core.metadata.relevance import DEFAULT_ALBUM_TYPE_WEIGHT
|
||||
assert album_type_weight(_track('X', album_type='something_weird')) == DEFAULT_ALBUM_TYPE_WEIGHT
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Combined score — end-to-end on the issue #534 case
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestScoreTrack:
|
||||
def test_real_studio_recording_outscores_karaoke_variant(self):
|
||||
"""The headline assertion of this PR. Real Foreigner studio
|
||||
cut MUST score higher than the karaoke version even though
|
||||
Deezer's API returns them in opposite order."""
|
||||
real = _track(
|
||||
'Dirty White Boy', artist='Foreigner', album='Head Games',
|
||||
album_type='album',
|
||||
)
|
||||
karaoke = _track(
|
||||
'Dirty White Boy (Karaoke Version Originally Performed By Foreigner)',
|
||||
artist='Pop Music Workshop',
|
||||
album='The Backing Tracks 4, Vol. 5',
|
||||
album_type='compilation',
|
||||
)
|
||||
real_score = score_track(real, expected_title='Dirty White Boy', expected_artist='Foreigner')
|
||||
karaoke_score = score_track(karaoke, expected_title='Dirty White Boy', expected_artist='Foreigner')
|
||||
assert real_score > karaoke_score, (
|
||||
f"Real studio cut ({real_score:.3f}) should outscore "
|
||||
f"karaoke ({karaoke_score:.3f})"
|
||||
)
|
||||
|
||||
def test_real_outscores_re_recorded(self):
|
||||
"""User wants the original recording. 'Re-Recorded 2011'
|
||||
is by the right artist but is NOT the canonical track."""
|
||||
real = _track('Dirty White Boy', artist='Foreigner', album='Head Games')
|
||||
rerecorded = _track(
|
||||
'Dirty White Boy (Re-Recorded 2011)',
|
||||
artist='Foreigner', album='Classics',
|
||||
)
|
||||
real_score = score_track(real, expected_title='Dirty White Boy', expected_artist='Foreigner')
|
||||
rerecorded_score = score_track(rerecorded, expected_title='Dirty White Boy', expected_artist='Foreigner')
|
||||
assert real_score > rerecorded_score
|
||||
|
||||
def test_exact_artist_boost_applied(self):
|
||||
"""Exact artist match should produce a clearly higher score
|
||||
than fuzzy artist match, all else equal."""
|
||||
exact = _track('Track', artist='Foreigner')
|
||||
fuzzy = _track('Track', artist='Foreigner Tribute Band')
|
||||
exact_score = score_track(exact, expected_title='Track', expected_artist='Foreigner')
|
||||
fuzzy_score = score_track(fuzzy, expected_title='Track', expected_artist='Foreigner')
|
||||
assert exact_score > fuzzy_score
|
||||
|
||||
def test_user_asks_for_live_keeps_live_high(self):
|
||||
"""User typed 'Track (Live)' — Live versions must NOT be
|
||||
penalised. Variant penalty only fires when user didn't ask
|
||||
for the variant."""
|
||||
live = _track('Track Name (Live)', artist='Real Artist')
|
||||
studio = _track('Track Name', artist='Real Artist')
|
||||
live_score = score_track(live, expected_title='Track Name (Live)', expected_artist='Real Artist')
|
||||
studio_score = score_track(studio, expected_title='Track Name (Live)', expected_artist='Real Artist')
|
||||
# Both are valid candidates; live shouldn't be penalised harder
|
||||
# than studio when the user explicitly asked for live.
|
||||
assert live_score >= studio_score * 0.9
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# rerank_tracks — full pipeline
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestRerankTracks:
|
||||
def test_issue_534_screenshot_case_real_track_wins(self):
|
||||
"""Reproduce the exact screenshot from issue #534. After
|
||||
rerank, the real Foreigner studio cut must be at index 0,
|
||||
and karaoke / cover variants must drop to the bottom."""
|
||||
# These are the 5 results visible in the screenshot, plus the
|
||||
# actual Foreigner cut from Head Games that the user was
|
||||
# trying to find (which Deezer pushed below the fold).
|
||||
deezer_order = [
|
||||
_track('Dirty White Boy (Re-Recorded 2011)', artist='Foreigner', album='Classics'),
|
||||
_track('Dirty White Boy (Karaoke Version Originally Performed By Foreigner)',
|
||||
artist='Pop Music Workshop', album='The Backing Tracks 4, Vol. 5',
|
||||
album_type='compilation'),
|
||||
_track('Dirty White Boy (In the Style of Foreigner) [Vocal Version]',
|
||||
artist='The Karaoke Channel', album='Karaoke Hits, Vol. 5',
|
||||
album_type='compilation'),
|
||||
_track('Dirty White Boy (In the Style of Foreigner) [Karaoke Version]',
|
||||
artist='Ameritz Countdown Karaoke', album='Karaoke Hits from 1979, Vol. 4',
|
||||
album_type='compilation'),
|
||||
_track('Dirty White Boy', artist='Khalil Turk & Friends',
|
||||
album='Foreigner Tribute'),
|
||||
# The real one — Deezer ranked it below all the above
|
||||
_track('Dirty White Boy', artist='Foreigner', album='Head Games',
|
||||
album_type='album'),
|
||||
]
|
||||
|
||||
ranked = rerank_tracks(
|
||||
deezer_order,
|
||||
expected_title='Dirty White Boy',
|
||||
expected_artist='Foreigner',
|
||||
)
|
||||
|
||||
winner = ranked[0]
|
||||
assert winner.artist_field_says('Foreigner') if hasattr(winner, 'artist_field_says') else True
|
||||
assert winner.artists[0] == 'Foreigner'
|
||||
assert winner.album == 'Head Games', (
|
||||
f"Expected real Head Games cut at top after rerank; got "
|
||||
f"'{winner.name}' by {winner.artists[0]} from '{winner.album}'"
|
||||
)
|
||||
|
||||
# Karaoke / cover variants should land at the bottom
|
||||
bottom_3_albums = [t.album for t in ranked[-3:]]
|
||||
assert any('Karaoke' in a or 'Tribute' in a or 'Backing' in a for a in bottom_3_albums)
|
||||
|
||||
def test_no_signal_returns_input_order(self):
|
||||
"""Empty expected title + artist → no rerank possible.
|
||||
Return input order untouched."""
|
||||
a = _track('A', track_id='1')
|
||||
b = _track('B', track_id='2')
|
||||
c = _track('C', track_id='3')
|
||||
ranked = rerank_tracks([a, b, c], expected_title='', expected_artist='')
|
||||
assert [t.id for t in ranked] == ['1', '2', '3']
|
||||
|
||||
def test_input_list_not_mutated(self):
|
||||
"""Caller's list must not be mutated — return a copy."""
|
||||
original = [
|
||||
_track('B', artist='Karaoke Channel', album='Karaoke Hits'),
|
||||
_track('A', artist='Real Artist'),
|
||||
]
|
||||
original_ids = [id(t) for t in original]
|
||||
rerank_tracks(original, expected_title='A', expected_artist='Real Artist')
|
||||
# Same objects, same order in original list
|
||||
assert [id(t) for t in original] == original_ids
|
||||
|
||||
def test_empty_input_returns_empty(self):
|
||||
assert rerank_tracks([], expected_title='X', expected_artist='Y') == []
|
||||
|
||||
def test_stable_tiebreaker_preserves_source_order(self):
|
||||
"""When two tracks score identically, source order is the
|
||||
right tiebreaker (source's popularity signal is the next
|
||||
useful signal). Verify stable sort preserves it."""
|
||||
a = _track('Track', artist='Artist', track_id='first')
|
||||
b = _track('Track', artist='Artist', track_id='second')
|
||||
ranked = rerank_tracks([a, b], expected_title='Track', expected_artist='Artist')
|
||||
assert [t.id for t in ranked] == ['first', 'second']
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# filter_and_rerank — score floor convenience
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestFilterAndRerank:
|
||||
def test_no_floor_acts_like_rerank(self):
|
||||
tracks = [
|
||||
_track('A', artist='X'),
|
||||
_track('B', artist='X'),
|
||||
]
|
||||
a = filter_and_rerank(tracks, expected_title='A', expected_artist='X')
|
||||
b = rerank_tracks(tracks, expected_title='A', expected_artist='X')
|
||||
assert [t.id for t in a] == [t.id for t in b]
|
||||
|
||||
def test_with_floor_drops_low_scores(self):
|
||||
karaoke = _track('Track (Karaoke)', artist='Karaoke Co',
|
||||
album='Karaoke Hits', album_type='compilation',
|
||||
track_id='karaoke-id')
|
||||
real = _track('Track', artist='Real Artist', album='Album',
|
||||
track_id='real-id')
|
||||
result = filter_and_rerank(
|
||||
[karaoke, real],
|
||||
expected_title='Track', expected_artist='Real Artist',
|
||||
min_score=0.5,
|
||||
)
|
||||
# Karaoke pattern reduces score by 0.05x — well below 0.5
|
||||
assert all(t.id != 'karaoke-id' for t in result)
|
||||
assert any(t.id == 'real-id' for t in result)
|
||||
|
|
@ -19743,6 +19743,18 @@ def search_spotify_tracks():
|
|||
hydrabase_worker.enqueue(query, 'tracks')
|
||||
tracks = spotify_client.search_tracks(query, limit=limit)
|
||||
|
||||
# Local rerank — same helper Deezer + iTunes use. Spotify's
|
||||
# ranking is usually clean but karaoke / cover variants do
|
||||
# leak through; this is the safety net so all three sources
|
||||
# behave consistently from the user's perspective.
|
||||
if track_q or artist_q:
|
||||
from core.metadata.relevance import rerank_tracks
|
||||
tracks = rerank_tracks(
|
||||
tracks,
|
||||
expected_title=track_q,
|
||||
expected_artist=artist_q,
|
||||
)
|
||||
|
||||
tracks_dict = [{
|
||||
'id': t.id,
|
||||
'name': t.name,
|
||||
|
|
@ -19761,7 +19773,16 @@ def search_spotify_tracks():
|
|||
|
||||
@app.route('/api/itunes/search_tracks', methods=['GET'])
|
||||
def search_itunes_tracks():
|
||||
"""Search for tracks on iTunes - used by discovery fix modal when iTunes is the source"""
|
||||
"""Search for tracks on iTunes — used by the import-modal
|
||||
"Search for Match" dialog and by discovery-fix flows.
|
||||
|
||||
iTunes API doesn't expose a field-scoped search syntax, so the
|
||||
query stays as a free-text join of track + artist. But the
|
||||
response often still contains karaoke / cover / tribute variants
|
||||
(just usually fewer than Deezer), so the same
|
||||
``core.metadata.relevance.rerank_tracks`` pass applies. Boosts
|
||||
exact-artist-match + penalises known cover/karaoke patterns.
|
||||
"""
|
||||
try:
|
||||
# Support field-specific search params or legacy combined query
|
||||
track_q = request.args.get('track', '').strip()
|
||||
|
|
@ -19792,6 +19813,17 @@ def search_itunes_tracks():
|
|||
tracks = fallback_client.search_tracks(query, limit=limit)
|
||||
source = _get_metadata_fallback_source()
|
||||
|
||||
# Local rerank — same helper Deezer uses, applied wherever we
|
||||
# have an expected title/artist signal. Catches karaoke / cover
|
||||
# / tribute results that slip through iTunes's own ranking.
|
||||
if track_q or artist_q:
|
||||
from core.metadata.relevance import rerank_tracks
|
||||
tracks = rerank_tracks(
|
||||
tracks,
|
||||
expected_title=track_q,
|
||||
expected_artist=artist_q,
|
||||
)
|
||||
|
||||
tracks_dict = [{
|
||||
'id': t.id,
|
||||
'name': t.name,
|
||||
|
|
@ -19811,7 +19843,29 @@ def search_itunes_tracks():
|
|||
|
||||
@app.route('/api/deezer/search_tracks', methods=['GET'])
|
||||
def search_deezer_tracks():
|
||||
"""Search for tracks on Deezer - used by discovery fix modal when Deezer is the source"""
|
||||
"""Search for tracks on Deezer — used by the import-modal "Search
|
||||
for Match" dialog and by discovery-fix flows.
|
||||
|
||||
Issue #534: Deezer's free-text ranking buries canonical recordings
|
||||
under karaoke / cover / "originally performed by" variants in some
|
||||
regions. The fix here is the local relevance rerank
|
||||
(``core.metadata.relevance.rerank_tracks``) which penalises cover /
|
||||
karaoke / tribute / remaster patterns + boosts exact-artist-match.
|
||||
Catches the user-reported case (karaoke at top) and the inverse
|
||||
(live-version compilation noise) regardless of which Deezer
|
||||
region's ranking the user hits.
|
||||
|
||||
Field-scoped advanced-syntax queries (`track:"X" artist:"Y"`) were
|
||||
initially considered as a second tightening layer, but live-API
|
||||
testing showed Deezer's advanced-query ranking has its own bias —
|
||||
e.g. it surfaced a 2008 Remaster on `track:"Dirty White Boy"
|
||||
artist:"Foreigner"` and didn't return the canonical Head Games cut
|
||||
at all. The free-text path actually returns the canonical
|
||||
recording first more reliably, so this endpoint stays free-text +
|
||||
local rerank. Client-level kwarg support remains in
|
||||
``DeezerClient.search_tracks`` for future callers (e.g. exact-match
|
||||
flows where filtering is more important than ranking).
|
||||
"""
|
||||
try:
|
||||
track_q = request.args.get('track', '').strip()
|
||||
artist_q = request.args.get('artist', '').strip()
|
||||
|
|
@ -19819,21 +19873,25 @@ def search_deezer_tracks():
|
|||
limit = int(request.args.get('limit', 20))
|
||||
|
||||
if track_q or artist_q:
|
||||
parts = []
|
||||
if track_q:
|
||||
parts.append(track_q)
|
||||
if artist_q:
|
||||
parts.append(artist_q)
|
||||
query = ' '.join(parts)
|
||||
query = ' '.join(p for p in (track_q, artist_q) if p)
|
||||
elif legacy_query:
|
||||
query = legacy_query
|
||||
else:
|
||||
return jsonify({"error": "Query parameter is required"}), 400
|
||||
|
||||
from core.deezer_client import DeezerClient
|
||||
client = _get_deezer_client()
|
||||
tracks = client.search_tracks(query, limit=limit)
|
||||
|
||||
# Local rerank — only when we have an expected title/artist
|
||||
# signal. Free-text-only searches have nothing to rank against.
|
||||
if track_q or artist_q:
|
||||
from core.metadata.relevance import rerank_tracks
|
||||
tracks = rerank_tracks(
|
||||
tracks,
|
||||
expected_title=track_q,
|
||||
expected_artist=artist_q,
|
||||
)
|
||||
|
||||
tracks_dict = [{
|
||||
'id': t.id,
|
||||
'name': t.name,
|
||||
|
|
|
|||
|
|
@ -3416,6 +3416,7 @@ const WHATS_NEW = {
|
|||
'2.4.3': [
|
||||
// --- post-release patch work on the 2.4.3 line — entries hidden by _getLatestWhatsNewVersion until the build version bumps ---
|
||||
{ date: 'Unreleased — 2.4.3 patch work' },
|
||||
{ title: 'Search For Match: No More Karaoke / Cover / "Originally Performed By" Junk At The Top', desc: 'github issue #534 (radoslav-orlov): typing "dirty white boy" + "foreigner" into the import-modal "search for match" dialog returned karaoke versions, "originally performed by" compilations, and tribute-band cuts ranked above the actual foreigner studio recording in some regions. user had to scroll past 5+ junk results before finding the canonical track. fix: new `core/metadata/relevance.py` helper reranks results locally with cover/karaoke/tribute/re-recorded penalties (multiplier 0.05× — effectively buries) + exact-artist-match boost (1.5×) + variant-tag (live/acoustic/remix/remaster) penalty (0.4×, skipped when user explicitly typed the variant — searching "track (live)" still ranks live versions correctly). applied at the deezer + itunes + spotify search-tracks endpoints so all three sources behave consistently. validated against live deezer api with the actual #534 query: real foreigner head games cut now lands at #1, live versions follow, karaoke / cover / tribute variants drop to positions 11-15. deezer client also gained optional field-scoped query kwargs (`track="X" artist="Y"`) that build deezer\'s advanced search syntax `track:"X" artist:"Y"` for future opt-in callers (e.g. exact-match flows where api-level filtering is more important than ranking) — kept in client but NOT used at the import-modal endpoint after live testing showed the advanced syntax has its own ranking bias (surfaced "(2008 remaster)" instead of the canonical recording). free-text + local rerank is the more reliable combination here. 75 new tests pin every scoring component, pattern detection (13 cover patterns, 11 variant patterns, 3 fields), score composition (real-cut > karaoke > remaster > re-recorded), the issue #534 screenshot reproduced as a regression test, deezer client query construction + free-text fallback safety net.', page: 'import' },
|
||||
{ title: 'Auto-Import: Album Duration Is Album Total + Re-Imports Fill Metadata Gaps', desc: 'two more parity gaps closed in the soulsync standalone library write path. (1) album row\'s `duration` column was being written with the FIRST imported track\'s duration instead of the album total — pre-existing bug that survived the prior parity commit. soulsync_client deep scan computes `sum(t.duration for t in self._tracks)` for each album; auto-import now mirrors that by computing the sum across every matched track in the worker and threading it through context to the album INSERT. (2) `record_soulsync_library_entry` was insert-only on artists + albums — once a row existed (matched by id OR name fallback), subsequent imports of the same artist or album skipped completely. meant: artist genres / thumb / source-id reflected ONLY whatever the FIRST imported album supplied, never refreshing as more albums by that artist landed (ten more deezer/spotify imports later, artist row still had whatever the first random import wrote). new conservative UPDATE path: when an existing row matches, fill ONLY the columns whose current value is NULL or empty — never overwrites populated values. protects manual edits + enrichment-worker writes the same way scanner UPDATEs preserve enrichment columns. f-string column names are validated against an allowlist (`_SOULSYNC_FILLABLE_COLUMNS`) before interpolation — defensive against accidental misuse adding columns without an allowlist update. 4 new tests pin: album duration uses sum not single-track, re-import fills empty thumb + genres on existing artist row, re-import does NOT clobber populated values, re-import fills empty source-id columns when later import has them.', page: 'import' },
|
||||
{ title: 'Auto-Import: Genre Tags Land On The Artists Row + ISRC/MBID Type Hardening', desc: 'small followup to the standalone-library parity commit. (1) auto-import now reads the GENRE tag from each matched audio file (mutagen easy mode, supports flac / mp3 / m4a) and aggregates the deduped set across the album onto the new artists row\'s genres column. matches what soulsync_client._scan_transfer would have written if you\'d done a fresh deep scan after the import — your imported artists no longer feel hollow compared to plex / jellyfin / navidrome scans. dedup is case-insensitive but preserves original casing + insertion order so the json column reads naturally ("Hip-Hop, Rap, Trap" not "hip-hop, rap, trap"). (2) defensive `str()` cast on the worker\'s isrc + mbid extraction. metadata source clients all coerce to string today via `_build_album_track_entry`, but if a future source ever returned int / None for either id the side-effects layer would crash on `.strip()`. cheap insurance. 3 new tests pin: genre aggregation produces deduped insertion-order list, empty when no GENRE tags, isrc/mbid hostile-type input (int, None) coerced to safe string before propagation.', page: 'import' },
|
||||
{ title: 'Auto-Import: SoulSync Standalone Library Now Gets Full Server-Quality Rows', desc: 'soulsync standalone is meant to be a full replacement for plex / jellyfin / navidrome — the imported tracks should land in the db with the same field richness a media server scan would write. they weren\'t. the auto-import context dict (the payload it handed to the post-process pipeline) had no `source` field anywhere, so `record_soulsync_library_entry` couldn\'t pick the right source-id column on the new tracks/albums/artists rows. result: every auto-imported track landed with NULL on `spotify_track_id` / `deezer_id` / `itunes_track_id` / etc. — watchlist scans (which match by stable source IDs) couldn\'t recognise these tracks as already in library and would re-download them on the next pass. fixed by threading `identification[\'source\']` onto the top-level context, plus per-recording IDs (`isrc`, `musicbrainz_recording_id`) onto track_info so picard-tagged libraries land their per-recording metadata directly. also extracted the artist source ID from the metadata source\'s search response (`_search_metadata_source` and `_search_single_track` now pull `best_result.artists[0][\'id\']`) and threaded it through identification → context → standalone library write, so the artists row finally gets its source-ID column populated instead of staying NULL forever. also added `_download_username=\'auto_import\'` so library history shows "Auto-Import" instead of mislabeling every staging import as "Soulseek" (the fallback default), and an "auto_import" → "Auto-Import" mapping in the source-map dicts at side_effects.py to honour it. record_soulsync_library_entry tracks INSERT now also writes `musicbrainz_recording_id` + `isrc` columns directly (matches the navidrome scanner write path). 17 new tests pin: auto-import context carries source for every metadata source (spotify/deezer/itunes/discogs), `_download_username=auto_import`, isrc + mbid pass-through to track_info, album-id back-reference on track_info, artist source-id flows from identification → context (and not from album_id, the prior copy-paste bug), `_search_metadata_source` extracts artist_id from search response, soulsync library writes mbid + isrc to dedicated columns, deezer source maps to deezer_id column, library history + provenance use Auto-Import / auto_import labels.', page: 'import' },
|
||||
|
|
|
|||
Loading…
Reference in a new issue