13 Commits (dev)

Author	SHA1	Message	Date
Broque Thomas	96e6ba0ed7	Preserve Navidrome album cover art ... Expose Navidrome album coverArt as a Subsonic getCoverArt thumbnail so library refreshes keep a real album-art URL. Preserve existing album thumb_url when an incoming server album has no thumbnail, preventing manual or server-corrected covers from being cleared and later replaced by loose missing-cover searches. Add regression tests for Navidrome album thumbnails and DB thumb preservation.	1 day ago
Broque Thomas	c02d51d60d	Plex: trigger_library_scan + is_library_scanning use auto-detected section — fixes #535 ... # Bug Plex servers with the music library named anything other than "Music" (Música, Musique, Musik, Musica, 音乐, موسيقى, etc.) hit this error after every import cycle: soulsync.plex_client - ERROR - Failed to trigger library scan for 'Music': Invalid library section: Music soulsync.web_scan_manager - ERROR - Failed to initiate PLEX library scan via web Side effect: `wishlist.processing` kept reporting "Missing from media server after sync" for tracks that DID import correctly, so they got perpetually re-added to the wishlist. # Root cause `_find_music_library` correctly auto-detects the music section by `section.type == 'artist'` and stores it on `self.music_library` — works for any locale because the type is language-neutral. Read methods (`get_artists`, etc.) route through `_get_music_sections` which returns `[self.music_library]`, so they never had the bug. But `trigger_library_scan` and `is_library_scanning` ignored `self.music_library` and called `self.server.library.section(library_name)` directly with the hardcoded `"Music"` default. `server.library.section('Music')` raises `NotFound` on any server whose section isn't literally named "Music". # Fix Both methods now prefer `self.music_library` first, fall back to literal `library_name` lookup only when auto-detection hasn't populated the cached reference (test fixtures, edge cases). `is_library_scanning`'s activity-feed match also corrected to filter by the resolved section's actual title — the prior code matched `library_name.lower() in activity_title.lower()` which defaults to "music" and would never match activities for non-English sections. `trigger_library_scan`'s success log line now surfaces the actual section title (`Música`) instead of the unused `library_name` default ("Music") — confusing when debugging on non-English servers. # Tests added (13) `tests/media_server/test_plex_non_english_section_name.py`: - `test_uses_auto_detected_section_regardless_of_locale` — parametrised across 6 locale variants (Música, Musique, Musik, Musica, 音乐, موسيقى). Each verifies trigger_library_scan calls the auto-detected section's `update()`, NOT a literal-name fallback. Stub raises AssertionError on `server.library.section()` so a regression that re-introduces the fallback fails loudly. - `test_falls_back_to_literal_lookup_when_no_auto_detection` — backward compat: music_library=None → literal lookup as before. - `test_explicit_library_name_arg_used_only_when_no_auto_detection` — auto-detected wins over explicit kwarg when both available. - `test_logs_correct_section_label_on_success` — log line surfaces resolved section title. - 4 symmetric tests for is_library_scanning covering refreshing-attr check, activity-feed title match, no-match for unrelated sections, fallback path. # Verification - 13 new tests pass - 84/84 media_server tests pass (no regression in the existing Plex / Jellyfin / Navidrome suite) - 2458 full suite passes (+13 from baseline) - Ruff clean	2 weeks ago
Broque Thomas	9f2813fce4	Add cross-section dedup to all-libraries listing layer ... Followup to the all-libraries-mode commit. Without dedup, a Plex Home family where two users both have "Drake" in their music libraries would see "Drake" twice in SoulSync's library list — Plex returns distinct ratingKeys for each section's copy of the same artist. Dedup design — applied selectively, NOT everywhere: - ``_dedupe_artists(artists)``: groups by lowercased title, picks the canonical entry by ``leafCount`` (more tracks wins). Active ONLY in all-libraries mode; single-library mode is a no-op fast path with zero behavior change. - ``_dedupe_albums(albums)``: same but keys on (lowercased parentTitle, lowercased title) so two artists with identically-titled albums (e.g. self-titled releases) stay separate. Applied to: - ``get_all_artists()`` — public listing for the library view - ``get_library_stats()`` — count matches what user sees in the list Deliberately NOT applied to: - ``get_all_artist_ids()`` / ``get_all_album_ids()`` — these feed removal detection (compare returned ratingKey set against DB-linked ratingKeys to decide what's been removed). Deduping here would falsely flag non-canonical ratingKeys as "removed" and prune SoulSync's DB tracks that are linked to them. Pinned by two CRITICAL tests. - ``_all_tracks()`` — track count stays raw because the same track in two sections IS two distinct files / Plex entries, not a logical duplicate. - ``_search_general()`` and ``search_tracks`` Stage 1/2 — search results stay raw so cross-section matches aren't lost. Stage 1 may miss cross-section tracks for the same artist but Stage 2's server-wide track search catches them. Logging: when raw vs deduped artist counts differ, ``get_all_artists`` logs both so users can see "Found 4697 artists across all music sections (4521 unique after cross-section dedup)" — surfaces the overlap clearly. Tests: 8 new tests in test_plex_all_libraries.py pin: - canonical pick by leafCount (artists + albums) - case-insensitive name match - single-library no-op path (zero behavior change for those users) - album dedup keys on (artist, title) so different-artist same-title albums stay separate - ``get_all_artists`` listing applies dedup - ``get_all_artist_ids`` does NOT dedup (CRITICAL — removal detection) - ``get_all_album_ids`` does NOT dedup (CRITICAL — removal detection) - ``get_library_stats`` uses deduped counts for artists/albums but raw count for tracks Existing pre-stat test updated to use distinct mock instances — ``[MagicMock()] * 5`` creates five references to one mock which now correctly collapses under dedup. 71/71 media_server tests green, 2162/2162 full suite green. Honest known limitation acknowledged in WHATS_NEW + version modal: write-back (genre / poster / metadata updates) targets one ratingKey at a time — only updates the canonical section's copy of an artist if it exists in multiple. Other section's copy stays unchanged. Document and revisit if it matters.	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	6489244bcc	MS Cin/JohnBaumb honesty pass — drop dead wrappers, sync contract to reality ... Pre-review audit found premature abstraction + lying docstrings. Cut what isn't used, made the rest match what's actually shipped. (1) Engine: dropped 7 cross-server dispatch wrappers that had ZERO production callers (ensure_connection / get_all_artists / get_all_album_ids / search_tracks / trigger_library_scan / is_library_scanning / get_library_stats / get_recently_added_albums). Every consumer reaches the active client directly via sync_service._get_active_media_client() or engine.client(name). Engine surface shrinks to client(name) / active_client() / active_server / is_connected() (the one wrapper that has callers — 4 dashboard status sites) / configured_clients() / reload_config(). ~150 lines deleted, 5 dead-method tests removed. (2) Contract Protocol body trimmed to match REQUIRED_METHODS exactly (is_connected, ensure_connection, get_all_artists, get_all_album_ids). The other 5 methods that were declared in the Protocol "required" section weren't actually required — Plex doesn't implement get_recently_added_albums, Jellyfin doesn't implement search_tracks, SoulSync doesn't implement most of them. Static contract now matches runtime conformance test. Optional methods moved to a KNOWN_PER_SERVER_METHODS data-only listing with audited per-server coverage notes — discoverability without false promises. (3) Engine module docstring + __init__.py docstring no longer overclaim "33+ chains collapsed" — only 4 uniform-shape chains were collapsed; ~18 server-specific chains stay explicit per the "lift what's truly shared" standard. Phrasing now matches reality. (4) types.py docstring claimed TrackInfo.from_jellyfin_dict and TrackInfo.from_navidrome_dict exist as classmethods. They don't — only from_plex_track / from_plex_playlist do. Jellyfin and Navidrome construct TrackInfo inline at their call sites today. Docstring now honest about that + flags the lift as a clean followup. (5) Engine line 95 comment "backward-compat for source-specific reaches" was misleading — there is no legacy alternative being preserved; engine.client(name) IS the canonical access pattern. Section header rewritten. Tests: 2121 pass (was 2126; -5 dead-method pin tests).	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	49f7679eef	MS Cin-1 + Cin-2: Explicit contract inheritance + generic accessors ... Apply the Cin-1 / Cin-2 pattern from the download refactor PR to the media server engine PR before review. Cin-1 — explicit inheritance: - PlexClient, JellyfinClient, NavidromeClient, SoulSyncClient now explicitly inherit MediaServerClient instead of relying on structural typing alone. Pre-change a reader of plex_client.py had no way to know the class was supposed to satisfy the contract. - Removed the engine + registry re-exports from core/media_server/__init__.py to break the circular import that the inheritance change introduced (importing the package now triggered a chain that loaded clients before their base class resolved). Submodules import directly: from core.media_server.engine import MediaServerEngine, etc. - Conformance test now also asserts isinstance() / issubclass() against MediaServerClient — drift in any class fails at the test boundary instead of at runtime. Cin-2 — generic accessors + singleton: - engine.configured_clients() — replaces the legacy per-server `if X and X.is_connected(): clients[name] = X` chains in web_server.py. - engine.reload_config(name=None) — generic dispatch, so callers pass the server name instead of reaching for plex_client.reload_config() directly. - get_media_server_engine() / set_media_server_engine() singleton factory matching the get_metadata_engine() / get_download_orchestrator() shape. web_server.py boots via set_media_server_engine(...) so factory + global handle share state. - 7 new tests pin the accessors + singleton behaviour.	3 weeks ago
Broque Thomas	6b54ca6598	Phase B: Add MediaServerEngine skeleton ... `MediaServerEngine` reads the active server from config + dispatches to the corresponding registered client. Per-server reaches still work through `engine.client(name)`. Required-method dispatch (is_connected, ensure_connection, get_all_artists, get_all_album_ids) returns safe defaults when the active client failed to initialize OR when the method raises. Optional-method dispatch (search_tracks, trigger_library_scan, is_library_scanning, get_library_stats, get_recently_added_albums) checks hasattr first — SoulSync standalone has no trigger_library_scan or get_library_stats, engine no-ops with appropriate defaults instead of forcing every client to declare stub methods. 10 new engine tests pin: active-server resolution, required dispatch routing, exception safety, missing-optional-method fallback shape. Suite still green (1951 passed). Engine isn't on any production code path yet — Phase C migrates the 33 web_server.py dispatch sites to call engine.method() instead of hand-branching by active_server name.	3 weeks ago
Broque Thomas	50fe4bec97	A4: Pin SoulSyncClient observable behavior ... 5 tests pin the SoulSync standalone client surface — the structurally-different one (no auth, no API, no library scan). is_connected just checks os.path.isdir(transfer_path). ensure_connection reloads config first so the user changing the transfer_path takes effect without a process restart. get_all_album_ids returns a set of MD5-hashed string ids matching cross-server uniform set semantics.	3 weeks ago
Broque Thomas	edcdaaa993	A3: Pin NavidromeClient observable behavior ... 4 tests pin the Navidrome client surface. Auth shape: base_url + username + password (no token model — salt generated per request). get_all_album_ids paginates getAlbumList2 and returns a set of string ids matching cross-server uniform set semantics.	3 weeks ago
Broque Thomas	5da2cfec46	A2: Pin JellyfinClient observable behavior ... 5 tests pin Jellyfin client surface. is_connected requires ALL four of base_url + api_key + user_id + music_library_id (stricter than Plex's is_connected). get_all_album_ids returns a set of string GUID ids matching the cross-server uniform set semantics.	3 weeks ago
Broque Thomas	c1da56b2c2	A1: Pin PlexClient observable behavior ... 6 tests pin the Plex client surface the engine will dispatch through after Phase B/C migrations: - is_connected returns False on no-server, True on server-present - is_fully_configured requires BOTH server AND music_library - get_all_artists empty list on not-connected, iterates music_library.searchArtists() when connected - get_all_album_ids returns a set of STRING ratingKey values (coerced from Plex ints so semantics match Jellyfin GUIDs + Navidrome string ids) Phase A pinning catches behavior drift during web_server.py dispatch-site migrations (Phase C) and engine adapter wiring (Phase B).	3 weeks ago
Broque Thomas	f702196dca	Phase 0: Add MediaServerClient contract + registry ... `core/media_server/` package with the Protocol contract that every media server client (Plex, Jellyfin, Navidrome, SoulSync standalone) satisfies, plus the registry that holds them. Required methods conservatively limited to the four every server truly implements today: is_connected, ensure_connection, get_all_artists, get_all_album_ids. Other generic methods (search_tracks, trigger_library_scan, get_recently_added_albums, etc.) are listed as OPTIONAL — present on most servers but not all (SoulSync has no library-scan API since it walks the filesystem directly; Jellyfin uses a different search shape). Phase B's engine adapters route around the gaps with per-server fallback instead of forcing every client to declare a no-op stub. Same registry shape as the download plugin registry — single source of truth for which servers exist + name resolution. Adding a 5th server (Subsonic, Emby, etc.) becomes one register call plus the new client class. 5 conformance tests pin every server class implements every required method. Plan doc at docs/media-server-engine-refactor-plan.md. Pure additive — no consumer routes through the contract or registry yet. Suite still green (1921 passed).	3 weeks ago