soulsync/tests/test_tidal_search_shortening.py
Broque Thomas a9f827ef42 Reject Tidal streams that silently downgrade from the requested quality
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.
2026-04-24 13:12:30 -07:00

164 lines
6.2 KiB
Python

"""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:
# 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)
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'])