Add query-shortening retry + qualifier guard to Tidal search
Tidal's search engine chokes on long queries with multiple qualifier
words (remix credits, edit labels, bonus-disc markers). User reported
case: "maduk transformations remixed fire away fred v remix" returns 0,
but shortening to "maduk transformations remixed fire away" works.
Behaviour change:
- On a 0-result search, retry with progressively-shortened variants
(capped at 5 total attempts, 100ms pause between).
- Variants (in priority order):
1. strip trailing "(...)" / "[...]"
2. strip all parentheticals/brackets
3-5. drop last 1 / 2 / 3 tokens
6. keep first half of tokens (rounded up)
- Dedupes so identical variants don't re-query.
Safety — qualifier-aware filter:
- Variant keywords (Live / Remix / Acoustic / Extended / Unplugged /
Instrumental / Karaoke / etc.) are extracted from the original query
using word-boundary match so "edit" doesn't match "edition" and
"mix" doesn't match "remixed".
- If the original query carries any qualifiers, fallback results MUST
contain those qualifiers in their track names — otherwise a shortened
query could silently downgrade "Song (Live)" to the studio "Song".
- Tracks that fail the filter are dropped. If no variant produces
qualifier-matching tracks, returns ([], []) — the same outcome as the
original code, so no regression.
Contract preservation:
- Never raises to caller (outer try/except catches orchestration errors).
- Returns ([], []) on any failure path, same as original.
- Original-query successes take the same code path as before — no
behavioural change for queries that already work.
- Defensive guards for None/empty/non-string query (early return).
Logging:
- Preserves original warning/error/info messages for back-compat log
scraping.
- Adds fallback-success INFO log ("Tidal fallback query succeeded: ...")
so successful retries are visible in production logs.
- Adds qualifier-filter INFO/DEBUG logs with kept/total counts.
- Per-attempt exception logs at DEBUG (not ERROR) to avoid noise when
retries succeed.
- Traceback preserved on final failure.
Tests (16 regression tests in tests/test_tidal_search_shortening.py):
- Skowl's reported query reaches his working variant within the cap.
- Paren/bracket stripping priority.
- Short queries produce no variants.
- All variants unique (dedup guard).
- Progressive token drops present for long queries.
- Qualifier extraction is word-bounded (no "edit" in "edition").
- Qualifier extraction is case-insensitive.
- Track name filter requires ALL qualifiers.
- Empty-qualifier list passes every track (original-query behaviour).
All 292 tests pass.
This commit is contained in:
Broque Thomas
parent
47ced912b9
commit
0d0bbf38c9
2 changed files with 358 additions and 6 deletions
|
|
@ -244,6 +244,98 @@ class TidalDownloadClient:
|
|||
logger.error(f"Tidal connection check failed: {e}")
|
||||
return False
|
||||
|
||||
# Words that distinguish a specific audio variant from the original track.
|
||||
# If any of these appear in a query, the fallback-retry results must also
|
||||
# contain them — otherwise we'd silently downgrade a "(Live)" or
|
||||
# "(Acoustic)" search to the studio version when shortened queries match
|
||||
# too broadly.
|
||||
_QUALIFIER_KEYWORDS = frozenset({
|
||||
'remix', 'mix', 'edit', 'version', 'dub', 'rmx', 'vip', 'cut',
|
||||
'rework', 'bootleg', 'flip',
|
||||
'live', 'concert', 'unplugged', 'acoustic', 'session',
|
||||
'instrumental', 'karaoke', 'demo', 'bonus',
|
||||
'extended', 'radio',
|
||||
})
|
||||
|
||||
@classmethod
|
||||
def _extract_qualifiers(cls, query: str) -> List[str]:
|
||||
"""Return the qualifier keywords that appear as whole words in the
|
||||
query (case-insensitive). Word-boundary match prevents false hits like
|
||||
"edit" matching "edition" or "mix" matching "remix"."""
|
||||
if not query:
|
||||
return []
|
||||
found = []
|
||||
q_lower = query.lower()
|
||||
for kw in cls._QUALIFIER_KEYWORDS:
|
||||
if re.search(r'\b' + re.escape(kw) + r'\b', q_lower):
|
||||
found.append(kw)
|
||||
return found
|
||||
|
||||
@staticmethod
|
||||
def _track_name_contains_qualifiers(track_name: str, qualifiers: List[str]) -> bool:
|
||||
"""True iff the track name contains every qualifier as a whole word."""
|
||||
if not qualifiers:
|
||||
return True
|
||||
if not track_name:
|
||||
return False
|
||||
name_lower = track_name.lower()
|
||||
for kw in qualifiers:
|
||||
if not re.search(r'\b' + re.escape(kw) + r'\b', name_lower):
|
||||
return False
|
||||
return True
|
||||
|
||||
@staticmethod
|
||||
def _generate_shortened_queries(original: str) -> List[str]:
|
||||
"""Generate progressively-shorter variants of a search query.
|
||||
|
||||
Tidal's search engine chokes on long queries with lots of qualifiers
|
||||
(remix credits, edit labels, bonus-disc markers). When the original
|
||||
returns 0 results, we retry with shortened variants in order of
|
||||
conservativeness — the first variant that returns results wins.
|
||||
|
||||
Variants are returned in priority order. Dedupes against the original
|
||||
and against previously-added variants so we never issue duplicate
|
||||
requests.
|
||||
"""
|
||||
variants: List[str] = []
|
||||
seen = {original.strip().lower()}
|
||||
|
||||
def _add(candidate: str) -> None:
|
||||
candidate = candidate.strip()
|
||||
if candidate and candidate.lower() not in seen:
|
||||
variants.append(candidate)
|
||||
seen.add(candidate.lower())
|
||||
|
||||
# 1. Strip a trailing parenthetical/bracketed suffix.
|
||||
# "Song (Radio Edit)" → "Song"
|
||||
_add(re.sub(r'\s*[\(\[][^\)\]]*[\)\]]\s*$', '', original))
|
||||
|
||||
# 2. Strip ALL parentheticals/brackets (mid-string too).
|
||||
# "Song (feat X) [Remix]" → "Song"
|
||||
_add(re.sub(r'\s*[\(\[][^\)\]]*[\)\]]', ' ', original))
|
||||
|
||||
tokens = original.split()
|
||||
|
||||
# 3. Drop the last token — covers trailing 1-word modifiers
|
||||
# (e.g. "… Remix", "… Extended").
|
||||
if len(tokens) >= 3:
|
||||
_add(' '.join(tokens[:-1]))
|
||||
|
||||
# 4. Drop the last two tokens.
|
||||
if len(tokens) >= 4:
|
||||
_add(' '.join(tokens[:-2]))
|
||||
|
||||
# 5. Drop the last three tokens — covers "fred v remix" style
|
||||
# 3-word modifiers common in remix/bonus track names.
|
||||
if len(tokens) >= 5:
|
||||
_add(' '.join(tokens[:-3]))
|
||||
|
||||
# 6. Aggressive: keep roughly the first half (rounded up).
|
||||
if len(tokens) >= 7:
|
||||
_add(' '.join(tokens[:len(tokens) // 2 + 1]))
|
||||
|
||||
return variants
|
||||
|
||||
async def search(self, query: str, timeout: int = None, progress_callback=None) -> Tuple[List[TrackResult], List[AlbumResult]]:
|
||||
"""
|
||||
Search Tidal for tracks (async, Soulseek-compatible interface).
|
||||
|
|
@ -255,21 +347,117 @@ class TidalDownloadClient:
|
|||
logger.warning("Tidal not available for search (not authenticated)")
|
||||
return ([], [])
|
||||
|
||||
# Defensive guard — None/empty query would blow up the shortener's
|
||||
# .strip() call. Match the original behaviour (log + empty tuple).
|
||||
if not query or not isinstance(query, str):
|
||||
logger.warning(f"Invalid Tidal search query: {query!r}")
|
||||
return ([], [])
|
||||
|
||||
logger.info(f"Searching Tidal for: {query}")
|
||||
|
||||
# Outer try/except preserves the original contract: any unexpected
|
||||
# error returns ([], []) so the caller can fall back to other sources
|
||||
# instead of crashing. Traceback is logged once, not per-attempt.
|
||||
try:
|
||||
# Build the retry ladder: original query, then progressively-shortened
|
||||
# variants. Capped at 5 total requests to avoid hammering Tidal on
|
||||
# genuinely-missing tracks, while still allowing enough variants to
|
||||
# cover multi-word trailing modifiers like remix credits.
|
||||
queries_to_try = [query] + self._generate_shortened_queries(query)
|
||||
queries_to_try = queries_to_try[:5]
|
||||
|
||||
# Qualifier-aware safety net: if the original query contains variant
|
||||
# keywords (Live, Remix, Acoustic, Extended, etc.), fallback results
|
||||
# MUST still contain those qualifiers in their track names. Otherwise
|
||||
# a shortened query could silently downgrade "Song (Live)" to the
|
||||
# studio "Song" and the caller would download the wrong variant.
|
||||
required_qualifiers = self._extract_qualifiers(query)
|
||||
|
||||
tidal_tracks: list = []
|
||||
successful_query: Optional[str] = None
|
||||
last_error: Optional[Exception] = None
|
||||
# Tracks whether ANY fallback attempt returned broader matches that
|
||||
# got rejected by the qualifier filter — used to give an accurate
|
||||
# "no qualifier-matching variant" log message at the end instead of
|
||||
# a generic "0 results".
|
||||
any_fallback_filtered_out = False
|
||||
|
||||
loop = asyncio.get_event_loop()
|
||||
for attempt_idx, attempt_query in enumerate(queries_to_try):
|
||||
try:
|
||||
q_copy = attempt_query
|
||||
|
||||
def _search():
|
||||
results = self.session.search(query, models=[tidalapi.media.Track], limit=50)
|
||||
return results.get('tracks', []) if isinstance(results, dict) else []
|
||||
def _search(q=q_copy):
|
||||
results = self.session.search(q, models=[tidalapi.media.Track], limit=50)
|
||||
return results.get('tracks', []) if isinstance(results, dict) else []
|
||||
|
||||
tidal_tracks = await loop.run_in_executor(None, _search)
|
||||
found = await loop.run_in_executor(None, _search)
|
||||
|
||||
if found:
|
||||
# Fallback attempts get qualifier-filtered. We trust the
|
||||
# original query to return only appropriate matches, but
|
||||
# shortened queries are more permissive and can return
|
||||
# wrong-variant tracks (e.g. studio when Live was asked
|
||||
# for). Drop any result whose title doesn't carry all
|
||||
# original qualifier words.
|
||||
is_fallback = attempt_idx > 0
|
||||
if is_fallback and required_qualifiers:
|
||||
filtered = [
|
||||
t for t in found
|
||||
if self._track_name_contains_qualifiers(getattr(t, 'name', ''), required_qualifiers)
|
||||
]
|
||||
if filtered:
|
||||
tidal_tracks = filtered
|
||||
successful_query = attempt_query
|
||||
logger.info(
|
||||
f"Tidal fallback kept {len(filtered)}/{len(found)} tracks "
|
||||
f"after qualifier filter {required_qualifiers} for '{attempt_query}'"
|
||||
)
|
||||
break
|
||||
else:
|
||||
any_fallback_filtered_out = True
|
||||
logger.debug(
|
||||
f"Tidal fallback '{attempt_query}' returned {len(found)} tracks "
|
||||
f"but none matched original qualifiers {required_qualifiers} — "
|
||||
f"trying next variant"
|
||||
)
|
||||
if attempt_idx < len(queries_to_try) - 1:
|
||||
await asyncio.sleep(0.1)
|
||||
continue
|
||||
else:
|
||||
tidal_tracks = found
|
||||
successful_query = attempt_query
|
||||
break
|
||||
|
||||
if attempt_idx < len(queries_to_try) - 1:
|
||||
logger.debug(f"Tidal returned 0 results for '{attempt_query}' — trying shortened variant")
|
||||
# Small pause so we're not hammering Tidal with rapid retries
|
||||
await asyncio.sleep(0.1)
|
||||
except Exception as e:
|
||||
last_error = e
|
||||
logger.debug(f"Tidal search attempt {attempt_idx + 1} failed: {e}")
|
||||
|
||||
if not tidal_tracks:
|
||||
logger.warning(f"No Tidal results for: {query}")
|
||||
if last_error is not None:
|
||||
import traceback
|
||||
tb_str = ''.join(traceback.format_exception(
|
||||
type(last_error), last_error, last_error.__traceback__
|
||||
))
|
||||
logger.error(
|
||||
f"Tidal search failed after {len(queries_to_try)} attempts: {last_error}\n{tb_str}"
|
||||
)
|
||||
elif any_fallback_filtered_out:
|
||||
logger.warning(
|
||||
f"No Tidal results for '{query}' — fallbacks found broader matches but "
|
||||
f"none preserved required qualifiers {required_qualifiers}"
|
||||
)
|
||||
else:
|
||||
logger.warning(f"No Tidal results for: {query}")
|
||||
return ([], [])
|
||||
|
||||
if successful_query and successful_query != query:
|
||||
logger.info(f"Tidal fallback query succeeded: '{successful_query}' (original: '{query}')")
|
||||
|
||||
# Get configured quality for display
|
||||
quality_key = config_manager.get('tidal_download.quality', 'lossless')
|
||||
quality_info = QUALITY_MAP.get(quality_key, QUALITY_MAP['lossless'])
|
||||
|
|
@ -286,7 +474,11 @@ class TidalDownloadClient:
|
|||
return (track_results, [])
|
||||
|
||||
except Exception as e:
|
||||
logger.error(f"Tidal search failed: {e}")
|
||||
# Unhandled error in the retry orchestration itself (not in an
|
||||
# individual attempt, which is already caught above). Preserves
|
||||
# the original contract of returning ([], []) on any failure so
|
||||
# the caller's fallback chain isn't broken.
|
||||
logger.error(f"Tidal search orchestration failed: {e}")
|
||||
import traceback
|
||||
traceback.print_exc()
|
||||
return ([], [])
|
||||
|
|
|
|||
160
tests/test_tidal_search_shortening.py
Normal file
160
tests/test_tidal_search_shortening.py
Normal file
|
|
@ -0,0 +1,160 @@
|
|||
"""Regression tests for TidalDownloadClient._generate_shortened_queries.
|
||||
|
||||
The shortener's job: when Tidal's search chokes on a long query with
|
||||
qualifier suffixes ("... (Fred V Remix)"), produce progressively-shorter
|
||||
variants so the retry loop has a chance of finding results. These tests
|
||||
pin the expected retry ladder shape for common real-world query patterns.
|
||||
"""
|
||||
|
||||
import sys
|
||||
import types
|
||||
|
||||
|
||||
# Stub tidalapi before importing the module — it uses tidalapi.Quality at
|
||||
# import time, and we don't want to require the package for unit tests.
|
||||
if 'tidalapi' not in sys.modules:
|
||||
_fake = types.ModuleType('tidalapi')
|
||||
|
||||
class _FakeQuality:
|
||||
low_96k = 'low_96k'
|
||||
low_320k = 'low_320k'
|
||||
high_lossless = 'high_lossless'
|
||||
hi_res = 'hi_res'
|
||||
hi_res_lossless = 'hi_res_lossless'
|
||||
|
||||
_fake.Quality = _FakeQuality
|
||||
_fake.media = types.SimpleNamespace(Track=object)
|
||||
sys.modules['tidalapi'] = _fake
|
||||
|
||||
|
||||
from core.tidal_download_client import TidalDownloadClient # noqa: E402
|
||||
|
||||
|
||||
def _ladder(original, cap=5):
|
||||
"""Return the full retry ladder (original + variants), capped like the
|
||||
runtime does."""
|
||||
variants = TidalDownloadClient._generate_shortened_queries(original)
|
||||
return [original] + variants[: cap - 1]
|
||||
|
||||
|
||||
def test_skowl_reported_query_reaches_working_variant():
|
||||
"""The user-reported failing query needs to reach
|
||||
'maduk transformations remixed fire away' within the retry cap."""
|
||||
original = 'maduk transformations remixed fire away fred v remix'
|
||||
ladder = _ladder(original)
|
||||
assert 'maduk transformations remixed fire away' in ladder
|
||||
# And the original still starts the ladder
|
||||
assert ladder[0] == original
|
||||
|
||||
|
||||
def test_parenthesized_suffix_is_stripped_first():
|
||||
"""The cheapest, most obvious shortening should come first for
|
||||
parenthesized suffixes."""
|
||||
variants = TidalDownloadClient._generate_shortened_queries('Song (Radio Edit)')
|
||||
assert variants[0] == 'Song'
|
||||
|
||||
|
||||
def test_bracketed_suffix_is_stripped():
|
||||
variants = TidalDownloadClient._generate_shortened_queries('Song [Remix]')
|
||||
assert 'Song' in variants
|
||||
|
||||
|
||||
def test_short_queries_produce_no_variants():
|
||||
# 1 or 2-word queries have nothing useful to shorten
|
||||
assert TidalDownloadClient._generate_shortened_queries('one two') == []
|
||||
assert TidalDownloadClient._generate_shortened_queries('single') == []
|
||||
|
||||
|
||||
def test_variants_are_unique():
|
||||
# Dedup guard — no variant should duplicate the original or another variant
|
||||
original = 'Artist Title Club Mix Extended Version'
|
||||
variants = TidalDownloadClient._generate_shortened_queries(original)
|
||||
lower = [v.lower() for v in variants]
|
||||
assert len(lower) == len(set(lower))
|
||||
assert original.lower() not in lower
|
||||
|
||||
|
||||
def test_progressive_drops_appear_in_ladder():
|
||||
"""Drop-1, drop-2, drop-3 should all be present (in some order) for a
|
||||
long query that has no parentheses."""
|
||||
original = 'a b c d e f g h' # 8 tokens
|
||||
variants = TidalDownloadClient._generate_shortened_queries(original)
|
||||
# Drop-1 → 7 tokens; drop-2 → 6; drop-3 → 5
|
||||
token_counts = [len(v.split()) for v in variants]
|
||||
assert 7 in token_counts
|
||||
assert 6 in token_counts
|
||||
assert 5 in token_counts
|
||||
|
||||
|
||||
def test_empty_query_returns_empty_list():
|
||||
assert TidalDownloadClient._generate_shortened_queries('') == []
|
||||
|
||||
|
||||
# ── Qualifier guard ────────────────────────────────────────────────────────
|
||||
#
|
||||
# When the original query carries a variant marker like "Live", "Remix",
|
||||
# "Acoustic", fallback results must preserve that marker in the track name —
|
||||
# otherwise a shortened query would silently downgrade "Song (Live)" to the
|
||||
# studio "Song" and the caller would download the wrong variant.
|
||||
|
||||
|
||||
def test_extract_qualifiers_finds_whole_word_matches():
|
||||
# Word boundary: "remix" as a standalone word counts; "remixed" does not
|
||||
q = TidalDownloadClient._extract_qualifiers(
|
||||
'maduk transformations remixed fire away fred v remix'
|
||||
)
|
||||
assert 'remix' in q
|
||||
# "mix" is inside "remix/remixed" but not a whole word
|
||||
assert 'mix' not in q
|
||||
|
||||
|
||||
def test_extract_qualifiers_is_case_insensitive():
|
||||
q = TidalDownloadClient._extract_qualifiers('Song (LIVE at Wembley)')
|
||||
assert 'live' in q
|
||||
|
||||
|
||||
def test_extract_qualifiers_no_false_positives():
|
||||
# "edit" must not match "edition"; "mix" must not match "remixed";
|
||||
# "live" must not match "olive" / "deliver"
|
||||
q = TidalDownloadClient._extract_qualifiers('Deluxe Edition Delivering Olive')
|
||||
assert q == []
|
||||
|
||||
|
||||
def test_extract_qualifiers_empty_query():
|
||||
assert TidalDownloadClient._extract_qualifiers('') == []
|
||||
assert TidalDownloadClient._extract_qualifiers(None) == []
|
||||
|
||||
|
||||
def test_track_name_matches_when_all_qualifiers_present():
|
||||
assert TidalDownloadClient._track_name_contains_qualifiers(
|
||||
'Fire Away (Fred V Remix)', ['remix']
|
||||
)
|
||||
|
||||
|
||||
def test_track_name_rejects_when_qualifier_missing():
|
||||
# Studio version should NOT pass when "remix" is required
|
||||
assert not TidalDownloadClient._track_name_contains_qualifiers(
|
||||
'Fire Away', ['remix']
|
||||
)
|
||||
|
||||
|
||||
def test_track_name_requires_all_qualifiers():
|
||||
# "Live Acoustic" requires both
|
||||
assert TidalDownloadClient._track_name_contains_qualifiers(
|
||||
'Song (Live Acoustic)', ['live', 'acoustic']
|
||||
)
|
||||
# Missing one → rejected
|
||||
assert not TidalDownloadClient._track_name_contains_qualifiers(
|
||||
'Song (Live)', ['live', 'acoustic']
|
||||
)
|
||||
|
||||
|
||||
def test_track_name_empty_qualifiers_passes_everything():
|
||||
# When no qualifiers required, any track passes (original-query behaviour)
|
||||
assert TidalDownloadClient._track_name_contains_qualifiers('Anything', [])
|
||||
|
||||
|
||||
def test_track_name_qualifier_is_word_bounded():
|
||||
# "edit" qualifier must match "Edit" but not "Edition"
|
||||
assert TidalDownloadClient._track_name_contains_qualifiers('Song (Radio Edit)', ['edit'])
|
||||
assert not TidalDownloadClient._track_name_contains_qualifiers('Deluxe Edition', ['edit'])
|
||||
Loading…
Reference in a new issue