From 146bf32f234c2b61cec733110dec04645542ecea Mon Sep 17 00:00:00 2001 From: Jonathan Date: Mon, 13 Jul 2026 19:23:18 +0200 Subject: [PATCH] feat(worker): Last.fm artist.getSimilar as a second discovery SimilaritySource Adds LastfmSource, resolving Last.fm's often-empty artist mbid via the injected MB browser (name->mbid cached) so every emitted SimilarArtist.mbid is a real MusicBrainz artist MBID. Registered in build_similarity_sources, gated on lastfm.api_key so the ListenBrainz-only path is unchanged when unset. --- worker/lyra_worker/registry.py | 12 ++- worker/lyra_worker/similarity/_lastfm.py | 69 +++++++++++++++ worker/tests/test_lastfm_source.py | 108 +++++++++++++++++++++++ worker/tests/test_similarity_registry.py | 15 ++++ 4 files changed, 202 insertions(+), 2 deletions(-) create mode 100644 worker/lyra_worker/similarity/_lastfm.py create mode 100644 worker/tests/test_lastfm_source.py diff --git a/worker/lyra_worker/registry.py b/worker/lyra_worker/registry.py index f6ec332..352a983 100644 --- a/worker/lyra_worker/registry.py +++ b/worker/lyra_worker/registry.py @@ -12,6 +12,7 @@ from lyra_worker.adapters.youtube import YouTubeAdapter from lyra_worker.browser import MbBrowser from lyra_worker.probe import AudioProbe from lyra_worker.resolver import MbResolver +from lyra_worker.similarity._lastfm import LastfmSource from lyra_worker.similarity._listenbrainz import ListenBrainzSource from lyra_worker.similarity.base import SimilaritySource from lyra_worker.tagger import Tagger @@ -53,5 +54,12 @@ def build_probe() -> AudioProbe: def build_similarity_sources(config: dict) -> list[SimilaritySource]: """Similarity sources for discovery. ListenBrainz needs no credentials, so it is - always constructed; per-run reachability is checked via each source's health().""" - return [ListenBrainzSource(base_url=config.get("discover.listenBrainzUrl") or "")] + always constructed; per-run reachability is checked via each source's health(). + Last.fm is added only when an API key is configured.""" + sources: list[SimilaritySource] = [ + ListenBrainzSource(base_url=config.get("discover.listenBrainzUrl") or "") + ] + key = config.get("lastfm.api_key") + if key: + sources.append(LastfmSource(api_key=key, browser=MusicBrainzBrowser())) + return sources diff --git a/worker/lyra_worker/similarity/_lastfm.py b/worker/lyra_worker/similarity/_lastfm.py new file mode 100644 index 0000000..9001aee --- /dev/null +++ b/worker/lyra_worker/similarity/_lastfm.py @@ -0,0 +1,69 @@ +import requests + +from lyra_worker.similarity.base import SimilarArtist + +_BASE = "https://ws.audioscrobbler.com/2.0/" + + +class LastfmSource: + """SimilaritySource backed by Last.fm's artist.getsimilar. + + Last.fm rows often carry an empty mbid, so names are resolved to a real + MusicBrainz artist MBID via the injected browser. Unresolvable rows are + skipped -- every emitted SimilarArtist.mbid must be a real MB artist MBID, + since downstream keys on it. + """ + + name = "lastfm" + + def __init__(self, api_key, browser, base_url=_BASE, timeout: float = 15.0, + similar_limit: int = 50): + self._key = api_key + self._browser = browser # MbBrowser: .search_artist(name) -> list[ArtistHit] + self._base = base_url + self._timeout = timeout + self._limit = similar_limit + self._name_cache: dict[str, str | None] = {} + + def _get(self, method: str, **params): + res = requests.get( + self._base, + params={"method": method, "api_key": self._key, "format": "json", **params}, + timeout=self._timeout, + ) + return res.json() + + def health(self) -> bool: + try: + data = self._get("artist.getsimilar", artist="Radiohead", limit="1") + return not isinstance(data.get("error"), (int, float)) + except requests.RequestException: + return False + + def _resolve(self, name: str) -> str | None: + key = name.lower() + if key in self._name_cache: + return self._name_cache[key] + hits = self._browser.search_artist(name) + mbid = hits[0].mbid if hits else None + self._name_cache[key] = mbid + return mbid + + def similar_artists(self, mbid: str) -> list[SimilarArtist]: + data = self._get("artist.getsimilar", mbid=mbid, limit=str(self._limit)) + if isinstance(data.get("error"), (int, float)): + return [] + rows = data.get("similarartists", {}).get("artist", []) + if isinstance(rows, dict): + rows = [rows] + out: list[SimilarArtist] = [] + for row in rows: + name = row.get("name") or "" + cand = row.get("mbid") or None + if not cand: + cand = self._resolve(name) + if not cand or cand == mbid or not name: + continue + score = float(row.get("match") or 0) + out.append(SimilarArtist(mbid=cand, name=name, score=score)) + return out diff --git a/worker/tests/test_lastfm_source.py b/worker/tests/test_lastfm_source.py new file mode 100644 index 0000000..1360404 --- /dev/null +++ b/worker/tests/test_lastfm_source.py @@ -0,0 +1,108 @@ +from unittest.mock import MagicMock, patch + +import requests + +from lyra_worker.browser import ArtistHit +from lyra_worker.similarity._lastfm import LastfmSource +from lyra_worker.similarity.base import SimilarArtist + + +def _resp(payload, status=200): + r = MagicMock() + r.status_code = status + r.json.return_value = payload + return r + + +class _FakeBrowser: + """Records search_artist calls; resolves via a name -> [ArtistHit] map.""" + + def __init__(self, hits=None): + self._hits = dict(hits or {}) + self.calls = [] + + def search_artist(self, name): + self.calls.append(name) + return list(self._hits.get(name, [])) + + +def test_similar_artists_parses_and_uses_supplied_mbid(): + payload = {"similarartists": {"artist": [ + {"name": "Cand One", "mbid": "c1", "match": "0.9"}, + ]}} + browser = _FakeBrowser() + with patch("lyra_worker.similarity._lastfm.requests.get", return_value=_resp(payload)): + out = LastfmSource(api_key="k", browser=browser).similar_artists("seed") + assert out == [SimilarArtist("c1", "Cand One", 0.9)] + assert browser.calls == [] # mbid supplied, no resolution needed + + +def test_similar_artists_resolves_missing_mbid_and_caches(): + payload = {"similarartists": {"artist": [ + {"name": "Cand Two", "mbid": "", "match": "0.5"}, + {"name": "Cand Two", "mbid": "", "match": "0.4"}, + ]}} + browser = _FakeBrowser(hits={"Cand Two": [ArtistHit(mbid="c2", name="Cand Two")]}) + with patch("lyra_worker.similarity._lastfm.requests.get", return_value=_resp(payload)): + out = LastfmSource(api_key="k", browser=browser).similar_artists("seed") + assert out == [ + SimilarArtist("c2", "Cand Two", 0.5), + SimilarArtist("c2", "Cand Two", 0.4), + ] + assert browser.calls == ["Cand Two"] # cached after first lookup + + +def test_similar_artists_skips_unresolvable_row(): + payload = {"similarartists": {"artist": [ + {"name": "Unknown Artist", "mbid": "", "match": "0.5"}, + {"name": "Cand One", "mbid": "c1", "match": "0.9"}, + ]}} + browser = _FakeBrowser(hits={}) # no hits for "Unknown Artist" + with patch("lyra_worker.similarity._lastfm.requests.get", return_value=_resp(payload)): + out = LastfmSource(api_key="k", browser=browser).similar_artists("seed") + assert out == [SimilarArtist("c1", "Cand One", 0.9)] + + +def test_similar_artists_skips_self(): + payload = {"similarartists": {"artist": [ + {"name": "Seed", "mbid": "seed", "match": "1.0"}, + {"name": "Cand One", "mbid": "c1", "match": "0.9"}, + ]}} + browser = _FakeBrowser() + with patch("lyra_worker.similarity._lastfm.requests.get", return_value=_resp(payload)): + out = LastfmSource(api_key="k", browser=browser).similar_artists("seed") + assert out == [SimilarArtist("c1", "Cand One", 0.9)] + + +def test_similar_artists_coerces_single_object(): + payload = {"similarartists": {"artist": {"name": "Cand One", "mbid": "c1", "match": "0.9"}}} + browser = _FakeBrowser() + with patch("lyra_worker.similarity._lastfm.requests.get", return_value=_resp(payload)): + out = LastfmSource(api_key="k", browser=browser).similar_artists("seed") + assert out == [SimilarArtist("c1", "Cand One", 0.9)] + + +def test_similar_artists_error_envelope_returns_empty_list(): + payload = {"error": 6, "message": "no artist"} + browser = _FakeBrowser() + with patch("lyra_worker.similarity._lastfm.requests.get", return_value=_resp(payload)): + out = LastfmSource(api_key="k", browser=browser).similar_artists("seed") + assert out == [] + + +def test_health_true_on_normal_response(): + payload = {"similarartists": {"artist": [{"name": "Radiohead", "mbid": "x", "match": "1.0"}]}} + with patch("lyra_worker.similarity._lastfm.requests.get", return_value=_resp(payload)): + assert LastfmSource(api_key="k", browser=_FakeBrowser()).health() is True + + +def test_health_false_on_error_envelope(): + payload = {"error": 10, "message": "invalid api key"} + with patch("lyra_worker.similarity._lastfm.requests.get", return_value=_resp(payload)): + assert LastfmSource(api_key="k", browser=_FakeBrowser()).health() is False + + +def test_health_false_on_request_exception(): + with patch("lyra_worker.similarity._lastfm.requests.get", + side_effect=requests.ConnectionError("down")): + assert LastfmSource(api_key="k", browser=_FakeBrowser()).health() is False diff --git a/worker/tests/test_similarity_registry.py b/worker/tests/test_similarity_registry.py index cb805e9..737b158 100644 --- a/worker/tests/test_similarity_registry.py +++ b/worker/tests/test_similarity_registry.py @@ -1,4 +1,5 @@ from lyra_worker.registry import build_similarity_sources +from lyra_worker.similarity._lastfm import LastfmSource from lyra_worker.similarity._listenbrainz import ListenBrainzSource @@ -12,3 +13,17 @@ def test_build_returns_listenbrainz_by_default(): def test_base_url_override_from_config(): src = build_similarity_sources({"discover.listenBrainzUrl": "http://lb.local"})[0] assert src._base == "http://lb.local" + + +def test_lastfm_added_when_api_key_configured(): + sources = build_similarity_sources({"lastfm.api_key": "secret-key"}) + names = [s.name for s in sources] + assert names == ["listenbrainz", "lastfm"] + lastfm = next(s for s in sources if s.name == "lastfm") + assert isinstance(lastfm, LastfmSource) + assert lastfm._key == "secret-key" + + +def test_lastfm_omitted_without_api_key(): + sources = build_similarity_sources({}) + assert all(s.name != "lastfm" for s in sources)