From a9f827ef420c35eaf4ac5e63f6917e43c147b1fa Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Fri, 24 Apr 2026 12:55:28 -0700 Subject: [PATCH 1/3] Reject Tidal streams that silently downgrade from the requested quality MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported on Discord by Netti93: with Tidal configured for "HiRes only" and "Allow Quality Fallback" disabled, tracks were still downloading successfully — as m4a 320kbps files. Some "successful" downloads were less than half the file size of the same track pulled via Tidarr/tiddl from the same Tidal account. Root cause: Tidal's API silently degrades to the best quality your account + the track + your region permits. Setting `session.audio_quality = Quality.hi_res_lossless` and calling `track.get_stream()` on a track that's only available in AAC returns an AAC stream with no error. The downloader wrote the m4a file to disk, the ~7MB size sailed past the 100KB stub threshold, and the download reported success. The pre-existing "verify quality wasn't silently downgraded" block only LOGGED a warning when this happened; it did not fail the tier. Two knock-on effects: - Users with "HiRes only, no fallback" got m4a files anyway, which defeats the setting entirely. - The worker-level fallback chain (hires → lossless → high → low) couldn't advance past the first tier, because every tier "succeeded" at whatever Tidal happened to serve. Fix: after `track.get_stream()`, compare `stream.audio_quality` against the tier we asked for using a rank-based ordering: LOW < HIGH < LOSSLESS < HI_RES < HI_RES_LOSSLESS - Same tier or higher → accept (so the occasional Tidal upgrade doesn't get rejected just because it's not an exact match). - Lower tier → reject THIS tier. The loop `continue`s and the next fallback tier is tried, or the whole download fails honestly when the user has fallback disabled. The existing final-error log already has a hint directing users to enable fallback if they want automatic Lossless substitution. - Unrecognized `audioQuality` value (e.g. a new Tidal tier we haven't mapped) → reject conservatively, so the next fallback tier gets a chance and the diagnostic log names the unknown value. Why the rank-based approach instead of strict equality: Tidal's API doesn't technically promise an exact-tier match on serving; on tracks that are flagged in its catalog as a higher tier, it can serve higher than the session setting. Rejecting higher-than-asked quality would be user-hostile. And the `HI_RES` (legacy MQA) value — not in tidalapi's modern `Quality` enum but possibly still present on old catalog entries — needs to rank below `HI_RES_LOSSLESS`: users asking for true lossless HiRes should reject MQA since MQA is a lossy format. tidalapi's `Quality` enum is a `str` subclass whose VALUES (not member names) match what the Tidal API returns in the `audioQuality` field (e.g. `Quality.hi_res_lossless.value == 'HI_RES_LOSSLESS'`, `Quality.low_320k.value == 'HIGH'`). Both sides of the comparison are coerced to `str` before use, so the check is robust to whichever tidalapi version exposes the served quality as an enum or a plain string. The check is extracted as `_verify_stream_tier(stream, q_info, q_key) -> (ok, reason)` at module scope — a pure function with no I/O, unit-tested independently. Ten tests: match, three upgrade cases (LOSSLESS → HI_RES_LOSSLESS, LOSSLESS → HI_RES, LOW → any higher), three downgrade cases (the reported HiRes → AAC, HiRes Lossless → MQA HiRes, Lossless → AAC), one unrecognized-tier case, and two defensive paths for older tidalapi builds without `audio_quality` on the stream object and for QUALITY_MAP entries that lack `tidal_quality` (e.g. tidalapi wasn't importable at module load). Test stub updated to use uppercase `Quality` values matching real tidalapi so case-sensitivity regressions get caught. Also removed the old codec-string-based warning block — the new tier check is strictly stronger, and keeping the warning around would just be dead code waiting to drift out of sync. Deliberately NOT tackling in this PR (documented as follow-ups): - Bit-depth verification of HiRes FLAC files via mutagen. The `stream.audio_quality` tier check catches the main "HiRes requested, got AAC" case; bit-depth would only matter if Tidal labeled a stream HI_RES_LOSSLESS but served a 16-bit FLAC (`Stream.bit_depth` isn't reliable for this — tidalapi defaults missing `bitDepth` fields to 16, so a trust-the-stream check would spuriously reject valid HiRes whenever Tidal omits the field). A proper fix runs mutagen post-download to inspect the actual file, then decides whether to delete + retry the next tier — a whole new failure mode with design trade-offs that deserve their own PR. The support logs don't show this happening. - The "manual remap still says Not Found" symptom. Might be downstream of this same bug (silent-AAC "success" hitting a later rejection), might be a separate task-state issue. Not guessing without logs from the retry path. - Quality-aware stub threshold. 100KB is a reasonable floor for real stub/preview detection and there's no evidence the universal threshold is misfiring in the wild. Field-verified status: desk-verified via unit tests and empirical checks against a live tidalapi import (confirming the `Quality` enum's str-subclass behavior). Not yet smoke-tested end-to-end against a real Tidal account with a HiRes-only-no-fallback setting — Netti93 or anyone else with that config should notice either the fix working (non-HiRes tracks fail honestly with a clear log line) or any regression before wider release. Files: - core/tidal_download_client.py — new `_verify_stream_tier` helper and `_QUALITY_RANK` table at module scope, called in the download loop after the stream is fetched and before any bandwidth is spent. Removed the old inline codec-based warning since the new check supersedes it. - tests/test_tidal_stream_tier_verification.py — ten tests covering match / upgrade / downgrade / unknown / defensive paths. - tests/test_tidal_search_shortening.py — fake `Quality` values brought in line with tidalapi's real values so both files share a consistent stub regardless of pytest collection order. - webui/static/helper.js — WHATS_NEW entry under 2.40 describing the rank-based tier comparison. Reported on Discord by Netti93 — the "same account works via Tidarr" comparison narrowed the cause to SoulSync's download path rather than an account/region issue. --- core/tidal_download_client.py | 104 ++++++++++-- tests/test_tidal_search_shortening.py | 14 +- tests/test_tidal_stream_tier_verification.py | 162 +++++++++++++++++++ webui/static/helper.js | 1 + 4 files changed, 261 insertions(+), 20 deletions(-) create mode 100644 tests/test_tidal_stream_tier_verification.py diff --git a/core/tidal_download_client.py b/core/tidal_download_client.py index 31884e5c..8e7e718d 100644 --- a/core/tidal_download_client.py +++ b/core/tidal_download_client.py @@ -76,6 +76,86 @@ if tidalapi is not None: QUALITY_MAP['hires']['tidal_quality'] = tidalapi.Quality.hi_res_lossless +# Ordering of Tidal's audioQuality values, worst to best. Used to accept +# tier upgrades (Tidal serving higher than the user asked) while still +# rejecting downgrades. Values are the strings tidalapi's `Quality` enum +# exposes — and the strings Tidal's API returns in the `audioQuality` +# field. `HI_RES` (legacy MQA) isn't in the modern `Quality` enum but +# may still come back for old catalog tracks; we rank it below +# `HI_RES_LOSSLESS` so it's treated as a downgrade when the user asked +# for true HiRes lossless. +_QUALITY_RANK = { + 'LOW': 1, + 'HIGH': 2, + 'LOSSLESS': 3, + 'HI_RES': 4, + 'HI_RES_LOSSLESS': 5, +} + + +def _verify_stream_tier(stream, q_info: dict, q_key: str) -> Tuple[bool, Optional[str]]: + """Return ``(True, None)`` when the tier Tidal actually served is + acceptable (same as requested, or a higher tier), ``(False, reason)`` + when Tidal silently downgraded. + + Tidal's API degrades quality without raising: ask for HI_RES_LOSSLESS + on a track that's only in LOW_320K and you get LOW_320K back with no + error. The downloader used to accept that and write the resulting + AAC file, which defeated "HiRes only" with no fallback and made the + worker's fallback chain ineffective (every tier "succeeded" at the + first one that returned anything). + + We accept upgrades because Tidal occasionally serves a higher tier + than requested on tracks flagged as such in its catalog — rejecting + a higher quality than asked for would be user-hostile. + + Defensive paths: + - No ``audio_quality`` on the stream (older tidalapi builds): pass + through, let the pre-existing codec / file-size guards decide. + - QUALITY_MAP entry without ``tidal_quality`` (tidalapi wasn't + importable at module load): pass through for the same reason. + - Unrecognized served quality value (new Tidal tier we haven't + mapped yet): reject, surfacing a "can't verify" reason so the + next tier gets a chance or the final diagnostic names the + unknown value. + """ + served = getattr(stream, 'audio_quality', None) + expected = q_info.get('tidal_quality') + if served is None or expected is None: + return True, None + + # Both sides may be enum instances (str subclass) or plain strings; + # coerce to str to compare values only. + served_str = str(served) + expected_str = str(expected) + + if served_str == expected_str: + return True, None + + served_rank = _QUALITY_RANK.get(served_str) + expected_rank = _QUALITY_RANK.get(expected_str) + + if expected_rank is None: + # Shouldn't happen — every entry in QUALITY_MAP resolves to a + # known tier. If it does, don't reject valid downloads. + return True, None + + if served_rank is None: + return False, ( + f"{q_key}: Tidal returned unrecognized audioQuality " + f"'{served_str}' — can't verify the tier matches '{expected_str}'" + ) + + if served_rank >= expected_rank: + return True, None + + return False, ( + f"{q_key}: Tidal served '{served_str}' instead of " + f"'{expected_str}' — account tier, track licensing, " + f"or region doesn't permit {q_key} for this track" + ) + + class TidalDownloadClient: """ Tidal download client using tidalapi. @@ -654,6 +734,13 @@ class TidalDownloadClient: logger.warning(f"Quality {q_key} returned no stream, trying next") quality_error_reasons.append(reason) continue + + ok, reason = _verify_stream_tier(stream, q_info, q_key) + if not ok: + logger.warning(reason) + quality_error_reasons.append(reason) + continue + logger.info(f"Got Tidal stream at quality: {q_key}") except Exception as e: reason = f"{q_key}: {type(e).__name__}: {e}" @@ -672,7 +759,8 @@ class TidalDownloadClient: download_url = urls[0] - # Determine file extension from manifest + # Determine file extension from manifest codec (HiRes FLAC + # can arrive wrapped in MP4 — unwrapped at Step 4). codec = manifest.get_codecs() if codec and 'flac' in codec.lower(): extension = 'flac' @@ -683,20 +771,6 @@ class TidalDownloadClient: else: extension = q_info.get('extension', 'flac') - # Verify quality wasn't silently downgraded: if HiRes was requested but the - # codec/manifest points to standard FLAC, log a clear warning. - if q_key == 'hires' and codec: - codec_lower = codec.lower() - if 'flac' in codec_lower or 'alac' in codec_lower: - # HiRes should be 24-bit — we can't confirm bit-depth from the codec - # string alone, but we log the received codec so users can diagnose. - logger.info(f"HiRes stream codec: {codec} (verify file bit-depth after download)") - elif 'mp4a' in codec_lower or 'aac' in codec_lower: - logger.warning( - f"HiRes requested but received AAC stream (codec: {codec}) — " - f"account may not have HiRes subscription or track isn't available in HiRes" - ) - # Build output filename safe_name = re.sub(r'[<>:"/\\|?*]', '_', display_name) out_filename = f"{safe_name}.{extension}" diff --git a/tests/test_tidal_search_shortening.py b/tests/test_tidal_search_shortening.py index 50920fad..334b3610 100644 --- a/tests/test_tidal_search_shortening.py +++ b/tests/test_tidal_search_shortening.py @@ -16,11 +16,15 @@ 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' + # Values mirror the real tidalapi Quality enum (the strings the + # Tidal API returns in `audioQuality`). Keeping these honest + # lets sibling tests that actually compare quality values rely + # on the same stub regardless of pytest collection order. + low_96k = 'LOW' + low_320k = 'HIGH' + high_lossless = 'LOSSLESS' + hi_res = 'HI_RES' + hi_res_lossless = 'HI_RES_LOSSLESS' _fake.Quality = _FakeQuality _fake.media = types.SimpleNamespace(Track=object) diff --git a/tests/test_tidal_stream_tier_verification.py b/tests/test_tidal_stream_tier_verification.py new file mode 100644 index 00000000..73898a55 --- /dev/null +++ b/tests/test_tidal_stream_tier_verification.py @@ -0,0 +1,162 @@ +"""Tests for `_verify_stream_tier` — the guard that rejects silent Tidal +quality downgrades so the fallback chain (or "HiRes only" with fallback +disabled) behaves the way users configure it to. + +Without this check, a user with "HiRes only, no quality fallback" who +asks Tidal for a track that's only available in AAC 320kbps would +receive the 320kbps stream silently — Tidal never raises, it just +serves the highest tier available — and the downloader would accept +the m4a file and report success. Reported by Netti93. + +Tiers ranked worst-to-best: + LOW < HIGH < LOSSLESS < HI_RES < HI_RES_LOSSLESS + +Accepting matches and upgrades, rejecting downgrades, rejecting +unrecognized values. + +Note on the fake Quality values: tidalapi's real Quality enum has +VALUES that differ from the member names (e.g., `low_320k.value == +'HIGH'`, `high_lossless.value == 'LOSSLESS'`). The stub mirrors real +values so the tests catch case-sensitivity regressions. +""" + +import sys +import types + + +if 'tidalapi' not in sys.modules: + _fake = types.ModuleType('tidalapi') + + class _FakeQuality: + low_96k = 'LOW' + low_320k = 'HIGH' + high_lossless = '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 QUALITY_MAP, _verify_stream_tier # noqa: E402 + + +class _FakeStream: + """Minimal stand-in for tidalapi.media.Stream.""" + + def __init__(self, audio_quality=None): + if audio_quality is not None: + self.audio_quality = audio_quality + + +# --------------------------------------------------------------------------- +# Match — served quality is exactly what was requested +# --------------------------------------------------------------------------- + +def test_served_quality_matches_request(): + stream = _FakeStream(audio_quality='HI_RES_LOSSLESS') + ok, reason = _verify_stream_tier(stream, QUALITY_MAP['hires'], 'hires') + assert ok is True + assert reason is None + + +# --------------------------------------------------------------------------- +# Upgrades — Tidal serving a higher tier than requested is accepted +# --------------------------------------------------------------------------- + +def test_lossless_request_upgraded_to_hires_is_accepted(): + """If Tidal serves HI_RES_LOSSLESS on a LOSSLESS-tier request (rare + but possible on tracks flagged as such in Tidal's catalog), we take + the upgrade — rejecting a better-than-asked tier would be user- + hostile.""" + stream = _FakeStream(audio_quality='HI_RES_LOSSLESS') + ok, reason = _verify_stream_tier(stream, QUALITY_MAP['lossless'], 'lossless') + assert ok is True + assert reason is None + + +def test_lossless_request_upgraded_to_mqa_hires_is_accepted(): + stream = _FakeStream(audio_quality='HI_RES') + ok, reason = _verify_stream_tier(stream, QUALITY_MAP['lossless'], 'lossless') + assert ok is True + assert reason is None + + +def test_low_request_upgraded_to_any_higher_tier_is_accepted(): + stream = _FakeStream(audio_quality='LOSSLESS') + ok, reason = _verify_stream_tier(stream, QUALITY_MAP['low'], 'low') + assert ok is True + assert reason is None + + +# --------------------------------------------------------------------------- +# Downgrades — the reported bug +# --------------------------------------------------------------------------- + +def test_hires_downgraded_to_aac_is_rejected(): + """The exact case Netti93 reported: asked HiRes, Tidal served + AAC 320kbps (`'HIGH'` in Tidal's API vocabulary).""" + stream = _FakeStream(audio_quality='HIGH') + ok, reason = _verify_stream_tier(stream, QUALITY_MAP['hires'], 'hires') + assert ok is False + assert 'HIGH' in reason + assert 'HI_RES_LOSSLESS' in reason + + +def test_hires_lossless_downgraded_to_mqa_hires_is_rejected(): + """User explicitly asked for HI_RES_LOSSLESS (true lossless HiRes). + Getting MQA-encoded HI_RES is a downgrade even though both are + "HiRes tier" marketing-wise — MQA is lossy.""" + stream = _FakeStream(audio_quality='HI_RES') + ok, reason = _verify_stream_tier(stream, QUALITY_MAP['hires'], 'hires') + assert ok is False + assert 'HI_RES_LOSSLESS' in reason + + +def test_lossless_downgraded_to_aac_is_rejected(): + stream = _FakeStream(audio_quality='HIGH') + ok, reason = _verify_stream_tier(stream, QUALITY_MAP['lossless'], 'lossless') + assert ok is False + assert 'LOSSLESS' in reason + + +# --------------------------------------------------------------------------- +# Unknown quality strings — reject conservatively +# --------------------------------------------------------------------------- + +def test_unknown_served_quality_is_rejected(): + """If Tidal introduces a new tier we haven't mapped yet, we can't + prove it's acceptable — reject rather than silently pass through, + so the next fallback tier gets a chance and the final diagnostic + log names the unknown value.""" + stream = _FakeStream(audio_quality='SPATIAL_360_DREAM_TIER') + ok, reason = _verify_stream_tier(stream, QUALITY_MAP['hires'], 'hires') + assert ok is False + assert 'SPATIAL_360_DREAM_TIER' in reason + assert 'unrecognized' in reason.lower() or 'can\'t verify' in reason.lower() + + +# --------------------------------------------------------------------------- +# Defensive — missing attributes must not spuriously fail downloads +# --------------------------------------------------------------------------- + +def test_stream_without_audio_quality_attr_is_accepted(): + """Older tidalapi versions may not expose audio_quality — treat as + "can't verify" and let pre-existing codec / file-size guards decide. + Better to miss a downgrade than break every Tidal download after a + library upgrade.""" + stream = _FakeStream() + assert not hasattr(stream, 'audio_quality') + ok, reason = _verify_stream_tier(stream, QUALITY_MAP['hires'], 'hires') + assert ok is True + assert reason is None + + +def test_quality_info_without_tidal_quality_is_accepted(): + """If QUALITY_MAP somehow lacks 'tidal_quality' (tidalapi failed to + import at module load), don't spuriously reject streams.""" + stream = _FakeStream(audio_quality='HI_RES_LOSSLESS') + ok, reason = _verify_stream_tier(stream, {'label': 'x'}, 'hires') + assert ok is True + assert reason is None diff --git a/webui/static/helper.js b/webui/static/helper.js index ff4a7871..9e65a2f8 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -3443,6 +3443,7 @@ const WHATS_NEW = { '2.40': [ // --- Search & Artists unification (in progress, not yet released) --- { date: 'Unreleased — Search & Artists unification', unreleased: true }, + { title: 'Tidal: Reject Silent Quality Downgrades', desc: 'Netti93 reported that with Tidal set to "HiRes only" and quality fallback disabled, tracks were still downloading successfully — as m4a 320kbps files. Root cause: Tidal\'s API silently serves whatever tier your account + the track + your region permits. Ask for HI_RES_LOSSLESS on a track that\'s only in LOW_320K and Tidal returns the AAC stream without raising. The downloader wrote the m4a to disk, the filesize cleared the 100KB stub threshold, and the download reported success. The worker-level fallback chain (hires → lossless → high → low) also never got a chance to advance, because every tier "succeeded" at the first one that returned anything. Fix: after getting the stream, compare stream.audio_quality against what we requested using a rank-based tier comparison (LOW < HIGH < LOSSLESS < HI_RES < HI_RES_LOSSLESS). Same tier or better = accept (so occasional Tidal upgrades don\'t get thrown away). Lower tier = treat this tier as failed, which lets the fallback chain advance when fallback is enabled or fails the whole download honestly when the user has "HiRes only, no fallback" configured. Unrecognized audioQuality values (a new Tidal tier we haven\'t mapped yet) are rejected conservatively so the final diagnostic log can name the unknown value. Older tidalapi builds without the audio_quality attribute fall through to the pre-existing codec / file-size guards so nothing regresses', page: 'downloads', unreleased: true }, { title: 'Search Source Picker Icon Row', desc: 'The Search page now has a row of source icons above the search bar — one per source (Spotify, Apple Music, Deezer, Discogs, Hydrabase, MusicBrainz, Music Videos, Soulseek). Typing searches only the currently-selected source instead of fanning out to every one by default. Click a different icon to switch; results come back on demand. The default icon on page load is your configured primary metadata source. Replaces the short-lived "Search from" dropdown that preceded this', page: 'search', unreleased: true }, { title: 'Per-Query Source Cache (No More Re-Fetching)', desc: 'Once you\'ve searched a source for a given query, switching back to it is instant — results are cached for the current query. A small dot on each source icon shows which ones already have cached results this query. Type a new query and the whole cache resets. Same behavior in the sidebar global search popover. Net effect: roughly 6-7x fewer API calls per search compared to the old default fan-out', page: 'search', unreleased: true }, { title: 'Global Search Widget Source Parity', desc: 'The sidebar Cmd+K / "/" search popover gained the same source icon row as the full Search page. Pick your source up front, see cache dots for already-fetched sources this query, and the rate-limit fallback banner appears if the backend substituted a different source than the one you clicked. Clicking the Soulseek icon hands off to the full Search page (raw file results need more room than the popover provides)', page: 'search', unreleased: true }, From b3afed1599790fce040ca0be9df62146876e9956 Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Fri, 24 Apr 2026 14:27:17 -0700 Subject: [PATCH 2/3] Fix Tidal device-auth link opening SoulSync instead of link.tidal.com MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "Link Tidal Account" device-flow UI displayed a verification URL like `link.tidal.com/XBXYT` that, when clicked, navigated back to the SoulSync origin (e.g. `http://localhost:8889/link.tidal.com/XBXYT`) instead of to Tidal's activation page. Root cause: tidalapi returns `login.verification_uri_complete` as a schemeless string. settings.js drops it straight into ``, and browsers treat schemeless hrefs as same-origin relative URLs. Normalize the URI in `start_device_auth` — if it doesn't already start with `http://` or `https://`, prepend `https://`. Same treatment for the `link.tidal.com/{user_code}` fallback so the defensive path stays well-formed too. --- core/tidal_download_client.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/core/tidal_download_client.py b/core/tidal_download_client.py index 8e7e718d..0823e56d 100644 --- a/core/tidal_download_client.py +++ b/core/tidal_download_client.py @@ -265,8 +265,16 @@ class TidalDownloadClient: login, future = self.session.login_oauth() self._device_auth_future = future + # tidalapi returns `verification_uri_complete` as a schemeless + # string like `link.tidal.com/ABCDE`. Passing that straight to + # an makes the browser treat it as a relative URL and + # route it back to the SoulSync origin, so normalize to a + # full https:// URL here. + raw_uri = login.verification_uri_complete or f"link.tidal.com/{login.user_code}" + if not raw_uri.startswith(('http://', 'https://')): + raw_uri = f"https://{raw_uri}" self._device_auth_link = { - 'verification_uri': login.verification_uri_complete or f"https://link.tidal.com/{login.user_code}", + 'verification_uri': raw_uri, 'user_code': login.user_code, } logger.info(f"Tidal device auth started — code: {login.user_code}") From 252121ca9658b9bf5de7ba0c7a19236ee5179378 Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Fri, 24 Apr 2026 16:02:15 -0700 Subject: [PATCH 3/3] Bump Spotify post-ban cooldown from 5 min to 30 min MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported on Discord by winecountrygames — Spotify auth granted, then re-banned for 4 hours within ~30 seconds, repeatedly. Trace from his captured log: < 12:05 [pre-log] Spotify ban active when log starts 15:21:27 First ban EXPIRED → 5-minute post-ban cooldown begins 15:26:27 Cooldown ends, spotify_client.is_authenticated() probe allowed again → client initialized 15:26:59 First Spotify API call after cooldown — get_artist_albums for an artist whose discography a background worker was enriching — gets 429 immediately with no Retry-After header → new ban activated for 14400s (4 hours) Root cause: `_POST_BAN_COOLDOWN = 300` (5 minutes) is shorter than Spotify's actual server-side memory of the previous offense. The cooldown exists specifically to prevent the "ban expires → we probe → re-ban" cycle (`spotify_client.py:65-68` documents that intent explicitly), but the value was wrong: Spotify's server still considered this user banned 5 minutes after our local ban window ended, so the very first call after cooldown got slapped. The 4-hour re-ban itself is correct behavior — `_BASE_MAX_RETRIES_BAN` fires when spotipy reports "max retries", which means the client exhausted its internal retry budget on 429s before raising. That's a severe-ban signal and a long default is the right response. Fix: bump `_POST_BAN_COOLDOWN` to 1800 seconds (30 min). This is the smallest change that addresses the immediate "re-probe → re-ban" loop in the report. 30 minutes is an empirical floor — long enough for Spotify to actually clear its server-side memory in the cases we've observed, short enough not to keep functional users locked out beyond necessary. Can be revisited if reports persist. What this PR does NOT fix (important context for the same user): This bump only helps the "ban expires → we re-probe → re-ban" loop. It does NOT help winecountrygames's other symptom — Spotify being banned within 30 seconds of his FIRST EVER authorization (no prior ban). That's a separate failure mode: on first auth, enrichment workers immediately fan out across the user's library (250 artists in his case), hammering Spotify endpoints with bulk get_artist_albums calls before any rate-limit feedback can land. Spotify's hidden per-endpoint daily quotas — which BoulderBadgeDad has empirically documented but the global rate limiter doesn't see — flag the burst and impose a multi-hour cooldown that LOOKS like a bot-detection ban to us. A proper fix needs a fresh-auth ramp-up: start with very low Spotify QPS for the first N minutes, scale up only if no rate-limit feedback arrives. That's a separate PR. Documented as additional follow-ups (NOT in this change): - Adaptive cooldown that scales with the size of the previous ban — a 4-hour MAX_RETRIES ban probably warrants a 1-hour cooldown, while a 60-second Retry-After-honored ban can resume in 5 minutes. The system already distinguishes these in `_set_global_rate_limit`, it just doesn't propagate the distinction to cooldown duration. - Probe-with-light-call pattern — make the first post-cooldown call a single inexpensive endpoint (`current_user`) rather than allowing a background worker's heavy `get_artist_albums` to be the canary. Failed probe extends cooldown silently instead of triggering a fresh 4-hour ban. - Fresh-auth ramp-up (per the limitation above). Files: - core/spotify_client.py — `_POST_BAN_COOLDOWN` 300 → 1800. Comment expanded to cite the report so the value isn't bumped back without context. - webui/static/helper.js — WHATS_NEW entry under 2.40 explaining the change for affected users. No tests added — the cooldown logic itself is unchanged, only the constant. Tests asserting on a constant value are theater. Reported on Discord by winecountrygames — his captured log made the "ban-expires-to-re-ban" timing chain unambiguous. --- core/spotify_client.py | 12 +++++++++--- webui/static/helper.js | 1 + 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/core/spotify_client.py b/core/spotify_client.py index 90c2f336..322e9021 100644 --- a/core/spotify_client.py +++ b/core/spotify_client.py @@ -63,9 +63,15 @@ _rate_limit_first_hit = 0 # Timestamp of the first hit in the current escalat _LONG_RATE_LIMIT_THRESHOLD = 60 # seconds # After a ban expires, wait this long before making any auth probe calls. -# This prevents the "immediate re-probe → re-ban" cycle where Spotify's server-side -# cooldown outlasts the Retry-After value they sent us. -_POST_BAN_COOLDOWN = 300 # 5 minutes +# This prevents the "immediate re-probe → re-ban" cycle where Spotify's +# server-side cooldown outlasts the Retry-After (or our default ban +# duration) we used. A user who'd just sat through a 4-hour MAX_RETRIES +# ban had it expire, hit our 5-minute cooldown, made a single +# get_artist_albums call 32 seconds after the cooldown ended, and got +# slapped with another 4-hour ban — the post-ban cooldown was too short +# for Spotify's server to forget the previous offense. 30 minutes is a +# better empirical floor; can be revisited if reports persist. +_POST_BAN_COOLDOWN = 1800 # 30 minutes # Escalation: if we get rate limited again within this window, increase ban duration _ESCALATION_WINDOW = 3600 # 1 hour — if re-limited within this, escalate diff --git a/webui/static/helper.js b/webui/static/helper.js index 9e65a2f8..e559977d 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -3443,6 +3443,7 @@ const WHATS_NEW = { '2.40': [ // --- Search & Artists unification (in progress, not yet released) --- { date: 'Unreleased — Search & Artists unification', unreleased: true }, + { title: 'Spotify: Longer Post-Ban Cooldown (30 min)', desc: 'A user reported their Spotify rate-limit ban expired after 4 hours, the system ran its 5-minute post-ban cooldown, and then 32 seconds after the cooldown ended a single get_artist_albums call from a background worker was hit with another 4-hour ban. Diagnosis: Spotify\'s server-side memory of the previous offense outlasted our 5-minute cooldown, so the very first call after cooldown got slapped immediately. The cooldown exists specifically to prevent the "ban expires → we probe → re-ban" cycle, but the value was too short. Bumped from 5 minutes to 30 minutes — same mechanism, just enough room for Spotify to actually forget. A more principled follow-up (adaptive cooldown that scales with the previous ban size, plus making the first post-cooldown call a single light probe rather than allowing background workers through) is documented as a future PR if reports persist after this bump', page: 'dashboard', unreleased: true }, { title: 'Tidal: Reject Silent Quality Downgrades', desc: 'Netti93 reported that with Tidal set to "HiRes only" and quality fallback disabled, tracks were still downloading successfully — as m4a 320kbps files. Root cause: Tidal\'s API silently serves whatever tier your account + the track + your region permits. Ask for HI_RES_LOSSLESS on a track that\'s only in LOW_320K and Tidal returns the AAC stream without raising. The downloader wrote the m4a to disk, the filesize cleared the 100KB stub threshold, and the download reported success. The worker-level fallback chain (hires → lossless → high → low) also never got a chance to advance, because every tier "succeeded" at the first one that returned anything. Fix: after getting the stream, compare stream.audio_quality against what we requested using a rank-based tier comparison (LOW < HIGH < LOSSLESS < HI_RES < HI_RES_LOSSLESS). Same tier or better = accept (so occasional Tidal upgrades don\'t get thrown away). Lower tier = treat this tier as failed, which lets the fallback chain advance when fallback is enabled or fails the whole download honestly when the user has "HiRes only, no fallback" configured. Unrecognized audioQuality values (a new Tidal tier we haven\'t mapped yet) are rejected conservatively so the final diagnostic log can name the unknown value. Older tidalapi builds without the audio_quality attribute fall through to the pre-existing codec / file-size guards so nothing regresses', page: 'downloads', unreleased: true }, { title: 'Search Source Picker Icon Row', desc: 'The Search page now has a row of source icons above the search bar — one per source (Spotify, Apple Music, Deezer, Discogs, Hydrabase, MusicBrainz, Music Videos, Soulseek). Typing searches only the currently-selected source instead of fanning out to every one by default. Click a different icon to switch; results come back on demand. The default icon on page load is your configured primary metadata source. Replaces the short-lived "Search from" dropdown that preceded this', page: 'search', unreleased: true }, { title: 'Per-Query Source Cache (No More Re-Fetching)', desc: 'Once you\'ve searched a source for a given query, switching back to it is instant — results are cached for the current query. A small dot on each source icon shows which ones already have cached results this query. Type a new query and the whole cache resets. Same behavior in the sidebar global search popover. Net effect: roughly 6-7x fewer API calls per search compared to the old default fan-out', page: 'search', unreleased: true },