From d62afbb2df5b71cb3678c4f86b80b5b56accce60 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sun, 28 Jun 2026 00:12:38 -0700 Subject: [PATCH 01/47] Release 2.8.0: Discord release notes --- RELEASE_2.8.0_discord.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 RELEASE_2.8.0_discord.md diff --git a/RELEASE_2.8.0_discord.md b/RELEASE_2.8.0_discord.md new file mode 100644 index 00000000..4b2b7c37 --- /dev/null +++ b/RELEASE_2.8.0_discord.md @@ -0,0 +1,13 @@ +**SoulSync 2.8.0** is out πŸŽ‰ a quality + reliability release. + +🧹 **The Unverified queue, finally under control** β€” if you saw thousands of "unverified" rows piling up, this is for you. the AcoustID scan stops duplicating history rows, a one-time reconcile on startup clears the existing backlog from your library (no re-scan), and a new 🧹 *Clean orphaned* button sweeps dead rows whose file is gone. (#934 β€” thanks @nick2000713 for #938) + +βœ‚οΈ **Preview Clip Cleanup** β€” a new Tools job that finds the ~30s preview clips the HiFi source sometimes hands back instead of the full song, then deletes them and re-wishlists the real version. each finding has a β–Ά Play button so you can confirm before approving. + +πŸ’Ώ **Album Completeness handles split albums** β€” an album split across multiple library rows no longer shows every fragment as falsely "incomplete"; it groups the validated fragments into one correct finding. (#936 β€” thanks @ragnarlotus) + +πŸ› **Fixes** β€” pasted YouTube cookies no longer throw `unsupported browser: "custom"` on Docker (thanks HellRa1SeR); longer remasters aren't quarantined as "truncated" anymore (#937, thanks @diegocade1); "Add to Wishlist" from a discography went from ~15–30s *per track* to instant; wishlist art renders for re-downloads; and **Clear Completed** is back on the Downloads page. + +⚑ **Performance** β€” trimmed the dashboard GPU usage that was hammering Firefox/Zen (and Background Particles are OFF by default now), plus bounded the runaway memory growth that could lock the app up on big libraries. (#935 / #802) + +enjoy! 🎢 From 77e3673c9c7ea8918178c5661f40f982becf7768 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sun, 28 Jun 2026 09:32:48 -0700 Subject: [PATCH 02/47] stats: normalize image URLs at cache-build time, not per /api/stats/cached read (#935) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit reported (radoslav-orlov): the Stats page hangs ~20s on a 16GB/HDD box, GET /api/stats/cached ?range=7d -> 200 in ~20000ms, while it's instant on Boulder's SSD. the endpoint is documented "instant" β€” it reads 3 small precomputed metadata blobs β€” but it then ran every top artist/album/track image through normalize_image_url ON THE READ. that fixer calls cache_url_for, which does a SQLite INSERT/UPDATE + commit under a global lock PER image. on an HDD each commit fsyncs and contends with the background image fetcher -> ~20s for ~50 images. (this is exactly the "image caching when we open a page" ramonskie flagged in the thread.) fix: do the normalization in the background ListeningStatsWorker when it builds the cache (_enrich_stats_items), so the cache stores browser-ready /api/image-cache/... URLs and the read does ZERO per-image work. the read-path fixer stays as a cheap no-op (normalize_image_url early- returns on already-proxied URLs), so an old raw-URL cache self-heals on the next rebuild with no broken art in between. the registration writes now happen once per rebuild, off the hot path. 2 tests (existing enrich test relaxed to truthy since the value is normalized now; new deterministic test stubs the fixer to prove every section's url is fixed at build time). 136 stats/listening/image tests green. --- core/listening_stats_worker.py | 13 ++++++-- tests/test_listening_stats_batch_queries.py | 33 ++++++++++++++++++++- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/core/listening_stats_worker.py b/core/listening_stats_worker.py index e2445aac..4010759b 100644 --- a/core/listening_stats_worker.py +++ b/core/listening_stats_worker.py @@ -291,6 +291,13 @@ class ListeningStatsWorker: if not (top_artists or top_albums or top_tracks): return + # Normalize image URLs HERE, at cache-build time, not on every /api/stats/cached + # read. normalize_image_url registers each URL in the image cache (a SQLite write + # under a lock) β€” doing that per-request made the "instant" stats endpoint take ~20s + # on HDD-backed installs (#935). Done once per background rebuild it's off the hot path, + # and the read just returns the already-browser-safe URLs. + from core.metadata import normalize_image_url as _fix_image + conn = None try: conn = self.db._get_connection() @@ -324,7 +331,7 @@ class ListeningStatsWorker: key = (artist.get('name') or '').lower() r = artist_rows.get(key) if r: - artist['image_url'] = r[1] or None + artist['image_url'] = _fix_image(r[1]) or None artist['id'] = r[2] artist['global_listeners'] = r[3] artist['global_playcount'] = r[4] @@ -356,7 +363,7 @@ class ListeningStatsWorker: key = (album.get('name') or '').lower() r = album_rows.get(key) if r: - album['image_url'] = r[1] or None + album['image_url'] = _fix_image(r[1]) or None album['id'] = r[2] album['artist_id'] = r[3] @@ -395,7 +402,7 @@ class ListeningStatsWorker: (track.get('artist') or '').lower()) r = track_rows.get(key) if r: - track['image_url'] = r[2] or None + track['image_url'] = _fix_image(r[2]) or None track['id'] = r[3] track['artist_id'] = r[4] except Exception as e: diff --git a/tests/test_listening_stats_batch_queries.py b/tests/test_listening_stats_batch_queries.py index ea79b4be..09c810c0 100644 --- a/tests/test_listening_stats_batch_queries.py +++ b/tests/test_listening_stats_batch_queries.py @@ -218,13 +218,44 @@ class TestEnrichStatsItems: album_by_name = {a["name"]: a for a in cache["top_albums"]} assert album_by_name["First Album"]["id"] == "al1" - assert album_by_name["First Album"]["image_url"] == "http://img/al1.jpg" + # image_url is normalized at build time now (#935) β€” it's enriched (truthy); + # exact form depends on the image-cache config, so don't pin the raw value. + assert album_by_name["First Album"]["image_url"] track_by_name = {t["name"]: t for t in cache["top_tracks"]} assert track_by_name["Alpha"]["id"] == "t1" assert track_by_name["Bravo"]["id"] == "t2" assert track_by_name["Alpha"]["artist_id"] == "a1" + def test_image_urls_normalized_at_build_time(self, db, worker, monkeypatch): + """#935: image URLs are run through the fixer HERE, at cache-build time, so the + /api/stats/cached read does zero per-image image-cache writes on the hot path + (those writes were the ~20s stats hang on HDD-backed installs). Deterministic + via a stub fixer so it doesn't depend on the real image-cache config.""" + _insert_track(db, "t1", "Alpha", "a1", "Band One", "al1", "First Album") + + seen = [] + + def _fake_fix(url): + seen.append(url) + return f"/api/image-cache/fixed::{url}" + + monkeypatch.setattr("core.metadata.normalize_image_url", _fake_fix) + + cache = { + "top_artists": [{"name": "Band One"}], + "top_albums": [{"name": "First Album"}], + "top_tracks": [{"name": "Alpha", "artist": "Band One"}], + } + worker._enrich_stats_items(cache) + + # every section's raw thumb_url was passed through the fixer, and the cached + # value is the fixed (browser-safe) url β€” so the read path has nothing to do. + assert cache["top_albums"][0]["image_url"] == "/api/image-cache/fixed::http://img/al1.jpg" + assert cache["top_artists"][0]["image_url"].startswith("/api/image-cache/fixed::") + assert cache["top_tracks"][0]["image_url"].startswith("/api/image-cache/fixed::") + assert any("al1" in str(s) for s in seen) + def test_unknown_entries_left_untouched(self, db, worker): _insert_track(db, "t1", "Real", "a1", "Real Band", "al1", "Real Album") From bcf99d76d3f6a2f912708686c2ec1293fba047f7 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sun, 28 Jun 2026 09:49:03 -0700 Subject: [PATCH 03/47] import page: share ONE staging scan across files/groups/hints (#935) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ramonskie's thread finding: opening the Import page fires staging files + groups + hints together, and each one independently os.walk'd the whole staging folder AND mutagen-read every file's tags β€” 3x the directory walk and 3x the per-file tag I/O on every page open (the import scan storm + memory spike on large staging folders). they all need the same per-file tag data, so scan ONCE: _scan_staging_records walks staging and reads each file's metadata a single time, returning per-file records that files/groups/hints all derive from in-memory. a short TTL (6s) + a lock means the three near-simultaneous page-open requests share one scan instead of each kicking off a full re-read; the lock also prevents concurrent full scans. hints now derives from the same read_staging_file_metadata the other two use (same underlying tags) instead of a separate read_tags pass. album/singles process drop the cache on completion so the list updates immediately after files leave staging. net: 1 walk + 1 tag-read-per-file on page open instead of 3. 2 tests (shared-scan: 3 endpoints = 1 read per file, not 3; hints updated to the shared reader) + autouse cache-clear fixture; 695 import/staging tests green. --- core/imports/routes.py | 178 +++++++++++++++++----------- tests/imports/test_import_routes.py | 56 ++++++++- 2 files changed, 160 insertions(+), 74 deletions(-) diff --git a/core/imports/routes.py b/core/imports/routes.py index 39674bba..09043aed 100644 --- a/core/imports/routes.py +++ b/core/imports/routes.py @@ -3,6 +3,8 @@ from __future__ import annotations import os +import threading +import time import uuid from concurrent.futures import as_completed from dataclasses import dataclass @@ -71,36 +73,86 @@ class ImportRouteRuntime: logger: Any = module_logger +# ── Shared staging scan ────────────────────────────────────────────────────── +# Opening the Import page fires staging files/groups/hints together; each used to +# os.walk the whole staging folder AND mutagen-read every file independently β€” 3Γ— +# the directory walk + 3Γ— the tag I/O on every page open (the import-page scan +# storm + memory spike, issue #935). They all need the same per-file tag data, so +# scan ONCE and let all three derive their views in-memory. A short TTL + a lock +# means the three near-simultaneous page-open requests (and any concurrent caller) +# share a single scan instead of each kicking off a full re-read. +_STAGING_SCAN_LOCK = threading.Lock() +_STAGING_SCAN_TTL = 6.0 # seconds β€” covers the page-open burst; re-scans after +_staging_scan_cache: Dict[str, Any] = {"path": None, "ts": 0.0, "records": None} + + +def _scan_staging_records(runtime: ImportRouteRuntime, staging_path: str) -> list[Dict[str, Any]]: + """Walk staging + read each audio file's tags ONCE, returning per-file records + that staging files/groups/hints all derive from. Briefly cached + locked so the + page-open trio shares a single scan rather than each re-walking and re-reading.""" + now = time.time() + cached = _staging_scan_cache + if (cached["records"] is not None and cached["path"] == staging_path + and (now - cached["ts"]) < _STAGING_SCAN_TTL): + return cached["records"] + + with _STAGING_SCAN_LOCK: + # Double-check: another request may have filled the cache while we waited. + now = time.time() + if (cached["records"] is not None and cached["path"] == staging_path + and (now - cached["ts"]) < _STAGING_SCAN_TTL): + return cached["records"] + + records: list[Dict[str, Any]] = [] + if os.path.isdir(staging_path): + for root, _dirs, filenames in os.walk(staging_path): + rel_dir = os.path.relpath(root, staging_path) + top_folder = rel_dir.split(os.sep)[0] if rel_dir != "." else None + for fname in filenames: + ext = os.path.splitext(fname)[1].lower() + if ext not in AUDIO_EXTENSIONS: + continue + full_path = os.path.join(root, fname) + rel_path = os.path.relpath(full_path, staging_path) + meta = runtime.read_staging_file_metadata(full_path, rel_path) + records.append({ + "filename": fname, "rel_path": rel_path, "full_path": full_path, + "extension": ext, "title": meta["title"], "album": meta["album"], + "artist": meta["artist"], "albumartist": meta["albumartist"], + "track_number": meta["track_number"], "disc_number": meta["disc_number"], + "top_folder": top_folder, + }) + + _staging_scan_cache.update({"path": staging_path, "ts": time.time(), "records": records}) + return records + + +def invalidate_staging_scan_cache() -> None: + """Drop the cached staging scan (call after an import moves/removes files so the + next files/groups/hints request reflects the new state immediately).""" + _staging_scan_cache.update({"path": None, "ts": 0.0, "records": None}) + + def staging_files(runtime: ImportRouteRuntime) -> tuple[Dict[str, Any], int]: """Scan the staging folder and return audio files with tag metadata.""" try: staging_path = runtime.get_staging_path() os.makedirs(staging_path, exist_ok=True) - files = [] - for root, _dirs, filenames in os.walk(staging_path): - for fname in filenames: - ext = os.path.splitext(fname)[1].lower() - if ext not in AUDIO_EXTENSIONS: - continue - full_path = os.path.join(root, fname) - rel_path = os.path.relpath(full_path, staging_path) - - meta = runtime.read_staging_file_metadata(full_path, rel_path) - - files.append( - { - "filename": fname, - "rel_path": rel_path, - "full_path": full_path, - "title": meta["title"], - "artist": meta["albumartist"] or meta["artist"] or "Unknown Artist", - "album": meta["album"], - "track_number": meta["track_number"], - "disc_number": meta["disc_number"], - "extension": ext, - } - ) + files = [ + { + "filename": r["filename"], + "rel_path": r["rel_path"], + "full_path": r["full_path"], + "title": r["title"], + "artist": r["albumartist"] or r["artist"] or "Unknown Artist", + "album": r["album"], + "track_number": r["track_number"], + "disc_number": r["disc_number"], + "extension": r["extension"], + } + for r in _scan_staging_records(runtime, staging_path) + ] files.sort(key=lambda f: f["filename"].lower()) return {"success": True, "files": files, "staging_path": staging_path}, 200 @@ -117,31 +169,23 @@ def staging_groups(runtime: ImportRouteRuntime) -> tuple[Dict[str, Any], int]: return {"success": True, "groups": []}, 200 album_groups = {} - for root, _dirs, filenames in os.walk(staging_path): - for fname in filenames: - ext = os.path.splitext(fname)[1].lower() - if ext not in AUDIO_EXTENSIONS: - continue - full_path = os.path.join(root, fname) - rel_path = os.path.relpath(full_path, staging_path) + for r in _scan_staging_records(runtime, staging_path): + album = r["album"] + artist = r["albumartist"] or r["artist"] + if not album or not artist: + continue - meta = runtime.read_staging_file_metadata(full_path, rel_path) - album = meta["album"] - artist = meta["albumartist"] or meta["artist"] - if not album or not artist: - continue - - key = (album.lower().strip(), artist.lower().strip()) - if key not in album_groups: - album_groups[key] = {"album": album.strip(), "artist": artist.strip(), "files": []} - album_groups[key]["files"].append( - { - "filename": fname, - "full_path": full_path, - "title": meta["title"], - "track_number": meta["track_number"], - } - ) + key = (album.lower().strip(), artist.lower().strip()) + if key not in album_groups: + album_groups[key] = {"album": album.strip(), "artist": artist.strip(), "files": []} + album_groups[key]["files"].append( + { + "filename": r["filename"], + "full_path": r["full_path"], + "title": r["title"], + "track_number": r["track_number"], + } + ) groups = [] for group in album_groups.values(): @@ -173,28 +217,15 @@ def staging_hints(runtime: ImportRouteRuntime) -> tuple[Dict[str, Any], int]: tag_albums = {} folder_hints = {} - for root, _dirs, filenames in os.walk(staging_path): - audio_files = [f for f in filenames if os.path.splitext(f)[1].lower() in AUDIO_EXTENSIONS] - if not audio_files: - continue + for r in _scan_staging_records(runtime, staging_path): + if r["top_folder"]: + folder_hints[r["top_folder"]] = folder_hints.get(r["top_folder"], 0) + 1 - rel_dir = os.path.relpath(root, staging_path) - if rel_dir != ".": - top_folder = rel_dir.split(os.sep)[0] - folder_hints[top_folder] = folder_hints.get(top_folder, 0) + len(audio_files) - - for fname in audio_files: - full_path = os.path.join(root, fname) - try: - tags = runtime.read_tags(full_path) - if tags: - album = (tags.get("album") or [None])[0] - artist = (tags.get("artist") or (tags.get("albumartist") or [None]))[0] - if album: - key = (album.strip(), (artist or "").strip()) - tag_albums[key] = tag_albums.get(key, 0) + 1 - except Exception as exc: - runtime.logger.debug("tag read failed: %s", exc) + album = r["album"] + artist = r["artist"] or r["albumartist"] + if album: + key = (album.strip(), (artist or "").strip()) + tag_albums[key] = tag_albums.get(key, 0) + 1 queries = [] seen_queries_lower = set() @@ -371,6 +402,11 @@ def album_process(runtime: ImportRouteRuntime, data: Dict[str, Any]) -> tuple[Di ) runtime.refresh_import_suggestions_cache() + # Files just left staging β€” drop the shared scan so the next files/groups/hints + # reflects reality immediately instead of waiting out the cache TTL. + if processed > 0: + invalidate_staging_scan_cache() + return {"success": True, "processed": processed, "total": len(matches), "errors": errors}, 200 except Exception as exc: runtime.logger.error("Error processing album import: %s", exc) @@ -506,6 +542,10 @@ def singles_process(runtime: ImportRouteRuntime, files: list[Dict[str, Any]]) -> ) runtime.refresh_import_suggestions_cache() + # Files just left staging β€” drop the shared scan so the list updates immediately. + if processed > 0: + invalidate_staging_scan_cache() + return {"success": True, "processed": processed, "total": len(files), "errors": errors}, 200 except Exception as exc: runtime.logger.error("Error processing singles import: %s", exc) diff --git a/tests/imports/test_import_routes.py b/tests/imports/test_import_routes.py index 34409e9c..d4b6823a 100644 --- a/tests/imports/test_import_routes.py +++ b/tests/imports/test_import_routes.py @@ -1,6 +1,8 @@ import os from concurrent.futures import Future +import pytest + import core.imports.routes as import_routes from core.imports.routes import ( ImportRouteRuntime, @@ -17,6 +19,15 @@ from core.imports.routes import ( ) +@pytest.fixture(autouse=True) +def _clear_staging_scan_cache(): + # The shared staging scan is cached at module level; clear it between tests so + # one test's scan can't satisfy another within the TTL. + import_routes.invalidate_staging_scan_cache() + yield + import_routes.invalidate_staging_scan_cache() + + class _FakeLogger: def __init__(self): self.debug_messages = [] @@ -181,14 +192,21 @@ def test_staging_hints_prefers_tag_queries_then_folder_queries(tmp_path): _touch(tmp_path / "Folder_Album" / "02.mp3") _touch(tmp_path / "Loose" / "track.flac") - def _read_tags(file_path): - if file_path.endswith("01.mp3") or file_path.endswith("02.mp3"): - return {"album": ["Tagged Album"], "artist": ["Tagged Artist"]} - return {} + def _empty(artist="", album="", track_number=0): + return {"title": "", "artist": artist, "albumartist": "", + "album": album, "track_number": track_number, "disc_number": 1} + + # hints now derives from the shared staging scan (read_staging_file_metadata), + # the same reader files/groups use β€” not a separate read_tags pass. + metadata = { + os.path.join("Folder_Album", "01.mp3"): _empty(artist="Tagged Artist", album="Tagged Album", track_number=1), + os.path.join("Folder_Album", "02.mp3"): _empty(artist="Tagged Artist", album="Tagged Album", track_number=2), + os.path.join("Loose", "track.flac"): _empty(), + } runtime = ImportRouteRuntime( get_staging_path=lambda: str(tmp_path), - read_tags=_read_tags, + read_staging_file_metadata=_metadata_for(metadata), logger=_FakeLogger(), ) @@ -606,3 +624,31 @@ def test_singles_process_requires_files(): assert status == 400 assert payload == {"success": False, "error": "No files provided"} + + +def test_staging_scan_is_shared_across_files_groups_hints(tmp_path): + """#935: opening Import fires files+groups+hints together; they must share ONE + staging scan (one walk + one tag read per file), not re-read every file 3Γ—.""" + _touch(tmp_path / "Album" / "01.mp3") + _touch(tmp_path / "Album" / "02.mp3") + + reads = [] + + def _meta(full_path, rel_path): + reads.append(rel_path) + return {"title": "T", "artist": "Artist", "albumartist": "Artist", + "album": "Album", "track_number": 1, "disc_number": 1} + + runtime = ImportRouteRuntime( + get_staging_path=lambda: str(tmp_path), + read_staging_file_metadata=_meta, + logger=_FakeLogger(), + ) + + # All three page-open endpoints, back to back (within the cache TTL). + staging_files(runtime) + staging_groups(runtime) + staging_hints(runtime) + + # 2 files Γ— ONE shared scan = 2 reads β€” not 6 (which is 2 files Γ— 3 endpoints). + assert sorted(reads) == [os.path.join("Album", "01.mp3"), os.path.join("Album", "02.mp3")] From 579617f861e61a912983c4366a01d4cd9142fb80 Mon Sep 17 00:00:00 2001 From: Siddharth Pradhan Date: Sun, 28 Jun 2026 13:36:02 -0400 Subject: [PATCH 04/47] normalize spotify credentials during Oauth --- Modelfile | 2 ++ core/metadata/registry.py | 13 +++++++++---- core/spotify_client.py | 26 +++++++++++++++++++++++++- web_server.py | 17 ++++++++++------- 4 files changed, 46 insertions(+), 12 deletions(-) create mode 100644 Modelfile diff --git a/Modelfile b/Modelfile new file mode 100644 index 00000000..f9a29148 --- /dev/null +++ b/Modelfile @@ -0,0 +1,2 @@ +FROM qwen2.5-coder:7b +PARAMETER num_ctx 16384 diff --git a/core/metadata/registry.py b/core/metadata/registry.py index a6c6ea39..946d84a3 100644 --- a/core/metadata/registry.py +++ b/core/metadata/registry.py @@ -230,14 +230,19 @@ def get_spotify_client_for_profile(profile_id: Optional[int] = None): return client try: - from core.spotify_client import SpotifyClient + from core.spotify_client import SpotifyClient, normalize_spotify_oauth_config from spotipy.oauth2 import SpotifyOAuth import spotipy + normalized_creds = normalize_spotify_oauth_config({ + "client_id": client_id, + "client_secret": client_secret, + "redirect_uri": redirect_uri, + }) auth_manager = SpotifyOAuth( - client_id=client_id, - client_secret=client_secret, - redirect_uri=redirect_uri, + client_id=normalized_creds.get("client_id", client_id), + client_secret=normalized_creds.get("client_secret", client_secret), + redirect_uri=normalized_creds.get("redirect_uri", redirect_uri), scope="user-library-read user-read-private playlist-read-private playlist-read-collaborative user-read-email user-follow-read", cache_path=cache_path, state=f"profile_{profile_id}", diff --git a/core/spotify_client.py b/core/spotify_client.py index b5318037..cf3859ca 100644 --- a/core/spotify_client.py +++ b/core/spotify_client.py @@ -13,6 +13,30 @@ from core.metadata.cache import get_metadata_cache logger = get_logger("spotify_client") + +def normalize_spotify_oauth_config(config: Optional[Dict[str, Any]]) -> Dict[str, Any]: + """Normalize Spotify OAuth config before building an auth manager. + + Spotify rejects values that include surrounding whitespace, quotes, or + newline characters. The settings UI can paste values that carry such + formatting, so we trim them before sending them to Spotify. + """ + if not isinstance(config, dict): + return {} + + normalized = {} + for key in ("client_id", "client_secret", "redirect_uri"): + value = config.get(key, "") + if isinstance(value, str): + value = value.strip().strip('"').strip("'") + if key == "redirect_uri": + value = value.rstrip("/") + normalized[key] = value + else: + normalized[key] = value + return normalized + + def _upgrade_spotify_image_url(url: str) -> str: """Upgrade a Spotify CDN image URL to the highest available resolution. @@ -697,7 +721,7 @@ class SpotifyClient: self._setup_client() def _setup_client(self): - config = config_manager.get_spotify_config() + config = normalize_spotify_oauth_config(config_manager.get_spotify_config()) if not config.get('client_id') or not config.get('client_secret'): logger.warning("Spotify credentials not configured") diff --git a/web_server.py b/web_server.py index 15b5de83..47e846c6 100644 --- a/web_server.py +++ b/web_server.py @@ -4692,11 +4692,13 @@ def _profile_spotify_oauth(profile_id_int): chooser so a user can't silently inherit whatever Spotify session is active in their browser (e.g. the admin's). Returns None if no app creds exist.""" from spotipy.oauth2 import SpotifyOAuth + from core.spotify_client import normalize_spotify_oauth_config creds = (get_database().get_profile_spotify(profile_id_int) or {}) - cfg = config_manager.get_spotify_config() - client_id = creds.get('client_id') or cfg.get('client_id') - client_secret = creds.get('client_secret') or cfg.get('client_secret') - redirect_uri = creds.get('redirect_uri') or cfg.get('redirect_uri', 'http://127.0.0.1:8888/callback') + cfg = normalize_spotify_oauth_config(config_manager.get_spotify_config()) + profile_creds = normalize_spotify_oauth_config(creds) + client_id = profile_creds.get('client_id') or cfg.get('client_id') + client_secret = profile_creds.get('client_secret') or cfg.get('client_secret') + redirect_uri = profile_creds.get('redirect_uri') or cfg.get('redirect_uri', 'http://127.0.0.1:8888/callback') if not client_id or not client_secret: return None return SpotifyOAuth( @@ -5095,7 +5097,7 @@ def spotify_callback(): pass try: - from core.spotify_client import SpotifyClient + from core.spotify_client import SpotifyClient, normalize_spotify_oauth_config from spotipy.oauth2 import SpotifyOAuth from config.settings import config_manager @@ -5125,7 +5127,7 @@ def spotify_callback(): raise Exception("Failed to exchange authorization code for access token") # Global callback (admin) - config = config_manager.get_spotify_config() + config = normalize_spotify_oauth_config(config_manager.get_spotify_config()) configured_uri = config.get('redirect_uri', "http://127.0.0.1:8888/callback") logger.info(f"Using redirect_uri for token exchange: {configured_uri}") @@ -35864,9 +35866,10 @@ def start_oauth_callback_servers(): try: from spotipy.oauth2 import SpotifyOAuth from config.settings import config_manager + from core.spotify_client import normalize_spotify_oauth_config # Get Spotify config - config = config_manager.get_spotify_config() + config = normalize_spotify_oauth_config(config_manager.get_spotify_config()) configured_uri = config.get('redirect_uri', "http://127.0.0.1:8888/callback") _oauth_logger.info(f"Using redirect_uri for token exchange: {configured_uri}") From df0b4d35957722ebfa50e031354e44424cd0c0f5 Mon Sep 17 00:00:00 2001 From: Siddharth Pradhan Date: Sun, 28 Jun 2026 13:36:47 -0400 Subject: [PATCH 05/47] remove modelfile --- Modelfile | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 Modelfile diff --git a/Modelfile b/Modelfile deleted file mode 100644 index f9a29148..00000000 --- a/Modelfile +++ /dev/null @@ -1,2 +0,0 @@ -FROM qwen2.5-coder:7b -PARAMETER num_ctx 16384 From b62d9b5b08447b13b7f2a0053922423165bc4f24 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sun, 28 Jun 2026 11:45:16 -0700 Subject: [PATCH 06/47] quality: recognize DSD (.dsf/.dff) as lossless + stop the false "truncated" flag (#939) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit diegocade1: DSD files (.dsf, ~500MB DSD64) were labeled "Low Quality" and nagged to upgrade. two independent causes, both fixed (additive β€” no existing format/behaviour changed): 1) DSF was an unrecognized format -> bottom 'unknown' tier -> "Low Quality": - source_map: map .dsf/.dff -> 'dsf' (also lights it up in AUDIO_EXTENSIONS, so Soulseek can match a DSF if one exists) - model.tier_score: 'dsf' base 102 (just above FLAC) β€” lands in the lossless range - probe_audio_quality: add a DSD branch returning format='dsf' (mutagen.dsf for .dsf detail; .dff classifies lossless without measured detail) instead of None - settings UI: DSD in RT_LOSSLESS_FORMATS + a "DSD (DSF / DFF)" option in the profile dropdown 2) the actual cause of the screenshot's findings β€” the truncation guard falsely called DSF "broken (only ~12% decodes)": ffmpeg decodes DSD to PCM at a different rate than the DSD container's 2.8 MHz, so astats samples Γ· container-rate massively under-counts. now detect_broken_audio skips the truncation check for DSD (silence detection still applies). 8 seam tests: dsf/dff -> 'dsf'; dsf tier in lossless range (with + without measured bitrate); is_dsd_path; and a contrast pair proving the same 12%-decode numbers flag a .flac but skip a .dsf. 230 quality/import/silence tests green, ruff + JS integrity clean. --- core/imports/file_ops.py | 16 ++++++++++ core/imports/silence.py | 22 +++++++++++--- core/quality/model.py | 1 + core/quality/source_map.py | 3 ++ tests/imports/test_silence_guard.py | 47 +++++++++++++++++++++++++++++ tests/quality/test_model.py | 19 ++++++++++++ tests/quality/test_source_map.py | 1 + webui/index.html | 1 + webui/static/settings.js | 2 +- 9 files changed, 106 insertions(+), 6 deletions(-) diff --git a/core/imports/file_ops.py b/core/imports/file_ops.py index 747d65e2..2f9f245a 100644 --- a/core/imports/file_ops.py +++ b/core/imports/file_ops.py @@ -360,6 +360,22 @@ def probe_audio_quality(file_path: str): sample_rate=getattr(audio.info, 'sample_rate', None), ) + if ext in ('dsf', 'dff'): + # DSD (DSD Stream File / DSDIFF) β€” 1-bit hi-res lossless (#939). mutagen + # reads .dsf (rate/bitrate/bit_depth); .dff has no mutagen reader, so it + # still classifies as the lossless 'dsf' tier just without measured detail. + sr = bd = br = None + if ext == 'dsf': + try: + from mutagen.dsf import DSF + info = DSF(file_path).info + sr = getattr(info, 'sample_rate', None) + bd = getattr(info, 'bits_per_sample', None) + br = info.bitrate // 1000 if getattr(info, 'bitrate', None) else None + except Exception: # noqa: S110 β€” unreadable DSF still classifies lossless, just without measured detail + pass + return AudioQuality(format='dsf', bitrate=br, sample_rate=sr, bit_depth=bd) + return None except Exception as e: logger.debug("probe_audio_quality failed for %s: %s", file_path, e) diff --git a/core/imports/silence.py b/core/imports/silence.py index ecbe048f..5fb63017 100644 --- a/core/imports/silence.py +++ b/core/imports/silence.py @@ -18,6 +18,7 @@ run, so a tooling problem never blocks a legitimate import. from __future__ import annotations +import os import re import subprocess from typing import Optional @@ -157,6 +158,14 @@ def measured_duration_from_astats(astats_stderr: str, sample_rate: int) -> Optio return int(m.group(1)) / float(sample_rate) +def is_dsd_path(file_path: str) -> bool: + """True for DSD audio (.dsf / .dff). The decoded-samples truncation check is + invalid for DSD: ffmpeg decodes DSD to PCM at a different rate than the DSD + container's 2.8 MHz, so samples Γ· container-sample-rate massively under-counts + and would falsely report the file as truncated (#939).""" + return os.path.splitext(str(file_path or ''))[1].lower() in ('.dsf', '.dff') + + def incomplete_audio_reason( measured_s: Optional[float], container_s: Optional[float], @@ -267,11 +276,14 @@ def detect_broken_audio( stderr = proc.stderr.decode("utf-8", errors="replace") if proc.stderr else "" - # Truncation check first (real audio far shorter than the container). - measured_s = measured_duration_from_astats(stderr, sample_rate) - reason = incomplete_audio_reason(measured_s, container_s, min_ratio=min_ratio) - if reason: - return reason + # Truncation check first (real audio far shorter than the container) β€” but + # NOT for DSD: the astats sample-count Γ· DSD-rate math is invalid there and + # would always false-positive (#939). Silence detection below still applies. + if not is_dsd_path(file_path): + measured_s = measured_duration_from_astats(stderr, sample_rate) + reason = incomplete_audio_reason(measured_s, container_s, min_ratio=min_ratio) + if reason: + return reason # Then silence-padding (mostly-silent file). return is_mostly_silent_reason(stderr, container_s, threshold=threshold) diff --git a/core/quality/model.py b/core/quality/model.py index 4fe1c0f9..498b0381 100644 --- a/core/quality/model.py +++ b/core/quality/model.py @@ -39,6 +39,7 @@ class AudioQuality: # matched target. Cross-format PRIORITY is decided solely by the user's # ranked-target list (target index), never by these numbers. format_base: dict[str, float] = { + 'dsf': 102.0, # DSD β€” 1-bit hi-res lossless, ranks at/above FLAC (#939) 'flac': 100.0, 'alac': 98.0, # lossless (Apple) 'wav': 95.0, diff --git a/core/quality/source_map.py b/core/quality/source_map.py index 9a834f4d..0af50eaa 100644 --- a/core/quality/source_map.py +++ b/core/quality/source_map.py @@ -44,6 +44,9 @@ _EXTENSION_FORMAT_MAP = { 'ogg': 'ogg', 'oga': 'ogg', 'opus': 'opus', 'wma': 'wma', + # DSD (DSD Stream File / DSDIFF) β€” 1-bit hi-res lossless (e.g. DSD64 β‰ˆ 11 Mbps). + # Both container types map to the single 'dsf' tier (#939). + 'dsf': 'dsf', 'dff': 'dsf', } # Audio extensions worth probing/classifying at all β€” derived from the map so diff --git a/tests/imports/test_silence_guard.py b/tests/imports/test_silence_guard.py index fe0d9235..5d24375a 100644 --- a/tests/imports/test_silence_guard.py +++ b/tests/imports/test_silence_guard.py @@ -6,7 +6,10 @@ parsers are tested here; the ffmpeg call is integration. import pytest +import core.imports.silence as silence_mod from core.imports.silence import ( + detect_broken_audio, + is_dsd_path, silence_ratio_from_output, is_mostly_silent_reason, measured_duration_from_astats, @@ -102,3 +105,47 @@ def test_no_incomplete_reason_for_full_file(): def test_no_incomplete_reason_when_unmeasurable(): assert incomplete_audio_reason(None, 188.4, min_ratio=0.85) is None assert incomplete_audio_reason(30.0, 0, min_ratio=0.85) is None + + +# ── DSD (#939): the samplesΓ·rate truncation math is invalid for DSD, so it must +# be skipped for .dsf/.dff (silence detection still applies). ── + +def test_is_dsd_path(): + assert is_dsd_path("/m/Album/01. Song.dsf") is True + assert is_dsd_path("/m/Album/01. Song.DFF") is True # case-insensitive + assert is_dsd_path("/m/Album/01. Song.flac") is False + assert is_dsd_path("") is False + assert is_dsd_path(None) is False + + +class _FakeProc: + def __init__(self, stderr): + self.stderr = stderr.encode("utf-8") + + +class _FakeInfo: + length = 330.0 # container says 330s + sample_rate = 44100 + + +def _patch_broken_pipeline(monkeypatch, astats_stderr): + """Make detect_broken_audio run against a canned 'truncated' ffmpeg result.""" + monkeypatch.setattr(silence_mod, "_ffmpeg_available", lambda: True) + monkeypatch.setattr("mutagen.File", lambda *_a, **_k: type("A", (), {"info": _FakeInfo()})()) + monkeypatch.setattr(silence_mod.subprocess, "run", lambda *_a, **_k: _FakeProc(astats_stderr)) + + +def test_truncation_flagged_for_normal_file(monkeypatch): + # ~40s decoded of a 330s container (12%) β†’ a normal file IS flagged truncated. + astats = "[Parsed_astats_0 @ 0x55] Number of samples: 1764000\n" # 1764000/44100 β‰ˆ 40s + _patch_broken_pipeline(monkeypatch, astats) + reason = detect_broken_audio("/m/Album/01. Song.flac", min_ratio=0.85) + assert reason and "Incomplete audio" in reason + + +def test_truncation_skipped_for_dsd(monkeypatch): + # Same 12%-decoding numbers, but a .dsf file must NOT be flagged β€” the math is + # invalid for DSD (ffmpeg decodes DSD to PCM at a different rate). #939 + astats = "[Parsed_astats_0 @ 0x55] Number of samples: 1764000\n" + _patch_broken_pipeline(monkeypatch, astats) + assert detect_broken_audio("/m/Album/01. Song.dsf", min_ratio=0.85) is None diff --git a/tests/quality/test_model.py b/tests/quality/test_model.py index a740f22c..f9aeed69 100644 --- a/tests/quality/test_model.py +++ b/tests/quality/test_model.py @@ -128,3 +128,22 @@ def test_v2_to_v3_preserves_order_and_maps_fields(): assert formats == ['flac', 'mp3'] # disabled mp3_192 omitted assert targets[0]['bit_depth'] == 24 assert targets[1]['min_bitrate'] == 320 + + +# ── DSD (#939): DSF must rank as lossless, never "Low Quality" below MP3 ── + +def test_dsf_ranks_in_lossless_range(): + dsf = AudioQuality('dsf', bitrate=11290).tier_score() + flac_cd = AudioQuality('flac', sample_rate=44100, bit_depth=16).tier_score() + mp3_320 = AudioQuality('mp3', bitrate=320).tier_score() + # DSD64 is hi-res lossless β€” at/above CD FLAC and well above any lossy format. + assert dsf >= flac_cd + assert dsf > mp3_320 + + +def test_dsf_without_measured_bitrate_still_lossless(): + # .dff has no mutagen reader, so it classifies as 'dsf' with no measured detail β€” + # it must still land in the lossless tier, not the 'unknown' floor. + dsf = AudioQuality('dsf').tier_score() + assert dsf > AudioQuality('mp3', bitrate=320).tier_score() + assert dsf > AudioQuality('unknown').tier_score() diff --git a/tests/quality/test_source_map.py b/tests/quality/test_source_map.py index b4d71c43..c17cd337 100644 --- a/tests/quality/test_source_map.py +++ b/tests/quality/test_source_map.py @@ -30,6 +30,7 @@ from core.quality.source_map import ( ("aiff", "wav"), ("aif", "wav"), # PCM β†’ wav tier ("wma", "wma"), ("alac", "alac"), + ("dsf", "dsf"), (".dsf", "dsf"), ("dff", "dsf"), # DSD β†’ dsf tier (#939) ("xyz", "unknown"), ("", "unknown"), (None, "unknown"), ]) def test_format_from_extension(ext, fmt): diff --git a/webui/index.html b/webui/index.html index cfc502c5..15a49c48 100644 --- a/webui/index.html +++ b/webui/index.html @@ -5100,6 +5100,7 @@ + diff --git a/webui/static/settings.js b/webui/static/settings.js index adfceafe..3d91f4b5 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -2094,7 +2094,7 @@ function deleteRankedTarget(i) { // Lossless formats take bit-depth + sample-rate constraints; lossy take a // minimum bitrate. Single source of truth for the add-target field toggle. -const RT_LOSSLESS_FORMATS = ['flac', 'alac', 'wav']; +const RT_LOSSLESS_FORMATS = ['flac', 'alac', 'wav', 'dsf']; const RT_LOSSY_FORMATS = ['mp3', 'aac', 'ogg', 'opus', 'wma']; // "group:" selections are a UI convenience: picking one + constraints expands // into individual per-format targets at that slot (the backend still works From c96135ee60e77a0f042e3154753f68cb5d6ba1b4 Mon Sep 17 00:00:00 2001 From: Siddharth Pradhan Date: Sun, 28 Jun 2026 14:56:28 -0400 Subject: [PATCH 07/47] add tests --- .../test_spotify_oauth_integration.py | 86 +++++++++++++++++++ tests/test_spotify_client.py | 76 ++++++++++++++++ 2 files changed, 162 insertions(+) create mode 100644 tests/metadata/test_spotify_oauth_integration.py create mode 100644 tests/test_spotify_client.py diff --git a/tests/metadata/test_spotify_oauth_integration.py b/tests/metadata/test_spotify_oauth_integration.py new file mode 100644 index 00000000..4bc809cd --- /dev/null +++ b/tests/metadata/test_spotify_oauth_integration.py @@ -0,0 +1,86 @@ +import unittest +from unittest.mock import patch, MagicMock +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[2])) + +from core.metadata.registry import get_spotify_client_for_profile + +# Mock config_manager as it's a global dependency +class MockConfigManager: + def __init__(self): + self.store = {} + + def get(self, key, default=None): + return self.store.get(key, default) + + def get_spotify_config(self): + return self.store.get('spotify', {}) + + def set(self, key, value): + self.store[key] = value + +class TestSpotifyOAuthIntegration(unittest.TestCase): + @patch('core.metadata.registry.get_spotify_client') + @patch('core.metadata.registry._profile_spotify_credentials_provider') + @patch('core.metadata.registry._get_config_value') + @patch('spotipy.oauth2.SpotifyOAuth') + @patch('core.spotify_client.normalize_spotify_oauth_config') + def test_get_spotify_client_for_profile_uses_normalized_config(self, mock_normalize, mock_spotify_oauth, mock_get_config, mock_creds_provider, mock_get_global): + # Set up mock config values + mock_creds_provider.return_value = { + "client_id": " original_client_id ", + "client_secret": " original_client_secret ", + "redirect_uri": "http://example.com/callback/" + } + + # Make sure the file exists check passes + with patch('os.path.exists', return_value=True): + # Set up mock for normalize_spotify_oauth_config to return cleaned values + mock_normalize.return_value = { + "client_id": "cleaned_client_id", + "client_secret": "cleaned_client_secret", + "redirect_uri": "http://example.com/callback" + } + + # Call the function under test with profile_id=2 (to bypass global client) + get_spotify_client_for_profile(profile_id=2) + + # Assert that normalize_spotify_oauth_config was called with the original config + mock_normalize.assert_any_call({ + "client_id": " original_client_id ", + "client_secret": " original_client_secret ", + "redirect_uri": "http://example.com/callback/" + }) + + # Assert that SpotifyOAuth was initialized with the normalized config + mock_spotify_oauth.assert_called_once() + args, kwargs = mock_spotify_oauth.call_args + self.assertEqual(kwargs['client_id'], "cleaned_client_id") + self.assertEqual(kwargs['client_secret'], "cleaned_client_secret") + self.assertEqual(kwargs['redirect_uri'], "http://example.com/callback") + self.assertEqual(kwargs['state'], 'profile_2') + + @patch('core.metadata.registry.get_spotify_client') + @patch('core.metadata.registry._profile_spotify_credentials_provider') + @patch('core.metadata.registry._get_config_value') + @patch('spotipy.oauth2.SpotifyOAuth') + @patch('core.spotify_client.normalize_spotify_oauth_config') + def test_get_spotify_client_for_profile_handles_no_config(self, mock_normalize, mock_spotify_oauth, mock_get_config, mock_creds_provider, mock_get_global): + # Simulate no spotify config + mock_creds_provider.return_value = {} + mock_get_config.side_effect = lambda key, default: "" if "client" in key else "http://127.0.0.1:8888/callback" + + # Ensure os.path.exists returns False so it doesn't try to use cache + with patch('os.path.exists', return_value=False): + # Call the function under test + get_spotify_client_for_profile(profile_id=2) + + # It still reaches get_spotify_client() which is mocked. + # In registry.py, the fallbacks for client_id/client_secret result in calls to get_spotify_client(). + mock_get_global.assert_called() + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/test_spotify_client.py b/tests/test_spotify_client.py new file mode 100644 index 00000000..cf9210ee --- /dev/null +++ b/tests/test_spotify_client.py @@ -0,0 +1,76 @@ +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + +from core.spotify_client import normalize_spotify_oauth_config + +def test_normalization(): + # Normal case with leading/trailing whitespace and quotes + config = { + "client_id": " client_id ", + "client_secret": " client_secret ", + "redirect_uri": "http://127.0.0.1:8888/callback/" + } + expected = { + "client_id": "client_id", + "client_secret": "client_secret", + "redirect_uri": "http://127.0.0.1:8888/callback" + } + assert normalize_spotify_oauth_config(config) == expected + +def test_empty_values(): + # Empty input values + config = { + "client_id": "", + "client_secret": None, + "redirect_uri": "" + } + # When value is None, it falls into the else branch: normalized[key] = value + # value is None, so expected is None for client_secret + expected = { + "client_id": "", + "client_secret": None, + "redirect_uri": "" + } + assert normalize_spotify_oauth_config(config) == expected + +def test_missing_keys(): + # Input dictionary with missing keys + config = { + "client_id": "client_id" + } + # .get(key, "") means missing keys become "" + expected = { + "client_id": "client_id", + "client_secret": "", + "redirect_uri": "" + } + assert normalize_spotify_oauth_config(config) == expected + +def test_non_string_values(): + # Input dictionary with non-string values for the keys + config = { + "client_id": 123, + "client_secret": True, + "redirect_uri": None + } + # When value is not a string, it falls into the else branch: normalized[key] = value + expected = { + "client_id": 123, + "client_secret": True, + "redirect_uri": None + } + assert normalize_spotify_oauth_config(config) == expected + +def test_no_input(): + # Empty input dictionary + config = {} + # .get(key, "") means missing keys become "" + expected = { + "client_id": "", + "client_secret": "", + "redirect_uri": "" + } + assert normalize_spotify_oauth_config(None) == {} + assert normalize_spotify_oauth_config(config) == expected \ No newline at end of file From 4af7600fd5bac18e1717152cce9b4dd2eaa7d0d7 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sun, 28 Jun 2026 12:13:05 -0700 Subject: [PATCH 08/47] lossy copy: support all lossless formats, not just FLAC (#941) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit radoslav-orlov: "create lossy copies of lossless tracks" only recognized FLAC, even though ALAC/ WAV/AIFF/DSD are now quality-profile formats. the FLAC knowledge was hardcoded in 3 separate places (the import path, the Lossy Converter scan, and the fix executor) β€” exactly how a format gets added in one spot but not another. kettui-style fix β€” one canonical seam both sites route through, instead of 3 more string edits: - new core/quality/lossless.py: is_lossless_format / is_lossless_audio_path (pure; injects a codec probe for the ambiguous .m4a/.mp4 β€” ALAC vs AAC β€” so the decision stays testable with no I/O), LOSSLESS_FORMATS (single source of truth, derived-consistent with model.tier_score), and the lossy_output_would_overwrite_source safety invariant. - create_lossy_copy + the Lossy Converter scan + repair_worker._fix_missing_lossy_copy all route through it. SQL pre-filters by candidate extensions, then each file is confirmed (probing .m4a). - SAFETY: a lossy copy must never be written over its own source β€” an .m4a ALAC source + AAC target lands on the same .m4a path, and ffmpeg runs with -y. all three sites now bail on the overwrite case BEFORE ffmpeg (the existing delete-original guard was too late β€” the source was already clobbered). dropped a vestigial mutagen FLAC import; updated FLAC-only UI strings. 19 tests: full seam coverage (formats, the .m4a ALAC/AAC probe branch, candidate extensions, the overwrite guard), a tier-model consistency test that fails if the lossless set drifts, and import- site wiring tests β€” WAV now converts (was rejected), and the .m4a-ALAC+AAC overwrite case proves ffmpeg NEVER runs. 286 quality/import/repair tests green, ruff clean. --- core/imports/file_ops.py | 31 ++++++- core/quality/lossless.py | 103 ++++++++++++++++++++++ core/repair_jobs/lossy_converter.py | 49 ++++++++--- core/repair_worker.py | 8 ++ tests/imports/test_import_file_ops.py | 58 ++++++++++++ tests/quality/test_lossless.py | 121 ++++++++++++++++++++++++++ 6 files changed, 355 insertions(+), 15 deletions(-) create mode 100644 core/quality/lossless.py create mode 100644 tests/quality/test_lossless.py diff --git a/core/imports/file_ops.py b/core/imports/file_ops.py index 2f9f245a..89903108 100644 --- a/core/imports/file_ops.py +++ b/core/imports/file_ops.py @@ -524,14 +524,29 @@ def downsample_hires_flac(final_path, context): return None +def m4a_codec(path): + """Codec of an .m4a/.mp4 container ('alac', 'aac', …) or None β€” lets the + lossless check tell ALAC (lossless) from AAC (lossy), since both are .m4a.""" + try: + from mutagen.mp4 import MP4 + return (getattr(MP4(path).info, 'codec', '') or '').lower() or None + except Exception: + return None + + def create_lossy_copy(final_path): - """Convert a FLAC file to a lossy copy using the configured codec.""" - from mutagen.flac import FLAC + """Convert a lossless file (FLAC / ALAC / WAV / AIFF / DSD) to a lossy copy + using the configured codec. Non-lossless inputs are skipped (#941).""" + from core.quality.lossless import ( + is_lossless_audio_path, + lossy_output_would_overwrite_source, + ) if not config_manager.get("lossy_copy.enabled", False): return None - if os.path.splitext(final_path)[1].lower() != ".flac": + # Was FLAC-only; now any lossless source. .m4a is probed (ALAC vs AAC). + if not is_lossless_audio_path(final_path, probe_codec=m4a_codec): return None codec = config_manager.get("lossy_copy.codec", "mp3").lower() @@ -560,6 +575,16 @@ def create_lossy_copy(final_path): out_basename = out_basename.replace(original_quality, quality_label) out_path = os.path.join(os.path.dirname(out_path), out_basename) + # Safety invariant: never write the lossy copy over its own source (an .m4a + # ALAC source + AAC target lands on the same .m4a path). ffmpeg runs with -y, + # so this guard MUST precede it β€” the later delete-original guard is too late. + if lossy_output_would_overwrite_source(final_path, out_path): + logger.info( + f"[Lossy Copy] Skipping β€” {codec.upper()} output would overwrite the " + f"source: {os.path.basename(final_path)}" + ) + return None + ffmpeg_bin = shutil.which("ffmpeg") if not ffmpeg_bin: local = os.path.join(os.path.dirname(__file__), "tools", "ffmpeg") diff --git a/core/quality/lossless.py b/core/quality/lossless.py new file mode 100644 index 00000000..df84dcab --- /dev/null +++ b/core/quality/lossless.py @@ -0,0 +1,103 @@ +"""Single source of truth for "is this audio lossless?" + the lossy-copy +overwrite invariant. + +The "create a lossy copy of lossless tracks" feature lives in two places (the +import post-processing path and the Lossy Converter repair job). Both used to +hardcode ``.flac``, which is exactly how ALAC/WAV/DSD ended up being quality- +profile options but NOT lossy-copy sources (#941). The knowledge of which +formats are lossless now lives HERE, derived from the same format names the +quality model ranks, so adding a format lights it up in both sites at once and +they can never drift. + +Two seams, both pure (no I/O) so they're unit-testable without real files: + +* :func:`is_lossless_format` / :func:`is_lossless_audio_path` β€” eligibility. + ``.m4a``/``.mp4`` are ambiguous (ALAC=lossless, AAC=lossy) and can only be told + apart by codec, so the path check delegates them to an *injected* ``probe_codec`` + callable β€” the file I/O stays at the edge, the decision stays pure. +* :func:`lossy_output_would_overwrite_source` β€” the safety invariant: a lossy copy + must NEVER be written over its own source (which becomes possible once ``.m4a`` + is eligible and the target codec is AAC β†’ same ``.m4a`` path). +""" + +from __future__ import annotations + +import os +from typing import Any, Callable, Optional + +from core.quality.source_map import AUDIO_EXTENSIONS, format_from_extension + +# Canonical lossless format set. MUST stay in sync with the lossless tiers in +# core.quality.model.tier_score and the frontend RT_LOSSLESS_FORMATS β€” the +# consistency test in tests/quality/test_lossless.py pins the tier agreement. +LOSSLESS_FORMATS = frozenset({'flac', 'alac', 'wav', 'dsf'}) + +# Container extensions that may hold EITHER lossless (ALAC) or lossy (AAC) audio β€” +# only a codec probe can decide, so they're never lossless on extension alone. +_AMBIGUOUS_EXTS = frozenset({'m4a', 'mp4'}) + + +def is_lossless_format(fmt: Any) -> bool: + """True when a unified format name (as returned by ``format_from_extension``) + is lossless. ``'aac'`` is False here β€” an ALAC-in-m4a file reports format + ``'aac'`` by extension and must be resolved by codec, not by name.""" + return str(fmt or '').lower() in LOSSLESS_FORMATS + + +# Extensions worth checking as possibly-lossless (a caller can pre-filter SQL by +# these, then confirm each via is_lossless_audio_path). Derived from the format +# map so it can't drift: every extension whose format is lossless, plus the +# ambiguous ALAC containers. Leading dot included (e.g. '.flac', '.m4a'). +LOSSLESS_CANDIDATE_EXTENSIONS = frozenset( + e for e in AUDIO_EXTENSIONS + if is_lossless_format(format_from_extension(e)) or e.lstrip('.') in _AMBIGUOUS_EXTS +) + + +def _ext(path: Any) -> str: + return os.path.splitext(str(path or ''))[1].lower().lstrip('.') + + +def is_lossless_audio_path( + path: Any, + *, + probe_codec: Optional[Callable[[str], Optional[str]]] = None, +) -> bool: + """True when the file at ``path`` is lossless. + + Unambiguous extensions (flac/wav/aiff/dsf/dff/alac) are decided by extension. + ``.m4a``/``.mp4`` are decided by ``probe_codec(path)`` (returns the codec, e.g. + ``'alac'`` or ``'aac'``) β€” without a probe they're treated as NOT lossless, so + a missing probe can never misclassify an AAC file as lossless. + """ + ext = _ext(path) + if is_lossless_format(format_from_extension(ext)): + return True + if ext in _AMBIGUOUS_EXTS and probe_codec is not None: + try: + codec = (probe_codec(str(path)) or '').lower() + except Exception: + return False + return 'alac' in codec + return False + + +def lossy_output_would_overwrite_source(source_path: Any, output_path: Any) -> bool: + """True when the computed lossy-copy output path is the source file itself. + + Safety invariant: the converter must skip (never run ffmpeg with ``-y``) when + this is True, or it would destroy the original. Happens when an ``.m4a`` ALAC + source is converted with the AAC codec (output is also ``.m4a``).""" + if not source_path or not output_path: + return False + a = os.path.normcase(os.path.normpath(str(source_path))) + b = os.path.normcase(os.path.normpath(str(output_path))) + return a == b + + +__all__ = [ + 'LOSSLESS_FORMATS', + 'is_lossless_format', + 'is_lossless_audio_path', + 'lossy_output_would_overwrite_source', +] diff --git a/core/repair_jobs/lossy_converter.py b/core/repair_jobs/lossy_converter.py index da1f1646..7e14adbd 100644 --- a/core/repair_jobs/lossy_converter.py +++ b/core/repair_jobs/lossy_converter.py @@ -1,13 +1,19 @@ -"""Lossy Converter Job β€” finds FLAC files that don't have a lossy copy. +"""Lossy Converter Job β€” finds lossless files that don't have a lossy copy. -Scans the library for FLAC files without a corresponding lossy copy alongside +Scans the library for lossless files without a corresponding lossy copy alongside them, and creates a finding for each. The fix action converts the file using ffmpeg with the user's configured codec/bitrate settings. """ import os +from core.imports.file_ops import m4a_codec from core.library.path_resolver import resolve_library_file_path +from core.quality.lossless import ( + LOSSLESS_CANDIDATE_EXTENSIONS, + is_lossless_audio_path, + lossy_output_would_overwrite_source, +) from core.repair_jobs import register_job from core.repair_jobs.base import JobContext, JobResult, RepairJob from utils.logging_config import get_logger @@ -21,6 +27,16 @@ CODEC_MAP = { } +def _lossless_ext_where(col: str) -> str: + """SQL pre-filter matching files whose extension *might* be lossless. The + final decision (including ALAC-in-.m4a, which needs a codec probe) is made + per-file by is_lossless_audio_path. Extensions are trusted constants from the + quality model, never user input β€” safe to interpolate.""" + return '(' + ' OR '.join( + f"LOWER({col}) LIKE '%{ext}'" for ext in sorted(LOSSLESS_CANDIDATE_EXTENSIONS) + ) + ')' + + def _resolve_file_path(file_path, transfer_folder, download_folder=None, config_manager=None): """Backwards-compat wrapper. Use ``resolve_library_file_path`` directly.""" return resolve_library_file_path( @@ -35,15 +51,15 @@ def _resolve_file_path(file_path, transfer_folder, download_folder=None, config_ class LossyConverterJob(RepairJob): job_id = 'lossy_converter' display_name = 'Lossy Converter' - description = 'Finds FLAC files without a lossy copy' + description = 'Finds lossless files without a lossy copy' help_text = ( - 'Scans your library for FLAC files that don\'t already have a lossy copy ' + 'Scans your library for lossless files (FLAC/ALAC/WAV/AIFF/DSD) that don\'t already have a lossy copy ' '(MP3, Opus, or AAC) alongside them.\n\n' 'Uses the codec setting from your Lossy Copy configuration on the Settings ' 'page. Enable Lossy Copy in Settings first, then run this job to find FLAC ' 'files missing a lossy copy.\n\n' 'Each finding can be fixed individually or in bulk β€” the fix action converts ' - 'the FLAC file using ffmpeg at your configured bitrate.\n\n' + 'the lossless file using ffmpeg at your configured bitrate.\n\n' 'Requires ffmpeg to be installed.' ) icon = 'repair-icon-lossy' @@ -81,14 +97,14 @@ class LossyConverterJob(RepairJob): try: conn = context.db._get_connection() cursor = conn.cursor() - cursor.execute(""" + cursor.execute(f""" SELECT t.id, t.title, ar.name, al.title, t.file_path, al.thumb_url, ar.thumb_url FROM tracks t LEFT JOIN artists ar ON ar.id = t.artist_id LEFT JOIN albums al ON al.id = t.album_id WHERE t.file_path IS NOT NULL AND t.file_path != '' - AND LOWER(t.file_path) LIKE '%.flac' + AND {_lossless_ext_where('t.file_path')} """) tracks = cursor.fetchall() except Exception as e: @@ -104,7 +120,7 @@ class LossyConverterJob(RepairJob): context.update_progress(0, total) if context.report_progress: context.report_progress( - phase=f'Scanning {total} FLAC files for missing {quality_label} copies...', + phase=f'Scanning {total} lossless files for missing {quality_label} copies...', total=total ) @@ -135,8 +151,17 @@ class LossyConverterJob(RepairJob): if not resolved or not os.path.exists(resolved): continue + # Confirm it's actually lossless β€” the SQL pre-filter lets .m4a through, + # which is ALAC (lossless) OR AAC (lossy); only a codec probe decides. + if not is_lossless_audio_path(resolved, probe_codec=m4a_codec): + continue + # Check if lossy copy already exists out_path = os.path.splitext(resolved)[0] + out_ext + # Never offer to convert a file onto itself (e.g. .m4a ALAC + AAC target + # lands on the same path) β€” that conversion would destroy the original. + if lossy_output_would_overwrite_source(resolved, out_path): + continue if os.path.exists(out_path): continue @@ -159,7 +184,7 @@ class LossyConverterJob(RepairJob): file_path=file_path, title=f'No {quality_label} copy: {title or "Unknown"}', description=( - f'FLAC file "{title}" by {artist_name or "Unknown"} does not have ' + f'Lossless file "{title}" by {artist_name or "Unknown"} does not have ' f'a {quality_label} copy alongside it' ), details={ @@ -195,7 +220,7 @@ class LossyConverterJob(RepairJob): context.report_progress( scanned=total, total=total, phase='Complete', - log_line=f'Found {result.findings_created} FLAC files without {quality_label} copies', + log_line=f'Found {result.findings_created} lossless files without {quality_label} copies', log_type='success' if result.findings_created == 0 else 'info' ) @@ -208,10 +233,10 @@ class LossyConverterJob(RepairJob): try: conn = context.db._get_connection() cursor = conn.cursor() - cursor.execute(""" + cursor.execute(f""" SELECT COUNT(*) FROM tracks WHERE file_path IS NOT NULL AND file_path != '' - AND LOWER(file_path) LIKE '%.flac' + AND {_lossless_ext_where('file_path')} """) row = cursor.fetchone() return row[0] if row else 0 diff --git a/core/repair_worker.py b/core/repair_worker.py index 55b682f7..e1ff10c7 100644 --- a/core/repair_worker.py +++ b/core/repair_worker.py @@ -3373,6 +3373,14 @@ class RepairWorker: return {'success': False, 'error': f'Source file not found: {file_path}'} out_path = os.path.splitext(resolved)[0] + out_ext + # Safety invariant: ffmpeg runs with -y, so refuse to convert a file onto + # itself (an .m4a ALAC source + AAC target shares the .m4a path) β€” that + # would destroy the original lossless file (#941). + from core.quality.lossless import lossy_output_would_overwrite_source + if lossy_output_would_overwrite_source(resolved, out_path): + return {'success': False, + 'error': f'{codec.upper()} output would overwrite the source file; ' + f'choose a different lossy codec'} if os.path.exists(out_path): return {'success': True, 'action': 'already_exists', 'message': f'{quality_label} copy already exists'} diff --git a/tests/imports/test_import_file_ops.py b/tests/imports/test_import_file_ops.py index a23df527..a5734e6c 100644 --- a/tests/imports/test_import_file_ops.py +++ b/tests/imports/test_import_file_ops.py @@ -244,3 +244,61 @@ def test_atomic_helper_cleans_temp_and_keeps_source_on_failure(tmp_path, monkeyp assert src.exists() # source preserved on failure assert not dst.exists() # no partial final file assert not list(dstdir.glob(".*ssync-tmp")) # temp cleaned up + + +# ── #941: create_lossy_copy now accepts all lossless sources + never overwrites them ── + +import core.imports.file_ops as _fo + + +def _enable_lossy(monkeypatch, codec="mp3", bitrate="320"): + cfg = {"lossy_copy.enabled": True, "lossy_copy.codec": codec, + "lossy_copy.bitrate": bitrate, "lossy_copy.delete_original": False} + monkeypatch.setattr(_fo.config_manager, "get", lambda k, d=None: cfg.get(k, d)) + monkeypatch.setattr(_fo, "get_audio_quality_string", lambda _p: None) + + +def test_create_lossy_copy_rejects_non_lossless(monkeypatch, tmp_path): + _enable_lossy(monkeypatch) + src = tmp_path / "song.mp3" + src.write_bytes(b"id3") + assert _fo.create_lossy_copy(str(src)) is None # lossy input β†’ nothing to do + + +def test_create_lossy_copy_now_accepts_wav(monkeypatch, tmp_path): + """Was FLAC-only; a WAV must now pass the gate and convert (#941).""" + _enable_lossy(monkeypatch, codec="mp3") + monkeypatch.setattr(_fo.shutil, "which", lambda _name: "/usr/bin/ffmpeg") + monkeypatch.setattr("mutagen.File", lambda *_a, **_k: None) # skip tag write + + seen = {} + + def _fake_run(cmd, **_kw): + seen["cmd"] = cmd + open(cmd[-1], "wb").write(b"fake-mp3") # ffmpeg "writes" the output + return types.SimpleNamespace(returncode=0, stderr="") + + monkeypatch.setattr(_fo.subprocess, "run", _fake_run) + + src = tmp_path / "song.wav" + src.write_bytes(b"RIFF....WAVE") + out = _fo.create_lossy_copy(str(src)) + assert out and out.endswith(".mp3") # WAV passed the gate + converted + assert str(src) in seen["cmd"] # ffmpeg got the .wav input + + +def test_create_lossy_copy_skips_when_output_would_overwrite_source(monkeypatch, tmp_path): + """REGRESSION: .m4a ALAC source + AAC codec β†’ output is the same .m4a path. + ffmpeg (-y) must NEVER run, or it would destroy the original lossless file.""" + _enable_lossy(monkeypatch, codec="aac", bitrate="256") + monkeypatch.setattr(_fo, "m4a_codec", lambda _p: "alac") # source IS ALAC (lossless) + + ran = {"called": False} + monkeypatch.setattr(_fo.subprocess, "run", + lambda *_a, **_k: ran.__setitem__("called", True)) + + src = tmp_path / "track.m4a" + src.write_bytes(b"....ALAC....") + out = _fo.create_lossy_copy(str(src)) + assert out is None # skipped β€” output would overwrite source + assert ran["called"] is False # the original was never touched diff --git a/tests/quality/test_lossless.py b/tests/quality/test_lossless.py new file mode 100644 index 00000000..acb274c9 --- /dev/null +++ b/tests/quality/test_lossless.py @@ -0,0 +1,121 @@ +"""The canonical "is this lossless?" seam + the lossy-copy overwrite invariant +(#941). All pure β€” no files, no ffmpeg β€” so the decision and the safety guard are +unit-testable in isolation. Both the import path (create_lossy_copy) and the Lossy +Converter repair job route through these, so the same rules drive both.""" + +from core.quality.lossless import ( + LOSSLESS_FORMATS, + LOSSLESS_CANDIDATE_EXTENSIONS, + is_lossless_format, + is_lossless_audio_path, + lossy_output_would_overwrite_source, +) +from core.quality.model import AudioQuality +from core.repair_jobs.lossy_converter import _lossless_ext_where + + +# ── is_lossless_format ── + +def test_lossless_formats_recognized(): + for fmt in ('flac', 'alac', 'wav', 'dsf', 'FLAC', 'Dsf'): + assert is_lossless_format(fmt) is True + + +def test_lossy_formats_not_lossless(): + for fmt in ('mp3', 'aac', 'ogg', 'opus', 'wma', 'unknown', '', None): + assert is_lossless_format(fmt) is False + + +# ── is_lossless_audio_path (the ambiguity is the whole point) ── + +def test_unambiguous_extensions_decided_by_extension(): + for path in ('/m/a.flac', '/m/a.wav', '/m/a.wave', '/m/a.aiff', '/m/a.aif', + '/m/a.dsf', '/m/a.dff', '/m/a.alac', '/m/A.FLAC'): + assert is_lossless_audio_path(path) is True + + +def test_lossy_extensions_are_not_lossless(): + for path in ('/m/a.mp3', '/m/a.ogg', '/m/a.opus', '/m/a.wma', '/m/a.aac'): + assert is_lossless_audio_path(path) is False + + +def test_m4a_without_probe_is_not_lossless(): + # The safe default: with no codec probe, an .m4a can't be proven lossless, so + # an AAC file is never misclassified as lossless and converted/deleted. + assert is_lossless_audio_path('/m/a.m4a') is False + assert is_lossless_audio_path('/m/a.mp4') is False + + +def test_m4a_alac_is_lossless_via_probe(): + assert is_lossless_audio_path('/m/a.m4a', probe_codec=lambda _p: 'alac') is True + assert is_lossless_audio_path('/m/a.mp4', probe_codec=lambda _p: 'ALAC') is True + + +def test_m4a_aac_is_not_lossless_via_probe(): + assert is_lossless_audio_path('/m/a.m4a', probe_codec=lambda _p: 'mp4a.40.2') is False + + +def test_probe_exception_is_not_lossless(): + def _boom(_p): + raise RuntimeError("probe failed") + assert is_lossless_audio_path('/m/a.m4a', probe_codec=_boom) is False + + +# ── LOSSLESS_CANDIDATE_EXTENSIONS (the SQL pre-filter set) ── + +def test_candidate_extensions_cover_lossless_plus_ambiguous(): + for e in ('.flac', '.wav', '.aiff', '.dsf', '.dff', '.alac', '.m4a', '.mp4'): + assert e in LOSSLESS_CANDIDATE_EXTENSIONS + # raw lossy extensions must NOT be candidates + for e in ('.mp3', '.aac', '.ogg', '.opus', '.wma'): + assert e not in LOSSLESS_CANDIDATE_EXTENSIONS + + +def test_sql_where_clause_matches_candidates_only(): + where = _lossless_ext_where('t.file_path') + assert "LIKE '%.flac'" in where and "LIKE '%.dsf'" in where and "LIKE '%.m4a'" in where + assert "LIKE '%.mp3'" not in where and "LIKE '%.aac'" not in where + + +# ── lossy_output_would_overwrite_source (the safety invariant) ── + +def test_overwrite_detected_when_paths_equal(): + assert lossy_output_would_overwrite_source('/m/Album/01.m4a', '/m/Album/01.m4a') is True + + +def test_overwrite_detected_after_normalization(): + assert lossy_output_would_overwrite_source('/m/Album/../Album/01.m4a', '/m/Album/01.m4a') is True + + +def test_no_overwrite_for_different_extension(): + assert lossy_output_would_overwrite_source('/m/Album/01.flac', '/m/Album/01.mp3') is False + assert lossy_output_would_overwrite_source('/m/Album/01.m4a', '/m/Album/01.mp3') is False + + +def test_overwrite_guard_handles_empty(): + assert lossy_output_would_overwrite_source('', '/x.mp3') is False + assert lossy_output_would_overwrite_source('/x.flac', '') is False + + +# ── anti-drift: the seam must agree with the quality tier model ── + +def test_lossless_set_consistent_with_tier_model(): + """If a format is in LOSSLESS_FORMATS it must out-rank every lossy format in + tier_score β€” guards against the two lists drifting apart (the whole reason + this seam exists).""" + lossy = ('mp3', 'aac', 'ogg', 'opus', 'wma') + worst_lossless = min(AudioQuality(f, bitrate=11290, sample_rate=44100, bit_depth=16).tier_score() + for f in LOSSLESS_FORMATS) + best_lossy = max(AudioQuality(f, bitrate=320).tier_score() for f in lossy) + assert worst_lossless > best_lossy + + +# ── regression: the exact bug class this guards (overwrite the original) ── + +def test_regression_m4a_alac_to_aac_would_overwrite_and_is_blocked(): + """An .m4a ALAC source converted with the AAC codec lands on the SAME .m4a + path. The guard must catch it so ffmpeg -y never destroys the original.""" + src = '/library/Sade/Diamond Life/01. Smooth Operator.m4a' # ALAC + out = src # AAC target β†’ .m4a + assert is_lossless_audio_path(src, probe_codec=lambda _p: 'alac') is True + assert lossy_output_would_overwrite_source(src, out) is True # β†’ callers skip From 8aa8fcb94ab022fd2b7b27841316c874d119224f Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sun, 28 Jun 2026 12:18:57 -0700 Subject: [PATCH 09/47] =?UTF-8?q?test:=20drift=20guard=20=E2=80=94=20front?= =?UTF-8?q?end=20lossless=20lists=20must=20match=20backend=20LOSSLESS=5FFO?= =?UTF-8?q?RMATS=20(#941)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the frontend keeps its own copy of the lossless set (settings.js RT_LOSSLESS_FORMATS + the index.html quality-profile dropdown) β€” runtime-fetching a yearly-changing list from the backend isn't worth the coupling. but that duplication IS the exact root cause of #941 (a format added to one place, not another). so instead of unifying, pin it: two tests parse the frontend lists and assert they match the backend LOSSLESS_FORMATS. adding a new lossless format now fails CI until it's added everywhere, instead of silently shipping a half-wired feature. verified the guard catches drift (not a tautology): a simulated backend-only 'ape' addition makes the equality fail. 18 lossless tests green, ruff clean. --- tests/quality/test_lossless.py | 42 ++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/quality/test_lossless.py b/tests/quality/test_lossless.py index acb274c9..0b311f15 100644 --- a/tests/quality/test_lossless.py +++ b/tests/quality/test_lossless.py @@ -119,3 +119,45 @@ def test_regression_m4a_alac_to_aac_would_overwrite_and_is_blocked(): out = src # AAC target β†’ .m4a assert is_lossless_audio_path(src, probe_codec=lambda _p: 'alac') is True assert lossy_output_would_overwrite_source(src, out) is True # β†’ callers skip + + +# ── cross-language drift guard: the frontend lossless lists must match the backend ── +# (#941's root cause: a format added to the quality profile but not the lossy-copy +# side. The lists are physically separate by choice β€” runtime-fetching a yearly- +# changing set isn't worth the coupling β€” so a test makes the drift impossible to +# ship silently instead.) + +import re +from pathlib import Path + +_ROOT = Path(__file__).resolve().parent.parent.parent +_SETTINGS_JS = _ROOT / "webui" / "static" / "settings.js" +_INDEX_HTML = _ROOT / "webui" / "index.html" + + +def _js_array(text, name): + m = re.search(rf"{name}\s*=\s*\[([^\]]*)\]", text) + assert m, f"{name} not found" + return set(re.findall(r"'([^']+)'", m.group(1))) + + +def test_frontend_rt_lossless_formats_matches_backend(): + js = _SETTINGS_JS.read_text(encoding="utf-8") + frontend = _js_array(js, "RT_LOSSLESS_FORMATS") + assert frontend == set(LOSSLESS_FORMATS), ( + f"settings.js RT_LOSSLESS_FORMATS {frontend} != backend LOSSLESS_FORMATS " + f"{set(LOSSLESS_FORMATS)} β€” add the new format to BOTH" + ) + + +def test_quality_profile_dropdown_offers_every_lossless_format(): + html = _INDEX_HTML.read_text(encoding="utf-8") + m = re.search(r'(.*?)', html, re.DOTALL) + assert m, "Lossless optgroup not found in index.html" + # concrete per-format option values (skip the 'group:lossless' convenience entry) + option_values = {v for v in re.findall(r'value="([^"]+)"', m.group(1)) + if not v.startswith("group:")} + assert set(LOSSLESS_FORMATS) <= option_values, ( + f"index.html lossless dropdown {option_values} is missing " + f"{set(LOSSLESS_FORMATS) - option_values}" + ) From b72febbf1c1bea8a9480cea1d7250c8e6fdb9a15 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sun, 28 Jun 2026 12:32:36 -0700 Subject: [PATCH 10/47] manual search: INJECT the exact pasted-link track, don't rely on text search surfacing it (#932) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit reopened by diegocade1: pasting a Qobuz track link still showed unrelated results. the earlier fix (b1f061a) only BUBBLED the linked track to the top β€” but a pasted link is resolved to an "artist title" text query and searched, and for an obscure track ("foreign lavennew" by colacola) that text search returns broad lookalikes ("Foreign Bird", "Foreign Spies", …) and never the actual track. nothing to bubble β†’ user sees junk. fix: since the link is already resolved via get_track(id), fetch that exact track AS a downloadable result and inject it at the top (Qobuz downloads by id, so the result is fully usable). the text search still runs for alternatives. - QobuzClient.get_track_result(id): get_track + _qobuz_to_track_result; None on any failure. - _qobuz_to_track_result gains require_streamable (default True for bulk search). the link fetch passes False: track/get may OMIT the streamable flag, which would default-False and wrongly drop the exact track the user explicitly asked for. (this closes the one shape assumption that couldn't be verified against a live Qobuz API β€” the track is no longer gated on it.) - track_link.inject_linked_track_first(tracks, linked_result, id): pure seam β€” prepend the fetched result + drop any search duplicate; falls back to the bubble when no result was fetched. - manual-search endpoint fetches linked_result defensively (getattr 'get_track_result') and calls the seam. Tidal/HiFi (get_track returns a dict but the converter wants an object β€” shape mismatch) have no get_track_result, so they keep the existing bubble path: NO regression. 14 tests: inject puts the fetched track first when search missed it / dedups a search copy / falls back to bubble / str-safe id / noop; get_track_result convert/none/exception; and the REAL converter builds a valid downloadable result from a track/get dict that OMITS streamable (search path still rejects it). 85 track-link/qobuz tests green, ruff clean. --- core/downloads/track_link.py | 17 +++++ core/qobuz_client.py | 34 +++++++-- tests/downloads/test_track_link.py | 109 +++++++++++++++++++++++++++++ web_server.py | 29 ++++++-- 4 files changed, 178 insertions(+), 11 deletions(-) diff --git a/core/downloads/track_link.py b/core/downloads/track_link.py index 3565a5ff..039ddcbf 100644 --- a/core/downloads/track_link.py +++ b/core/downloads/track_link.py @@ -38,6 +38,23 @@ def bubble_linked_track_first(tracks: List[Any], link_track_id: str) -> List[Any target = str(link_track_id) return sorted(tracks, key=lambda t: linked_track_id(t) != target) + +def inject_linked_track_first( + tracks: List[Any], linked_result: Any, link_track_id: str +) -> List[Any]: + """Put the EXACT linked track first. + + When ``linked_result`` is the track fetched directly by id, prepend it and + drop any search duplicate of it β€” so an obscure track a text search never + surfaced is still present and downloadable (#932). When it's None (the source + can't fetch one), fall back to bubbling a matching search result. Pure.""" + if not link_track_id: + return tracks + target = str(link_track_id) + if linked_result is not None: + return [linked_result] + [t for t in tracks if linked_track_id(t) != target] + return bubble_linked_track_first(tracks, target) + # host substring β†’ download source id. Only ID-downloadable streaming sources. _HOSTS = ( ('tidal.com', 'tidal'), diff --git a/core/qobuz_client.py b/core/qobuz_client.py index e90e934a..f82e6da4 100644 --- a/core/qobuz_client.py +++ b/core/qobuz_client.py @@ -715,6 +715,26 @@ class QobuzClient(DownloadSourcePlugin): logger.error(f"Error getting Qobuz track {track_id}: {e}") return None + def get_track_result(self, track_id): + """Fetch ONE track by ID and convert it to a downloadable TrackResult. + + Used when a pasted Qobuz link must inject the EXACT track: a text search + for an obscure track's name often doesn't surface it at all, so bubbling + the matching id is a no-op (#932). Returns None on any failure so the + caller falls back to the text-search path.""" + try: + track = self.get_track(track_id) + if not track: + return None + quality_key = quality_tier_for_source('qobuz', default='lossless') + quality_info = QOBUZ_QUALITY_MAP.get(quality_key, QOBUZ_QUALITY_MAP['lossless']) + # require_streamable=False: this is the exact track the user linked β€” don't + # let a missing/absent 'streamable' flag in track/get drop it (#932). + return self._qobuz_to_track_result(track, quality_info, require_streamable=False) + except Exception as e: + logger.debug(f"get_track_result failed for Qobuz {track_id}: {e}") + return None + # ===================== Playlists & Favorites ===================== # # Qobuz playlist sync surface β€” mirrors the Tidal client contract @@ -1012,14 +1032,20 @@ class QobuzClient(DownloadSourcePlugin): traceback.print_exc() return ([], []) - def _qobuz_to_track_result(self, track: Dict, quality_info: dict) -> Optional[TrackResult]: - """Convert Qobuz track dict to TrackResult (Soulseek-compatible format).""" + def _qobuz_to_track_result(self, track: Dict, quality_info: dict, + require_streamable: bool = True) -> Optional[TrackResult]: + """Convert Qobuz track dict to TrackResult (Soulseek-compatible format). + + ``require_streamable`` gates bulk SEARCH results on the ``streamable`` flag + (filters noise). For a track the user explicitly pasted a link to, pass + False: ``track/get`` may omit the flag (which would default-False and wrongly + drop the exact track they asked for), and they should get to try it (#932).""" track_id = track.get('id') if not track_id: return None - # Check if track is streamable - if not track.get('streamable', False): + # Check if track is streamable (skipped for explicit by-id link fetches). + if require_streamable and not track.get('streamable', False): return None performer = track.get('performer', {}) diff --git a/tests/downloads/test_track_link.py b/tests/downloads/test_track_link.py index 06cbc1bb..b91e49e4 100644 --- a/tests/downloads/test_track_link.py +++ b/tests/downloads/test_track_link.py @@ -122,3 +122,112 @@ def test_bubble_noop_when_nothing_matches_or_empty(): assert bubble_linked_track_first([a, b], '296427754') == [a, b] # unchanged assert bubble_linked_track_first([], '296427754') == [] assert bubble_linked_track_first([a, b], '') == [a, b] + + +# ── inject the EXACT fetched track first (#932 reopen: text search misses obscure tracks) ── + +from core.downloads.track_link import inject_linked_track_first + + +def test_inject_puts_fetched_track_first_when_search_missed_it(): + # The reported case: the linked track isn't among the fuzzy text-search + # results at all, so the directly-fetched result is injected at the top. + fetched = _result('296427754') + search = [_result('111'), _result('222')] # unrelated lookalikes + out = inject_linked_track_first(search, fetched, '296427754') + assert out[0] is fetched + assert len(out) == 3 + + +def test_inject_dedups_a_search_copy_of_the_linked_track(): + fetched = _result('296427754') + dup = _result('296427754') # search also returned it + search = [_result('111'), dup, _result('222')] + out = inject_linked_track_first(search, fetched, '296427754') + assert out[0] is fetched + # exactly one element carries the linked id, and it's the injected one + assert [linked_track_id(t) for t in out].count('296427754') == 1 + assert all(t is not dup for t in out) # the search copy (by identity) was dropped + assert len(out) == 3 + + +def test_inject_falls_back_to_bubble_without_a_fetched_result(): + exact = _result('296427754') + out = inject_linked_track_first([_result('111'), exact, _result('222')], None, '296427754') + assert linked_track_id(out[0]) == '296427754' # bubbled, not injected + assert len(out) == 3 + + +def test_inject_is_noop_without_a_link_id(): + search = [_result('111')] + assert inject_linked_track_first(search, _result('x'), '') == search + + +def test_inject_int_track_id_is_str_safe(): + fetched = _result('296427754') + out = inject_linked_track_first([_result('111')], fetched, 296427754) + assert out[0] is fetched + + +# ── QobuzClient.get_track_result: fetch by id β†’ downloadable TrackResult ── + +from core.qobuz_client import QobuzClient + + +def _bare_qobuz(): + return QobuzClient.__new__(QobuzClient) # no network / __init__ + + +def test_get_track_result_converts_fetched_track(monkeypatch): + client = _bare_qobuz() + sentinel = object() + monkeypatch.setattr(client, 'get_track', lambda tid: {'id': tid, 'streamable': True}) + monkeypatch.setattr(client, '_qobuz_to_track_result', + lambda track, qi, require_streamable=True: sentinel) + assert client.get_track_result('296427754') is sentinel + + +def test_get_track_result_none_when_track_unavailable(monkeypatch): + client = _bare_qobuz() + monkeypatch.setattr(client, 'get_track', lambda tid: None) + assert client.get_track_result('nope') is None + + +def test_get_track_result_none_on_exception(monkeypatch): + client = _bare_qobuz() + def _boom(_tid): + raise RuntimeError('api down') + monkeypatch.setattr(client, 'get_track', _boom) + assert client.get_track_result('x') is None + + +# ── #932 hardening: a pasted-link fetch must not be dropped by a missing +# 'streamable' flag (track/get may omit it). Exercises the REAL converter. ── + +_QOBUZ_TRACK = {'id': 296427754, 'title': 'foreign lavennew', + 'performer': {'name': 'colacola'}, 'duration': 180} + + +def test_converter_rejects_non_streamable_on_search_path(): + # Default (bulk search) still filters on streamable β€” unchanged behaviour. + client = _bare_qobuz() + qi = {'codec': 'flac', 'bitrate': 1411} + assert client._qobuz_to_track_result(dict(_QOBUZ_TRACK), qi) is None # no streamable flag + + +def test_converter_builds_for_link_fetch_without_streamable(): + client = _bare_qobuz() + qi = {'codec': 'flac', 'bitrate': 1411} + r = client._qobuz_to_track_result(dict(_QOBUZ_TRACK), qi, require_streamable=False) + assert r is not None + assert linked_track_id(r) == '296427754' # carries the id β†’ injectable + downloadable + + +def test_get_track_result_survives_missing_streamable_flag(monkeypatch): + # The feared track/get shape (no 'streamable') must STILL yield the track β€” + # this is the end-to-end gap that couldn't be confirmed against a live API. + client = _bare_qobuz() + monkeypatch.setattr(client, 'get_track', lambda _tid: dict(_QOBUZ_TRACK)) + r = client.get_track_result('296427754') + assert r is not None + assert linked_track_id(r) == '296427754' diff --git a/web_server.py b/web_server.py index 15b5de83..4db814c2 100644 --- a/web_server.py +++ b/web_server.py @@ -7506,6 +7506,7 @@ def manual_search_for_task(task_id): link = parse_download_track_link(query) link_source = None link_track_id = None + linked_result = None # the EXACT track fetched by id, injected into results # A pasted SoundCloud link can't be turned into an "artist title" query # and searched β€” unlisted/private tracks aren't searchable. Instead force # the SoundCloud source and keep the URL as the query; the SoundCloud @@ -7535,6 +7536,18 @@ def manual_search_for_task(task_id): query = clean_q source = _src link_source, link_track_id = _src, _tid + # Fetch the EXACT linked track as a downloadable result to inject β€” + # a text search for an obscure track's name often doesn't surface it + # at all, so we can't rely on it being in the search results (#932). + # Defensive: only sources that expose get_track_result (Qobuz today); + # others fall back to the bubble path below β€” never worse than before. + _link_client = download_orchestrator.client(_src) if download_orchestrator else None + if _link_client is not None and hasattr(_link_client, 'get_track_result'): + try: + linked_result = _link_client.get_track_result(_tid) + except Exception as _lr_err: + logger.debug("[Manual Search] get_track_result failed for %s %s: %s", + _src, _tid, _lr_err) if source != 'all': if source not in valid_source_ids: @@ -7595,13 +7608,15 @@ def manual_search_for_task(task_id): "error": error, }) + "\n" continue - # Pasted-link exact match: bubble the track whose source id - # matches the link to the top so the user sees the exact version - # first. Reads _source_metadata['track_id'] (TrackResult has no - # top-level id) β€” the old getattr(t,'id') always missed (#932). - if src_name == link_source and link_track_id and tracks: - from core.downloads.track_link import bubble_linked_track_first - tracks = bubble_linked_track_first(tracks, link_track_id) + # Pasted-link exact match (#932): the linked source's search may + # not surface an obscure linked track at all, so INJECT the exact + # track (fetched by id) at the top and drop any search duplicate of + # it. If we couldn't fetch it (source has no get_track_result, or it + # failed), fall back to bubbling a matching search result β€” never + # worse than before. + if src_name == link_source and link_track_id: + from core.downloads.track_link import inject_linked_track_first + tracks = inject_linked_track_first(tracks, linked_result, link_track_id) serialized = [] for t in tracks: s = _serialize_candidate(t, source_override=src_name) From d8718994516d3abe71315ecaed00efd53188384d Mon Sep 17 00:00:00 2001 From: dev Date: Sun, 28 Jun 2026 21:50:21 +0200 Subject: [PATCH 11/47] Apply saved appearance on app startup --- webui/static/init.js | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/webui/static/init.js b/webui/static/init.js index aea68fab..aec092d7 100644 --- a/webui/static/init.js +++ b/webui/static/init.js @@ -274,6 +274,34 @@ function applyReduceEffects(enabled) { } })(); +async function bootstrapServerAppearanceSettings() { + try { + const response = await fetch('/api/settings', { credentials: 'same-origin' }); + const settings = await response.json(); + if (!response.ok || !settings || typeof settings !== 'object' || settings.error) return; + + const appearance = settings.ui_appearance || {}; + const preset = appearance.accent_preset || '#1db954'; + const custom = appearance.accent_color || '#1db954'; + const accent = preset === 'custom' ? custom : preset; + applyAccentColor(accent); + + if (Object.prototype.hasOwnProperty.call(appearance, 'particles_enabled')) { + applyParticlesSetting(appearance.particles_enabled !== false); + } + if (Object.prototype.hasOwnProperty.call(appearance, 'worker_orbs_enabled')) { + applyWorkerOrbsSetting(appearance.worker_orbs_enabled !== false); + } + if (localStorage.getItem('soulsync-reduce-effects') === null) { + applyReduceEffects(appearance.reduce_effects === true); + } + } catch (error) { + console.warn('Could not bootstrap appearance settings:', error); + } +} + +bootstrapServerAppearanceSettings(); + // ── Profile System ───────────────────────────────────────────── let currentProfile = null; const PROFILE_CONTEXT_CHANGED_EVENT = 'ss:webui-profile-context-changed'; From d4e2dccd73ab3031e5e4337247722b2355b9907b Mon Sep 17 00:00:00 2001 From: dev Date: Sun, 28 Jun 2026 21:55:47 +0200 Subject: [PATCH 12/47] Render saved appearance before CSS paints --- web_server.py | 73 +++++++++++++++++++++++++++++++++++++++++++++++- webui/index.html | 9 +++++- 2 files changed, 80 insertions(+), 2 deletions(-) diff --git a/web_server.py b/web_server.py index 826c3fbe..84713700 100644 --- a/web_server.py +++ b/web_server.py @@ -303,6 +303,77 @@ app.config['SEND_FILE_MAX_AGE_DEFAULT'] = 0 if DEV_STATIC_NO_CACHE else 31536000 import time as _cache_bust_time _STATIC_CACHE_BUST = str(int(_cache_bust_time.time())) +def _valid_hex_color(value, fallback='#1db954'): + value = str(value or '').strip() + return value if re.fullmatch(r'#[0-9a-fA-F]{6}', value) else fallback + + +def _hex_to_rgb(hex_color): + color = _valid_hex_color(hex_color) + return tuple(int(color[i:i + 2], 16) for i in (1, 3, 5)) + + +def _rgb_to_hsl(r, g, b): + rn, gn, bn = r / 255, g / 255, b / 255 + max_c = max(rn, gn, bn) + min_c = min(rn, gn, bn) + lightness = (max_c + min_c) / 2 + if max_c == min_c: + return 0, 0, lightness + + delta = max_c - min_c + saturation = delta / (2 - max_c - min_c) if lightness > 0.5 else delta / (max_c + min_c) + if max_c == rn: + hue = ((gn - bn) / delta + (6 if gn < bn else 0)) / 6 + elif max_c == gn: + hue = ((bn - rn) / delta + 2) / 6 + else: + hue = ((rn - gn) / delta + 4) / 6 + return hue, saturation, lightness + + +def _hsl_to_rgb(hue, saturation, lightness): + if saturation == 0: + value = round(lightness * 255) + return value, value, value + + def hue_to_rgb(p, q, t): + if t < 0: + t += 1 + if t > 1: + t -= 1 + if t < 1 / 6: + return p + (q - p) * 6 * t + if t < 1 / 2: + return q + if t < 2 / 3: + return p + (q - p) * (2 / 3 - t) * 6 + return p + + q = lightness * (1 + saturation) if lightness < 0.5 else lightness + saturation - lightness * saturation + p = 2 * lightness - q + return ( + round(hue_to_rgb(p, q, hue + 1 / 3) * 255), + round(hue_to_rgb(p, q, hue) * 255), + round(hue_to_rgb(p, q, hue - 1 / 3) * 255), + ) + + +def _initial_appearance_context(): + preset = config_manager.get('ui_appearance.accent_preset', '#1db954') + custom = config_manager.get('ui_appearance.accent_color', '#1db954') + accent = _valid_hex_color(custom if preset == 'custom' else preset) + r, g, b = _hex_to_rgb(accent) + hue, saturation, lightness = _rgb_to_hsl(r, g, b) + light = _hsl_to_rgb(hue, saturation, min(lightness + 0.16, 0.95)) + neon = _hsl_to_rgb(hue, min(saturation + 0.1, 1.0), min(lightness + 0.30, 0.95)) + return { + 'initial_accent_color': accent, + 'initial_accent_rgb': f'{r}, {g}, {b}', + 'initial_accent_light_rgb': f'{light[0]}, {light[1]}, {light[2]}', + 'initial_accent_neon_rgb': f'{neon[0]}, {neon[1]}, {neon[2]}', + } + @app.context_processor def _inject_static_cache_bust(): @@ -319,7 +390,7 @@ def _inject_static_cache_bust(): static_v = str(max(mtimes)) except Exception: static_v = _STATIC_CACHE_BUST - return {'static_v': static_v} + return {'static_v': static_v, **_initial_appearance_context()} @app.context_processor diff --git a/webui/index.html b/webui/index.html index 9690da8b..a5d1b4f8 100644 --- a/webui/index.html +++ b/webui/index.html @@ -7,12 +7,19 @@ SoulSync - Music Sync & Manager - + + {{ vite_assets('head')|safe }} From 70dba77711411a15205efd870553b99254aba149 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sun, 28 Jun 2026 13:07:29 -0700 Subject: [PATCH 13/47] spotify oauth: keep the redirect_uri trailing slash (follow-up to #942) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #942's normalize_spotify_oauth_config trimmed whitespace/quotes (good β€” those can't be part of a real credential) but ALSO rstrip("/")'d the redirect_uri. that's unsafe: Spotify matches the redirect URI EXACTLY against the app's dashboard registration, and a trailing slash is a legitimate part of a URI. stripping it would silently break anyone who registered '…/callback/' (we'd send '…/callback' β†’ INVALID_CLIENT: Invalid redirect URI) β€” trading one failure mode for a sneakier one the user can't diagnose (SoulSync no longer sends what they typed). drop the rstrip; keep the whitespace/quote trim. the value is now preserved verbatim apart from unambiguous paste garbage. flipped the test that asserted the strip to assert the slash is kept (and that whitespace/quotes around it are still trimmed), + a dedicated regression guard. the #942 integration test mocks normalize, so it's unaffected. 262 spotify/oauth tests green. credit: builds on HellRa1SeR's #942. --- core/spotify_client.py | 18 +++++++++++------- tests/test_spotify_client.py | 22 ++++++++++++++++++---- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/core/spotify_client.py b/core/spotify_client.py index cf3859ca..d8e5df8e 100644 --- a/core/spotify_client.py +++ b/core/spotify_client.py @@ -17,9 +17,16 @@ logger = get_logger("spotify_client") def normalize_spotify_oauth_config(config: Optional[Dict[str, Any]]) -> Dict[str, Any]: """Normalize Spotify OAuth config before building an auth manager. - Spotify rejects values that include surrounding whitespace, quotes, or - newline characters. The settings UI can paste values that carry such - formatting, so we trim them before sending them to Spotify. + Spotify rejects values that include surrounding whitespace or quotes, and the + settings UI can paste values carrying such formatting, so we trim those β€” they + can never be part of a real credential. + + We deliberately do NOT strip a trailing slash from ``redirect_uri``: Spotify + matches the redirect URI EXACTLY against the app's dashboard registration, and + a trailing slash is a legitimate part of a URI. Stripping it would silently + break anyone who registered ``…/callback/`` (we'd send ``…/callback`` β†’ + "INVALID_CLIENT: Invalid redirect URI"). So the value is preserved verbatim + apart from the unambiguous whitespace/quote garbage (#942 follow-up). """ if not isinstance(config, dict): return {} @@ -28,10 +35,7 @@ def normalize_spotify_oauth_config(config: Optional[Dict[str, Any]]) -> Dict[str for key in ("client_id", "client_secret", "redirect_uri"): value = config.get(key, "") if isinstance(value, str): - value = value.strip().strip('"').strip("'") - if key == "redirect_uri": - value = value.rstrip("/") - normalized[key] = value + normalized[key] = value.strip().strip('"').strip("'") else: normalized[key] = value return normalized diff --git a/tests/test_spotify_client.py b/tests/test_spotify_client.py index cf9210ee..86820eef 100644 --- a/tests/test_spotify_client.py +++ b/tests/test_spotify_client.py @@ -6,19 +6,33 @@ sys.path.insert(0, str(Path(__file__).resolve().parents[1])) from core.spotify_client import normalize_spotify_oauth_config def test_normalization(): - # Normal case with leading/trailing whitespace and quotes + # Whitespace + quotes are stripped (paste garbage); the redirect_uri's + # trailing slash is PRESERVED β€” Spotify matches it exactly against the app + # dashboard, so stripping it could break a valid registration (#942 follow-up). config = { - "client_id": " client_id ", + "client_id": ' "client_id" ', "client_secret": " client_secret ", - "redirect_uri": "http://127.0.0.1:8888/callback/" + "redirect_uri": " http://127.0.0.1:8888/callback/ " } expected = { "client_id": "client_id", "client_secret": "client_secret", - "redirect_uri": "http://127.0.0.1:8888/callback" + "redirect_uri": "http://127.0.0.1:8888/callback/" # slash kept } assert normalize_spotify_oauth_config(config) == expected + +def test_trailing_slash_on_redirect_uri_is_preserved(): + """Regression guard: Spotify requires an EXACT redirect-URI match against the + app dashboard, so a trailing slash a user registered must NOT be stripped β€” + stripping it would send '…/callback' and trigger INVALID_CLIENT (#942).""" + with_slash = {"client_id": "x", "client_secret": "y", + "redirect_uri": "http://127.0.0.1:8888/callback/"} + without_slash = {"client_id": "x", "client_secret": "y", + "redirect_uri": "http://127.0.0.1:8888/callback"} + assert normalize_spotify_oauth_config(with_slash)["redirect_uri"] == "http://127.0.0.1:8888/callback/" + assert normalize_spotify_oauth_config(without_slash)["redirect_uri"] == "http://127.0.0.1:8888/callback" + def test_empty_values(): # Empty input values config = { From e2317de0a43af6c5939c79ec7c269a70fe497d92 Mon Sep 17 00:00:00 2001 From: dev Date: Sun, 28 Jun 2026 22:25:28 +0200 Subject: [PATCH 14/47] Remove dead settings CSS --- webui/static/style.css | 335 ----------------------------------------- 1 file changed, 335 deletions(-) diff --git a/webui/static/style.css b/webui/static/style.css index ac4bcbfe..e4e741b0 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -3389,66 +3389,6 @@ body.helper-mode-active #dashboard-activity-feed:hover { color: #a78bfa; /* Usenet β€” violet */ } -/* Lidarr-style intro hero card on Indexers & Downloaders tab */ -.ind-hero { - padding: 18px 22px; - background: linear-gradient(135deg, rgba(var(--accent-rgb), 0.08), rgba(35, 35, 35, 0.6)); - border: 1px solid rgba(var(--accent-rgb), 0.15); -} - -.ind-hero-title { - font-size: 13px; - font-weight: 700; - letter-spacing: 0.8px; - text-transform: uppercase; - color: rgb(var(--accent-rgb)); - margin-bottom: 12px; -} - -.ind-hero-flow { - display: flex; - flex-wrap: wrap; - align-items: center; - gap: 10px; - margin-bottom: 12px; - font-size: 13px; - color: rgba(255, 255, 255, 0.85); -} - -.ind-hero-step { - display: inline-flex; - align-items: center; - gap: 8px; - padding: 6px 12px; - background: rgba(255, 255, 255, 0.04); - border: 1px solid rgba(255, 255, 255, 0.08); - border-radius: 999px; -} - -.ind-hero-step-num { - width: 20px; - height: 20px; - border-radius: 50%; - background: rgba(var(--accent-rgb), 0.2); - color: rgb(var(--accent-rgb)); - font-weight: 700; - font-size: 11px; - display: inline-flex; - align-items: center; - justify-content: center; -} - -.ind-hero-arrow { - color: rgba(var(--accent-rgb), 0.55); - font-weight: 700; -} - -.ind-hero-sub { - font-size: 12.5px; - color: rgba(255, 255, 255, 0.55); - line-height: 1.55; -} - .ind-hero-warning { margin-top: 14px; padding: 12px 14px; @@ -3939,23 +3879,6 @@ body.helper-mode-active #dashboard-activity-feed:hover { font-size: 12px; color: rgba(255, 255, 255, 0.85); } -.info-icon { - display: inline-flex; - align-items: center; - justify-content: center; - width: 16px; - height: 16px; - flex: 0 0 auto; - font-size: 12px; - line-height: 1; - color: rgba(var(--accent-rgb), 0.85); - cursor: pointer; - user-select: none; - transition: color 0.15s ease; -} -.info-icon:hover { color: rgba(var(--accent-rgb), 1); } -.setting-help-body { margin-top: 4px; } - /* Supported Formats */ .supported-formats { color: rgba(255, 255, 255, 0.7); @@ -4343,213 +4266,6 @@ body.helper-mode-active #dashboard-activity-feed:hover { background: rgba(99, 179, 237, 0.3); } -.quality-tier { - margin-bottom: 16px; - padding: 12px; - background: linear-gradient(135deg, rgba(255, 255, 255, 0.04), rgba(255, 255, 255, 0.01)); - border: 1px solid rgba(255, 255, 255, 0.08); - border-radius: 12px; - transition: all 0.2s ease; -} - -.quality-tier:hover { - border-color: rgba(255, 255, 255, 0.12); -} - -.quality-tier-header { - display: flex; - justify-content: space-between; - align-items: center; - margin-bottom: 10px; -} - -.quality-tier-name { - color: rgba(255, 255, 255, 0.95); - font-size: 12px; - font-weight: 600; -} - -.quality-tier-priority { - color: rgba(255, 255, 255, 0.45); - font-size: 10px; - font-weight: 500; -} - -.quality-tier-sliders { - padding-left: 24px; - opacity: 1; - transition: opacity 0.3s ease; -} - -.quality-tier-sliders.disabled { - opacity: 0.35; - pointer-events: none; -} - -.flac-bit-depth-selector { - padding: 8px 12px; - transition: opacity 0.3s ease; -} - -.flac-bit-depth-selector.disabled { - opacity: 0.35; - pointer-events: none; -} - -.flac-bit-depth-selector label { - display: block; - margin-bottom: 6px; - color: rgba(255, 255, 255, 0.7); - font-size: 11px; -} - -.bit-depth-buttons { - display: flex; - gap: 8px; -} - -.bit-depth-btn { - flex: 1; - padding: 7px 12px; - background: linear-gradient(135deg, rgba(255, 255, 255, 0.04), rgba(255, 255, 255, 0.02)); - border: 1px solid rgba(255, 255, 255, 0.1); - border-radius: 10px; - color: rgba(255, 255, 255, 0.7); - font-size: 11px; - font-weight: 500; - cursor: pointer; - transition: all 0.3s cubic-bezier(0.4, 0, 0.2, 1); -} - -.bit-depth-btn.active { - background: linear-gradient(135deg, - rgba(var(--accent-rgb), 0.2) 0%, - rgba(var(--accent-light-rgb), 0.12) 100%); - border-color: rgba(var(--accent-rgb), 0.4); - color: rgb(var(--accent-light-rgb)); - font-weight: 600; - box-shadow: 0 4px 16px rgba(var(--accent-rgb), 0.15), inset 0 1px 0 rgba(255, 255, 255, 0.05); -} - -.bit-depth-btn:hover { - background: rgba(255, 255, 255, 0.08); - border-color: rgba(255, 255, 255, 0.15); - color: #ffffff; - transform: translateY(-1px); -} - -.bit-depth-btn.active:hover { - background: linear-gradient(135deg, - rgba(var(--accent-rgb), 0.28) 0%, - rgba(var(--accent-light-rgb), 0.18) 100%); -} - -.slider-group { - margin-top: 8px; -} - -.slider-group label { - display: block; - margin-bottom: 8px; - color: rgba(255, 255, 255, 0.7); - font-size: 11px; -} - -.dual-slider-container { - position: relative; - height: 40px; - margin: 10px 0; -} - -.range-slider { - position: absolute; - width: 100%; - height: 4px; - -webkit-appearance: none; - appearance: none; - background: transparent; - outline: none; - pointer-events: none; -} - -.range-slider::-webkit-slider-track { - width: 100%; - height: 4px; - background: rgba(255, 255, 255, 0.08); - border-radius: 2px; -} - -.range-slider::-webkit-slider-thumb { - -webkit-appearance: none; - appearance: none; - width: 16px; - height: 16px; - border-radius: 50%; - background: linear-gradient(135deg, rgb(var(--accent-rgb)), rgb(var(--accent-light-rgb))); - cursor: pointer; - pointer-events: all; - border: 2px solid #0a0a0a; - box-shadow: 0 2px 8px rgba(0, 0, 0, 0.4), 0 0 8px rgba(var(--accent-rgb), 0.2); -} - -.range-slider::-webkit-slider-thumb:hover { - background: linear-gradient(135deg, rgb(var(--accent-light-rgb)), #22e968); - transform: scale(1.15); - box-shadow: 0 2px 10px rgba(0, 0, 0, 0.4), 0 0 12px rgba(var(--accent-rgb), 0.3); -} - -.range-slider::-moz-range-track { - width: 100%; - height: 4px; - background: rgba(255, 255, 255, 0.08); - border-radius: 2px; -} - -.range-slider::-moz-range-thumb { - width: 16px; - height: 16px; - border-radius: 50%; - background: linear-gradient(135deg, rgb(var(--accent-rgb)), rgb(var(--accent-light-rgb))); - cursor: pointer; - pointer-events: all; - border: 2px solid #0a0a0a; - box-shadow: 0 2px 8px rgba(0, 0, 0, 0.4), 0 0 8px rgba(var(--accent-rgb), 0.2); -} - -.range-slider::-moz-range-thumb:hover { - background: linear-gradient(135deg, rgb(var(--accent-light-rgb)), #22e968); - transform: scale(1.15); - box-shadow: 0 2px 10px rgba(0, 0, 0, 0.4), 0 0 12px rgba(var(--accent-rgb), 0.3); -} - -.range-slider-track { - position: absolute; - top: 18px; - width: 100%; - height: 4px; - background: rgba(var(--accent-rgb), 0.25); - border-radius: 2px; - pointer-events: none; -} - -.slider-values { - display: flex; - justify-content: space-between; - align-items: center; - margin-top: 8px; - color: rgba(255, 255, 255, 0.5); - font-size: 11px; - font-weight: 500; -} - -.slider-values span:first-child, -.slider-values span:last-child { - color: rgb(var(--accent-light-rgb)); - font-weight: 700; -} - -/* ===== END QUALITY PROFILE STYLES ===== */ - .test-button { background: rgba(var(--accent-rgb), 0.1); color: rgb(var(--accent-light-rgb)); @@ -4603,12 +4319,6 @@ body.helper-mode-active #dashboard-activity-feed:hover { transform: translateY(0) scale(0.97); } -.settings-actions { - display: flex; - justify-content: center; - padding-top: 20px; -} - /* Additional Settings Components */ .path-input-group { display: flex; @@ -55841,18 +55551,6 @@ tr.tag-diff-same { flex-wrap: wrap; } -/* ── Quality section ── */ -#settings-page .quality-tier { - background: rgba(255, 255, 255, 0.02); - border: 1px solid rgba(255, 255, 255, 0.05); - border-radius: 12px; - margin-bottom: 10px; - transition: border-color 0.2s; -} -#settings-page .quality-tier:hover { - border-color: rgba(255, 255, 255, 0.1); - box-shadow: 0 2px 12px rgba(0, 0, 0, 0.12); -} #settings-page .preset-button { border-radius: 8px; font-size: 0.84em; @@ -55902,30 +55600,6 @@ tr.tag-diff-same { padding: 0 18px 12px; } -/* ── Save button β€” centered, sticky feel ── */ -#settings-page .settings-actions { - max-width: 760px; - width: 100%; - margin: 0 auto; - padding: 24px 0 16px; -} -#settings-page .settings-actions .save-button { - width: 100%; - padding: 12px; - border-radius: 10px; - font-size: 0.92em; - font-weight: 600; - transition: all 0.25s cubic-bezier(0.4, 0, 0.2, 1); -} -#settings-page .settings-actions .save-button:hover { - transform: translateY(-1px); - box-shadow: 0 4px 20px rgba(var(--accent-rgb, 29, 185, 84), 0.35); -} -#settings-page .settings-actions .save-button:active { - transform: scale(0.99); - box-shadow: 0 2px 8px rgba(var(--accent-rgb, 29, 185, 84), 0.2); -} - /* ── Path input groups (dir paths with Unlock buttons) ── */ #settings-page .path-input-group { flex: 1; @@ -56028,12 +55702,6 @@ tr.tag-diff-same { box-shadow: 0 2px 16px rgba(0, 0, 0, 0.12); } -/* ── Path inputs with lock buttons ── */ -#settings-page .form-group .path-input-wrapper { - flex: 1; - max-width: 340px; -} - /* ── Hybrid source priority list (drag and drop) ── */ .hybrid-source-list { display: flex; @@ -56415,9 +56083,6 @@ tr.tag-diff-same { /* Accordion headers */ #settings-page .stg-service > .stg-service-header { padding: 12px 14px; } #settings-page .stg-service > .stg-service-body { padding: 0 0 8px; } - /* Save button */ - #settings-page .settings-actions { padding: 16px 8px; } - #settings-page .settings-actions .save-button { width: 100%; } /* Section titles */ #settings-page .settings-group > h3 { padding: 24px 0 10px; } } From 2fb142dded68b8fd2cf577b8861899ac7f3ff677 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sun, 28 Jun 2026 13:41:38 -0700 Subject: [PATCH 15/47] jellyfin scan: page the bulk fetch so the no-progress watchdog can't false-stall MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Discord (DXP4800 NAS, 7148 tracks): library updates kept dying with "Update appears stuck β€” no progress for 300s (last phase: Fetching all tracks in bulk...)". not actually hung. root cause: the bulk track/album fetch used a single 10000-item page, so a whole library came back in ONE request that emitted NO progress while in flight. the watchdog (database_update_health) kills a job with no progress for 300s β€” so on a slow server that one silent request tripped it even though it was alive, not stuck. raising the timeout cap only buys the silent request more rope; a bigger library or slower disk just needs a higher number. the per-batch progress line also only ran when there was a NEXT page, so a sub-page-size library reported nothing at all. fix: extract a pure paginate_all_items seam (core/library/bulk_paginate.py) that pages in 1000s and reports progress after EVERY page β€” so the watchdog is fed on a cadence set by page size, not library size, and can't starve mid-fetch no matter how big the library. both Jellyfin bulk loops (tracks + albums, same defect) now route through it. preserves the failure-shrink resilience (halve to a floor, then give up). does NOT change what's fetched β€” same query, fields, items. note: changes nothing about WHICH tracks come back; only how they're paged + that every page reports. keep the raised cap on dev as a margin β€” this is the actual fix. Plex/Navidrome don't share the pattern (checked). 9 seam tests incl. the watchdog-feed invariant (progress count scales with N/page_size, never one call for the whole library) + the sub-page regression + failure-shrink. 467 jellyfin/library tests green, ruff clean. --- core/jellyfin_client.py | 102 +++++++--------------------- core/library/bulk_paginate.py | 93 +++++++++++++++++++++++++ tests/library/test_bulk_paginate.py | 95 ++++++++++++++++++++++++++ 3 files changed, 212 insertions(+), 78 deletions(-) create mode 100644 core/library/bulk_paginate.py create mode 100644 tests/library/test_bulk_paginate.py diff --git a/core/jellyfin_client.py b/core/jellyfin_client.py index fa4f61b0..663c212f 100644 --- a/core/jellyfin_client.py +++ b/core/jellyfin_client.py @@ -5,6 +5,7 @@ from datetime import datetime import json from utils.logging_config import get_logger from config.settings import config_manager +from core.library.bulk_paginate import paginate_all_items # Shared dataclasses live in the neutral media_server package β€” every # server client used to define a near-identical XTrackInfo / @@ -512,12 +513,8 @@ class JellyfinClient(MediaServerClient): try: # SIMPLIFIED APPROACH: Fetch all tracks, then all albums separately (robust and fast) logger.info("Fetching all tracks in bulk...") - all_tracks = [] - start_index = 0 - limit = 10000 - consecutive_failures = 0 - - while True: + + def _fetch_tracks_page(start_index, limit): params = { 'ParentId': self.music_library_id, 'IncludeItemTypes': 'Audio', @@ -528,41 +525,19 @@ class JellyfinClient(MediaServerClient): 'StartIndex': start_index, 'Limit': limit } - response = self._make_request(f'/Users/{self.user_id}/Items', params) - - if not response: - consecutive_failures += 1 - # Wait before retrying β€” the server may still be processing the timed-out request - time.sleep(5) - if limit > 1000: - limit = limit // 2 - consecutive_failures = 0 # Reset β€” give the smaller batch a fair chance - logger.warning(f"Track fetch failed - reducing batch size to {limit}") - continue - elif consecutive_failures >= 2: - logger.warning("Multiple track fetch failures at minimum batch size - stopping") - break - else: - logger.warning("Track fetch failed at minimum batch size - retrying once") - continue + return response.get('Items', []) if response else None # None = failed page + + # Page in modest chunks so progress is reported every page β€” a single + # huge silent request used to trip the 300s no-progress watchdog on + # slow servers even though it was alive (see bulk_paginate docstring). + all_tracks = paginate_all_items( + _fetch_tracks_page, + report_progress=self._progress_callback, + label="tracks", + on_retry_wait=lambda: time.sleep(5), + ) - consecutive_failures = 0 - batch_tracks = response.get('Items', []) - if not batch_tracks: - break - - all_tracks.extend(batch_tracks) - - if len(batch_tracks) < limit: - break - - start_index += limit - progress_msg = f"Fetched {len(all_tracks)} tracks so far..." - logger.info(f" {progress_msg} (batch size: {limit})") - if self._progress_callback: - self._progress_callback(progress_msg) - # Group tracks by album ID for instant lookup self._track_cache = {} for track_data in all_tracks: @@ -578,12 +553,8 @@ class JellyfinClient(MediaServerClient): # STEP 2: Fetch all albums in bulk (same proven pattern) logger.info("Fetching all albums in bulk...") - all_albums = [] - start_index = 0 - limit = 10000 - consecutive_failures = 0 - - while True: + + def _fetch_albums_page(start_index, limit): params = { 'ParentId': self.music_library_id, 'IncludeItemTypes': 'MusicAlbum', @@ -594,41 +565,16 @@ class JellyfinClient(MediaServerClient): 'StartIndex': start_index, 'Limit': limit } - response = self._make_request(f'/Users/{self.user_id}/Items', params) - - if not response: - consecutive_failures += 1 - # Wait before retrying β€” the server may still be processing the timed-out request - time.sleep(5) - if limit > 1000: - limit = limit // 2 - consecutive_failures = 0 # Reset β€” give the smaller batch a fair chance - logger.warning(f"Album fetch failed - reducing batch size to {limit}") - continue - elif consecutive_failures >= 2: - logger.warning("Multiple album fetch failures at minimum batch size - stopping") - break - else: - logger.warning("Album fetch failed at minimum batch size - retrying once") - continue + return response.get('Items', []) if response else None # None = failed page + + all_albums = paginate_all_items( + _fetch_albums_page, + report_progress=self._progress_callback, + label="albums", + on_retry_wait=lambda: time.sleep(5), + ) - consecutive_failures = 0 - batch_albums = response.get('Items', []) - if not batch_albums: - break - - all_albums.extend(batch_albums) - - if len(batch_albums) < limit: - break - - start_index += limit - progress_msg = f"Fetched {len(all_albums)} albums so far..." - logger.info(f" {progress_msg} (batch size: {limit})") - if self._progress_callback: - self._progress_callback(progress_msg) - # Group albums by artist ID for instant lookup self._album_cache = {} for album_data in all_albums: diff --git a/core/library/bulk_paginate.py b/core/library/bulk_paginate.py new file mode 100644 index 00000000..de423219 --- /dev/null +++ b/core/library/bulk_paginate.py @@ -0,0 +1,93 @@ +"""Paginate a bulk media-server fetch while feeding the no-progress watchdog. + +The library scan fetches every track/album by paging a server API. The DB-update +watchdog (``core/database_update_health.py``) kills a job that reports no progress +for 300s. The old Jellyfin fetch used a single 10 000-item page, so a whole +library came back in ONE request that emitted NO progress while it was in flight β€” +on a slow server that single request exceeded 300s and the watchdog declared the +job "stuck" even though it was alive, not hung (Discord: DXP4800 NAS, 7148 tracks, +"Fetching all tracks in bulk…"). + +``paginate_all_items`` pages at a size chosen so progress is emitted on a cadence +set by the PAGE SIZE, not the library size β€” the watchdog is fed every page, so it +can never starve mid-fetch regardless of how big the library is. It is pure: all +I/O lives in the injected ``fetch_page``, so the pagination + progress + failure- +shrink logic is unit-testable without a server. + +This does NOT change WHAT is fetched (same query, same fields, same items) β€” only +how it's paged and that every page reports progress (the old loop skipped progress +on the final/only page, which is the entire bug for a sub-page-size library). +""" + +from __future__ import annotations + +from typing import Any, Callable, List, Optional + +# Page size for bulk library fetches. Small enough that a single request stays +# well under the 300s no-progress watchdog even on a slow NAS, and that progress +# is reported every page. NOT a performance knob β€” a resilience/observability one. +DEFAULT_PAGE_SIZE = 1000 + +# Floor the failure-shrink can reach before giving up β€” a server that can't return +# even this many items in one request is genuinely struggling. +DEFAULT_MIN_PAGE_SIZE = 250 + + +def paginate_all_items( + fetch_page: Callable[[int, int], Optional[List[Any]]], + *, + report_progress: Optional[Callable[[str], None]] = None, + label: str = "items", + page_size: int = DEFAULT_PAGE_SIZE, + min_page_size: int = DEFAULT_MIN_PAGE_SIZE, + on_retry_wait: Optional[Callable[[], None]] = None, +) -> List[Any]: + """Page through ``fetch_page(start_index, limit)`` until the server is drained. + + ``fetch_page`` returns the page's items (a list, possibly empty = end), or + ``None`` to signal a FAILED request (timeout/error) β€” on failure the page size + is halved down to ``min_page_size`` and retried, then abandoned after two + consecutive failures at the floor. + + Progress is reported after EVERY non-empty page (including the final/only one), + so a no-progress watchdog is fed on a cadence set by ``page_size`` β€” never by + the total library size. Returns every item gathered. + """ + items: List[Any] = [] + start_index = 0 + limit = page_size + consecutive_failures = 0 + + while True: + batch = fetch_page(start_index, limit) + + if batch is None: # failed request + consecutive_failures += 1 + if on_retry_wait is not None: + on_retry_wait() + if limit > min_page_size: + limit = max(min_page_size, limit // 2) + consecutive_failures = 0 # give the smaller batch a fair chance + continue + if consecutive_failures >= 2: + break # struggling at the floor β€” stop with what we have + continue + + consecutive_failures = 0 + if not batch: + break # drained + + items.extend(batch) + # Feed the watchdog on EVERY page β€” this is the line the old loop only ran + # when there was a *next* page, so a sub-page-size library reported nothing. + if report_progress is not None: + report_progress(f"Fetched {len(items)} {label} so far...") + + if len(batch) < limit: + break # last (partial) page + start_index += limit + + return items + + +__all__ = ["paginate_all_items", "DEFAULT_PAGE_SIZE", "DEFAULT_MIN_PAGE_SIZE"] diff --git a/tests/library/test_bulk_paginate.py b/tests/library/test_bulk_paginate.py new file mode 100644 index 00000000..3310a5ab --- /dev/null +++ b/tests/library/test_bulk_paginate.py @@ -0,0 +1,95 @@ +"""Bulk-fetch pagination seam β€” pages a server fetch while feeding the no-progress +watchdog every page (the DXP4800 "Fetching all tracks in bulk… stuck 300s" bug). +Pure: a fake fetch_page stands in for the server, a list records progress calls.""" + +import math + +from core.library.bulk_paginate import ( + paginate_all_items, + DEFAULT_PAGE_SIZE, +) + + +def _server(total, *, fail_at=None, fail_times=0): + """A fake server holding `total` items. Returns the right page slice for + (start_index, limit). Optionally returns None (a failed request) the first + `fail_times` times it's called at offset `fail_at`.""" + items = list(range(total)) + state = {"fails": 0} + + def fetch_page(start_index, limit): + if fail_at is not None and start_index == fail_at and state["fails"] < fail_times: + state["fails"] += 1 + return None + return items[start_index:start_index + limit] + + return fetch_page + + +def test_returns_every_item_across_pages(): + out = paginate_all_items(_server(7148), page_size=1000) + assert out == list(range(7148)) + + +def test_progress_fed_every_page_not_once_for_whole_library(): + # The watchdog-feed invariant: progress count scales with N/page_size, so the + # gap between progress beats is one page β€” never the whole library. A single + # 7148-track library used to emit ZERO progress (one 10k page) and stall. + calls = [] + paginate_all_items(_server(7148), page_size=1000, report_progress=calls.append) + assert len(calls) == math.ceil(7148 / 1000) == 8 + + +def test_sub_page_library_still_reports_progress(): + # Regression: the old loop skipped progress on the final/only page, so a + # library smaller than one page reported nothing β†’ watchdog starved. + calls = [] + out = paginate_all_items(_server(500), page_size=1000, report_progress=calls.append) + assert out == list(range(500)) + assert len(calls) == 1 # was 0 before the fix + + +def test_exact_multiple_of_page_size(): + calls = [] + out = paginate_all_items(_server(2000), page_size=1000, report_progress=calls.append) + assert out == list(range(2000)) + assert len(calls) == 2 # two full pages, both reported + + +def test_empty_library(): + calls = [] + out = paginate_all_items(_server(0), page_size=1000, report_progress=calls.append) + assert out == [] + assert calls == [] + + +def test_no_progress_callback_is_safe(): + assert paginate_all_items(_server(2500), page_size=1000) == list(range(2500)) + + +def test_failed_page_shrinks_then_succeeds(): + # First request at offset 0 fails once; the helper halves the page size, retries, + # and still returns everything β€” the slow-server resilience path. + waits = [] + out = paginate_all_items( + _server(1500, fail_at=0, fail_times=1), + page_size=1000, min_page_size=250, + on_retry_wait=lambda: waits.append(1), + ) + assert out == list(range(1500)) + assert waits == [1] # waited once before the retry + + +def test_gives_up_after_repeated_failures_at_floor(): + # A page that always fails at the floor must terminate (not loop forever) and + # return what was gathered β€” here nothing, since it fails on the first page. + def always_fail(_start, _limit): + return None + out = paginate_all_items(always_fail, page_size=250, min_page_size=250) + assert out == [] + + +def test_default_page_size_is_watchdog_safe(): + # A guard on the constant itself: the default must be far below a library size + # that would fit in one request, so progress is always paged. + assert DEFAULT_PAGE_SIZE <= 1000 From 60dee1b4d82e1d6208fb3eedc6619c9ac2fb5d6b Mon Sep 17 00:00:00 2001 From: dev Date: Sun, 28 Jun 2026 22:47:37 +0200 Subject: [PATCH 16/47] Align source settings card spacing --- webui/static/style.css | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/webui/static/style.css b/webui/static/style.css index e4e741b0..1cfc2a35 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -68589,3 +68589,14 @@ body.app-locked > *:not(#launch-pin-overlay):not(#login-overlay):not(script):not .reid-replace-text em { color: #7f879e; font-style: normal; font-size: 11.5px; } .reid-footer-actions { display: flex; gap: 10px; } #reid-confirm-btn:disabled { opacity: 0.4; cursor: not-allowed; filter: grayscale(0.4); } + +/* Settings cleanup: source settings uses the same inner-card spacing as Retry Logic. */ +#settings-page .settings-section-body:not(.collapsed) > .settings-group.source-settings-wrapper { + padding: 0 !important; + display: flex; + flex-direction: column; + gap: 14px; +} +#settings-page .settings-section-body:not(.collapsed) > .settings-group.source-settings-wrapper > .settings-subcard { + margin: 0 !important; +} From 0276aa8764c7bf3f52670bf75685d4d47019b534 Mon Sep 17 00:00:00 2001 From: dev Date: Sun, 28 Jun 2026 22:52:09 +0200 Subject: [PATCH 17/47] Update settings page overhaul --- webui/index.html | 978 +++++++++++++++++++++++---------------- webui/static/settings.js | 250 ++++++++-- webui/static/style.css | 350 +++++++++++++- 3 files changed, 1129 insertions(+), 449 deletions(-) diff --git a/webui/index.html b/webui/index.html index a5d1b4f8..46df8390 100644 --- a/webui/index.html +++ b/webui/index.html @@ -2148,7 +2148,7 @@
- @@ -3908,14 +3908,13 @@
-
+

Settings

-

Configure services, downloads, and preferences

- +
@@ -3923,7 +3922,7 @@
- + @@ -3937,6 +3936,7 @@

API Configuration

+

Metadata Source

@@ -3950,9 +3950,9 @@ -
-
-
Where artist, album, and track metadata comes from:
+ i +
- -
-
Runs the background metadata enrichment worker on the no-auth Spotify source instead of this connected account β€” keeping bulk enrichment off your official API quota (and dodging rate-limit bans), so your account is reserved for interactive search and playlist sync. On by default. Turn off to enrich through your connected account instead. Works even with no account connected. Note: the no-auth source can't supply artist genres.
+
+ + i
+
@@ -4571,81 +4573,17 @@
-
-

Download Settings

-
- These are container-internal paths. Only modify them if you know what you're doing β€” incorrect values will break downloads. -
+ + +