diff --git a/core/search/by_id.py b/core/search/by_id.py
index 7f55a314..a5189a25 100644
--- a/core/search/by_id.py
+++ b/core/search/by_id.py
@@ -6,13 +6,14 @@ entity up *directly* on the owning source — no scoring, no guessing.
Design notes
------------
-- **Source-agnostic input.** A full URL carries its source in the domain
+- **Links only.** A full URL carries its source in the domain
(``open.spotify.com`` → spotify, ``musicbrainz.org`` → musicbrainz, …)
and its kind in the path (``/album/`` vs ``/track/``), so it resolves to
- exactly one lookup. A *bare* ID is disambiguated by format where we can
- (UUID → MusicBrainz, 22-char base62 → Spotify) and otherwise fanned out
- across the numeric-ID sources (Deezer, iTunes); the first source that
- returns a hit wins. Paste the full link to remove that ambiguity.
+ exactly one unambiguous lookup. The ``spotify:album:ID`` URI is accepted
+ too since it's equally explicit. Bare IDs are intentionally rejected: a
+ bare number like ``525046`` carries no source and no entity type, so it
+ would resolve to whatever album/track happens to own that id on some
+ source — often an unrelated entity. Paste the link instead.
- **Reuses existing per-source get-by-id.** Spotify/iTunes/MusicBrainz all
expose ``get_album``; Deezer exposes ``get_album_metadata``; all four
@@ -32,7 +33,6 @@ an injected ``client_resolver`` (defaulting to the orchestrator's
from __future__ import annotations
import logging
-import re
from typing import Any, Callable, NamedTuple, Optional
from urllib.parse import parse_qs, urlparse
@@ -44,15 +44,6 @@ logger = logging.getLogger(__name__)
# return raw API shapes and aren't metadata-link sources, so they're omitted.
SUPPORTED_SOURCES = ('spotify', 'itunes', 'musicbrainz', 'deezer')
-_UUID_RE = re.compile(
- r'^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$', re.I
-)
-_SPOTIFY_ID_RE = re.compile(r'^[0-9A-Za-z]{22}$')
-_NUMERIC_RE = re.compile(r'^\d+$')
-
-# Numeric-ID sources, tried in this order when a bare number is pasted.
-_NUMERIC_SOURCES = ('deezer', 'itunes')
-
# Domains we recognize — used to detect a pasted URL even when the user
# omitted the scheme (e.g. "open.spotify.com/album/…").
_KNOWN_HOSTS = (
@@ -64,9 +55,10 @@ _KNOWN_HOSTS = (
class LookupTarget(NamedTuple):
"""One (source, kind, id) lookup to attempt.
- ``kind`` is ``'album'`` or ``'track'`` when the input pins it (a URL path
- or URI type), or ``None`` when unknown — the resolver then tries album
- first, then track.
+ ``kind`` is ``'album'`` or ``'track'`` — always pinned by the URL path or
+ URI type (links-only input). The ``Optional`` typing is kept defensively:
+ the resolver falls back to album-then-track if a future parser path ever
+ yields ``None``.
"""
source: str
@@ -139,14 +131,12 @@ def _parse_url(raw: str) -> list[LookupTarget]:
return []
-def parse_metadata_identifier(
- raw: str, preferred_source: Optional[str] = None
-) -> list[LookupTarget]:
- """Parse a pasted link/ID into an ordered list of lookup targets.
+def parse_metadata_identifier(raw: str) -> list[LookupTarget]:
+ """Parse a pasted provider link (or ``spotify:`` URI) into lookup targets.
- Returns the candidates to try in order; the first that resolves wins.
- ``preferred_source`` (the user's active source) only reorders the
- fan-out for ambiguous *bare numeric* IDs.
+ Links only — a bare ID has no source/type and is rejected (returns ``[]``).
+ A URL resolves to exactly one target; the list type is kept for the
+ ``spotify:`` URI path and future multi-target patterns.
"""
raw = (raw or '').strip()
if not raw:
@@ -164,34 +154,9 @@ def parse_metadata_identifier(
)
if looks_like_url:
url = raw if '://' in raw else f'https://{raw}'
- targets = _parse_url(url)
- if targets:
- return targets
- # Fall through: maybe a bare id slipped in with a stray scheme.
-
- # Bare identifier — disambiguate by format.
- if _UUID_RE.match(raw):
- # MusicBrainz MBID: could be release / release-group (album) or
- # recording (track). Try album first, then track.
- return [LookupTarget('musicbrainz', 'album', raw),
- LookupTarget('musicbrainz', 'track', raw)]
-
- if _NUMERIC_RE.match(raw):
- # Numeric IDs collide across Deezer/iTunes — fan out, first hit wins.
- order = list(_NUMERIC_SOURCES)
- if preferred_source in order:
- order.remove(preferred_source)
- order.insert(0, preferred_source)
- targets: list[LookupTarget] = []
- for src in order:
- targets.append(LookupTarget(src, 'album', raw))
- targets.append(LookupTarget(src, 'track', raw))
- return targets
-
- if _SPOTIFY_ID_RE.match(raw):
- return [LookupTarget('spotify', 'album', raw),
- LookupTarget('spotify', 'track', raw)]
+ return _parse_url(url)
+ # Bare ID (or anything we don't recognize as a link) — rejected.
return []
@@ -286,28 +251,37 @@ def _fetch_track(client: Any, source: str, identifier: str) -> Optional[dict]:
return client.get_track_details(identifier)
-def _empty_result(raw: str, source: str = '') -> dict:
+# Shown in the dropdown's empty state so the user knows what to do next.
+_MSG_NOT_A_LINK = (
+ 'Paste a full link from Spotify, Apple Music, MusicBrainz, or Deezer '
+ '(a bare ID is ambiguous).'
+)
+_MSG_NOT_FOUND = "Couldn't resolve that link — double-check it's correct."
+
+
+def _empty_result(raw: str, source: str = '', message: str = '') -> dict:
return {
'source': source,
'albums': [],
'tracks': [],
'available': False,
'query': raw,
+ 'message': message,
}
def resolve_identifier(
raw: str,
deps: Any,
- preferred_source: Optional[str] = None,
client_resolver: Optional[Callable[[str], Any]] = None,
) -> dict:
- """Resolve a pasted link/ID to a single album or track card.
+ """Resolve a pasted provider link to a single album or track card.
Returns a dropdown-compatible dict:
- ``{source, albums, tracks, available, query}``. ``available`` is True iff
- a source returned a hit. The first resolving target wins, so the result
- carries exactly one card (and the ``source`` that owns it).
+ ``{source, albums, tracks, available, query, message}``. ``available`` is
+ True iff a source returned a hit; the first resolving target wins, so the
+ result carries exactly one card (and the ``source`` that owns it).
+ ``message`` is a user-facing hint when nothing resolved.
``client_resolver`` maps a source name to a client (or None). It defaults
to the orchestrator's ``resolve_client``; tests inject fakes.
@@ -318,10 +292,10 @@ def resolve_identifier(
def client_resolver(source: str) -> Any: # noqa: E306
return resolve_client(source, deps)[0]
- targets = parse_metadata_identifier(raw, preferred_source=preferred_source)
+ targets = parse_metadata_identifier(raw)
if not targets:
- logger.info(f"Link/ID resolve: unrecognized identifier {raw!r}")
- return _empty_result(raw, preferred_source or '')
+ logger.info(f"Link/ID resolve: not a recognized link {raw!r}")
+ return _empty_result(raw, message=_MSG_NOT_A_LINK)
for target in targets:
try:
@@ -361,4 +335,4 @@ def resolve_identifier(
)
logger.info(f"Link/ID resolve: no source resolved {raw!r}")
- return _empty_result(raw, preferred_source or '')
+ return _empty_result(raw, source=targets[0].source, message=_MSG_NOT_FOUND)
diff --git a/tests/search/test_search_by_id.py b/tests/search/test_search_by_id.py
index 131525ff..e27264fc 100644
--- a/tests/search/test_search_by_id.py
+++ b/tests/search/test_search_by_id.py
@@ -135,42 +135,23 @@ def test_parse_spotify_url_without_scheme_or_www():
# ---------------------------------------------------------------------------
-# parse_metadata_identifier — bare IDs
+# parse_metadata_identifier — bare IDs are rejected (links only)
# ---------------------------------------------------------------------------
-def test_parse_bare_uuid_is_musicbrainz_album_then_track():
- mbid = 'a1b2c3d4-e5f6-7890-abcd-ef1234567890'
- out = by_id.parse_metadata_identifier(mbid)
- assert out == [
- LookupTarget('musicbrainz', 'album', mbid),
- LookupTarget('musicbrainz', 'track', mbid),
- ]
+def test_parse_bare_numeric_id_rejected():
+ # The footgun case (#775 follow-up): a bare number has no source/type, so
+ # it must NOT resolve to whatever album happens to own that id.
+ assert by_id.parse_metadata_identifier('525046') == []
-def test_parse_bare_base62_is_spotify_album_then_track():
- sid = '4aawyAB9vmqN3uQ7FjRGTy' # 22 chars, has letters
- out = by_id.parse_metadata_identifier(sid)
- assert out == [
- LookupTarget('spotify', 'album', sid),
- LookupTarget('spotify', 'track', sid),
- ]
+def test_parse_bare_uuid_rejected():
+ assert by_id.parse_metadata_identifier(
+ 'a1b2c3d4-e5f6-7890-abcd-ef1234567890'
+ ) == []
-def test_parse_bare_numeric_fans_out_deezer_then_itunes():
- out = by_id.parse_metadata_identifier('302127')
- assert out == [
- LookupTarget('deezer', 'album', '302127'),
- LookupTarget('deezer', 'track', '302127'),
- LookupTarget('itunes', 'album', '302127'),
- LookupTarget('itunes', 'track', '302127'),
- ]
-
-
-def test_parse_bare_numeric_biases_preferred_source_first():
- out = by_id.parse_metadata_identifier('302127', preferred_source='itunes')
- # iTunes pulled to the front of the fan-out.
- assert out[0] == LookupTarget('itunes', 'album', '302127')
- assert out[1] == LookupTarget('itunes', 'track', '302127')
+def test_parse_bare_base62_rejected():
+ assert by_id.parse_metadata_identifier('4aawyAB9vmqN3uQ7FjRGTy') == []
def test_parse_empty_and_garbage_return_empty():
@@ -301,40 +282,35 @@ def test_resolve_deezer_album_uses_get_album_metadata():
assert client.album_calls == ['302127']
-def test_resolve_kind_unknown_falls_back_album_then_track():
- # Bare base62 → spotify, kind unknown. Album returns None, track resolves.
+def test_resolve_track_link():
+ # A track URL pins kind=track — only get_track_details is called.
client = _FakeClient(album=None, track={
'id': 't1', 'name': 'Creep', 'artists': ['Radiohead'],
'album': {'name': 'Pablo Honey'}, 'duration_ms': 238000,
})
- sid = '4aawyAB9vmqN3uQ7FjRGTy'
res = by_id.resolve_identifier(
- sid, deps=None, client_resolver=_resolver_from({'spotify': client}),
+ 'https://open.spotify.com/track/t1', deps=None,
+ client_resolver=_resolver_from({'spotify': client}),
)
assert res['available'] is True
assert res['tracks'][0]['name'] == 'Creep'
assert res['albums'] == []
- assert client.album_calls == [sid] # tried album first
- assert client.track_calls == [sid] # then track
+ assert client.album_calls == [] # kind pinned to track — no album probe
+ assert client.track_calls == ['t1']
-def test_resolve_numeric_fanout_first_hit_wins():
- # Deezer has no such id (returns None), iTunes does — resolver moves on.
- deezer = _FakeClient(album=None, track=None)
- itunes = _FakeClient(album={'id': '99', 'name': 'Thriller',
- 'artists': ['Michael Jackson'], 'images': [],
- 'release_date': '1982', 'total_tracks': 9,
- 'album_type': 'album', 'external_urls': {}})
+def test_resolve_bare_id_rejected_with_hint():
+ # The #775 follow-up regression: a bare number must not resolve; it
+ # returns not-found with a link hint instead of an unrelated album.
+ called = []
res = by_id.resolve_identifier(
- '99', deps=None,
- client_resolver=_resolver_from({'deezer': deezer, 'itunes': itunes}),
+ '525046', deps=None,
+ client_resolver=lambda s: called.append(s), # must never be invoked
)
- assert res['available'] is True
- assert res['source'] == 'itunes'
- assert res['albums'][0]['name'] == 'Thriller'
- # Deezer was tried (album + track) before iTunes resolved.
- assert deezer.album_calls == ['99']
- assert deezer.track_calls == ['99']
+ assert res['available'] is False
+ assert res['albums'] == [] and res['tracks'] == []
+ assert 'link' in res['message'].lower()
+ assert called == [] # no source was even probed
def test_resolve_unavailable_client_is_skipped():
@@ -345,7 +321,9 @@ def test_resolve_unavailable_client_is_skipped():
)
assert res['available'] is False
assert res['albums'] == [] and res['tracks'] == []
- assert res['source'] == ''
+ # The source we tried is reported even on miss.
+ assert res['source'] == 'spotify'
+ assert res['message']
def test_resolve_client_exception_does_not_propagate():
diff --git a/web_server.py b/web_server.py
index ac01991b..c077de35 100644
--- a/web_server.py
+++ b/web_server.py
@@ -5728,27 +5728,26 @@ def enhanced_search():
@app.route('/api/enhanced-search/by-id', methods=['POST'])
def enhanced_search_by_id():
- """Resolve a pasted metadata link/ID to a single album or track (#775).
+ """Resolve a pasted metadata link to a single album or track (#775).
- Source-agnostic: a provider URL (Spotify/Apple/MusicBrainz/Deezer) or a
- bare ID is looked up directly on the owning source via its get-by-id —
- no fuzzy search, no scoring. Returns the same dropdown shape the normal
- enhanced search renders, plus the resolving ``source`` so the frontend
- can route downloads/imports through the existing flow.
+ A provider URL (Spotify/Apple/MusicBrainz/Deezer) is looked up directly
+ on the owning source via its get-by-id — no fuzzy search, no scoring. The
+ domain pins the source and the path pins album-vs-track, so it's
+ unambiguous. Returns the same dropdown shape the normal enhanced search
+ renders, plus the resolving ``source`` so the frontend can route
+ downloads/imports through the existing flow.
- Body: ``{"query": "", "source": ""?}``. The
- optional ``source`` only biases the fan-out for ambiguous bare numeric
- IDs (Deezer vs iTunes).
+ Body: ``{"query": ""}``. Links only — a
+ bare ID is rejected with a hint, since it carries no source or type.
"""
data = request.get_json() or {}
raw = (data.get('query') or '').strip()
- preferred = (data.get('source') or '').strip().lower() or None
if not raw:
return jsonify(_search_by_id._empty_result(''))
try:
deps = _build_search_deps()
- result = _search_by_id.resolve_identifier(raw, deps, preferred_source=preferred)
+ result = _search_by_id.resolve_identifier(raw, deps)
return jsonify(result)
except Exception as e:
logger.error(f"Link/ID resolve error: {e}")
diff --git a/webui/index.html b/webui/index.html
index 323bfe3f..80dac941 100644
--- a/webui/index.html
+++ b/webui/index.html
@@ -2201,15 +2201,15 @@
-
+
🔗
diff --git a/webui/static/search.js b/webui/static/search.js
index 81627b93..2daff1bf 100644
--- a/webui/static/search.js
+++ b/webui/static/search.js
@@ -300,10 +300,7 @@ function initializeSearchModeToggle() {
const resp = await fetch('/api/enhanced-search/by-id', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
- body: JSON.stringify({
- query: raw,
- source: searchController.state.activeSource,
- }),
+ body: JSON.stringify({ query: raw }),
});
data = await resp.json();
} catch (err) {
@@ -329,11 +326,14 @@ function initializeSearchModeToggle() {
}
_renderFromState(searchController.state);
} else {
- // No source resolved the link — show the empty state.
+ // Not a link, or nothing resolved — surface the backend's hint
+ // via a toast (non-destructive) and show the empty state.
if (loadingState) loadingState.classList.add('hidden');
if (resultsContainer) resultsContainer.classList.add('hidden');
if (emptyState) emptyState.classList.remove('hidden');
showDropdown();
+ const msg = (data && data.message) || 'No match for that link.';
+ if (typeof showToast === 'function') showToast(msg, 'warning');
}
}