fix(tidal): honour version field in matching and back off on rate limits
Three related fixes to Tidal track matching and downloading:
1. version-field handling — Tidal stores remix/live/edit qualifiers in a
dedicated `track.version` attribute (e.g. name="Emerge",
version="Junkie XL Remix"), not in the track name. The qualifier
filter and the matcher only looked at name/album, so the exact
recording was discarded. Fold `version` into both the qualifier
haystack and the candidate title passed to MusicMatchingEngine.
2. divergent-version penalty — once versions are visible, OTHER cuts of
the same base become candidates ("(Shazam Remix)" vs "(southstar
Remix)"). Neither title is a prefix of the other, so the prefix-based
version check missed them and the raw ratio stayed high off the
shared base. Apply a heavy penalty when both titles carry different
version descriptors so the wrong cut can't outscore the threshold.
3. rate-limit backoff — the trackManifests endpoint is aggressively
rate-limited; a bare request failed 429 instantly, burned the quality
tier, re-queued the track and hammered again (a self-amplifying
storm). Honour Retry-After / exponential backoff with a bounded retry
count and shutdown-aware sleep.
Adds unit + end-to-end tests for all three.
This commit is contained in:
Jeff Theroux
parent
f4dbaea68b
commit
3c06bd03c0
5 changed files with 462 additions and 27 deletions
|
|
@ -214,6 +214,22 @@ class MusicMatchingEngine:
|
|||
# Standard similarity
|
||||
standard_ratio = SequenceMatcher(None, str1, str2).ratio()
|
||||
|
||||
# Version vocabulary, shared by the prefix check and the divergent
|
||||
# check below.
|
||||
remaster_keywords = ['remaster', 'remastered']
|
||||
different_version_keywords = [
|
||||
'remix', 'mix', 'rmx', # Remixes (different song)
|
||||
'live', 'live at', 'live from', # Live versions (different recording)
|
||||
'acoustic', 'unplugged', # Acoustic versions (different arrangement)
|
||||
'slowed', 'reverb', 'sped up', 'speed up', # TikTok edits (different)
|
||||
'radio edit', 'radio version', # Radio edits (different cut)
|
||||
'single edit', # Single edits (different cut)
|
||||
'album edit', # Album edits (different cut)
|
||||
'instrumental', 'karaoke', # Instrumental (different)
|
||||
'extended', 'extended version', # Extended (different length)
|
||||
'demo', 'rough cut', # Demos (different recording)
|
||||
]
|
||||
|
||||
# STRICT VERSION CHECKING: Different versions should score LOW
|
||||
# This prevents "Song Title" from matching "Song Title (Remix)" during sync
|
||||
shorter, longer = (str1, str2) if len(str1) <= len(str2) else (str2, str1)
|
||||
|
|
@ -223,23 +239,6 @@ class MusicMatchingEngine:
|
|||
# Extract the extra content
|
||||
extra_content = longer[len(shorter):].strip()
|
||||
|
||||
# Check if the extra content looks like version info
|
||||
# Separate remasters from other versions - they should be treated differently
|
||||
remaster_keywords = ['remaster', 'remastered']
|
||||
|
||||
different_version_keywords = [
|
||||
'remix', 'mix', 'rmx', # Remixes (different song)
|
||||
'live', 'live at', 'live from', # Live versions (different recording)
|
||||
'acoustic', 'unplugged', # Acoustic versions (different arrangement)
|
||||
'slowed', 'reverb', 'sped up', 'speed up', # TikTok edits (different)
|
||||
'radio edit', 'radio version', # Radio edits (different cut)
|
||||
'single edit', # Single edits (different cut)
|
||||
'album edit', # Album edits (different cut)
|
||||
'instrumental', 'karaoke', # Instrumental (different)
|
||||
'extended', 'extended version', # Extended (different length)
|
||||
'demo', 'rough cut', # Demos (different recording)
|
||||
]
|
||||
|
||||
# Normalize extra content for comparison
|
||||
extra_normalized = extra_content.lower().strip(' -()[]')
|
||||
|
||||
|
|
@ -261,6 +260,38 @@ class MusicMatchingEngine:
|
|||
logger.debug(f"Version mismatch detected: '{str1}' vs '{str2}' (keyword: '{keyword}') - applying heavy penalty")
|
||||
return 0.30
|
||||
|
||||
# STRICT VERSION CHECKING (divergent case): two DIFFERENT versions of
|
||||
# the same base — e.g. "Song (Shazam Remix)" vs "Song (southstar
|
||||
# Remix)", or "...live at pukkelpop" vs "...live at wembley". Both
|
||||
# carry a version descriptor, so neither is a prefix of the other and
|
||||
# the prefix check above misses them; the raw ratio then stays high off
|
||||
# the shared base. Without this, when the requested version is absent a
|
||||
# different cut of the same song can outscore the threshold and get
|
||||
# downloaded. A correct same-version match is identical after
|
||||
# normalisation and already returned 1.0 above, so a both-versioned
|
||||
# pair that survives to here with high base overlap is a genuinely
|
||||
# different cut. (Remasters are intentionally excluded — the prefix
|
||||
# branch gives them the lenient 0.75 so re-mastered cuts still match.)
|
||||
def _versions_in(s: str) -> frozenset:
|
||||
return frozenset(
|
||||
kw for kw in different_version_keywords
|
||||
if re.search(r'\b' + re.escape(kw) + r'\b', s))
|
||||
|
||||
v1, v2 = _versions_in(str1), _versions_in(str2)
|
||||
if v1 and v2 and standard_ratio >= 0.5:
|
||||
# Strip the version words; what remains is base + distinguishing
|
||||
# descriptor (remixer / performance / year).
|
||||
def _strip_versions(s: str) -> str:
|
||||
for kw in different_version_keywords:
|
||||
s = re.sub(r'\b' + re.escape(kw) + r'\b', ' ', s)
|
||||
return re.sub(r'\s+', ' ', s).strip()
|
||||
|
||||
if v1 != v2 or _strip_versions(str1) != _strip_versions(str2):
|
||||
logger.debug(
|
||||
f"Divergent version detected: '{str1}' vs '{str2}' "
|
||||
f"- applying heavy penalty")
|
||||
return 0.30
|
||||
|
||||
return standard_ratio
|
||||
|
||||
def duration_similarity(self, duration1: int, duration2: int) -> float:
|
||||
|
|
|
|||
|
|
@ -302,18 +302,21 @@ class TidalDownloadClient(DownloadSourcePlugin):
|
|||
|
||||
@classmethod
|
||||
def _track_matches_qualifiers(cls, track, qualifiers: List[str]) -> bool:
|
||||
"""Issue #589 — qualifier check must inspect both track.name AND
|
||||
track.album.name. For MTV Unplugged-style releases the live /
|
||||
unplugged signal lives in the album title, not the track title.
|
||||
A track passes if every required qualifier appears as a whole
|
||||
word in either the track name OR its album name.
|
||||
"""Issue #589 — qualifier check must inspect track.name, track.version
|
||||
AND track.album.name. Tidal stores remix/live/edit qualifiers in a
|
||||
dedicated `version` attribute (e.g. name="Emerge", version="Junkie XL
|
||||
Remix"), and for MTV Unplugged-style releases the live / unplugged
|
||||
signal lives in the album title. A track passes if every required
|
||||
qualifier appears as a whole word in the track name, its version, OR
|
||||
its album name.
|
||||
"""
|
||||
if not qualifiers:
|
||||
return True
|
||||
track_name = (getattr(track, 'name', '') or '').lower()
|
||||
version = (getattr(track, 'version', '') or '').lower()
|
||||
album = getattr(track, 'album', None)
|
||||
album_name = (getattr(album, 'name', '') or '').lower() if album else ''
|
||||
haystack = f"{track_name} {album_name}".strip()
|
||||
haystack = f"{track_name} {version} {album_name}".strip()
|
||||
if not haystack:
|
||||
return False
|
||||
for kw in qualifiers:
|
||||
|
|
@ -475,6 +478,17 @@ class TidalDownloadClient(DownloadSourcePlugin):
|
|||
def _tidal_to_track_result(self, track, quality_info: dict) -> TrackResult:
|
||||
artist_name = track.artist.name if track.artist else 'Unknown Artist'
|
||||
title = track.name or 'Unknown Title'
|
||||
# Tidal keeps remix/live/edition qualifiers in a separate `version`
|
||||
# attribute (e.g. name="Emerge", version="Junkie XL Remix"). The
|
||||
# matcher only scores `title`, so without folding the version in,
|
||||
# every remix/live/edit candidate presents as the bare base track
|
||||
# and MusicMatchingEngine's strict version check rejects it against
|
||||
# a "... (Junkie XL Remix)" request. Append the version (unless it's
|
||||
# already in the name) so the candidate title is the full
|
||||
# "Title (Version)".
|
||||
version = getattr(track, 'version', None)
|
||||
if version and version.strip() and version.strip().lower() not in title.lower():
|
||||
title = f"{title} ({version.strip()})"
|
||||
album_name = track.album.name if track.album else None
|
||||
|
||||
duration_ms = int(track.duration * 1000) if track.duration else None
|
||||
|
|
@ -548,6 +562,62 @@ class TidalDownloadClient(DownloadSourcePlugin):
|
|||
|
||||
return init_uri, segment_uris
|
||||
|
||||
# Tidal aggressively rate-limits the trackManifests endpoint. When a
|
||||
# batch fans out, a bare request fails 429 instantly, the quality tier is
|
||||
# burned, the track is re-queued, and the retry hammers again — a
|
||||
# self-amplifying storm (thousands of 429s, downloads stalled, only the
|
||||
# lowest tier squeaking through). Honour the server's Retry-After (or back
|
||||
# off exponentially) so downloads pace themselves to whatever rate Tidal
|
||||
# allows instead of slamming the wall and instant-failing.
|
||||
_MANIFEST_MAX_RETRIES = 5
|
||||
_MANIFEST_BACKOFF_BASE = 2.0 # seconds → 2, 4, 8, 16, 32 (capped)
|
||||
_MANIFEST_BACKOFF_CAP = 30.0
|
||||
|
||||
@classmethod
|
||||
def _retry_after_seconds(cls, response, attempt: int) -> float:
|
||||
"""Backoff delay for a rate-limited response: honour a numeric
|
||||
Retry-After header when present, else exponential backoff (capped)."""
|
||||
retry_after = response.headers.get('Retry-After') if response is not None else None
|
||||
if retry_after:
|
||||
try:
|
||||
return min(max(float(retry_after), 0.0), cls._MANIFEST_BACKOFF_CAP)
|
||||
except (TypeError, ValueError):
|
||||
pass
|
||||
return min(cls._MANIFEST_BACKOFF_BASE * (2 ** attempt), cls._MANIFEST_BACKOFF_CAP)
|
||||
|
||||
def _sleep_with_shutdown(self, delay: float) -> bool:
|
||||
"""Sleep up to `delay` seconds in small slices. Returns True if a
|
||||
shutdown was requested mid-sleep so the caller can bail early."""
|
||||
slept = 0.0
|
||||
while slept < delay:
|
||||
if self.shutdown_check and self.shutdown_check():
|
||||
return True
|
||||
chunk = min(0.5, delay - slept)
|
||||
time.sleep(chunk)
|
||||
slept += chunk
|
||||
return False
|
||||
|
||||
def _get_with_rate_limit_retry(self, url: str, *, params=None, headers=None,
|
||||
timeout: int = 20):
|
||||
"""HTTP GET that backs off and retries on 429 (and transient 5xx),
|
||||
honouring Retry-After. Returns the final Response (the caller still
|
||||
calls raise_for_status); a persistent rate-limit after all retries
|
||||
returns the last 429 response so existing error handling applies."""
|
||||
response = None
|
||||
for attempt in range(self._MANIFEST_MAX_RETRIES + 1):
|
||||
response = http_requests.get(url, params=params, headers=headers, timeout=timeout)
|
||||
if response.status_code != 429 and response.status_code < 500:
|
||||
return response
|
||||
if attempt >= self._MANIFEST_MAX_RETRIES:
|
||||
return response
|
||||
delay = self._retry_after_seconds(response, attempt)
|
||||
logger.warning(
|
||||
f"Tidal returned {response.status_code} on manifest fetch — backing off "
|
||||
f"{delay:.1f}s (attempt {attempt + 1}/{self._MANIFEST_MAX_RETRIES})")
|
||||
if self._sleep_with_shutdown(delay):
|
||||
return response
|
||||
return response
|
||||
|
||||
def _get_hls_manifest(self, track_id: int, quality: str = 'lossless') -> Optional[Dict]:
|
||||
q_info = HLS_QUALITY_MAP.get(quality, HLS_QUALITY_MAP['lossless'])
|
||||
formats = q_info['formats']
|
||||
|
|
@ -573,7 +643,7 @@ class TidalDownloadClient(DownloadSourcePlugin):
|
|||
}
|
||||
|
||||
try:
|
||||
response = http_requests.get(url, params=params, headers=headers, timeout=20)
|
||||
response = self._get_with_rate_limit_retry(url, params=params, headers=headers, timeout=20)
|
||||
response.raise_for_status()
|
||||
data = response.json()
|
||||
except http_requests.HTTPError as e:
|
||||
|
|
|
|||
100
tests/matching/test_divergent_version.py
Normal file
100
tests/matching/test_divergent_version.py
Normal file
|
|
@ -0,0 +1,100 @@
|
|||
"""Divergent-version matching: two DIFFERENT versions of the same base
|
||||
title must NOT match.
|
||||
|
||||
Context: Tidal stores remix/live/edit qualifiers in a dedicated `version`
|
||||
field which `_tidal_to_track_result` now folds into the candidate title so
|
||||
the matcher can see it. That fix made the *correct* version win — but it
|
||||
also makes OTHER versions of the same song visible ("We Are The People
|
||||
(Shazam Remix)" vs "(southstar Remix)"). Neither title is a prefix of the
|
||||
other, so the original prefix-based version check missed them and the raw
|
||||
ratio stayed high (~0.8) off the shared base. Without discrimination, when
|
||||
the requested version is absent a different remix could outscore the
|
||||
threshold and the wrong cut would be downloaded.
|
||||
|
||||
These pin: different descriptors reject, the correct one still wins, and
|
||||
the existing original-vs-version / remaster behaviour is preserved.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from core.matching_engine import MusicMatchingEngine
|
||||
|
||||
me = MusicMatchingEngine()
|
||||
|
||||
|
||||
# ── similarity_score: divergent version tails (already-normalised input) ──
|
||||
|
||||
def test_different_remix_descriptors_rejected():
|
||||
assert me.similarity_score(
|
||||
'we are the people shazam remix',
|
||||
'we are the people southstar remix',
|
||||
) == 0.30
|
||||
|
||||
|
||||
def test_different_live_performances_rejected():
|
||||
assert me.similarity_score(
|
||||
'all night live at pukkelpop',
|
||||
'all night live at wembley',
|
||||
) == 0.30
|
||||
|
||||
|
||||
def test_different_version_types_same_base_rejected():
|
||||
# Same song, different version TYPE (remix vs live) — different cut.
|
||||
assert me.similarity_score('song title remix', 'song title live') == 0.30
|
||||
|
||||
|
||||
def test_same_base_non_version_tails_not_penalised():
|
||||
# "one" / "two" are not version words — leave the raw ratio alone.
|
||||
assert me.similarity_score('song one', 'song two') != 0.30
|
||||
|
||||
|
||||
# ── regression: original-vs-version + remaster behaviour preserved ──
|
||||
|
||||
def test_original_vs_remix_still_rejected():
|
||||
assert me.similarity_score('we are the people', 'we are the people remix') == 0.30
|
||||
|
||||
|
||||
def test_remaster_still_light_penalty():
|
||||
assert me.similarity_score('song title', 'song title remastered') == 0.75
|
||||
|
||||
|
||||
def test_identical_titles_still_perfect():
|
||||
assert me.similarity_score('emerge junkie xl remix', 'emerge junkie xl remix') == 1.0
|
||||
|
||||
|
||||
# ── end-to-end via score_track_match (raw titles, real weighting) ──
|
||||
|
||||
_ARTIST = 'Empire Of The Sun'
|
||||
|
||||
|
||||
def test_wrong_remix_scored_below_threshold():
|
||||
# Requested Shazam Remix, only a different remix available → must land
|
||||
# well under the 0.55/0.60 acceptance gate so it is never downloaded.
|
||||
conf, _ = me.score_track_match(
|
||||
'We Are The People (Shazam Remix)', [_ARTIST], 344_000,
|
||||
'We Are The People (southstar Remix)', [_ARTIST], 236_000,
|
||||
)
|
||||
assert conf < 0.55, f'wrong remix scored {conf:.2f}, should be < 0.55'
|
||||
|
||||
|
||||
def test_correct_remix_still_wins():
|
||||
conf, _ = me.score_track_match(
|
||||
'We Are The People (Shazam Remix)', [_ARTIST], 344_000,
|
||||
'We Are The People (Shazam Remix)', [_ARTIST], 344_000,
|
||||
)
|
||||
assert conf >= 0.90, f'correct remix scored {conf:.2f}, should be >= 0.90'
|
||||
|
||||
|
||||
@pytest.mark.parametrize('wanted,candidate', [
|
||||
('We Are The People (Shazam Remix)', 'We Are The People (ARTBAT Remix)'),
|
||||
('All Night (Live @ Pukkelpop)', 'All Night (Umek Remix)'),
|
||||
('Emerge (Junkie XL Remix)', 'Emerge (DFA Version)'),
|
||||
])
|
||||
def test_wrong_version_below_correct(wanted, candidate):
|
||||
artist = 'X'
|
||||
wrong, _ = me.score_track_match(wanted, [artist], 0, candidate, [artist], 0)
|
||||
right, _ = me.score_track_match(wanted, [artist], 0, wanted, [artist], 0)
|
||||
assert right > wrong
|
||||
assert wrong < 0.60, f'{candidate!r} scored {wrong:.2f} vs {wanted!r}'
|
||||
|
|
@ -19,12 +19,14 @@ from unittest.mock import MagicMock
|
|||
from core.tidal_download_client import TidalDownloadClient
|
||||
|
||||
|
||||
def _make_track(name: str, album_name: str = ''):
|
||||
def _make_track(name: str, album_name: str = '', version: str = ''):
|
||||
"""Build a minimal duck-typed track object matching what the Tidal
|
||||
SDK returns: a `name` attribute and an `album` attribute with its
|
||||
own `name`."""
|
||||
SDK returns: a `name` attribute, a `version` attribute (remix/live/
|
||||
edit qualifier — separate from the title), and an `album` attribute
|
||||
with its own `name`."""
|
||||
track = MagicMock()
|
||||
track.name = name
|
||||
track.version = version
|
||||
track.album = MagicMock()
|
||||
track.album.name = album_name
|
||||
return track
|
||||
|
|
@ -121,3 +123,121 @@ def test_extract_qualifiers_picks_up_live_unplugged():
|
|||
quals = TidalDownloadClient._extract_qualifiers('Shy Away (MTV Unplugged Live)')
|
||||
assert 'live' in quals
|
||||
assert 'unplugged' in quals
|
||||
|
||||
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
# track.version — Tidal stores the remix/live/edit qualifier in a
|
||||
# dedicated `version` attribute, NOT in track.name. Real-world: the
|
||||
# exact recording is present in the search results but was discarded
|
||||
# because neither the qualifier filter nor the matcher looked at
|
||||
# `version`. Cases below are real Tidal tracks (id in comments).
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
|
||||
def test_qualifier_in_version_field_passes():
|
||||
# Tidal 124341 — name="Emerge", version="Junkie XL Remix"
|
||||
track = _make_track('Emerge', album_name='#1', version='Junkie XL Remix')
|
||||
assert TidalDownloadClient._track_matches_qualifiers(track, ['remix']) is True
|
||||
|
||||
|
||||
def test_qualifier_in_version_field_radio_version():
|
||||
# Tidal 122127 — name="Black Horse And The Cherry Tree",
|
||||
# version="Radio Version"
|
||||
track = _make_track('Black Horse And The Cherry Tree', version='Radio Version')
|
||||
assert TidalDownloadClient._track_matches_qualifiers(track, ['version']) is True
|
||||
|
||||
|
||||
def test_qualifier_missing_from_name_version_and_album_fails():
|
||||
# The studio cut: nothing carries "remix" anywhere — must still reject.
|
||||
track = _make_track('Emerge', album_name='#1', version='')
|
||||
assert TidalDownloadClient._track_matches_qualifiers(track, ['remix']) is False
|
||||
|
||||
|
||||
def test_empty_version_does_not_pollute_haystack():
|
||||
# Regression guard: a falsy version must not let unrelated qualifiers
|
||||
# match (e.g. via a stringified mock).
|
||||
track = _make_track('Emerge', album_name='#1', version='')
|
||||
assert TidalDownloadClient._track_matches_qualifiers(track, ['live']) is False
|
||||
|
||||
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
# _tidal_to_track_result — folds `version` into the candidate title so
|
||||
# the matcher (which scores title only) can see the qualifier.
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
|
||||
def _make_full_track(name, artist, version='', album='', duration=240,
|
||||
isrc='X', tid=1):
|
||||
track = MagicMock()
|
||||
track.id = tid
|
||||
track.name = name
|
||||
track.version = version
|
||||
track.artist = MagicMock()
|
||||
track.artist.name = artist
|
||||
track.artist.id = 99
|
||||
track.album = MagicMock()
|
||||
track.album.name = album
|
||||
track.duration = duration
|
||||
track.track_num = 1
|
||||
track.isrc = isrc
|
||||
track.bpm = 0
|
||||
track.copyright = ''
|
||||
return track
|
||||
|
||||
|
||||
_QINFO = {'codec': 'flac', 'bitrate': 1411}
|
||||
|
||||
|
||||
def test_version_folded_into_title():
|
||||
track = _make_full_track('Emerge', 'Fischerspooner', version='Junkie XL Remix')
|
||||
result = TidalDownloadClient._tidal_to_track_result(None, track, _QINFO)
|
||||
assert result.title == 'Emerge (Junkie XL Remix)'
|
||||
|
||||
|
||||
def test_no_version_leaves_title_unchanged():
|
||||
track = _make_full_track('Emerge', 'Fischerspooner', version='')
|
||||
result = TidalDownloadClient._tidal_to_track_result(None, track, _QINFO)
|
||||
assert result.title == 'Emerge'
|
||||
|
||||
|
||||
def test_version_already_in_name_not_duplicated():
|
||||
# Some Tidal tracks redundantly carry the version in the name too;
|
||||
# don't produce "Song (Remix) (Remix)".
|
||||
track = _make_full_track('Song (Remix)', 'Artist', version='Remix')
|
||||
result = TidalDownloadClient._tidal_to_track_result(None, track, _QINFO)
|
||||
assert result.title == 'Song (Remix)'
|
||||
|
||||
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
# End-to-end: folding version in is what lets MusicMatchingEngine accept
|
||||
# the exact recording. Before the fix the bare name scores below the
|
||||
# 0.60 gate; after it, the full title matches. Real repro cases.
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
|
||||
import pytest
|
||||
from core.matching_engine import MusicMatchingEngine
|
||||
|
||||
|
||||
# (wanted title, artist, tidal name, tidal version)
|
||||
_REPROS = [
|
||||
('Emerge (Junkie XL Remix)', 'Fischerspooner', 'Emerge', 'Junkie XL Remix'),
|
||||
('We Are The People (Shazam Remix)', 'Empire Of The Sun', 'We Are The People', 'Shazam Remix'),
|
||||
('Black Horse And The Cherry Tree (Radio Version)', 'KT Tunstall', 'Black Horse And The Cherry Tree', 'Radio Version'),
|
||||
('All Night (Live @ Pukkelpop)', 'Parov Stelar', 'All Night', 'Live @ Pukkelpop'),
|
||||
('Fleur de Lille (Extended)', 'Parov Stelar', 'Fleur de Lille', 'Extended'),
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize('wanted,artist,tidal_name,tidal_version', _REPROS)
|
||||
def test_version_fold_lets_matcher_accept(wanted, artist, tidal_name, tidal_version):
|
||||
me = MusicMatchingEngine()
|
||||
track = _make_full_track(tidal_name, artist, version=tidal_version)
|
||||
folded = TidalDownloadClient._tidal_to_track_result(None, track, _QINFO).title
|
||||
|
||||
# Before the fix the candidate title was the bare Tidal name → rejected.
|
||||
bare_conf, _ = me.score_track_match(wanted, [artist], 0, tidal_name, [artist], 0)
|
||||
# After the fix it's "Name (Version)" → accepted.
|
||||
folded_conf, _ = me.score_track_match(wanted, [artist], 0, folded, [artist], 0)
|
||||
|
||||
assert folded_conf >= 0.60, f'{wanted!r}: folded {folded_conf:.2f} should clear 0.60'
|
||||
assert folded_conf > bare_conf, (
|
||||
f'{wanted!r}: folding version in ({folded_conf:.2f}) must beat bare name '
|
||||
f'({bare_conf:.2f})')
|
||||
|
|
|
|||
114
tests/test_tidal_rate_limit_backoff.py
Normal file
114
tests/test_tidal_rate_limit_backoff.py
Normal file
|
|
@ -0,0 +1,114 @@
|
|||
"""Tidal manifest fetch backs off on HTTP 429 instead of instant-failing.
|
||||
|
||||
Tidal aggressively rate-limits the trackManifests endpoint. The bare
|
||||
request previously failed 429 immediately, burning the quality tier and
|
||||
re-queueing the track, which hammered Tidal again — a self-amplifying
|
||||
storm (thousands of 429s, downloads stalled). These pin the backoff:
|
||||
retry on 429, honour Retry-After, give up after a bounded number of
|
||||
attempts, bail on shutdown, and never retry a normal 4xx.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from core.tidal_download_client import TidalDownloadClient
|
||||
|
||||
|
||||
def _client():
|
||||
# Bypass __init__ (which builds a tidalapi session + mkdirs); we only
|
||||
# exercise the pure HTTP-retry helpers.
|
||||
c = TidalDownloadClient.__new__(TidalDownloadClient)
|
||||
c.shutdown_check = None
|
||||
return c
|
||||
|
||||
|
||||
def _resp(status, retry_after=None):
|
||||
r = MagicMock()
|
||||
r.status_code = status
|
||||
r.headers = {'Retry-After': str(retry_after)} if retry_after is not None else {}
|
||||
return r
|
||||
|
||||
|
||||
# ── _retry_after_seconds ──────────────────────────────────────────────
|
||||
|
||||
def test_exponential_backoff_without_header():
|
||||
assert TidalDownloadClient._retry_after_seconds(_resp(429), 0) == 2.0
|
||||
assert TidalDownloadClient._retry_after_seconds(_resp(429), 1) == 4.0
|
||||
assert TidalDownloadClient._retry_after_seconds(_resp(429), 2) == 8.0
|
||||
|
||||
|
||||
def test_backoff_capped():
|
||||
# 2 * 2**10 = 2048 → capped at 30.
|
||||
assert TidalDownloadClient._retry_after_seconds(_resp(429), 10) == 30.0
|
||||
|
||||
|
||||
def test_retry_after_header_honoured():
|
||||
assert TidalDownloadClient._retry_after_seconds(_resp(429, retry_after=7), 0) == 7.0
|
||||
|
||||
|
||||
def test_retry_after_header_capped():
|
||||
assert TidalDownloadClient._retry_after_seconds(_resp(429, retry_after=999), 0) == 30.0
|
||||
|
||||
|
||||
def test_garbage_retry_after_falls_back_to_exponential():
|
||||
assert TidalDownloadClient._retry_after_seconds(_resp(429, retry_after='soon'), 1) == 4.0
|
||||
|
||||
|
||||
# ── _get_with_rate_limit_retry ────────────────────────────────────────
|
||||
|
||||
def test_retries_then_succeeds():
|
||||
c = _client()
|
||||
responses = [_resp(429), _resp(429), _resp(200)]
|
||||
with patch('core.tidal_download_client.http_requests.get',
|
||||
side_effect=responses) as g, \
|
||||
patch('core.tidal_download_client.time.sleep') as sleep:
|
||||
out = c._get_with_rate_limit_retry('http://x')
|
||||
assert out.status_code == 200
|
||||
assert g.call_count == 3
|
||||
assert sleep.call_count >= 2 # backed off before each retry
|
||||
|
||||
|
||||
def test_non_429_4xx_returns_immediately_no_retry():
|
||||
c = _client()
|
||||
with patch('core.tidal_download_client.http_requests.get',
|
||||
side_effect=[_resp(403)]) as g, \
|
||||
patch('core.tidal_download_client.time.sleep') as sleep:
|
||||
out = c._get_with_rate_limit_retry('http://x')
|
||||
assert out.status_code == 403
|
||||
assert g.call_count == 1
|
||||
sleep.assert_not_called()
|
||||
|
||||
|
||||
def test_gives_up_after_max_retries():
|
||||
c = _client()
|
||||
with patch('core.tidal_download_client.http_requests.get',
|
||||
side_effect=[_resp(429)] * 20) as g, \
|
||||
patch('core.tidal_download_client.time.sleep'):
|
||||
out = c._get_with_rate_limit_retry('http://x')
|
||||
assert out.status_code == 429
|
||||
# initial try + MAX_RETRIES retries
|
||||
assert g.call_count == TidalDownloadClient._MANIFEST_MAX_RETRIES + 1
|
||||
|
||||
|
||||
def test_transient_5xx_is_retried():
|
||||
c = _client()
|
||||
with patch('core.tidal_download_client.http_requests.get',
|
||||
side_effect=[_resp(503), _resp(200)]) as g, \
|
||||
patch('core.tidal_download_client.time.sleep'):
|
||||
out = c._get_with_rate_limit_retry('http://x')
|
||||
assert out.status_code == 200
|
||||
assert g.call_count == 2
|
||||
|
||||
|
||||
def test_shutdown_aborts_backoff():
|
||||
c = _client()
|
||||
c.shutdown_check = lambda: True # shutdown requested
|
||||
with patch('core.tidal_download_client.http_requests.get',
|
||||
side_effect=[_resp(429), _resp(200)]) as g, \
|
||||
patch('core.tidal_download_client.time.sleep'):
|
||||
out = c._get_with_rate_limit_retry('http://x')
|
||||
# First call 429 → enters backoff → shutdown → returns the 429 without
|
||||
# making the second request.
|
||||
assert out.status_code == 429
|
||||
assert g.call_count == 1
|
||||
Loading…
Reference in a new issue