Merge pull request #566 from Nezreka/fix/debug-info-music-source-and-missing-services

Fix Copy Debug Info music_source + surface missing services

core/debug_info.py

@@ -14,7 +14,12 @@ from pathlib import Path
 from flask import jsonify, request
 
 from config.settings import config_manager
-from core.metadata.registry import get_spotify_client
+from core.metadata.registry import (
+    get_spotify_client,
+    get_primary_source,
+    is_hydrabase_enabled,
+)
+from core.metadata.status import get_spotify_status
 
 logger = logging.getLogger(__name__)
 
@@ -183,21 +188,47 @@ def get_debug_info():
         info['paths']['music_videos_path'] = music_videos_path
         info['paths']['music_videos_path_exists'] = os.path.isdir(music_videos_path)
 
-    # Services from status cache
+    # Services. `_status_cache` only carries 'media_server' and 'soulseek'
-    spotify_cache = _status_cache.get('spotify', {})
+    # (no 'spotify' key) so anything we used to read from `spotify_cache`
+    # silently defaulted to the missing-value fallback — that's the
+    # "music_source: unknown" bug. Spotify status now comes from the
+    # canonical `get_spotify_status` accessor; primary metadata source
+    # comes from `get_primary_source` (which already accounts for the
+    # auth-fallback chain — Spotify drops back to Deezer when not
+    # authenticated).
     media_server_cache = _status_cache.get('media_server', {})
     soulseek_cache = _status_cache.get('soulseek', {})
+    spotify_status = _safe_check(lambda: get_spotify_status(spotify_client=spotify_client), default={})
+    if not isinstance(spotify_status, dict):
+        spotify_status = {}
     info['services'] = {
-        'music_source': spotify_cache.get('source', 'unknown'),
+        'music_source': _safe_check(get_primary_source, default='unknown') or 'unknown',
-        'spotify_connected': spotify_cache.get('connected', False),
+        'spotify_connected': bool(spotify_status.get('connected', False)),
-        'spotify_rate_limited': spotify_cache.get('rate_limited', False),
+        'spotify_rate_limited': bool(spotify_status.get('rate_limited', False)),
         'media_server_type': media_server_cache.get('type', 'none'),
         'media_server_connected': media_server_cache.get('connected', False),
         'soulseek_connected': soulseek_cache.get('connected', False),
         'download_source': config_manager.get('download_source.mode', 'hybrid'),
         'tidal_connected': _safe_check(lambda: bool(tidal_client and tidal_client.is_authenticated())),
         'qobuz_connected': _safe_check(lambda: bool(qobuz_enrichment_worker and qobuz_enrichment_worker.client and qobuz_enrichment_worker.client.is_authenticated())),
+        'hydrabase_connected': _safe_check(is_hydrabase_enabled),
+        # YouTube is URL-based via yt-dlp — no auth, always reachable as
+        # long as the binary is installed. Surfaced so the debug dump
+        # documents that YouTube is one of the available download sources
+        # rather than implying it doesn't exist.
+        'youtube_available': True,
     }
+    # HiFi instance count — separate from connection status because each
+    # instance is its own independent endpoint with its own auth state.
+    info['services']['hifi_instance_count'] = _safe_check(
+        lambda: len(get_database().get_hifi_instances()), default=0
+    )
+    # Always-available public metadata sources (no auth, no per-user
+    # connection state). Listed so the debug dump reflects the full
+    # metadata surface SoulSync queries from, not just the auth-gated ones.
+    info['services']['always_available_metadata_sources'] = [
+        'deezer', 'itunes', 'musicbrainz',
+    ]
 
     # Enrichment workers
     workers = {}
@@ -300,7 +331,11 @@ def get_debug_info():
     # API rate monitor — current calls/min, 24h totals, peaks, rate limit events
     try:
         from core.api_call_tracker import api_call_tracker
-        from core.metadata.status import get_spotify_status
+        # `get_spotify_status` is already imported at module level. A
+        # local re-import here would make Python treat the name as a
+        # function-scoped local for the WHOLE body, breaking the lambda
+        # at the top of get_debug_info that closes over the module-level
+        # binding (Python 3.12 NameError on free variables).
         rates = api_call_tracker.get_all_rates()
         info['api_rates'] = rates
         # Rich 24h debug summary with peaks, totals, per-endpoint breakdown, events

tests/test_debug_info_services.py

@@ -0,0 +1,167 @@
+"""Pin the `info['services']` block returned by /api/debug-info.
+
+Pre-fix the `music_source` field always rendered as "unknown" because
+the code read `_status_cache.get('spotify', {})` — but the cache only
+ever holds 'media_server' and 'soulseek' keys, so the fallback always
+fired. Same problem (silently) for `spotify_connected` and
+`spotify_rate_limited`. Hydrabase was missing entirely.
+
+Fix routes those reads through the canonical accessors:
+- `music_source` → `core.metadata.registry.get_primary_source` (which
+  already accounts for the auth-fallback chain — Spotify → Deezer when
+  unauthenticated)
+- `spotify_connected` / `spotify_rate_limited` →
+  `core.metadata.status.get_spotify_status`
+- `hydrabase_connected` → `core.metadata.registry.is_hydrabase_enabled`
+- `youtube_available` → constant True (URL-based, no auth)
+- `hifi_instance_count` → `db.get_hifi_instances`
+- `always_available_metadata_sources` → static list of public-API
+  sources (Deezer / iTunes / MusicBrainz)
+"""
+
+from __future__ import annotations
+
+from contextlib import contextmanager
+from unittest.mock import patch
+
+import pytest
+
+
+@pytest.fixture
+def app_test_client():
+    import web_server
+    web_server.app.config['TESTING'] = True
+    with web_server.app.test_client() as client:
+        yield client
+
+
+@contextmanager
+def _patched_endpoint(
+    primary_source='spotify',
+    spotify_status=None,
+    hydrabase_enabled=False,
+    primary_source_raises=False,
+    spotify_status_raises=False,
+    hydrabase_raises=False,
+):
+    """Patch the three module-level lookups inside `core.debug_info`
+    and yield. Each can either return a fixed value or raise — the
+    `*_raises` flags select which."""
+    if spotify_status is None:
+        spotify_status = {'connected': True, 'rate_limited': False}
+
+    def _boom(*_a, **_k):
+        raise RuntimeError('forced failure for test')
+
+    primary_patch = patch(
+        'core.debug_info.get_primary_source',
+        side_effect=_boom if primary_source_raises else None,
+        return_value=None if primary_source_raises else primary_source,
+    )
+    spotify_patch = patch(
+        'core.debug_info.get_spotify_status',
+        side_effect=_boom if spotify_status_raises else None,
+        return_value=None if spotify_status_raises else spotify_status,
+    )
+    hydrabase_patch = patch(
+        'core.debug_info.is_hydrabase_enabled',
+        side_effect=_boom if hydrabase_raises else None,
+        return_value=None if hydrabase_raises else hydrabase_enabled,
+    )
+    with primary_patch, spotify_patch, hydrabase_patch:
+        yield
+
+
+def _services(client):
+    resp = client.get('/api/debug-info')
+    assert resp.status_code == 200
+    return resp.get_json()['services']
+
+
+def test_music_source_uses_primary_source_not_status_cache(app_test_client):
+    """The bug: music_source always read 'unknown' because it pulled
+    from a non-existent 'spotify' key in `_status_cache`. Fix routes
+    it through `get_primary_source` which is the actual authority."""
+    with _patched_endpoint(primary_source='tidal'):
+        services = _services(app_test_client)
+    assert services['music_source'] == 'tidal'
+
+
+def test_music_source_falls_back_to_unknown_when_lookup_raises(app_test_client):
+    """Defensive: if `get_primary_source` itself blows up, the field
+    still renders as 'unknown' rather than crashing the whole endpoint."""
+    with _patched_endpoint(primary_source_raises=True):
+        services = _services(app_test_client)
+    assert services['music_source'] == 'unknown'
+
+
+def test_spotify_connected_uses_get_spotify_status(app_test_client):
+    """`spotify_connected` was reading `_status_cache.get('spotify', {})`,
+    which never had the key. Routed through `get_spotify_status` now."""
+    with _patched_endpoint(spotify_status={'connected': True, 'rate_limited': False}):
+        services = _services(app_test_client)
+    assert services['spotify_connected'] is True
+
+
+def test_spotify_rate_limited_uses_get_spotify_status(app_test_client):
+    with _patched_endpoint(spotify_status={'connected': True, 'rate_limited': True}):
+        services = _services(app_test_client)
+    assert services['spotify_rate_limited'] is True
+
+
+def test_spotify_status_lookup_failure_does_not_break_endpoint(app_test_client):
+    """`get_spotify_status` raises → both spotify_* fields default to
+    False rather than 500'ing the whole debug dump."""
+    with _patched_endpoint(spotify_status_raises=True):
+        services = _services(app_test_client)
+    assert services['spotify_connected'] is False
+    assert services['spotify_rate_limited'] is False
+
+
+def test_hydrabase_connected_present(app_test_client):
+    """Hydrabase status was never surfaced in debug info even though
+    it's an active metadata source. Now reported."""
+    with _patched_endpoint(hydrabase_enabled=True):
+        services = _services(app_test_client)
+    assert services['hydrabase_connected'] is True
+
+
+def test_hydrabase_disconnected_when_disabled(app_test_client):
+    with _patched_endpoint(hydrabase_enabled=False):
+        services = _services(app_test_client)
+    assert services['hydrabase_connected'] is False
+
+
+def test_hydrabase_lookup_failure_defaults_false(app_test_client):
+    with _patched_endpoint(hydrabase_raises=True):
+        services = _services(app_test_client)
+    assert services['hydrabase_connected'] is False
+
+
+def test_youtube_available_always_true(app_test_client):
+    """YouTube is URL-based via yt-dlp, no auth, always available.
+    Surfaced so the dump documents it as a download source."""
+    with _patched_endpoint():
+        services = _services(app_test_client)
+    assert services['youtube_available'] is True
+
+
+def test_always_available_metadata_sources_listed(app_test_client):
+    """Public-API metadata sources (no auth, no per-user state) listed
+    so the debug dump reflects the full metadata surface."""
+    with _patched_endpoint():
+        services = _services(app_test_client)
+    available = services['always_available_metadata_sources']
+    assert 'deezer' in available
+    assert 'itunes' in available
+    assert 'musicbrainz' in available
+
+
+def test_hifi_instance_count_present(app_test_client):
+    """HiFi instance count exposed because each instance is a separate
+    endpoint with its own auth state — single connected/disconnected
+    bool wouldn't capture the actual config."""
+    with _patched_endpoint():
+        services = _services(app_test_client)
+    assert 'hifi_instance_count' in services
+    assert isinstance(services['hifi_instance_count'], int)

webui/static/helper.js

@@ -3416,6 +3416,7 @@ const WHATS_NEW = {
     '2.5.1': [
         // --- post-release patch work on the 2.5.1 line — entries hidden by _getLatestWhatsNewVersion until the build version bumps ---
         { date: 'Unreleased — 2.5.1 patch work' },
+        { title: 'Help & Docs: Copy Debug Info Now Reports The Right Music Source + Lists All Services', desc: 'the music_source field always rendered as "unknown" because the code read `_status_cache.get(\'spotify\', {})` — but the cache only has \'media_server\' and \'soulseek\' keys, so the lookup always fell through. same silent miss for spotify_connected and spotify_rate_limited. fix routes those reads through the canonical accessors: `get_primary_source()` for music source (which already accounts for the spotify→deezer auth fallback), `get_spotify_status()` for connection + rate-limit state. also added hydrabase_connected (was missing entirely), youtube_available (always true — yt-dlp + url-based, no auth), hifi_instance_count (separate from connection because each instance is its own endpoint with its own auth), and an always_available_metadata_sources list (deezer / itunes / musicbrainz — public apis, no auth) so the dump reflects the full metadata surface. while in there: removed a local `from core.metadata.status import get_spotify_status` re-import that was making python 3.12 treat the name as a function-scoped local, breaking the new lambda above it (NameError on free variable). 11 new tests at the endpoint boundary pin music_source, spotify_*, hydrabase_*, youtube_available, always_available_metadata_sources, hifi_instance_count, and the defensive paths when each lookup raises.', page: 'tools' },
         { title: 'Download Discography: Skips Tracks Already In Your Library', desc: 'discord report (skowl): clicking download discography on the same artist twice re-queued every track instead of skipping the half already on disk. trace: the endpoint added each track via `add_to_wishlist`, which dedups against the wishlist itself but never checks the library — once a downloaded track leaves the wishlist the next click re-inserts it. fix: same library-ownership check the discography backfill repair job already runs (`db.check_track_exists` at confidence ≥ 0.7). format-agnostic — name + artist + album, no extension comparison — so blasphemy mode (flac → mp3 with original deleted) doesn\'t false-miss. exception during the check returns "not owned" so a transient db hiccup doesn\'t silently nuke the discography fetch (a redundant wishlist add is cheap, a missed track isn\'t). per-album response carries a new `tracks_skipped_owned` counter alongside the artist / content / wishlist skips. 10 new tests at the helper boundary.', page: 'discover' },
         { title: 'Download Discography: No More Cross-Artist Tracks Or Unwanted Remixes', desc: 'issue #559: download discography pulled in tracks from compilations / appears-on albums where the artist was only featured on one or two tracks — every other track on those albums got added too. also ignored your watchlist "include remixes / live / acoustic / instrumental" settings, so one-off discography downloads kept stuffing your wishlist with remix ladders. fix: per-track filter at the endpoint. drops tracks where the requested artist isn\'t named in the track\'s artists list (keeps features, drops unrelated compilation entries). honors `watchlist.global_include_*` settings the same way the discography backfill repair job already does. per-album response carries new skip counts so the ui can show how much got filtered. 21 new tests at the helper boundary.', page: 'discover' },
         { title: 'Album Completeness: "Could Not Determine Album Folder" Error Now Tells You What To Fix', desc: 'github issue #558 (gabistek, navidrome on docker / arch host): clicking auto-fill or fix selected on the album completeness findings page returned a flat "could not determine album folder from existing tracks" error with no diagnostic. trace: the path resolver in `core/library/path_resolver.py` probes transfer + download + `library.music_paths` config + plex api library locations to map db-recorded paths to actual files on disk. for plex users the api auto-discovers the mount paths (per #476). navidrome\'s subsonic api doesn\'t expose filesystem paths at all (only folder names via `getMusicFolders`), and navidrome\'s native rest api on top of that doesn\'t expose them either — there is no api signal we can probe. so for navidrome users in docker, if the path navidrome reports (`/music/artist/album/track.flac`) doesn\'t exist as-is in the soulsync container view AND the user hasn\'t manually configured settings → library → music paths, the resolver returns none and the fix workflow bailed silently. fix: lifted the resolver into a diagnostic-aware variant (`resolve_library_file_path_with_diagnostic` returning a `(resolved, ResolveAttempt)` tuple) that records what was tried — raw-path-existed, base-dirs-probed, whether config_manager / plex_client were wired up. repair_worker uses the diagnostic to render a multi-part error: names the active media server, shows one sample db-recorded path the album\'s tracks have, lists every base directory the resolver actually probed, and points at settings → library → music paths as the actionable fix. user can now read the error and know exactly what to mount or configure. no auto-probing of common docker conventions — too speculative, could resolve to wrong dirs on the suffix-walk if conventional paths happen to contain a partial collision. backwards compatible: legacy `resolve_library_file_path` kept as a thin wrapper that drops the attempt, every existing call site unchanged. 12 new tests pin: tuple shape, raw-path short-circuit attempt fields, base-dirs listed even on walk failure, had-flags reflect caller inputs, error renders active server name + sample path + base dirs, distinguishes empty-base-dirs vs tried-and-failed cases, settings hint always present, defensive against none attempt + missing sample + missing config_manager.', page: 'tools' },