diff --git a/core/hifi_client.py b/core/hifi_client.py index 4c2ed260..f748f13e 100644 --- a/core/hifi_client.py +++ b/core/hifi_client.py @@ -984,14 +984,16 @@ class HiFiClient(DownloadSourcePlugin): continue # Preview guard #1 (pre-download): a preview manifest serves only ~30s of - # segments for a full-length track. Catch it from the EXTINF sum before - # wasting the download, so the orchestrator falls through to the next source. + # segments for a full-length track. A preview means THIS SOURCE only has a + # preview of the track — lower quality tiers are the SAME preview — so abort + # HiFi entirely and let the orchestrator fall through to the next SOURCE + # (soulseek/youtube/…), rather than landing a lower-tier preview. manifest_s = float(manifest_info.get('manifest_duration') or 0) if is_short_audio(manifest_s, expected_s): logger.warning( - "HiFi served a PREVIEW manifest at %s for '%s' (%.0fs of %.0fs) — skipping", - q_key, display_name, manifest_s, expected_s) - continue + "HiFi has only a PREVIEW of '%s' (manifest %.0fs of %.0fs at %s) — " + "failing HiFi so the next source is tried", display_name, manifest_s, expected_s, q_key) + return None extension = manifest_info['extension'] safe_name = re.sub(r'[<>:"/\\|?*]', '_', display_name) @@ -1087,11 +1089,15 @@ class HiFiClient(DownloadSourcePlugin): bits_per_sample=(props[1] if props else 0), channels=(props[2] if props else 0)) if fake: + # A preview at this tier means the SOURCE only has a preview — every + # lower tier is the same 30s clip (and the lossy ones dodge the + # bitrate check). Abort HiFi so the orchestrator tries the next + # SOURCE, instead of cascading down into an accepted lower-tier preview. logger.warning( - "HiFi served a PREVIEW/truncated file at %s for '%s' (%s) — rejecting", - q_key, display_name, why) + "HiFi has only a PREVIEW of '%s' (%s at %s) — failing HiFi so the " + "next source is tried", display_name, why, q_key) out_path.unlink(missing_ok=True) - continue + return None logger.info(f"HiFi download complete ({q_key}): {out_path} " f"({final_size / (1024*1024):.1f} MB)") diff --git a/tests/test_hifi_preview_guard.py b/tests/test_hifi_preview_guard.py index d8d7a38b..ed085933 100644 --- a/tests/test_hifi_preview_guard.py +++ b/tests/test_hifi_preview_guard.py @@ -96,7 +96,9 @@ def test_download_sync_skips_preview_manifests_and_never_downloads(tmp_path, mon result = c._download_sync('dl1', 12345, 'The Weeknd - Save Your Tears') assert result is None # → orchestrator falls back - assert tiers # it did consult the manifest(s) + # A preview means the SOURCE only has a preview — every lower tier is the same clip, + # so it must ABORT on the first preview, not cascade down into a lower-tier preview. + assert tiers == ['lossless'] def test_download_sync_proceeds_past_the_gate_for_a_full_manifest(tmp_path, monkeypatch): @@ -117,6 +119,27 @@ def test_download_sync_proceeds_past_the_gate_for_a_full_manifest(tmp_path, monk assert seg_calls # it got PAST the preview gate to download +def test_download_sync_aborts_on_a_faked_full_length_file_no_tier_cascade(tmp_path, monkeypatch): + # The real #895 case: manifest + container claim FULL length, but the finished file + # decodes to 30s. It must abort HiFi (return None) on the first tier — NOT drop to the + # lossy 'high' tier (the same 30s preview, which dodges the bitrate check). + monkeypatch.setattr(hc, 'config_manager', _Cfg()) + c = _bare_client(tmp_path) + c.get_track_info = lambda tid: {'duration_s': 215} + tiers = [] + c._get_hls_manifest = lambda tid, quality='lossless': ( + tiers.append(quality) or + {'segment_uris': ['seg'], 'init_uri': None, 'extension': 'flac', 'manifest_duration': 215.0}) + c._download_segment_with_retry = lambda url: b'\x00' * 200_000 # > MIN_AUDIO_SIZE + c._demux_flac = lambda i, o: o.write_bytes(b'\x00' * 200_000) # produce the 'flac' + c._probe_real_seconds = lambda p: 30.0 # decodes to 30s + c._probe_audio_seconds = lambda p: 215.0 # faked container claim + c._flac_props = lambda p: (44100, 16, 2) + + result = c._download_sync('dl1', 12345, 'The Weeknd - Save Your Tears') + assert result is None and tiers == ['lossless'] # aborted, did NOT try 'high' + + def test_download_sync_does_not_reject_when_track_length_unknown(tmp_path, monkeypatch): monkeypatch.setattr(hc, 'config_manager', _Cfg()) c = _bare_client(tmp_path)