Three nits I missed in the prior pass:
(1) Two more exception swallows still at logger.debug — the
get_recently_added_albums wrapper and the configured_clients
inner loop. My earlier sed only matched single-line patterns.
Both now log at warning level so a broken Plex / Jellyfin
surfaces in the boot log instead of silently returning [].
(2) registry.py module docstring still claimed it replaced
"33 hand-maintained dispatch sites" — same overclaim I fixed
on the engine docstring. Server-specific chains stay explicit
in web_server.py per the "lift what's truly shared" standard;
the registry just owns name → client lookup. Docstring rewritten
to match reality.
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.
`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).
