1216 Commits (f3e9581aad594a074e761263a7635be0701ddbb6)

Author	SHA1	Message	Date
Broque Thomas	f3e9581aad	Bump version to 2.5.3	2 weeks ago
Broque Thomas	2f284efa57	Retag now re-embeds LYRICS tag instead of leaving it empty ... Discord report (netti93). The download flow runs `enhance_file_metadata` (clears all tags) then `generate_lrc_file` (writes .lrc sidecar AND embeds USLT). The retag flow only ran the first half — `enhance_file_metadata` cleared USLT and there was no follow-up to restore it. Two coordinated fixes (no new setting per kettui scope discipline — user described it as "might even be an idea," consistency was the load-bearing ask). Fix 1 — retag calls generate_lrc_file after enhance `core/library/retag.py:execute_retag` now invokes `deps.generate_lrc_file` right after the `enhance_file_metadata` call, mirroring the download pipeline. New `generate_lrc_file` field on `RetagDeps`, defaults to None for backward compat with any test caller that builds RetagDeps without it. Web_server's `_build_retag_deps()` factory wires in the real `core.metadata.lyrics.generate_lrc_file`. Placement matters — runs BEFORE `safe_move_file` so the helper sees the audio file at its current path with its existing sidecar (which retag hasn't moved yet). After the embed, the audio file gets moved with USLT now present; the sidecar move step that follows is unaffected. Fix 2 — create_lrc_file re-embeds from existing sidecar `core/lyrics_client.py:create_lrc_file` used to early-return True when an .lrc / .txt sidecar already existed (skipping the LRClib fetch). For the retag case the sidecar is already there, so the shortcut hit and USLT was never re-written. Now the helper reads the existing sidecar and calls `_embed_lyrics` with its content before returning. Empty / unreadable sidecars short-circuit silently — defensive, no crash. Download flow unaffected because no sidecar exists at fetch time. 7 boundary tests pin: existing .lrc triggers re-embed, existing .txt triggers re-embed, empty sidecar skips embed, unreadable sidecar swallows error, no sidecar falls through to LRClib (download path regression guard), RetagDeps.generate_lrc_file field accepted, field optional for backward compat. Full suite: 3120 passed.	2 weeks ago
Broque Thomas	083355ec8c	Persist Find & Add selections as permanent server-playlist match overrides ... Closes #585. When a Spotify source track had a versioned suffix not present in the local file ("Iron Man - 2012 - Remaster" vs "Iron Man"), the auto-matcher missed the pair. User could click Find & Add to pick the right local file — that worked, file got added to the Plex playlist — but the source track stayed in Missing while the added file appeared in Extra, because the matcher kept no record of the user-confirmed pairing. On the next sync the source track re-tried to download. Fix: every Find & Add selection now writes a (spotify_track_id → server_track_id) override into sync_match_cache at confidence=1.0. The matching algorithm runs an override pass BEFORE the existing exact and fuzzy passes, so any user-confirmed pair short-circuits straight to "matched" without going through title normalization. Covers every mismatch class — dash-suffix remasters, covers / karaoke, alt masters, cross-language titles, typo'd local files. - core/sync/match_overrides.py (new) — pure helpers resolve_match_overrides + record_manual_match. 18 boundary tests pin: cache hits, cache misses falling through to normal matching, stale-cache (server track removed) handled gracefully, str/int id coercion, partial cache hits, defensive against non-dict inputs and DB exceptions. - web_server.py — get_server_playlist_tracks runs the override pre-pass before exact/fuzzy matching. server_playlist_add_track accepts source_track_id + source_title + source_artist and persists the override after every successful add (Plex / Jellyfin / Navidrome). source_track_id added to source_tracks payload so the frontend has it. - webui/static/pages-extra.js — _serverSelectTrack sends source_track_id + source_title + source_artist when adding a track from a mirrored playlist context. - Sync match cache schema unchanged — already had UNIQUE (spotify_track_id, server_source) which fits the override semantics perfectly. Manual overrides distinguished from auto-discovered matches by confidence=1.0. Full suite: 3010 passed.	2 weeks ago
Broque Thomas	f4cff78f13	Quarantine management — list, approve, delete, recover ... Closes #584. Quarantined files used to sit in ss_quarantine/ with a thin sidecar — no UI, no recovery, no way to see what got dropped. This adds the management surface the user needs without going to the filesystem. UI: new "Quarantine" button on the downloads page header opens a modal with every quarantined file (filename, expected track/artist, reason, when, size). Three actions per row: - Approve (one-click): restores the file, re-runs the post-process pipeline with ONLY the failing check skipped, lands in the library with full tags + lyrics + scan - Recover (legacy fallback): moves to Staging for thin-sidecar entries that lack the embedded context Approve needs - Delete: permanent removal of file + sidecar Per-check bypass: context['_skip_quarantine_check'] = 'integrity' / 'acoustid' / 'bit_depth'. Skips ONLY the named check — other quality gates stay live. No blanket bypass-all flag. Sidecar expansion: move_to_quarantine now persists the full json-serializable context via serialize_quarantine_context (drops non-JSON-safe values, walks nested dicts/lists/sets, str-coerces unknown objects) plus the trigger name. Existing thin sidecars are detected and routed to Recover instead of Approve. Pure helpers in core/imports/quarantine.py: list_quarantine_entries / delete_quarantine_entry / approve_quarantine_entry / recover_to_staging / serialize_quarantine_context. 27 tests pin every shape: orphan files / orphan sidecars / corrupt sidecars / collision-safe filename restoration / full-context vs thin-sidecar dispatch / json round-trip safety. Four new endpoints in web_server.py — thin glue around the helpers: GET /api/quarantine/list, DELETE /api/quarantine/<id>, POST /api/quarantine/<id>/approve, POST /api/quarantine/<id>/recover. Download modal status differentiates "🛡️ Quarantined" from "❌ Failed" so recoverable files are visible at a glance — checked against the error_message text, no schema change needed. Pipeline changes are three minimal per-check conditionals at the existing quarantine sites in core/imports/pipeline.py. Each move_to_quarantine call now passes its trigger name so the sidecar records which check fired. Full suite: 2992 passed.	2 weeks ago
Broque Thomas	8a11a660af	Extract manual import route handlers ... Move the remaining manual import endpoint logic out of web_server.py and into core.imports.routes behind ImportRouteRuntime. The Flask endpoints now stay as thin compatibility wrappers for album/track search, album match/process, single-file import processing, and batched singles processing. Keep legacy test patch points intact by re-exporting build_album_import_match_payload from web_server and routing singles_process through an injected process_single_import_file callable. This preserves existing route-level monkeypatch behavior while keeping the extracted helper testable. Add focused helper coverage for Hydrabase enqueueing, search limit clamping, album match payload forwarding, album import side effects, single-file worker outcomes, malformed manual matches, and singles aggregation/injected-worker behavior. Verification: py_compile and git diff --check passed locally; bundled-Python smoke covered the extracted helpers. Claude reran the project tests and reported all tests passing.	2 weeks ago
Broque Thomas	d703d33178	Extract import staging route helpers ... Move import staging files/groups/hints/suggestions controller logic out of web_server.py and into core.imports.routes behind an ImportRouteRuntime dependency object. Keep the existing Flask routes as thin compatibility wrappers so the UI endpoint surface stays unchanged. Add focused tests for staging file filtering, album grouping, hint generation, cached suggestions, empty missing staging paths, and error payloads from failed path/metadata reads. Verification: py_compile passed for web_server.py, core/imports/routes.py, and tests/imports/test_import_routes.py. A bundled-Python smoke pass covered the extracted helper behavior; pytest was not available in this Windows shell because the bundled Python lacks pytest and the repo venv is WSL/Linux-only here.	2 weeks ago
BoulderBadgeDad	dddf761d0b	Merge pull request #388 from kettui/feature/vite-webapp ... Lay the groundwork for webui React transition	2 weeks ago
Broque Thomas	fc366184b2	Raise discography limit from 50 to 200 ... Discord report: prolific artists (Bach, Beatles complete box, deep dance/electronic catalogues) only showed ~50 entries in the "Download Discography" modal. `MetadataLookupOptions(limit=50, max_pages=0)` was hardcoded at three call sites. Spotify's `max_pages=0` already paginates through everything (per-page is clamped to 10 internally), so Spotify-primary users were unaffected. But Deezer / iTunes / Discogs / Hydrabase all honor the outer `limit` as a hard cap, so non-Spotify users were silently clipped. Bump `limit` to 200 at all three call sites — matches iTunes's and Discogs's own internal caps and covers near-everyone's full catalogue. Spotify behavior unchanged. - web_server.py:9221 — discography endpoint (modal) - web_server.py:8700 — artist-detail discography view - core/artist_source_detail.py:129 — source-specific artist detail	2 weeks ago
Antti Kettunen	32bf52cc18	Extract WebUI asset helpers ... - move Vite manifest handling and SPA route rules into core/webui - keep web_server.py focused on Flask route wiring - add tests for asset rendering and manifest reload behavior - keep image URL normalization coverage alongside the metadata helpers	2 weeks ago
Antti Kettunen	d8f8c6b95c	Convert dev launcher to Python ... - Replace the shell convenience script with a cross-platform Python launcher. - Keep dev.sh as a Unix compatibility wrapper. - Let the direct backend bind with host and port overrides. - Update the root and webui README guidance for the new launcher. - Preserve the backend startup behavior used by the old dev flow.	2 weeks ago
Antti Kettunen	147a09035c	Remove stale initial page rendering hooks ... - Drop unused _resolve_webui_initial_* helpers from web_server.py. - Remove template-side initial_nav_page and initial_client_page conditionals. - Keep Vite asset injection and runtime page activation in the client.	2 weeks ago
Antti Kettunen	686bfcc749	Drop server-rendered webui page state ... Remove the Flask route-to-page helpers and stop passing initial active-page flags into the shell template. The web UI now renders static page and nav markup, while the client-side shell remains responsible for establishing active page state after load. This keeps the hybrid Flask + Vite asset setup intact while reducing duplicated route/page ownership logic in the backend template layer. Also added a previously missing /stream path to the spa exclusions	2 weeks ago
Antti Kettunen	736f243d5c	Simplify webui Vite asset injection	2 weeks ago
Antti Kettunen	d98dcd8606	Initial Vite app scaffolding & issues page impl ... - File-based routing with tanstack router - Persist top-level navigation state in url, even for most legacy pages - Striving for an intuitive and simple folder structure where route-related code is colocated, but the amount of files is still kept to a minimum - Replace native fetch with `ky` - Familiar api, but more polished	2 weeks ago
Broque Thomas	89246a7304	Write artist.jpg to artist folder so Navidrome shows real photos ... Closes #572 (rhwc). Navidrome has no API for setting an artist image — it reads `artist.jpg` (or `folder.jpg`) from the artist folder during library scans. SoulSync's `update_artist_poster` for Navidrome was a no-op, so users only ever saw album-art-derived thumbnails as the artist photo. - new "Write Artist Image" button on artist detail page - POST /api/artist/<id>/write-image-to-disk derives the artist folder from any track's resolved file_path (reuses _resolve_library_file_path so docker mount translation + library.music_paths probes from #558 apply), fetches the photo from the configured metadata source priority chain, downloads with content-type validation, writes atomically via `<filename>.tmp + os.replace` - when active server is Navidrome, triggers a library scan immediately so the file is picked up - respects existing artist.jpg (frontend prompts before overwriting) so user-supplied photos aren't clobbered - works for plex / jellyfin too as a fallback layer — both servers also read artist.jpg from disk 26 tests pin the pure helpers in core/library/artist_image.py: folder derivation (trailing sep / empty / non-string), URL picking (missing attr / whitespace / non-string), download (non-image content-type / 404 / timeout / empty body), atomic write (replace / temp-cleanup-on-failure / overwrite guard / missing folder).	2 weeks ago
Broque Thomas	641c72d7f1	Bump version to 2.5.2	2 weeks ago
Broque Thomas	6ce185491d	Add per-download Audit Trail modal to Library History ... - new "Audit" button on each download row in the library history modal opens a second modal visualizing the download lifecycle as an interactive horizontal stepper (request → source → match → verify → process → place) with click-to-expand detail cards - hero header with album art + track title + meta line + status pills (source / quality / acoustid result) - three tabs: Lifecycle / Tags / Lyrics - Tags tab reads the audio file live via mutagen at audit-open time via new GET /api/library/history/<id>/file-tags endpoint; file is the single source of truth so background enrichment writes (audiodb / lastfm / genius / replaygain / lyrics fetch) show up too. flat key/value rows stacked vertically (label-above- value) so long MBIDs / URLs / joined genre lists wrap cleanly. source IDs grouped per-service into 2-col sub-card grid. - Lyrics tab renders the full transcript with dimmed timecodes. - post-processing step infers observable changes from source-vs- final state (format conversion, file rename via tag template, folder template). - "Download History" button also added to the Downloads page batch panel header so it's reachable outside the dashboard. - mobile responsive: tabs + stepper scroll horizontally, modal goes full-screen, hero stacks below 480px. 19 helper tests pin the mutagen reader: id3 (TIT2/TPE1/TALB + TXXX + USLT + APIC), vorbis (FLAC dict + _id/_url passthrough), file metadata (format / bitrate / duration), defensive paths (empty / missing file / mutagen returns None / mutagen raises), stringify edge cases (list / tuple / int / frame-with-text / whitespace).	2 weeks ago
Broque Thomas	1715e4d52f	Bump version to 2.5.1	2 weeks ago
Broque Thomas	4892baf8d4	Skip already-owned tracks during download discography ... - new track_already_owned helper wraps db.check_track_exists at the same confidence threshold the discography backfill repair job uses (0.7) — name+artist+album, format-agnostic so blasphemy-mode libraries (flac → mp3 + delete original) match correctly - endpoint runs the check after the artist + content-type filters and before add_to_wishlist, so a second discography click on the same artist no longer re-queues every track that already downloaded - per-album response carries a new tracks_skipped_owned counter alongside the existing artist/content/wishlist skip categories Discord report (Skowl).	2 weeks ago
Broque Thomas	d4ad5bf57f	Filter cross-artist + content-type tracks during download discography ... - drop tracks where the requested artist isn't named in track.artists (keeps features, drops compilation / appears_on contamination) - honor watchlist.global_include_live/remixes/acoustic/instrumentals the same way the discography backfill repair job already does - surface per-album skip counts in the ndjson stream (artist mismatch + content filter) so the ui can show what was filtered Closes #559.	2 weeks ago
Broque Thomas	698ecc99f0	Import history: Clear History button now sweeps stuck 'processing' rows ... Reported: Clear History button on the Import page left zombie rows behind. Every survivor showed "⧗ Processing" status from 2-9 days ago. Trace: `_record_in_progress` inserts a `status='processing'` row up-front so the UI can render the in-flight import while it runs; `_finalize_result` updates it to `completed`/`failed` when the import finishes. When the worker is killed mid-import (server restart, crash), the row never gets finalized — stays at `processing` forever. The clear-history endpoint's SQL `DELETE ... WHERE status IN (...)` listed every terminal status but omitted `processing`, so zombies survived every click. Fix: add `processing` to the delete list, but guard against nuking genuinely-live imports by intersecting against the worker's `_snapshot_active()` map — any folder hash currently registered in `_active_imports` is excluded from the delete via an `AND folder_hash NOT IN (...)` clause. `pending_review` deliberately left out so user still has to approve/reject those explicitly. One endpoint touched (`/api/auto-import/clear-completed` in web_server.py). No worker changes — guard reuses the existing `_snapshot_active()` method that the UI poller already calls. 5 new tests in `tests/imports/test_auto_import_clear_completed_endpoint.py`: - Zombie `processing` rows swept, live `processing` row preserved (folder_hash currently in `_active_imports` survives) - Response count matches actual delete count - Empty active-set branch (unparameterized DELETE) — pinned because an empty SQL `IN ()` would be a syntax error - Worker-unavailable returns 500 (pre-existing guard not regressed) - `pending_review` rows always survive — never auto-swept Full pytest sweep: 2758 passed (one pre-existing flaky timing test on `test_import_singles_parallel.py` failed under full-suite CPU load, passes in isolation in 2.95s — unrelated to this change).	2 weeks ago
Broque Thomas	4fb9f38798	Your Albums: selectable wishlist modal + Tidal album resolution ... Two-part fix to the Your Albums "Download Missing" flow on Discover. Part A — UX redesign The prior `downloadMissingYourAlbums()` ran a per-album loop that fired direct-download tasks via `openDownloadMissingModalForYouTube`. Reported as silently failing — "Queuing 2/2" toast with no actual transfer activity. Even when downloads worked, bypassing the wishlist meant no retry / dedup / rate-limit / source-fallback handling. Replaced with a selectable-grid modal mirroring the Download Discography pattern from the library page. Click the download button → opens a checkbox grid showing every missing album (cover, title, artist, year, track count, source) → user picks what they actually want → click "Add to Wishlist" → each album's tracks get resolved + queued through the existing wishlist auto-download processor. NDJSON progress stream renders ✓/✗ per album. New JS helpers: - `_openYourAlbumsBatchModal(missingAlbums)` — builds the modal - `_renderYourAlbumsBatchCard(row, index)` — per-album card - `_yourAlbumsBatchSelectAll(select)` — bulk toggle - `_updateYourAlbumsBatchFooterCount()` — live count + button text - `_closeYourAlbumsBatchModal()` — overlay teardown - `_startYourAlbumsBatchAddToWishlist()` — submit handler, NDJSON progress consumer - `_yourAlbumsPickSource(album)` — picks the single best source-id per row (priority: spotify → deezer → tidal → discogs) Reuses the `.discog-*` CSS classes from the library Download Discography modal — no new CSS. Reuses the existing `/api/artist/<id>/download-discography` endpoint. The endpoint's URL artist_id param is functionally unused (per-album payload carries everything — verified by reading the endpoint body), so the modal posts with placeholder `your-albums` and gets multi-artist resolution for free without backend changes. Part B — Tidal album resolution Reported as the original bug: clicking download on Tidal-only albums did nothing because `/api/discover/album/<source>/<album_id>` had no `tidal` branch and `tidal_client` had no `get_album_tracks` method. `core/tidal_client.py`: new `get_album_tracks(album_id, limit=None)` method. Two-phase: cursor-walk `/v2/albums/<id>/relationships/items?include=items` for track refs + position metadata (`meta.trackNumber` + `meta.volumeNumber`), batch-hydrate via existing `_get_tracks_batch` for artist/album names. Returns `Track` objects with `track_number` and `disc_number` attached. Sort by (disc, track) so multi-disc compilations render in album order. `web_server.py`: new `'tidal'` source branch in `/api/discover/album/<source>/<album_id>`. Resolves album metadata via `get_album`, tracks via `get_album_tracks`, cover art via inline `?include=coverArt` lookup. Same response shape as Spotify/Deezer branches. `webui/static/discover.js`: - `tidal_album_id` added to `trySources` for the single-album click flow (`openYourAlbumDownload`) - Same source picker drives the new batch modal - Virtual-id generation includes `tidal_album_id` so Tidal-only albums get stable identifiers across discover-album-* / your- albums-* contexts 10 new tests in `tests/test_tidal_album_tracks.py` pin: - Single-page walk + hydration - Multi-page cursor chain - Multi-disc sort order (disc 1 → 2 in track order each) - `limit` short-circuit at page boundary - No-token short-circuit (no API call) - HTTP error returns empty - 429 raises (propagates to `rate_limited` decorator for retry) - Forward-compat type filter (skips non-track entries) - Partial-batch hydration failure containment - Empty-album short-circuit (no batch call) Full pytest: 2693 passed.	2 weeks ago
Broque Thomas	6fe85f2f37	Server playlist sync: append mode (preserve user-added tracks) ... Discord report (CJFC, 2026-04-26): syncing a Spotify playlist to the server overwrote anything manually added to the server-side playlist. The fix adds a per-sync mode picker next to the Sync button on the playlist details modal — Replace (default, current delete-recreate behavior) or Append only (preserves existing tracks, only adds new ones). Useful when the source platform caps playlist size and the user is manually building beyond it on the server. Implementation: * New `append_to_playlist(name, tracks)` method on Plex / Jellyfin / Navidrome clients. Each uses the server's NATIVE append API: - Plex: `existing_playlist.addItems(new_tracks)` - Jellyfin: `POST /Playlists/<id>/Items?Ids=...&UserId=...` - Navidrome: Subsonic `updatePlaylist?songIdToAdd=...` Falls back to `create_playlist` when the playlist doesn't exist yet (first sync). No delete-recreate, no backup playlist created (preserves playlist creation date + metadata + non-soulsync-managed tracks). * Dedup-by-server-native-id (ratingKey for Plex, GUID for Jellyfin, song-id for Navidrome) — never re-adds a track already on the playlist. Server-native identity, not fuzzy title+artist match, so it can't false-collide. * `sync_service.sync_playlist` accepts `sync_mode='replace'|'append'` kwarg. Single if/else branch dispatches to `append_to_playlist` or `update_playlist`. Threaded through `core/discovery/sync.run_sync_task` and the `/api/sync/start` HTTP handler. Validation on the API rejects unknown mode strings (defaults to 'replace'). * Frontend: per-playlist `<select id="sync-mode-${id}">` rendered next to the Sync button in both modal renderers (sync-spotify.js for Spotify playlists, sync-services.js for Deezer ARL playlists). `startPlaylistSync` reads the select at click time; missing select (other callers like discover.js) defaults to 'replace' so backward compat preserved without per-call-site updates. * SoulSync standalone has no playlist methods at all and the modal hides the Sync button entirely on it via `_isSoulsyncStandalone` — dispatch never reaches that path, no defensive fallback needed. 15 new tests pin per-server append behavior: - missing playlist → create_playlist delegation - dedup filtering (existing IDs skipped, only new tracks added) - empty new-track set short-circuits without API call - failure paths return False without raising - contract listing (KNOWN_PER_SERVER_METHODS includes 'append_to_playlist'; Plex / Jellyfin / Navidrome all implement) Plus tests/discovery/test_discovery_sync.py fake `sync_playlist` fixture got `sync_mode='replace'` default to match the new signature (was breaking after the kwarg add; now passing). WHATS_NEW entry under new '2.6.0' block (hidden by `_getLatestWhatsNewVersion` until next release bump). Closes CJFC discord request.	2 weeks ago
Broque Thomas	1d6e213b16	version bump	2 weeks ago
Broque Thomas	f28f9808db	Tidal: surface Favorite Tracks as virtual playlist (issue #502) ... Adds the user's Tidal favorited tracks ("My Collection" in the Tidal app) as a virtual playlist alongside their real playlists, mirroring how Spotify's "Liked Songs" is treated. Reporter (yug1900) located the working endpoint after the prior `/v2/favorites?filter[type]=TRACKS` attempt returned empty data — that endpoint is scoped to collections the third-party app created itself, not personal favorites. Real endpoint: GET /v2/userCollectionTracks/me/relationships/items ?countryCode=US&locale=en-US&include=items Cursor-paginated (20 per page, follow `links.next` with `page[cursor]=...` until exhausted). Response only carries track-level attributes — artist + album NAMES come back as relationship-link stubs, not embedded data. Implementation: * Two-phase fetch — `_iter_collection_track_ids` walks the cursor chain to enumerate every track id (cheap, IDs only), then `get_collection_tracks` batch-hydrates 20 IDs at a time through the existing `_get_tracks_batch` helper which already knows how to `include=artists,albums`. No duplication of the JSON:API artist/album parse, no new dataclass shape. * Virtual playlist `tidal-favorites` appended to the end of `/api/tidal/playlists`. ID intentionally has no colon — sync-services.js renderer interpolates IDs into CSS selectors via template literals (`#tidal-card-${p.id} .foo`) and a `:` would parse as a CSS pseudo-class operator. * `tidal_client.get_playlist("tidal-favorites")` recognizes the virtual id and dispatches to the collection path internally, so every per-id consumer gets it for free: detail endpoint, mirror auto-refresh automation, "build Spotify discovery from Tidal playlist" flow. OAuth scope expansion: * Added `collection.read` to both OAuth flows (the `core/tidal_client.py::authenticate` standalone path AND the `web_server.py::auth_tidal` web flow — they were independent scope strings that both needed updating). * Added `prompt=consent` to both flows — without it Tidal silently returns a token carrying only the ORIGINAL scope set even after re-authentication, because Tidal treats the existing authorization as still valid. * New `disconnect()` method + `POST /api/tidal/disconnect` endpoint + Disconnect button next to Authenticate in Settings → Connections → Tidal — required for users whose existing token predates the scope expansion (forces a clean grant). Reconnect-needed UI hint: * `_collection_needs_reconnect` flag set on 401/403 from the collection endpoint, cleared on next successful walk, NOT set on 5xx (transient server errors must not falsely tell the user to reconnect). * Listing endpoint reads the flag and surfaces a placeholder card titled "Favorite Tracks (reconnect Tidal to enable)" with a description pointing at Settings, so the user has something visible to act on instead of a silently missing row. Diagnostic logging — collection request URL + response status + first 300 bytes of body now logged at info level so future "why is my collection empty" reports can be diagnosed from app.log without needing live reproduction. 22 new tests pin: cursor walk (full chain, max-ids cap mid-page + at page boundary), auth gates (no token / 401 / 403 all bail clean), reconnect-flag lifecycle (set on 401/403, cleared on next successful walk, NOT set on 5xx), forward-compat type filter (non-track entries skipped), count helper, batch hydration delegation + chunking at the 20-per-batch cap, partial-batch failure containment, virtual-id dispatch (real playlist ids still flow through the normal path). Closes #502.	2 weeks ago
Broque Thomas	402d851cac	Deezer search: drop advanced-syntax at endpoint, free-text + rerank wins ... Live-API verification revealed advanced-syntax queries hurt more than they help on this endpoint. Switching the import-modal Deezer search back to free-text + local rerank. # What live testing showed Hit Deezer's public API with both query forms for the issue #534 case (`Dirty White Boy` + `Foreigner`): **Free-text (`q=Dirty White Boy Foreigner`):** - Returns 21 results - Real Foreigner Head Games studio cut at #1 - Live versions at #2-10 - Karaoke / cover variants at #11-15 **Advanced (`q=track:"Dirty White Boy" artist:"Foreigner"`):** - Returns 12 results - "(2008 Remaster)" at #1 — canonical Head Games cut MISSING from top 8 entirely - Live + alt-album versions follow Advanced syntax DOES filter karaoke at the API level (none in the 12-result set vs. 5 at positions 11-15 in free-text), but it has its own ranking bias that surfaces remasters / "Best Of" cuts ahead of the canonical recording. Net regression for the user- facing goal. # Fix 1. Endpoint reverts to free-text query with local rerank applied. 2. Local rerank gains "remaster" / "remastered" / "reissue" patterns under VARIANT_TAG_PATTERNS (soft 0.4× penalty — user may want them but they shouldn't outrank the original). 3. Client kwarg support (`track=` / `artist=` / `album=`) preserved for future opt-in callers (e.g. exact-match flows where API- level filtering matters more than ranking). # Verified end-to-end against live Deezer API Re-ran the exact #534 case through the live API + new rerank. Top 15 results post-rerank: 1. Dirty White Boy — Foreigner — Head Games ← REAL CUT AT TOP 2-10. Various Live versions 11-15. Karaoke / cover / tribute variants ← BURIED Real Foreigner Head Games studio cut at #1, exactly the user's ask. # Tests - `test_relevance.py` — variant tag patterns extended; existing tests still pass (50 tests). - `test_search_match_endpoints.py::test_joins_track_and_artist_into_free_text_query` — replaces `test_passes_track_and_artist_as_kwargs`; verifies endpoint sends free-text join, NOT field-scoped kwargs (the prior test asserted the wrong direction now). - Karaoke-burying assertion at the endpoint still pins the user-visible behaviour. - Client kwarg path tests untouched (still pin advanced-syntax construction for future opt-in callers). # Verification - 75 relevance + endpoint + query tests pass - 2445 full suite passes - Ruff clean - Live Deezer API shows real cut at #1 post-rerank	2 weeks ago
Broque Thomas	1cc37081a6	Fix Deezer search relevance — issue #534 ... # Background User reported (#534) that the import-modal "Search for Match" dialog returned irrelevant results when Deezer was the metadata source. Searching `Dirty White Boy` + `Foreigner` returned 5+ karaoke / "originally performed by" / "in the style of" / "re-recorded" / tribute-band results ranked above the actual Foreigner studio cut from Head Games. User had to scroll past the junk every time, or fall back to iTunes search which is much slower. # Root cause — two layers 1. **Endpoint joined `track + artist` into free-text query.** `/api/deezer/search_tracks` was passing `q=Dirty White Boy Foreigner` to Deezer's `/search/track` API. Deezer fuzzy-matches that string across title / lyrics / artist / album / contributors and orders by global popularity — anything that appears across many compilations outranks the canonical recording. 2. **No local rerank.** None of the search-modal endpoints applied any post-filtering. Deezer's API order shipped straight to the user. # Fix — same architectural shape Cin would build ## Layer 1: field-scoped query at the client boundary `core/deezer_client.py::search_tracks()` now accepts optional `track`, `artist`, `album` kwargs. When provided, builds Deezer's advanced search syntax: `q=track:"X" artist:"Y" album:"Z"`. Massive relevance improvement because each term matches the right field instead of fuzzy-matching everywhere. Backward compat preserved: legacy free-text `query=` callers still work unchanged. Field-scoped path takes precedence when both are provided. Empty input fast-fails without an API call. Embedded double-quotes stripped (Deezer's syntax has no escape mechanism). ## Layer 2: provider-neutral relevance reranker New `core/metadata/relevance.py` module — pure-function rerank over the canonical `Track` dataclass. Composable scoring: - **Cover/karaoke patterns** (multiplier 0.05, effectively buries): matches "karaoke", "originally performed by", "in the style of", "made famous by", "tribute", "vocal version", "backing track", "cover version", "re-recorded", "cover by", etc. across title, album, AND artist fields. Catches the screenshot's exact junk: artist credits like "Pop Music Workshop" / "The Karaoke Channel" / "Foreigner Tribute Band". - **Variant tags** (multiplier 0.4): live / acoustic / demo / instrumental / remix / radio edit / club mix etc. — softer penalty since the user MAY want them. Skipped entirely when the expected_title contains the same tag (so searching "Track (Live)" still ranks Live versions first). - **Exact artist boost** (multiplier 1.5): primary artist exactly matches expected_artist after normalisation. Single strongest signal for "this is the canonical recording". - **Title + artist similarity** via SequenceMatcher (parentheticals + punctuation stripped before comparison). - **Album-type weighting**: album=1.0 > single/ep=0.85 > compilation=0.7. Compilations are more likely tribute / karaoke repackages. Each component is a standalone function so tests pin them individually without standing up the full pipeline. ## Wired at three search-modal endpoints - `/api/deezer/search_tracks` — uses both layers (field-scoped query + rerank). - `/api/itunes/search_tracks` — uses rerank only (iTunes API has no advanced-syntax search, but karaoke / cover variants still leak through and need the local penalty). - `/api/spotify/search_tracks` — already builds field-scoped `track:X artist:Y` query; rerank added as the consistency safety net so all three sources behave the same from the user's perspective. Other Deezer call sites (matching engine, watchlist scanner, auto-import single-track ID) deliberately not touched in this PR — they have their own elaborate scoring pipelines tuned to their specific contexts and aren't surfacing the user-reported issue. Per Cin: "don't refactor beyond what the task requires." # Tests 71 new tests across 3 files: - `tests/metadata/test_relevance.py` (50 tests) — every scoring component pinned individually + the issue #534 screenshot reproduced as a regression test (real Foreigner cut wins after rerank, karaoke variants drop to bottom). - `tests/metadata/test_deezer_search_query.py` (14 tests) — advanced-syntax query construction, field-scoped wiring at the client boundary, free-text path unchanged, kwargs win when ambiguous, limit clamping, cache key consistency. - `tests/imports/test_search_match_endpoints.py` (7 tests) — end-to-end through Flask test client: Deezer endpoint passes kwargs not joined query; karaoke buried at bottom for all three sources; legacy query param still works without rerank. # Verification - 2441 full suite passes (+71 from baseline 2370) - 0 failures (the prior watchdog flake fix held) - Ruff clean across all changed files - JS parses clean (`node -c webui/static/helper.js`) # Architectural standards followed - **Logic at the right boundary.** Query construction lives in the client (every caller benefits from one change). Rerank lives in a neutral module (`core/metadata/relevance.py`) over the canonical `Track` dataclass — works for any source, not Deezer- specific. - **Explicit > implicit.** Every scoring rule has its own named function. Pattern tables are module-level constants tests can introspect. - **Scope discipline.** Audited every Deezer search call site; fixed the user-reported one + the consistent siblings. Did NOT speculatively normalise every Deezer call across the codebase. - **Backward compat.** Free-text `query=` callers untouched. Kwargs added to existing client method signature with safe defaults. - **Tests pin contract at correct boundary.** Pure-function rerank tests don't mock anything; client-query tests stub at `_api_get`; endpoint tests run through the real Flask app.	2 weeks ago
Broque Thomas	8a6ee7a2c7	Auto-import: bounded ThreadPoolExecutor + per-candidate UI state isolation ... # Concurrency model Pre-refactor concurrency was emergent + unbounded: - The worker's `_run` thread called `_scan_cycle` every 60s, processing candidates synchronously in a for-loop. - The `/api/auto-import/scan-now` endpoint spawned a fresh `threading.Thread(target=_scan_cycle)` per click — extra parallel scan cycles on top of the timer. - Multiple "Scan Now" clicks during in-flight processing → multiple threads racing on `_processing_paths` / `_folder_snapshots` state, no upper bound on concurrent scanners. - `stop()` didn't wait for in-flight processing — could leave file moves / tag writes / DB inserts mid-flight. Refactor to the pattern Cin uses elsewhere (`missing_download_executor`, `sync_executor`, `import_singles_executor` all use `ThreadPoolExecutor(max_workers=3, thread_name_prefix=...)`): - **One scan thread** — both timer + manual triggers go through `trigger_scan()`, gated by a non-blocking `_scan_lock`. Duplicate triggers no-op instead of stacking parallel scanners. - **Bounded executor** — `ThreadPoolExecutor` (default 3 workers, configurable via `auto_import.max_workers`) runs per-candidate work. Each candidate runs to completion in its own pool thread; up to N candidates run in parallel. - `_scan_and_submit()` is fast — just enumeration + executor submit, returns immediately, doesn't block on per-candidate work. - `_process_one_candidate(candidate)` holds the per-candidate logic identical to the old for-loop body, lifted into a method so the pool can run multiple instances concurrently. - `_submitted_hashes` set + lock dedupes candidates across the timer + manual triggers so a candidate already queued / running doesn't get re-submitted. - `stop()` calls `executor.shutdown(wait=True)` — clean shutdown, no orphaned file ops. # Per-candidate UI state isolation The executor refactor opened two concurrency holes that the old sequential model masked. Both fixed in this commit: 1. **Scalar UI fields stomped across pool workers.** Pre-refactor `_current_folder` / `_current_status` / `_current_track_*` were safe under the sequential model — only one candidate processed at a time, so the fields tracked the in-flight one. With three pool workers writing the same fields, the polling UI saw garbage like "Processing AlbumA, track 7/14: SongFromAlbumB". Replaced with `_active_imports: Dict[hash, _ActiveImport]` keyed on folder_hash, gated by `_active_lock`. Each pool worker owns its own entry. Helpers `_register_active` / `_update_active` / `_unregister_active` / `_snapshot_active` are the only API. 2. **Stats counters not thread-safe.** `self._stats[k] += 1` is read-modify-write — under load, parallel pool workers drop increments. New `_stats_lock` + `_bump_stat()` helper wraps every mutation. `get_status()` reads under the same lock and returns a copy. # Endpoint change `/api/auto-import/scan-now` no longer spawns its own scan thread — calls `auto_import_worker.trigger_scan()` (which routes through the shared lock + executor). Multiple clicks while a scan is in flight no-op deterministically. Endpoint still wraps the call in a daemon thread so the HTTP response returns immediately even if the staging walk is slow. # Backward compat The scalar `_current_folder` / `_current_status` / `_current_track_*` fields are preserved as **read-only properties** that resolve to the FIRST active import. The existing `get_status()` payload still includes those fields populated from the first entry — single-import UIs (and the test fixture) keep working unchanged. New `active_imports` array exposes the full multi-candidate state for parallel-aware UIs. # Behavior preserved - Per-candidate identify / match / process logic byte-identical - Live-progress state preserved (per candidate now) - Stability gate / already-processed dedup preserved - `_record_in_progress` / `_finalize_result` UI rows preserved - Tag-based loose-file grouping unchanged # Behavior changes - Multiple albums process IN PARALLEL up to `max_workers` - "Scan Now" while scan in progress no-ops (was: spawned another) - `stop()` waits for in-flight pool work via `shutdown(wait=True)` - Auto-import card now lists each in-flight album (one line per active import) instead of a single shared progress line # UI `webui/static/stats-automations.js`: - Progress widget reads `active_imports` array, renders one line per in-flight album with per-candidate status / track index - Falls back to the legacy summary line when payload doesn't carry `active_imports` (older backend) - Per-row "live processing" lookup now matches by `folder_hash` through the array instead of by `folder_name` against scalars # Tests added (`tests/imports/test_auto_import_executor.py`) - Pool config: default max_workers=3, configurable via constructor + via `auto_import.max_workers` config, floors at 1 - Scan lock: 5 concurrent `trigger_scan()` calls run only 1 scan while lock held; releases properly so subsequent triggers run - Executor dispatch: 5 candidates → 5 process calls via the pool - Bounded parallelism: max_workers=3 caps at 3 concurrent; max_workers=2 caps at 2 - Cross-trigger dedup: candidate submitted in scan A doesn't get re-submitted by scan B while still in-flight - Graceful shutdown: `stop()` blocks until in-flight pool work finishes - Per-candidate state isolation: 2 parallel workers updating their own candidate state don't interfere — each candidate's track_index / track_name / folder_name reads back exactly as written for that hash - `get_status()` returns coherent `active_imports` array with one entry per in-flight candidate; aggregate top-level `current_status` is 'processing' when any entry is processing - Unregister removes only that candidate, others stay visible - Stats counter thread-safety: 1000 parallel bumps land at 1000 (the read-modify-write race regresses without the lock) - `get_status()` stats snapshot is a copy, not a live reference # Verification - 17 new tests pass (executor + state isolation) - 2347 full suite passes (1 pre-existing flaky test — `test_watchdog_warns_about_stuck_workers` — passes in isolation, unrelated) - Ruff clean	2 weeks ago
Broque Thomas	f58f202d32	Fix manual album import losing source — issue #524 ... radoslav-orlov reported every imported album landing in the soulsync standalone library as "Unknown Artist" + the raw 10-digit album id as the title + 0 tracks. Audit traced it to the click handler in the import page dropping the source-of-the-album_id on its way to the backend match endpoint. Root cause: `importPageSelectAlbum(albumId)` (the onclick on every suggestion / search-result card) only passed the album_id string. The full search response carried `source`, `name`, and `artist` per row — the backend's `get_artist_album_tracks` needs source so it can route the lookup to the metadata source the id actually came from. Without it, the source chain tries each source's `get_album(id)` against an id shaped for a different source — a Deezer numeric id against Spotify's id format returns 404, against iTunes's collectionId range returns 404, etc. — and falls through to the failure-fallback dict in `get_artist_album_tracks`: { 'success': False, 'album': {'name': album_name or album_id, 'total_tracks': 0, 'release_date': '', ...}, # no artist field at all 'tracks': [], } That broken album dict then flowed through `build_album_import_context` → post-processing pipeline → `record_soulsync_library_entry`, writing "Unknown Artist" + album_id-as-title + 0 tracks rows into the soulsync standalone library tables. Why hybrid users hit it most: a Spotify-primary user searching for an album → search returns the Spotify result PLUS Deezer fallbacks (via `_search_albums_for_source`'s priority chain). Clicking a Deezer fallback row then sent only the Deezer id to /album/match without flagging that source — Spotify-first chain failed against the Deezer id and the broken fallback got written. Fix: Frontend (`webui/static/stats-automations.js`): - New `importPageState._albumLookup: { albumId: { id, name, artist, source } }` populated by both card renderers (`_renderSuggestionCard` + the search-results render block) before they emit the onclick. - `importPageSelectAlbum` reads source / name / artist from that cache and includes them in the match POST body, so the backend routes to the correct provider's `get_album` on the very first try. - `_escAttr` applied to album_id in the onclick (defensive — ids shouldn't contain quotes but `_escAttr` was already being used on every other field interpolated into onclick attributes). Backend (`web_server.py:import_album_match`): - Defensive log warning when source is missing from the request body. Catches any future regression where another caller (curl / third-party / new UI flow) drops source again — it'll show up as a visible warning in app.log instead of silently corrupting the library. Verification: - Full pytest suite: 2264 passed, 1 skipped, 0 failed - Ruff clean - JS syntax clean - Manual repro requires a real user flow (search albums on the import page → click one → import) which isn't covered by the existing unit tests; reviewer should verify against issue #524's steps before merge.	2 weeks ago
Broque Thomas	e20994e1c7	Manual picks: stream results, don't auto-retry, fix stuck-at-0% ... Three follow-on fixes to the manual-search candidates modal once people started actually using it: 1. NDJSON streaming. Manual search waited for every source to return before showing anything. Now streams one event per source as each completes — header line, source_results per source, done terminator. Frontend appends rows incrementally via response.body.getReader(). 2. Manual picks no longer auto-retry on failure. New _user_manual_pick flag set on the task in /download-candidate. Both monitor retry paths (not-in-live-transfers stuck + Errored state) bail on the flag. Surfaces the failure to the user instead of silently picking a different candidate via fresh search. 3. Non-Soulseek manual picks (youtube/tidal/qobuz/hifi/deezer/ soundcloud/lidarr) no longer stuck at "downloading 0%" forever. The live_transfers IF branch now marks manual-pick tasks failed directly when the engine reports Errored, instead of deferring to the monitor (which bails on manual picks). Engine fallback in else branch covers the rare race where the orchestrator's pre-populated transfer lookup is missing the entry. Plus a deadlock fix discovered along the way: the new failure path synchronously called on_download_completed while holding tasks_lock, which itself re-acquires the same Lock — non-reentrant threading.Lock self-deadlocked the polling thread. While wedged, every other endpoint that needed the lock (including /candidates → other failed rows couldn't open modals) hung waiting. Moved completion callbacks onto a daemon thread so the lock releases first. Plus failed/not_found/cancelled rows are now ALWAYS clickable (not just when the auto-search cached candidates) — the modal carries the manual search bar, which is the user's recourse for empty results. Plus manual download worker now runs on a dedicated thread instead of competing with the batch's 3-worker missing_download_executor pool — saturated batches no longer queue manual picks indefinitely. All scoped to manual picks via the _user_manual_pick flag — auto attempt flow byte-identical to before. Engine fallback gated on the flag too so auto attempts in the else branch keep the original do-nothing behavior (safety valve handles the stuck-forever case). Also dropped _handle_failed_download from web_server.py — defined but had no callers (dead code). 17 new unit tests pin the gate behavior: - engine fallback: Errored/Cancelled/Succeeded/InProgress transitions, manual-pick gate, terminal-state skip, soulseek skip, missing download_id skip, engine returning None, orchestrator exception - monitor: manual-pick skips not-in-live-transfers retry + Errored retry - IF-branch end-to-end: Errored marks failed, "Completed, Errored" hits failure branch, auto attempts defer to monitor Manual-search endpoint tests rewritten for NDJSON: 11 cases (validation, single-source dispatch, parallel "all" dispatch, one-event-per-source streaming shape, unconfigured-source skip + reject, header metadata, per-source exception isolation). Full suite 2259 passed, 1 skipped.	2 weeks ago
Broque Thomas	996575fab3	Add manual search to the failed-track candidates modal ... When an auto-download fails or returns "not found" with leftover candidates, the user can already click the status cell to open a modal showing those candidates and pick a different one. This adds a manual search bar to that modal — type any query, hit search, get a fresh round of results without having to bail out and start over from the main search page. Solves the case where the auto-query was bad (featured artist not in title, parentheticals like "(Remastered 2019)" tripping the matcher, slight artist-name variants, transliteration) but the file genuinely exists on the source. Frontend (downloads.js) - Added a manual-search section above the existing auto-candidates table inside the candidates modal. - Source picker is smart per download mode: - Single-source mode (soulseek-only / youtube-only / etc) shows a "Searching X" label, no dropdown. - Hybrid mode shows a dropdown with "All sources" default + every configured source. Picking "All" runs parallel searches across them and tags each result row with its source badge. - Only configured sources show up; unconfigured are hidden. - Validation: button disabled until query length >= 2, "Type at least 2 characters" hint until threshold crosses. - Loading state on search button while the request is in flight. - Manual results render in a separate table above the existing auto-candidates table, using the same row template (file / quality / size / duration / user / ⬇ button) so the renderer helper is shared. - Click ⬇ reuses the existing `downloadCandidate(taskId, candidate, trackName)` flow — same retry path, same AcoustID verification when the file lands, no shortcut around the safety net. - Re-running the search with a different query replaces the previous manual results. Backend (web_server.py) - Extended `GET /api/downloads/task/<id>/candidates` response with: - `download_mode` (e.g. 'hybrid', 'soulseek') - `available_sources` (list of configured source IDs + labels) - `source` field on each candidate (purely additive — frontend auto-renderer ignores it on legacy code paths, manual-search renderer uses it for the badge) - Added `POST /api/downloads/task/<id>/manual-search`: - Body: `{ query, source: 'all' | <source_id> }` - Validates query length (>=2 trimmed) → 400 - Validates source against the configured-sources gate → 400 (rejects unconfigured sources even when explicitly named) - For 'all': parallel `ThreadPoolExecutor` dispatch across every configured download source, merged results - For specific source: just that source - Returns same shape as `/candidates` so the frontend renderer is reused - New module-level helpers: `_STREAMING_SOURCE_NAMES`, `_infer_candidate_source`, `_serialize_candidate`, `_list_available_download_sources`. The existing `/candidates` endpoint also goes through `_serialize_candidate` so the source badge is consistent across both flows. Behavior preserved - Existing modal layout / candidates table / ⬇ button are byte-identical when the user doesn't use manual search. - `downloadCandidate()` JS function untouched. - `/candidates` and `/download-candidate` endpoints backwards-compatible — only NEW fields added, nothing changed or removed. Tests `tests/test_manual_search_endpoint.py` — 10 tests: - `test_manual_search_validates_query_length` - `test_manual_search_validates_source` (whitelist gate) - `test_manual_search_handles_task_not_found` (404) - `test_manual_search_dispatches_to_configured_source_only` - `test_manual_search_all_dispatches_parallel` - `test_manual_search_skips_unconfigured_sources` - `test_manual_search_rejects_unconfigured_source_explicitly` - `test_manual_search_returns_same_shape_as_candidates` - `test_manual_search_single_source_mode_lists_source` (verifies `available_sources` reflects the active mode) - `test_manual_search_isolates_per_source_exceptions` (one source throwing doesn't kill the merged result) 2242/2242 full suite green (was 2232 + 10 new). Ruff clean. JS parses clean.	2 weeks ago
Broque Thomas	d556ec0fa7	Bump version to 2.4.3 + make sidebar version dynamic ... - `_SOULSYNC_BASE_VERSION` 2.4.2 → 2.4.3 - helper.js — flip 2.4.3 WHATS_NEW header to "May 8, 2026 — 2.4.3 release"; bump fallback default from 2.4.2 → 2.4.3 - docker-publish.yml — manual-trigger default tag 2.4.2 → 2.4.3 Drive-by — make sidebar version + version-modal subtitle dynamic. The sidebar version button (`v2.4.1`) and version-modal subtitle (`Version 2.4.1 — Latest Changes`) were hardcoded text in the HTML. 2.4.2 shipped without these getting bumped — silent drift, easy to miss at every release. Added a Flask context_processor that injects `soulsync_version` and `soulsync_base_version` into every template, then templated the two hardcoded values: v{{ soulsync_base_version }} Version {{ soulsync_base_version }} — Latest Changes Now bumping `_SOULSYNC_BASE_VERSION` updates the UI everywhere it's rendered. No more "I forgot to bump the sidebar" at release. 2232/2232 full suite green. Ruff clean. JS parses clean.	2 weeks ago
Broque Thomas	959562f6b0	Delete Recently Added / Top Tracks / Forgotten Favorites / Familiar Favorites ... Owner decision: not worth shipping. The four library-driven personalized sections were stubbed returning [] for ages because their schema prereqs didn't exist; the prior commit re-enabled them by routing through a new `_select_library_tracks` helper. Owner reviewed and chose to delete the sections entirely instead. Removed everywhere: - `core/personalized_playlists.py` — `get_recently_added`, `get_top_tracks`, `get_forgotten_favorites`, `get_familiar_favorites` + the `_select_library_tracks` helper (no other callers; verified via grep). - `web_server.py` — 4 route handlers (`/api/discover/personalized/recently-added`, `top-tracks`, `forgotten-favorites`, `familiar-favorites`). - `webui/index.html` — 4 `<div class="discover-section">` blocks (`#personalized-recently-added`, `#personalized-top-tracks`, `#personalized-forgotten-favorites`, `#personalized-familiar-favorites`). - `webui/static/discover.js` — 4 load functions (`loadPersonalizedRecentlyAdded`, `loadPersonalizedTopTracks`, `loadPersonalizedForgottenFavorites`, `loadFamiliarFavorites`), plus their entries in `loadDiscoverPage`'s Promise.all, plus 4 module-level state vars + 6 dead branches across `openDownloadModalForDiscoverPlaylist` / `startDiscoverPlaylistSync` and the sync-progress / rehydrate dispatchers. - `webui/static/helper.js` — 4 tooltip / docs entries. - `webui/static/sync-spotify.js` — 1 stale rehydrate dispatcher branch (`discover_familiar_favorites`) caught during the global grep pass. - `tests/test_personalized_playlists_id_gate.py` — 3 library-method tests + the test infrastructure that supported them (`tracks` schema, `insert_library_track` helper). Documentation header updated to reflect the deletion. Net: -527 / +2 lines across 7 files. What stays: - Daily Mixes (also in personalized package, intentionally paused — separate decision). - Popular Picks + Hidden Gems + Discovery Shuffle (alive, not affected by this deletion). - All 14 tests in the personalized-playlists test file still pass. - The PersonalizedPlaylistsService lift from the prior commit (`_select_discovery_tracks` etc) — those are still in active use by the surviving discovery_pool methods. DISCOVER_TRACK_SELECTION_REVIEW.md at repo root contains historical references to the four deleted endpoints. Treated as historical context (same policy as WHATS_NEW), left alone. 2219/2219 full suite green (was 2222 - 3 deleted tests = 2219). JS parses clean, ruff clean.	2 weeks ago
Broque Thomas	6aafcaae93	Bump version to 2.4.2 ... - `web_server.py` — `_SOULSYNC_BASE_VERSION` 2.4.1 → 2.4.2 - `webui/static/helper.js` — flip the 2.4.2 WHATS_NEW header from "Unreleased — 2.4.2 dev cycle" to "May 7, 2026 — 2.4.2 release" so the per-version block stops being filtered out by `_getLatestWhatsNewVersion`. Also bumps the safety-net default inside that helper from 2.4.1 → 2.4.2. - `.github/workflows/docker-publish.yml` — manual-trigger default tag bumped to match. Drive-by fix: escaped a stray single quote in the `Internal: Download Engine` 2.4.2 entry that broke `node --check` on the file (`orchestrator.client('soulseek')` inside a single-quoted desc string silently terminated the string mid-entry). Pre-existing, unrelated to the bump but caught while validating JS parse for the release. VERSION_MODAL_SECTIONS not rotated in this commit — separate editorial pass.	3 weeks ago
Broque Thomas	1a2da016e4	Add download buttons + bulk action to artist top-tracks sidebar ... Closes #513 (s66jones). The artist detail page already showed a "Popular on Last.fm" sidebar — list of an artist's top tracks by playcount, with a play button per row but no download action. Issue #513 wanted a way to grab those tracks the same way zotify let users grab "top X songs" without pulling the full discography. Pulls from the configured primary metadata source (Spotify `artist_top_tracks`, Deezer `/artist/{id}/top`) when available, falls back to the existing Last.fm display-only mode for sources that don't expose popularity ranking (iTunes / Discogs / MusicBrainz). Source label in the section title shifts to match. Each row gets a hover-revealed download button that wishlists the single track via the existing /api/add-album-to-wishlist endpoint (preserves the track's real album metadata, so the wishlist worker later places the file in its proper album folder). A "Download All" footer button opens the standard download modal in PLAYLIST context, not album context — the virtual playlist_id is `top_tracks_<source>_<artistId>` which doesn't match any of the album-prefix checks in `startMissingTracksProcess` (downloads.js). That keeps `is_album_download=false`, so the master worker doesn't inject a wrapper context as `_explicit_album_context`. Each track downloads using its own real album metadata, files land in proper per-album folders on disk (not a fake "Top Tracks" folder). Backend additions: - `SpotifyClient.get_artist_top_tracks(artist_id, country, limit)` — wraps `spotipy.artist_top_tracks`, returns up to 10 tracks for the market (Spotify's API cap). UI-side limit trim only. - `DeezerClient.get_artist_top_tracks(artist_id, limit)` — wraps `/artist/{id}/top?limit=N`, converts Deezer's raw shape to the same Spotify-compatible dict layout (id, name, artists, album with album_type / total_tracks / images, duration_ms, track_number, disc_number) so downstream code doesn't branch on source. - `GET /api/artist/<id>/top-tracks` — dispatches to whichever client matches the primary source. Resolves per-source artist IDs from the DB row first (matching what /discography already does) so a Spotify ID in the URL still works when Deezer is primary, and vice versa. Returns `{success, source, tracks, resolved_artist_id}` on hit; `{success: False, reason: 'unsupported_source' | 'spotify_not_authenticated' | 'deezer_unavailable' | 'no_tracks_found'}` on miss so the frontend can decide whether to fall through to Last.fm. Frontend: - `_loadArtistTopTracks` tries the metadata source first, falls through to the legacy `/api/artist/0/lastfm-top-tracks` call if the source can't deliver. Section title and per-row UI shift based on which source answered. - New per-row `.hero-top-track-download` button (hover-revealed). - New `.hero-top-tracks-download-all` footer button — only visible when metadata-source mode rendered the list (Last.fm fallback hides it since rows have no track IDs to download). Tests: 10 new tests pin the client methods — - Spotify: returns track list, honors UI limit cap, returns empty when unauthed / artist_id missing / API throws. - Deezer: shape conversion to Spotify-compatible dict, empty when no data / artist_id missing, limit clamping at upper bound, default fallback when limit=0, malformed entries skipped. The Flask endpoint dispatcher itself isn't covered by the new test file because importing web_server at test-collection time spins up worker threads that race with caplog-using tests elsewhere in the suite (specifically test_library_reorganize_orchestrator). Endpoint verified manually; the underlying client methods (the load-bearing logic) are covered. 2204/2204 full suite green (was 2194 + 10 new).	3 weeks ago
Broque Thomas	9602d1827c	Final silent-exception sweep + ruff S110 lint guardrail — ~45 sites ... Catches the silent excepts the awk-based earlier sweeps missed: - Bare `except:` followed by `pass` (also swallows KeyboardInterrupt and SystemExit — actively wrong). Upgraded to `except Exception as e: logger.debug("...: %s", e)`. ~14 sites across connection_detect, soulseek_client, listenbrainz_manager, watchlist_scanner, youtube_client, navidrome_client, jellyfin_client, web_server. - `except Exception:` + pass that the awk pattern missed (e.g. multi-line or unusual whitespace). ~31 sites across automation_engine, database_update_worker, music_database, spotify_client, web_server, others. - 14 legitimate cleanup sites left silent with explicit `# noqa: S110` + comment explaining why (atexit handlers, finally-block conn.close calls). Logging during shutdown can itself crash because file handles get torn down before the handler fires. Also enables `S110` rule in pyproject.toml so this pattern fails CI going forward — drift fails at PR review instead of at runtime against a wedged worker thread. Tests path keeps S110 ignored (test fixtures legitimately use try-except-pass for cleanup). Adds a WHATS_NEW entry to helper.js summarizing the full #369 sweep. Verified: `python -m ruff check .` → All checks passed. Verified: `python -m pytest tests/` → 2188 passed. Closes #369	3 weeks ago
Broque Thomas	b0c58a0f91	Surface silent exceptions in web_server.py — 81 sites ... Replaces `except Exception: pass` blocks with `except Exception as e: logger.debug(...)` so failures are inspectable in the log instead of disappearing silently. Per JohnBaumb's request in #369. - Pattern is consistent: `logger.debug("<context>: %s", e)` with lazy formatter and 2-6 word context describing the operation. - 2 atexit handlers (lines 2977, 2983) intentionally left silent — the log file handles can be torn down before atexit fires, and a separate `_atexit_silence_shutdown_logger_errors` already exists for this exact reason. - No behavior changes; control flow is unchanged. Test suite green (2188 passed). Refs #369	3 weeks ago
Broque Thomas	620c41f1ac	Add "All Libraries (combined)" mode to PlexClient ... GitHub issue #505 (PopeBruhLXIX): users with multiple Plex music libraries (e.g. one per Plex Home user, or two folder roots split across separate library sections) only saw one library inside SoulSync because the connection settings forced you to pick a single library section. SoulSync's PlexClient stored exactly one ``self.music_library`` section reference and every read scanned only that one. This change adds an opt-in "All Libraries (combined)" dropdown option that flips the client into a server-wide read mode where every read method (``get_all_artists`` / ``get_all_album_ids`` / ``search_tracks`` / ``get_library_stats`` / etc) dispatches through ``server.library.search(libtype=...)`` instead of querying a single section. One Plex API call replaces N per-section iterations; Plex handles the aggregation server-side. Implementation: - ``ALL_LIBRARIES_SENTINEL`` (``'__all_libraries__'``) — module-level constant used as the saved DB preference value when the user picks the synthetic "All Libraries" entry. Detection is one string compare in ``_find_music_library`` / ``set_music_library_by_name``. Existing preferences (real library names) are unaffected. - ``self._all_libraries_mode`` (private flag) + ``is_all_libraries_mode()`` (public accessor for external callers). When True, ``music_library`` may stay None — ``is_fully_configured()`` recognizes the mode and still returns True so dispatch sites don't bail. - New private helpers ``_can_query``, ``_get_music_sections``, ``_all_artists``, ``_all_albums``, ``_all_tracks``, ``_search_general``, ``_search_artists_by_name``. Single dispatch point for the section-vs-server branch — every read method funnels through them so future drift fails at one place. - New public helpers for downstream callers: - ``get_recently_added_albums(maxresults, libtype)`` — used by DatabaseUpdateWorker's deep-scan recent-content sweep - ``get_recently_updated_albums(limit)`` — same - ``get_music_library_locations()`` — returns folder roots, used by web_server.py's file-path resolver - ``trigger_library_scan`` and ``is_library_scanning`` fan out across every music section in all-libraries mode. - ``get_available_music_libraries`` prepends a synthetic ``{'title': 'All Libraries (combined)', 'value': sentinel}`` entry ONLY when more than one music library exists. Single-library users don't get the extra option. ``value`` field is the canonical identifier the frontend submits to ``/api/plex/select-music-library`` (real libraries: title; synthetic: sentinel string). Backward- compatible — entries without ``value`` fall back to ``title``. Three crash points fixed in downstream consumers (would have failed during a deep scan after the user picked all-libraries mode): 1. ``database_update_worker.py:411`` — bailed out with "No music library found in Plex" because ``not self.media_client.music_library`` evaluated True in all-libraries mode (music_library is None there). Now uses ``is_fully_configured()`` which recognizes the mode. This was the root cause of the deep scan never starting. 2. ``database_update_worker.py:_get_recent_albums_plex`` — reached ``self.media_client.music_library.recentlyAdded()`` / ``.search()`` directly, AttributeError in all-libraries mode. Now routes through the new helper methods. 3. ``web_server.py:10947`` (file-path resolver) — accessed ``music_library.locations``; gated on ``music_library`` truthy so it didn't crash, but silently skipped all-libraries-mode locations. Now uses ``get_music_library_locations()`` which unions across sections. Plus polish: - ``/api/plex/clear-library`` also resets ``_all_libraries_mode`` so a fresh "select library" flow doesn't inherit stale mode state. - ``/api/plex/music-libraries`` surfaces "All Libraries (combined)" as ``current_library`` when in mode (settings UI displays correctly). - Frontend ``loadPlexMusicLibraries`` uses ``library.value || library.title`` so the sentinel-keyed option submits the sentinel string, not the human-readable label. Pre-select match handles both paths. Honest tradeoffs (documented as known limitations): - Same artist appearing in multiple Plex sections shows as separate entries in SoulSync (no dedup). Plex returns distinct ratingKeys for each. Cosmetic; revisit if it bites users. - Write-back (genre / poster updates) targets one ratingKey at a time — only updates that section's copy. Other sections' copies stay unchanged. - All-libraries mode includes any audiobook library that Plex classifies as ``type='artist'``. Edge case, opt-in only. Tests: 21 new tests in tests/media_server/test_plex_all_libraries.py pin both single-library mode (regression guard) and all-libraries mode for every refactored method. Existing test_plex_pinning.py fixture updated to initialize the new flag. 63/63 media_server tests green, 2148/2148 full suite green.	3 weeks ago
Broque Thomas	822759740d	Fix Download Discography pulling wrong artist + log routing ... Two fixes. (1) Discography endpoint now does server-side per-source ID resolution. When the user clicked Download Discography on a library artist, the endpoint received whichever artist ID the frontend happened to pick (spotify_artist_id || itunes_artist_id || deezer_id || library_db_id) and dispatched it as-is to whichever source it queried. If the picked ID didn't match the queried source's ID format, the lookup returned wrong-artist results (numeric ID collisions) or fell back to a fuzzy name search that picked a wrong artist. Two reproducible cases: - 50 Cent's library row had DB id 194687 — coincidentally a real Deezer artist ID for "Young Hot Rod". When the frontend's /enhanced fetch silently fell back to the DB id, the backend sent 194687 to Deezer, and Deezer returned Young Hot Rod's 50 albums in 50 Cent's discography modal. - Weird Al's library row had a stored Spotify ID. The frontend sent that to Deezer, which rejected the alphanumeric ID and fell back to fuzzy name search — which picked The Beatles somehow, returning 45 Beatles albums. The mechanism for per-source ID dispatch already exists in ``MetadataLookupOptions.artist_source_ids``, and the watchlist scanner already uses it; the on-demand discography endpoint just wasn't wired to it. Fix: when the URL artist_id matches a library row by ANY stored ID (DB id, spotify_artist_id, itunes_artist_id, deezer_id, or musicbrainz_id), pull every stored provider ID and pass them as ``artist_source_ids``. Each source gets its OWN stored ID regardless of which one the URL carries. When the URL ID is a non-library source-native ID and the row lookup misses entirely, behavior is identical to before (single-ID dispatch fallback). Logged the resolved per-source ID dict at INFO so future "wrong artist showed up" diagnostics are immediately legible in app.log. (2) Logger namespace fix in core/artists/quality.py and core/metadata/multi_source_search.py. Both modules used ``logging.getLogger(__name__)`` which resolves to ``core.artists.quality`` / ``core.metadata.multi_source_search`` — neither under the ``soulsync`` namespace where the file handler is wired. Result: every [Enhance], [MultiSourceSearch], and direct-lookup INFO line was being written to a logger with no handlers and silently dropped. App log showed the slow-request warning but no diagnostic detail. Switched both to ``get_logger()`` from utils.logging_config so the soulsync.* namespace picks them up. Same content, now actually lands in app.log. Confirmed working in live test: ``[Enhance] Direct lookup matched: deezer ID 1476162252 → 'Desastre'`` No behavior change in any other caller. Empty ``artist_source_ids`` (no library row matched) reaches lookup as ``None`` → identical to current single-ID dispatch path. Logger fix is pure routing — no content change.	3 weeks ago
Broque Thomas	3befe9349c	Direct ID lookup in Enhance Quality, like Download Discography ... Followup on the previous Enhance refactor. Multi-source parallel text search closed the worst case (users with no Spotify/Deezer getting "unknown artist - unknown album - unknown track" wishlist entries), but text search itself is still fragile against messy library tags: "Title (Live)", featured artists in the artist field, etc. Download Discography never had this problem because it resolves albums by stable ID, not by name. Enhance now does the same thing for tracks: for every metadata source the user has configured, if the library track has the corresponding stored ID (spotify_track_id / deezer_id / itunes_track_id / soul_id), call client.get_track_details(stored_id) directly and convert to the wishlist payload. First success wins. The user's configured primary source is tried first so a Deezer-primary user gets Deezer payloads on the wishlist entry (correct cover art / album shape) even when other sources also have stored IDs for the same track. Multi-source parallel text search stays as the fallback for tracks with no stored IDs (e.g. manually imported, never enriched). Empty- field rejection still gates the wishlist add. Implementation: - _STORED_ID_COLUMNS: source name → DB column mapping (Discogs intentionally omitted — release-based, no per-track IDs) - _enhanced_to_wishlist_payload: converts the get_track_details intermediate "enhanced" shape (artists as [str]) to wishlist shape (artists as [{'name': str}]). Spotify's raw_data is already in wishlist shape, returned as-is when detected (preserves full album.images that the enhanced top-level fields drop) - _try_direct_lookup_all_sources: iterates sources preferred-first, calls get_track_details on each that has both a stored ID and a configured client, returns first complete-metadata payload - spotify_client field removed from ArtistQualityDeps (no longer used — Spotify direct lookup now flows through the generic per-source loop using the entry from search_sources) - _try_upgrade_to_rich_payload removed (was Spotify-only with broken shape semantics for non-Spotify sources; search-fallback now uses _build_payload_from_track consistently) - get_primary_source() consulted to set the per-call preferred source for direct-lookup priority Also fixed a stale UI string: the Enhance modal toast read "Matching tracks to Spotify and adding to wishlist..." regardless of which sources were actually configured. Now reads "Matching tracks across metadata sources...". Tests: - _build_deps mirrors web_server._resolve_search_sources: passing spotify=spotify_obj auto-prepends ('spotify', spotify_obj) to search_sources (Spotify is always added when configured in prod) - 5 new tests pin the direct-lookup behavior: - test_direct_lookup_via_deezer_id_skips_text_search - test_direct_lookup_via_itunes_id_skips_text_search - test_direct_lookup_prefers_user_primary_source - test_direct_lookup_falls_through_to_text_search_when_no_stored_ids - test_direct_lookup_failure_falls_through_to_text_search - Reframed enhanced-format and search-fallback tests for the new payload-build path (no album-image side call, search-fallback uses _build_payload_from_track consistently) - 22/22 quality tests green, 2133/2133 full suite green.	3 weeks ago
Broque Thomas	7316646b01	Extract multi-source search; Enhance Quality matches Redownload coverage ... Track Redownload had been doing parallel multi-source metadata search across every configured source the whole time; Enhance Quality was running a single-source primary fallback that returned junk matches with empty fields when the primary was iTunes (Discord report: "unknown artist - unknown album - unknown track" wishlist entries for users with neither Spotify nor Deezer connected). Lift the redownload search into core/metadata/multi_source_search.py and point both flows at it. Same scoring, same per-source query optimization (Deezer's structured artist:/track: form), same current-match flagging via stored source IDs. ArtistQualityDeps now takes get_metadata_search_sources (returns [(name, client), ...] for every configured source) instead of the single-primary get_metadata_fallback_client + get_metadata_fallback_source. Spotify direct-lookup stays as a fast-path optimization (only Spotify exposes get_track_details(id) returning rich raw payload); when it doesn't fire, the multi-source parallel search picks the cross-source best match. Empty-field matches still rejected before wishlist add. Tests: _build_deps helper updated to accept the new search_sources contract while preserving fallback_client/fallback_source ergonomics. Reframed tests for the new semantics — direct-lookup is no longer gated on Spotify being the active primary; failure reason now lists every searched source. Added a test pinning the no-sources-configured prompt. 17/17 quality tests green, 2128/2128 full suite green.	3 weeks ago
Broque Thomas	4a27f3c245	Source-agnostic Enhance Quality flow + reject empty matches ... Discord report: clicking Enhance Quality on an artist with neither Spotify nor Deezer connected added tracks to the wishlist as "unknown artist - unknown album - unknown track". Root cause was structural. core/artists/quality.py had a hardcoded Spotify-direct → Spotify-search → iTunes-fallback chain that ignored the user's configured primary metadata source. When Spotify wasn't connected, every track fell through to an iTunes-only fallback that occasionally returned matches with empty fields (cleared the 0.7 confidence threshold but missing artist / album / title). Those empty strings propagated through the wishlist payload normalizer's truthy-check passthrough at core/wishlist/payloads.py:77-80 and the UI rendered them as "Unknown" defaults. Rewrote the flow source-agnostic: - ArtistQualityDeps gains get_metadata_fallback_source. Flow resolves the user's active primary source once up front. - New _build_payload_from_track helper produces the Spotify-shaped wishlist payload from any source's Track object — single place that knows how to construct it (replaces the duplicate construction in the Spotify-search and iTunes-fallback paths). - New _search_match helper does generic confidence-scored search against any client implementing search_tracks(query, limit). Same 0.7 threshold, same album-bonus weighting as before. - New _has_complete_metadata validator rejects matches with empty title / album / artists before they reach the wishlist. - _spotify_direct_lookup kept as a Spotify-only optimization (only Spotify exposes get_track_details(id) returning rich raw payload); other sources fall through to search. - Failure reason now names the active source: "No usable {source} match — connect another metadata source for better coverage". Result: Discogs users get a Discogs search. Hydrabase users get a Hydrabase search. iTunes users get an iTunes search with empty-field rejection. Spotify keeps its direct-lookup fast path. 6 new tests pin the architectural change: - Primary-source dispatch routes to the configured client (Discogs, not Spotify) when Spotify isn't primary - Spotify direct-lookup is gated on Spotify being the active primary (skipped when Discogs is configured even if track has spotify_track_id) - Empty title / album / artists fields all reject the match - Failure reason names the active source	3 weeks ago
Broque Thomas	e27ecb84f4	Final review-pass nits — class docstring, dead branch, dead imports, boot resilience ... Going line-by-line through the engine package + boot wiring. Five small things worth fixing before Cin reads it: (1) MediaServerEngine class docstring still claimed to be a "single entry point for cross-server library operations" — but the prior honesty pass cut all the cross-server dispatch wrappers because they had no callers. Class is really lookup + small accessors now. Docstring rewritten to match. (2) configured_clients() had a dead `not hasattr(client, 'is_connected')` branch. is_connected is in REQUIRED_METHODS so every client the registry yields here implements it. Branch removed; comment notes the reasoning. (3) types.py imported `datetime` and `Dict` but used neither — dead imports dropped. (4) types.py docstring claimed "all four servers" defined an XTrackInfo dataclass. Actually only Plex / Jellyfin / Navidrome did; SoulSync uses richer per-track wrappers. Fixed. (5) web_server.py boot: - media_server_engine added to the chained `= None` declaration so it's always defined before the try/except, defending against the rare path where engine init AND fallback both raise. - Outer engine init failure logger now uses exc_info=True for full traceback (boot-time issues are rare but worth diagnosing). - Nested fallback failure now logs explicitly instead of silently leaving media_server_engine as None. Tests: 2121 still pass.	3 weeks ago
Broque Thomas	860f9a0a8c	MS pre-review polish — encapsulation + visibility + tests ... Five tightening passes anticipating Cin / JohnBaumb's review nits: (1) Engine no longer reaches into ``registry._instances`` private attr. New public ``MediaServerRegistry.set_instance(name, client)`` method — engine constructor calls it for the ``clients=`` pre-built case so internal storage stays encapsulated. (2) Engine module docstring no longer overclaims. Originally said it "Replaces the historic 33+ if/elif chains" — but only the four uniform-shape ``is_connected`` chains were collapsed. The 19 chains that do server-specific work (Plex raw API vs Jellyfin / Navidrome client methods returning different shapes) stay explicit per the "lift what's truly shared" standard. Docstring rewritten to say exactly that. (3) Per-method exception swallows upgraded from ``logger.debug`` to ``logger.warning``. Returning safe defaults stays the right behavior for a read-side engine (Plex offline shouldn't crash the app), but silent debug-level swallowing made debugging hard — JohnBaumb pushed the download engine to surface real errors. Same treatment here: default still safe, but the warning tells you Plex is down. (4) ``_safe_init_media_client`` in web_server.py now logs the exception type + traceback. Broad ``except Exception`` is still intentional (any failure means that one server can't be used; the others stay up) but the boot log now distinguishes config errors (ConnectionError, AuthenticationError) from import / dependency failures. (5) Two new tests pin the encapsulation + fallback contracts: - ``test_engine_with_empty_clients_dict_is_safe_to_use`` — empty engine returns safe defaults on every method, doesn't raise. - ``test_engine_uses_registry_set_instance_not_private_attr`` — spy on registry.set_instance verifies engine uses the public method.	3 weeks ago
Broque Thomas	f230c93890	Merge remote-tracking branch 'origin/dev' into refactor/media-server-engine ... # Conflicts: # core/matching_engine.py # services/sync_service.py # web_server.py # webui/static/helper.js	3 weeks ago
Broque Thomas	397a1c73df	ID-first fallback for replace-track + remove-track too ... Same latent bug as add-track — replace-track and remove-track only looked up the Plex playlist by name. Plex deletes + recreates playlists on edit so the rating key the frontend cached can be stale, name lookups can also fail (special chars, encoding). Both now use the same id-first / name-fallback chain as the GET tracks endpoint, with a diagnostic log when both lookups fail. Pre-existing latent bug, not a refactor regression.	3 weeks ago
Broque Thomas	218af65606	ID-first fallback for server-playlist add-track + diagnostic logging ... The /api/server/playlist/<id>/add-track endpoint only looked up the target Plex playlist by name, but Plex deletes + recreates playlists on edit so the rating key the frontend cached can be stale. The companion GET /tracks endpoint already had id-first / name-fallback; add-track now does the same. Added a warning log on GET /tracks when BOTH lookups fail so the "all source items show Find & Add" symptom (which happens when server_tracks comes back empty) has a clear diagnostic in the log instead of silently rendering an empty server column. Not a refactor regression — the original code had the same name-only lookup. The mass-replace of `plex_client` → `media_server_engine.client('plex')` is byte-equivalent. Just surfacing the latent bug.	3 weeks ago
Broque Thomas	03d1c36637	Fallback to empty MediaServerEngine if init fails ... If MediaServerEngine init raised, ``media_server_engine`` was set to None. Every downstream caller (now that the per-server globals are gone) does ``media_server_engine.client('plex')`` style access — which would AttributeError on the None. Pre-refactor each per-server global had its own try/except so engine failure didn't take down per-server dispatch sites. Preserve that resilience by falling back to an empty MediaServerEngine — its ``client(name)`` returns None, downstream truthy checks treat that as "not configured" exactly the same way the legacy globals did.	3 weeks ago
Broque Thomas	a6bb5f5b43	MS Cin-5: Drop per-server globals — engine owns the clients ... Per-server web_server.py globals (plex_client / jellyfin_client / navidrome_client / soulsync_library_client) are gone. The engine now owns the per-server client instances; web_server.py constructs them inline into the engine init and routes everything through media_server_engine.client('<name>'). Multi-client consumers refactored to take the engine instead of separate per-server kwargs: - services/sync_service.py: PlaylistSyncService.__init__ now takes media_server_engine. Internal _get_active_media_client resolves the active server's client through self._engine.client(name) instead of the per-server self.X_client attributes. - core/listening_stats_worker.py: ListeningStatsWorker takes media_server_engine. The plex/jellyfin/navidrome dispatch in _poll collapses to engine.client(active_server) (gated to those three servers — SoulSync standalone has no listening data). - core/web_scan_manager.py: WebScanManager takes media_server_engine instead of the hand-keyed media_clients dict that drifted out of sync with the engine. - core/discovery/sync.py: SyncDeps holds media_server_engine instead of plex_client / jellyfin_client. Playlist-image dispatch routes through engine.client(name). Web_server.py: - Per-server globals removed from the chained `= None` init line + their try/except construction blocks. Replaced with a _safe_init_media_client(factory, name) helper that captures per-server init failures + passes the resulting clients straight into the MediaServerEngine init dict. - All construction sites (PlaylistSyncService, WebScanManager, ListeningStatsWorker, SyncDeps, library_check) updated to receive the engine instead of per-server clients. Test fixtures (tests/discovery/test_discovery_sync.py) gain a _FakeMediaServerEngine stub + the SyncDeps build helper passes that instead of separate plex/jellyfin clients.	3 weeks ago
Broque Thomas	1bc5017592	MS Cin-3 + Cin-4: Route web_server through engine instead of per-client globals ... Pre-change web_server.py had ~70 direct attribute reaches against the per-server globals (plex_client.X, jellyfin_client.X, navidrome_client.X, soulsync_library_client.X) plus ~60 standalone refs (truthy checks, media_client assignments, source-name tuples). The engine was wired but only used in 4 places, so most of the codebase still hand-dispatched — the exact "partially defeats the purpose of this refactor" critique Cin landed on the download PR initially. - All ~70 client.attribute reaches migrated to media_server_engine.client('<name>').attribute. The chains in web_server.py do server-specific work (Plex raw API, Jellyfin / Navidrome client methods, all returning different shapes), so the if/elif structure stays — but the per-server CLIENT REACH now goes through the engine like Cin's POC pattern intended. - All ~60 standalone refs migrated: - if plex_client → if media_server_engine.client('plex') - media_client = plex_client → media_client = media_server_engine.client('plex') - ('plex', plex_client) tuples → ('plex', media_server_engine.client('plex')) - Per-server globals (plex_client / jellyfin_client / navidrome_client / soulsync_library_client) kept for now — external modules (PlaylistSyncService, WebScanManager, ListeningStatsWorker, search library check, discovery sync deps) still take them as kwargs. Dropping them entirely needs a follow-up sweep across those modules. Suite green (1961 pass).	3 weeks ago