Surface metadata source on Import album results (#681)
Import album search silently fell through to the next source in
METADATA_SOURCE_PRIORITY when the configured primary returned zero
matches — intentional behavior shared with the auto-import worker
(see core/auto_import_worker.py:1316). With MusicBrainz selected and
a query MB couldn't resolve, users saw Deezer cards with no indication
their primary was bypassed.
Backend now echoes `primary_source` on /api/import/search/albums,
/api/import/search/tracks, and /api/import/staging/suggestions.
Frontend renders a per-card 'via {source}' badge when the served
source differs from the primary, plus a banner above the grid when
every card came from a fallback source. Fallback semantics unchanged.
Also collapses an inline duplicate of _renderSuggestionCard inside
importPageSearchAlbum into a single shared renderer.
Regression test pins the contract: response carries primary_source +
per-album source when the chain falls back.
This commit is contained in:
parent
94129d3099
commit
eba7f61e04
5 changed files with 97 additions and 24 deletions
|
|
@ -217,7 +217,12 @@ def staging_hints(runtime: ImportRouteRuntime) -> tuple[Dict[str, Any], int]:
|
|||
def staging_suggestions() -> tuple[Dict[str, Any], int]:
|
||||
"""Return cached import suggestions and readiness state."""
|
||||
cache = get_import_suggestions_cache()
|
||||
return {"success": True, "suggestions": cache["suggestions"], "ready": cache["built"]}, 200
|
||||
return {
|
||||
"success": True,
|
||||
"suggestions": cache["suggestions"],
|
||||
"ready": cache["built"],
|
||||
"primary_source": _get_primary_source(),
|
||||
}, 200
|
||||
|
||||
|
||||
def search_albums(runtime: ImportRouteRuntime, query: str, limit: int = 12) -> tuple[Dict[str, Any], int]:
|
||||
|
|
@ -228,11 +233,12 @@ def search_albums(runtime: ImportRouteRuntime, query: str, limit: int = 12) -> t
|
|||
return {"success": False, "error": "Missing query parameter"}, 400
|
||||
|
||||
limit = min(int(limit), 50)
|
||||
if runtime.get_primary_source() == "hydrabase" and runtime.hydrabase_worker and runtime.dev_mode_enabled:
|
||||
primary_source = runtime.get_primary_source()
|
||||
if primary_source == "hydrabase" and runtime.hydrabase_worker and runtime.dev_mode_enabled:
|
||||
runtime.hydrabase_worker.enqueue(query, "albums")
|
||||
|
||||
albums = runtime.search_import_albums(query, limit=limit)
|
||||
return {"success": True, "albums": albums}, 200
|
||||
return {"success": True, "albums": albums, "primary_source": primary_source}, 200
|
||||
except Exception as exc:
|
||||
runtime.logger.error("Error searching albums for import: %s", exc)
|
||||
return {"success": False, "error": str(exc)}, 500
|
||||
|
|
@ -362,11 +368,12 @@ def search_tracks(runtime: ImportRouteRuntime, query: str, limit: int = 10) -> t
|
|||
return {"success": False, "error": "Missing query parameter"}, 400
|
||||
|
||||
limit = min(int(limit), 30)
|
||||
if runtime.get_primary_source() == "hydrabase" and runtime.hydrabase_worker and runtime.dev_mode_enabled:
|
||||
primary_source = runtime.get_primary_source()
|
||||
if primary_source == "hydrabase" and runtime.hydrabase_worker and runtime.dev_mode_enabled:
|
||||
runtime.hydrabase_worker.enqueue(query, "tracks")
|
||||
|
||||
tracks = runtime.search_import_tracks(query, limit=limit)
|
||||
return {"success": True, "tracks": tracks}, 200
|
||||
return {"success": True, "tracks": tracks, "primary_source": primary_source}, 200
|
||||
except Exception as exc:
|
||||
runtime.logger.error("Error searching tracks for import: %s", exc)
|
||||
return {"success": False, "error": str(exc)}, 500
|
||||
|
|
|
|||
|
|
@ -207,6 +207,7 @@ def test_staging_suggestions_returns_cache_payload(monkeypatch):
|
|||
"get_import_suggestions_cache",
|
||||
lambda: {"suggestions": [{"album": "Album"}], "built": True},
|
||||
)
|
||||
monkeypatch.setattr(import_routes, "_get_primary_source", lambda: "deezer")
|
||||
|
||||
payload, status = staging_suggestions()
|
||||
|
||||
|
|
@ -215,6 +216,7 @@ def test_staging_suggestions_returns_cache_payload(monkeypatch):
|
|||
"success": True,
|
||||
"suggestions": [{"album": "Album"}],
|
||||
"ready": True,
|
||||
"primary_source": "deezer",
|
||||
}
|
||||
|
||||
|
||||
|
|
@ -303,7 +305,11 @@ def test_search_albums_enqueues_hydrabase_and_caps_limit():
|
|||
payload, status = search_albums(runtime, " Album ", 99)
|
||||
|
||||
assert status == 200
|
||||
assert payload == {"success": True, "albums": [{"id": "album-1"}]}
|
||||
assert payload == {
|
||||
"success": True,
|
||||
"albums": [{"id": "album-1"}],
|
||||
"primary_source": "hydrabase",
|
||||
}
|
||||
assert worker.enqueued == [("Album", "albums")]
|
||||
assert calls == [("Album", 50)]
|
||||
|
||||
|
|
@ -315,6 +321,28 @@ def test_search_albums_requires_query():
|
|||
assert payload == {"success": False, "error": "Missing query parameter"}
|
||||
|
||||
|
||||
def test_search_albums_exposes_primary_source_when_chain_falls_back():
|
||||
# Pins github issue #681: when the primary source returns nothing and the
|
||||
# silent fallback chain (intentional, see core/auto_import_worker.py:1316)
|
||||
# serves results from a different source, the response must carry both
|
||||
# `primary_source` (what the user configured) and per-album `source`
|
||||
# (what actually served the result) so the UI can warn the user.
|
||||
runtime = ImportRouteRuntime(
|
||||
get_primary_source=lambda: "musicbrainz",
|
||||
search_import_albums=lambda query, limit: [
|
||||
{"id": "deezer-1", "name": "Album", "source": "deezer"},
|
||||
],
|
||||
logger=_FakeLogger(),
|
||||
)
|
||||
|
||||
payload, status = search_albums(runtime, "Weapons of Mass Destruction", 12)
|
||||
|
||||
assert status == 200
|
||||
assert payload["success"] is True
|
||||
assert payload["primary_source"] == "musicbrainz"
|
||||
assert payload["albums"][0]["source"] == "deezer"
|
||||
|
||||
|
||||
def test_search_tracks_enqueues_hydrabase_and_caps_limit():
|
||||
worker = _FakeHydrabaseWorker()
|
||||
calls = []
|
||||
|
|
@ -329,7 +357,11 @@ def test_search_tracks_enqueues_hydrabase_and_caps_limit():
|
|||
payload, status = search_tracks(runtime, " Track ", 99)
|
||||
|
||||
assert status == 200
|
||||
assert payload == {"success": True, "tracks": [{"id": "track-1"}]}
|
||||
assert payload == {
|
||||
"success": True,
|
||||
"tracks": [{"id": "track-1"}],
|
||||
"primary_source": "hydrabase",
|
||||
}
|
||||
assert worker.enqueued == [("Track", "tracks")]
|
||||
assert calls == [("Track", 30)]
|
||||
|
||||
|
|
|
|||
|
|
@ -3414,6 +3414,8 @@ function closeHelperSearch() {
|
|||
// release time and add a real `date:` line at the top of the version block.
|
||||
const WHATS_NEW = {
|
||||
'2.5.9': [
|
||||
{ date: 'Unreleased — dev cycle' },
|
||||
{ title: 'Import search: show when results came from the fallback source', desc: 'if you picked MusicBrainz (or Discogs / iTunes / etc.) as your primary metadata source but the Import album search ended up serving Deezer cards, you had no idea — the chain silently fell through when the primary returned nothing. now each card shows a small "via Deezer" label when the source differs from your primary, and a banner above the grid spells it out when all results came from the fallback. backend behavior unchanged.' },
|
||||
{ date: 'May 21, 2026 — 2.5.9 release' },
|
||||
{ title: 'Now-playing modal: lyrics panel', desc: 'new lyrics panel below the player controls in the expanded now-playing modal. fetches from LRClib via /api/lyrics/fetch, but prefers the local .lrc / .txt sidecar files SoulSync drops next to your audio during post-processing so downloaded tracks show lyrics instantly with zero network. synced LRC (timestamped) highlights the active line and auto-scrolls it into the middle of the viewport on every audio timeupdate; plain text renders without highlighting. status chip shows whether the result came back Synced or Plain. panel is collapsed by default — click the Lyrics header to expand. cached per track so revisiting a track doesn\'t refetch.' },
|
||||
{ title: 'Now-playing modal: View Artist closes the modal first', desc: 'tapping View Artist on the expanded media player now closes the now-playing modal before navigating, so the artist page is actually visible instead of sitting under a modal you\'d have to manually dismiss. click is a no-op when no artist_id is attached to the current track.' },
|
||||
|
|
@ -3502,6 +3504,7 @@ const VERSION_MODAL_SECTIONS = [
|
|||
"Jellyfin full refresh repairs older media databases before importing tracks, so stale schemas no longer drop every track row",
|
||||
"Cache maintenance and full refresh retry transient SQLite disk I/O errors instead of failing the whole job on the first attempt",
|
||||
"Album Completeness rejects same-title releases from the wrong artist before importing anything",
|
||||
"Import album search now labels each card with the source that served it and warns when results came from a fallback instead of your primary",
|
||||
],
|
||||
usage_note: "version 2.5.9 focuses on safer matching and making the new release-based sources usable without disturbing existing source flows",
|
||||
},
|
||||
|
|
|
|||
|
|
@ -589,7 +589,8 @@ async function importPageMatchAutoGroup(groupIdx) {
|
|||
importPageState._autoGroupFilePaths = group.file_paths;
|
||||
|
||||
// Render results — user picks the right album
|
||||
grid.innerHTML = data.albums.map(a => _renderSuggestionCard(a)).join('');
|
||||
const banner = _renderImportFallbackBanner(data.albums, data.primary_source);
|
||||
grid.innerHTML = banner + data.albums.map(a => _renderSuggestionCard(a, data.primary_source)).join('');
|
||||
} else {
|
||||
grid.innerHTML = '<div style="color:#888;text-align:center;padding:20px;">No albums found — try searching manually</div>';
|
||||
}
|
||||
|
|
@ -625,28 +626,51 @@ async function importPageLoadSuggestions() {
|
|||
}
|
||||
|
||||
section.style.display = '';
|
||||
grid.innerHTML = data.suggestions.map(a => _renderSuggestionCard(a)).join('');
|
||||
const banner = _renderImportFallbackBanner(data.suggestions, data.primary_source);
|
||||
grid.innerHTML = banner + data.suggestions.map(a => _renderSuggestionCard(a, data.primary_source)).join('');
|
||||
} catch (err) {
|
||||
// Network error or server not ready — fail silently
|
||||
console.warn('Failed to load import suggestions:', err);
|
||||
}
|
||||
}
|
||||
|
||||
function _renderSuggestionCard(a) {
|
||||
function _renderSuggestionCard(a, primarySource) {
|
||||
// Cache the album lookup so importPageSelectAlbum can pull source +
|
||||
// name + artist on click (the onclick can only carry the ID string
|
||||
// — see github issue #524 root cause).
|
||||
importPageState._albumLookup[a.id] = {
|
||||
id: a.id, name: a.name || '', artist: a.artist || '', source: a.source || '',
|
||||
};
|
||||
// Surface the served source when it differs from the user's configured
|
||||
// primary — the search route silently falls through to the next source
|
||||
// in METADATA_SOURCE_PRIORITY when the primary returns nothing
|
||||
// (intentional design, see core/auto_import_worker.py:1316). Without
|
||||
// this badge the user has no idea their MusicBrainz / Discogs choice
|
||||
// got bypassed (github issue #681).
|
||||
const sourceBadge = (a.source && primarySource && a.source !== primarySource)
|
||||
? `<div class="import-page-album-card-source">via ${_esc((SOURCE_LABELS[a.source] || {}).text || a.source)}</div>`
|
||||
: '';
|
||||
return `<div class="import-page-album-card" onclick="importPageSelectAlbum('${_escAttr(a.id)}')">
|
||||
<img src="${a.image_url || '/static/placeholder-album.png'}" alt="${_escAttr(a.name)}" loading="lazy" onerror="this.src='/static/placeholder-album.png'">
|
||||
<div class="import-page-album-card-title" title="${_escAttr(a.name)}">${_esc(a.name)}</div>
|
||||
<div class="import-page-album-card-artist" title="${_escAttr(a.artist)}">${_esc(a.artist)}</div>
|
||||
<div class="import-page-album-card-meta">${a.total_tracks} tracks · ${a.release_date ? a.release_date.substring(0, 4) : ''}</div>
|
||||
${sourceBadge}
|
||||
</div>`;
|
||||
}
|
||||
|
||||
function _renderImportFallbackBanner(albums, primarySource) {
|
||||
if (!primarySource || !albums || !albums.length) return '';
|
||||
const allFallback = albums.every(a => a.source && a.source !== primarySource);
|
||||
if (!allFallback) return '';
|
||||
const servedSource = albums[0].source;
|
||||
const primaryLabel = (SOURCE_LABELS[primarySource] || {}).text || primarySource;
|
||||
const servedLabel = (SOURCE_LABELS[servedSource] || {}).text || servedSource;
|
||||
// Neutral wording — covers both live-search fallback (primary returned 0)
|
||||
// and cache-stale suggestions (primary changed since cache was built).
|
||||
return `<div class="import-page-fallback-banner">Showing ${_esc(servedLabel)} results — not from your primary source (${_esc(primaryLabel)}).</div>`;
|
||||
}
|
||||
|
||||
// --- Album Tab: Search ---
|
||||
|
||||
async function importPageSearchAlbum() {
|
||||
|
|
@ -666,20 +690,8 @@ async function importPageSearchAlbum() {
|
|||
grid.innerHTML = '<div style="color:#888;text-align:center;padding:20px;">No albums found</div>';
|
||||
return;
|
||||
}
|
||||
grid.innerHTML = data.albums.map(a => {
|
||||
// Cache album lookup so the click handler can include source
|
||||
// + name + artist on the match POST (see #524).
|
||||
importPageState._albumLookup[a.id] = {
|
||||
id: a.id, name: a.name || '', artist: a.artist || '', source: a.source || '',
|
||||
};
|
||||
return `
|
||||
<div class="import-page-album-card" onclick="importPageSelectAlbum('${_escAttr(a.id)}')">
|
||||
<img src="${a.image_url || '/static/placeholder-album.png'}" alt="${_escAttr(a.name)}" loading="lazy" onerror="this.src='/static/placeholder-album.png'">
|
||||
<div class="import-page-album-card-title" title="${_escAttr(a.name)}">${_esc(a.name)}</div>
|
||||
<div class="import-page-album-card-artist" title="${_escAttr(a.artist)}">${_esc(a.artist)}</div>
|
||||
<div class="import-page-album-card-meta">${a.total_tracks} tracks · ${a.release_date ? a.release_date.substring(0, 4) : ''}</div>
|
||||
</div>`;
|
||||
}).join('');
|
||||
const banner = _renderImportFallbackBanner(data.albums, data.primary_source);
|
||||
grid.innerHTML = banner + data.albums.map(a => _renderSuggestionCard(a, data.primary_source)).join('');
|
||||
document.getElementById('import-page-album-clear-btn').classList.remove('hidden');
|
||||
} catch (err) {
|
||||
grid.innerHTML = `<div style="color:#ef4444;text-align:center;padding:20px;">Error: ${err.message}</div>`;
|
||||
|
|
|
|||
|
|
@ -40320,6 +40320,25 @@ div.artist-hero-badge {
|
|||
margin-top: 4px;
|
||||
}
|
||||
|
||||
.import-page-album-card-source {
|
||||
font-size: 10px;
|
||||
color: rgba(255, 200, 100, 0.75);
|
||||
margin-top: 2px;
|
||||
font-style: italic;
|
||||
}
|
||||
|
||||
.import-page-fallback-banner {
|
||||
grid-column: 1 / -1;
|
||||
padding: 10px 14px;
|
||||
margin-bottom: 12px;
|
||||
background: rgba(255, 200, 100, 0.08);
|
||||
border: 1px solid rgba(255, 200, 100, 0.25);
|
||||
border-radius: 8px;
|
||||
color: rgba(255, 220, 170, 0.9);
|
||||
font-size: 12px;
|
||||
line-height: 1.4;
|
||||
}
|
||||
|
||||
/* Album hero (selected album) */
|
||||
.import-page-album-hero {
|
||||
display: flex;
|
||||
|
|
|
|||
Loading…
Reference in a new issue