From 4a768b7122c55c3dbab6389d948d3268b472c171 Mon Sep 17 00:00:00 2001 From: Jonathan Date: Wed, 15 Jul 2026 00:38:56 +0200 Subject: [PATCH] fix(discover): drop suggestions for non-artists (no core discography) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ListenBrainz surfaces producers/session players/band members (Max Martin, Dominic Howard) that co-occur in listens but have no releases of their own — they showed as bare rows with an empty page. _derive_albums now drops a surfaced artist (deletes their pending suggestion) when their MB discography has no core release-group (Album/Single/EP, no secondary type), catching both empty discographies and soundtrack/production-only credits, while keeping real singles/EP-only artists. On browse failure the suggestion is kept. Test fake browser gains default_core so scoring tests keep surfacing candidates. worker 249 tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- worker/lyra_worker/adapters/fakes.py | 13 +++++++-- worker/lyra_worker/discovery.py | 10 +++++++ worker/tests/test_discovery.py | 42 +++++++++++++-------------- worker/tests/test_discovery_albums.py | 29 ++++++++++++++++++ 4 files changed, 70 insertions(+), 24 deletions(-) diff --git a/worker/lyra_worker/adapters/fakes.py b/worker/lyra_worker/adapters/fakes.py index a97072b..fc815d3 100644 --- a/worker/lyra_worker/adapters/fakes.py +++ b/worker/lyra_worker/adapters/fakes.py @@ -72,17 +72,24 @@ class FailingAdapter(_BaseFake): class FakeMbBrowser: - """In-memory MbBrowser for tests. `releases` maps artist mbid -> [ReleaseGroupInfo].""" + """In-memory MbBrowser for tests. `releases` maps artist mbid -> [ReleaseGroupInfo]. + `default_core=True` returns a synthetic core Album for any artist not in `releases`, so + scoring tests don't trip the "no core discography → drop the suggestion" filter.""" - def __init__(self, artists=None, releases=None): + def __init__(self, artists=None, releases=None, default_core=False): self._artists = list(artists or []) self._releases = dict(releases or {}) + self._default_core = default_core def search_artist(self, name: str) -> list[ArtistHit]: return list(self._artists) def browse_release_groups(self, artist_mbid: str) -> list[ReleaseGroupInfo]: - return list(self._releases.get(artist_mbid, [])) + if artist_mbid in self._releases: + return list(self._releases[artist_mbid]) + if self._default_core: + return [ReleaseGroupInfo(f"rg-{artist_mbid}", "Album", "Album", (), "2000-01-01")] + return [] class FakeAudioProbe: diff --git a/worker/lyra_worker/discovery.py b/worker/lyra_worker/discovery.py index 0f1397e..4ebbabf 100644 --- a/worker/lyra_worker/discovery.py +++ b/worker/lyra_worker/discovery.py @@ -204,6 +204,16 @@ def _derive_albums(conn: psycopg.Connection, browser: MbBrowser, surfaced: list[ except Exception as e: # one artist's browse failure must not abort the sweep print(f"worker: discovery album browse failed for {artist['mbid']}: {e}", flush=True) continue + # Drop suggestions for "artists" with no real discography of their own — producers, + # session players, band members, soundtrack-only credits. ListenBrainz surfaces them + # (they co-occur in listens) but they have no core release-group to follow or want, and + # their page is empty. Require at least one core release (Album/Single/EP, no secondary + # type); on browse failure we keep the suggestion rather than drop a real artist. + if not any(is_core_release(g.primary_type, g.secondary_types) for g in groups): + with conn.cursor() as cur: + cur.execute("DELETE FROM \"DiscoverySuggestion\" WHERE status = 'pending' " + 'AND "artistMbid" = %s', (artist["mbid"],)) + continue core = [g for g in groups if _album_kind_ok(g, cfg.albums_only) and g.rg_mbid not in have] core.sort(key=lambda g: g.first_release_date or "", reverse=True) diff --git a/worker/tests/test_discovery.py b/worker/tests/test_discovery.py index cd9e01e..5c77347 100644 --- a/worker/tests/test_discovery.py +++ b/worker/tests/test_discovery.py @@ -26,7 +26,7 @@ def test_seeds_discovery_from_owned_album_artists(conn): # An artist you own an album by but don't follow becomes a discovery seed. _insert_library_item(conn, "Owned Artist", "lib-seed") src = FakeSimilaritySource(similar={"lib-seed": [SimilarArtist("c1", "New Find", 0.9)]}) - result = run_discovery(conn, [src], FakeMbBrowser(), DiscoveryConfig()) + result = run_discovery(conn, [src], FakeMbBrowser(default_core=True), DiscoveryConfig()) assert result.seeds == 1 assert [r[0] for r in _artist_rows(conn)] == ["c1"] # surfaced from the library seed with conn.cursor() as cur: # library seed throttled via DiscoverySeed, not WatchedArtist @@ -37,7 +37,7 @@ def test_seeds_discovery_from_owned_album_artists(conn): def test_library_seeding_can_be_disabled(conn): _insert_library_item(conn, "Owned Artist", "lib-seed") src = FakeSimilaritySource(similar={"lib-seed": [SimilarArtist("c1", "New Find", 0.9)]}) - result = run_discovery(conn, [src], FakeMbBrowser(), DiscoveryConfig(seed_from_library=False)) + result = run_discovery(conn, [src], FakeMbBrowser(default_core=True), DiscoveryConfig(seed_from_library=False)) assert result.seeds == 0 and _artist_rows(conn) == [] # library not seeded @@ -49,7 +49,7 @@ def test_owned_artist_is_not_suggested_as_a_candidate(conn): "s1": [SimilarArtist("c1", "Owned", 0.9), SimilarArtist("c2", "New", 0.8)], "c1": [], # the library seed itself finds nothing new here }) - run_discovery(conn, [src], FakeMbBrowser(), DiscoveryConfig()) + run_discovery(conn, [src], FakeMbBrowser(default_core=True), DiscoveryConfig()) assert [r[0] for r in _artist_rows(conn)] == ["c2"] # c1 excluded (already owned) @@ -60,7 +60,7 @@ def test_aggregates_scores_across_seeds(conn): "s1": [SimilarArtist("c1", "Shared", 0.6), SimilarArtist("c2", "Only1", 0.4)], "s2": [SimilarArtist("c1", "Shared", 0.5)], }) - result = run_discovery(conn, [src], FakeMbBrowser(), DiscoveryConfig()) + result = run_discovery(conn, [src], FakeMbBrowser(default_core=True), DiscoveryConfig()) assert result.artists == 2 rows = _artist_rows(conn) # c1 recommended by both seeds; each seed max-normalizes its top score to 1.0, so @@ -81,7 +81,7 @@ def test_normalizes_across_sources_of_different_scales(conn): lfm = FakeSimilaritySource(similar={"s1": [ SimilarArtist("c_lfm", "LFM Only", 0.9), SimilarArtist("c_shared", "Shared", 0.5)]}) lfm.name = "lastfm" - run_discovery(conn, [lb, lfm], FakeMbBrowser(), DiscoveryConfig()) + run_discovery(conn, [lb, lfm], FakeMbBrowser(default_core=True), DiscoveryConfig()) lb_only = _artist_score(conn, "c_lb")[0] # 5000/5000 = 1.0 lfm_only = _artist_score(conn, "c_lfm")[0] # 0.9/0.9 = 1.0 @@ -95,7 +95,7 @@ def test_skips_already_followed_artists(conn): insert_watched_artist(conn, mbid="c1", name="Already Followed") src = FakeSimilaritySource(similar={"s1": [SimilarArtist("c1", "X", 0.9), SimilarArtist("c2", "New", 0.8)]}) - run_discovery(conn, [src], FakeMbBrowser(), DiscoveryConfig()) + run_discovery(conn, [src], FakeMbBrowser(default_core=True), DiscoveryConfig()) assert [r[0] for r in _artist_rows(conn)] == ["c2"] # c1 filtered (followed) @@ -103,7 +103,7 @@ def test_min_score_filters_weak_candidates(conn): insert_watched_artist(conn, mbid="s1", name="Seed") src = FakeSimilaritySource(similar={"s1": [SimilarArtist("c1", "Strong", 0.9), SimilarArtist("c2", "Weak", 0.1)]}) - run_discovery(conn, [src], FakeMbBrowser(), DiscoveryConfig(min_score=0.5)) + run_discovery(conn, [src], FakeMbBrowser(default_core=True), DiscoveryConfig(min_score=0.5)) assert [r[0] for r in _artist_rows(conn)] == ["c1"] @@ -111,7 +111,7 @@ def test_dismissed_suggestion_is_never_revived(conn): insert_watched_artist(conn, mbid="s1", name="Seed") insert_discovery_suggestion(conn, kind="artist", artist_mbid="c1", status="dismissed", score=0.0) src = FakeSimilaritySource(similar={"s1": [SimilarArtist("c1", "X", 0.9)]}) - run_discovery(conn, [src], FakeMbBrowser(), DiscoveryConfig()) + run_discovery(conn, [src], FakeMbBrowser(default_core=True), DiscoveryConfig()) with conn.cursor() as cur: cur.execute('SELECT status::text, score FROM "DiscoverySuggestion" WHERE "artistMbid" = %s', ("c1",)) assert cur.fetchone() == ("dismissed", 0.0) # untouched — WHERE status='pending' blocked the update @@ -120,18 +120,18 @@ def test_dismissed_suggestion_is_never_revived(conn): def test_stamps_last_discovered_and_throttles(conn): insert_watched_artist(conn, mbid="s1", name="Seed") src = FakeSimilaritySource(similar={"s1": [SimilarArtist("c1", "X", 0.9)]}) - assert run_discovery(conn, [src], FakeMbBrowser(), DiscoveryConfig()).artists == 1 + assert run_discovery(conn, [src], FakeMbBrowser(default_core=True), DiscoveryConfig()).artists == 1 with conn.cursor() as cur: cur.execute('SELECT "lastDiscoveredAt" IS NOT NULL FROM "WatchedArtist" WHERE mbid = %s', ("s1",)) assert cur.fetchone()[0] is True # second run: seed not due (lastDiscoveredAt recent) -> no seeds -> nothing new - assert run_discovery(conn, [src], FakeMbBrowser(), DiscoveryConfig()).artists == 0 + assert run_discovery(conn, [src], FakeMbBrowser(default_core=True), DiscoveryConfig()).artists == 0 def test_unhealthy_source_contributes_nothing(conn): insert_watched_artist(conn, mbid="s1", name="Seed") src = FakeSimilaritySource(similar={"s1": [SimilarArtist("c1", "X", 0.9)]}, healthy=False) - assert run_discovery(conn, [src], FakeMbBrowser(), DiscoveryConfig()).artists == 0 + assert run_discovery(conn, [src], FakeMbBrowser(default_core=True), DiscoveryConfig()).artists == 0 assert _artist_rows(conn) == [] @@ -146,7 +146,7 @@ def test_source_with_raising_health_is_skipped_not_fatal(conn): return [] good = FakeSimilaritySource(similar={"s1": [SimilarArtist("c1", "X", 0.9)]}) - result = run_discovery(conn, [BoomSource(), good], FakeMbBrowser(), DiscoveryConfig()) + result = run_discovery(conn, [BoomSource(), good], FakeMbBrowser(default_core=True), DiscoveryConfig()) assert result.artists == 1 # good source still processed; boom skipped, no crash @@ -176,8 +176,8 @@ def test_score_sums_across_separate_chunks(conn): "s2": [SimilarArtist("c1", "Shared", 0.5)], }) cfg = DiscoveryConfig(chunk_size=1) - run_discovery(conn, [src], FakeMbBrowser(), cfg) # processes s1 (older) - run_discovery(conn, [src], FakeMbBrowser(), cfg) # processes s2 + run_discovery(conn, [src], FakeMbBrowser(default_core=True), cfg) # processes s1 (older) + run_discovery(conn, [src], FakeMbBrowser(default_core=True), cfg) # processes s2 # replace-not-accumulate would leave 0.5/seedCount1; contributions give the sum. assert _artist_score(conn, "c1") == (2.0, 2) # each seed max-normalizes to 1.0 @@ -193,7 +193,7 @@ def test_partial_sweep_preserves_other_seeds_contribution(conn): "s1": [SimilarArtist("c1", "Shared", 0.6)], "s2": [SimilarArtist("c1", "Shared", 0.5)], }) - run_discovery(conn, [src], FakeMbBrowser(), DiscoveryConfig()) # both -> 1.1/2 + run_discovery(conn, [src], FakeMbBrowser(default_core=True), DiscoveryConfig()) # both -> 1.1/2 assert _artist_score(conn, "c1") == (2.0, 2) # each seed max-normalizes to 1.0 # s1 due again (older than interval), s2 fresh; s1 now scores c1 at 0.7. @@ -202,7 +202,7 @@ def test_partial_sweep_preserves_other_seeds_contribution(conn): cur.execute('UPDATE "WatchedArtist" SET "lastDiscoveredAt" = now() WHERE mbid = %s', ("s2",)) conn.commit() src2 = FakeSimilaritySource(similar={"s1": [SimilarArtist("c1", "Shared", 0.7)]}) - run_discovery(conn, [src2], FakeMbBrowser(), DiscoveryConfig()) + run_discovery(conn, [src2], FakeMbBrowser(default_core=True), DiscoveryConfig()) assert _artist_score(conn, "c1") == (2.0, 2) # 1.0 (new s1) + 1.0 (kept s2), max-normalized @@ -215,7 +215,7 @@ def test_seed_dropoff_lowers_seedcount(conn): "s1": [SimilarArtist("c1", "Shared", 0.6)], "s2": [SimilarArtist("c1", "Shared", 0.5)], }) - run_discovery(conn, [src], FakeMbBrowser(), DiscoveryConfig()) + run_discovery(conn, [src], FakeMbBrowser(default_core=True), DiscoveryConfig()) assert _artist_score(conn, "c1") == (2.0, 2) # each seed max-normalizes to 1.0 # s1 re-swept but no longer finds c1 -> its contribution is removed, c1 recomputed. @@ -224,7 +224,7 @@ def test_seed_dropoff_lowers_seedcount(conn): cur.execute('UPDATE "WatchedArtist" SET "lastDiscoveredAt" = now() WHERE mbid = %s', ("s2",)) conn.commit() src2 = FakeSimilaritySource(similar={"s1": [SimilarArtist("c9", "Other", 0.9)]}) - run_discovery(conn, [src2], FakeMbBrowser(), DiscoveryConfig()) + run_discovery(conn, [src2], FakeMbBrowser(default_core=True), DiscoveryConfig()) assert _artist_score(conn, "c1") == (1.0, 1) # only s2 remains (max-normalized to 1.0) @@ -241,7 +241,7 @@ def test_evicts_contributions_from_removed_seed(conn): "s1": [SimilarArtist("c1", "Shared", 0.6)], "s2": [SimilarArtist("c1", "Shared", 0.5)], }) - run_discovery(conn, [src], FakeMbBrowser(), DiscoveryConfig()) + run_discovery(conn, [src], FakeMbBrowser(default_core=True), DiscoveryConfig()) assert _artist_score(conn, "c1") == (2.0, 2) # each seed max-normalizes to 1.0 # Unfollow s1 (no FK cascade on DiscoverySeedContribution). @@ -254,7 +254,7 @@ def test_evicts_contributions_from_removed_seed(conn): cur.execute('UPDATE "WatchedArtist" SET "lastDiscoveredAt" = now() - interval \'200 hours\' WHERE mbid = %s', ("s2",)) conn.commit() src2 = FakeSimilaritySource(similar={"s2": [SimilarArtist("c1", "Shared", 0.5)]}) - run_discovery(conn, [src2], FakeMbBrowser(), DiscoveryConfig()) + run_discovery(conn, [src2], FakeMbBrowser(default_core=True), DiscoveryConfig()) assert _artist_score(conn, "c1") == (1.0, 1) # s1's stale contribution evicted; s2 alone -> 1.0 with conn.cursor() as cur: @@ -271,7 +271,7 @@ def test_run_discovery_processes_at_most_chunk_size_seeds(conn): insert_watched_artist(conn, mbid=f"s{i}", name=f"Seed{i}") src = FakeSimilaritySource(similar={f"s{i}": [SimilarArtist(f"c{i}", f"Cand{i}", 0.9)] for i in range(4)}) cfg = DiscoveryConfig.from_config({"discover.chunkSize": "2"}) - result = run_discovery(conn, [src], FakeMbBrowser(), cfg) + result = run_discovery(conn, [src], FakeMbBrowser(default_core=True), cfg) assert result.seeds == 2 # only chunk_size seeds this call # exactly 2 seeds now have lastDiscoveredAt set with conn.cursor() as cur: diff --git a/worker/tests/test_discovery_albums.py b/worker/tests/test_discovery_albums.py index 7e5dddb..732ad40 100644 --- a/worker/tests/test_discovery_albums.py +++ b/worker/tests/test_discovery_albums.py @@ -12,6 +12,13 @@ def _album_rows(conn): return cur.fetchall() +def _artist_rows(conn): + with conn.cursor() as cur: + cur.execute("SELECT \"artistMbid\" FROM \"DiscoverySuggestion\" " + "WHERE kind = 'artist' AND status = 'pending' ORDER BY \"artistMbid\"") + return cur.fetchall() + + def _reasons(conn): with conn.cursor() as cur: cur.execute('SELECT "rgMbid", "albumReason" FROM "DiscoverySuggestion" ' @@ -120,6 +127,28 @@ def test_rederiving_keeps_wanted_album_suggestions(conn): assert cur.fetchone()[0] == 1 +def test_drops_artist_with_no_core_discography(conn): + # Two candidates: one real (has a core album), one a "producer" whose only release is a + # secondary-typed soundtrack (like Max Martin's & Juliet) — the latter must be dropped. + insert_watched_artist(conn, mbid="s1", name="Seed") + src = FakeSimilaritySource(similar={"s1": [ + SimilarArtist("real", "Real Artist", 0.9), SimilarArtist("producer", "Producer", 0.8)]}) + browser = FakeMbBrowser(releases={ + "real": [ReleaseGroupInfo("rg-r", "Record", "Album", (), "2020-01-01")], + "producer": [ReleaseGroupInfo("rg-st", "A Musical", "Album", ("Soundtrack",), "2019-01-01")], + }) + run_discovery(conn, [src], browser, DiscoveryConfig(albums_per_artist=1)) + assert [r[0] for r in _artist_rows(conn)] == ["real"] # producer dropped + + +def test_drops_artist_with_empty_discography(conn): + insert_watched_artist(conn, mbid="s1", name="Seed") + src = FakeSimilaritySource(similar={"s1": [SimilarArtist("member", "Band Member", 0.9)]}) + browser = FakeMbBrowser(releases={"member": []}) # no release groups at all + run_discovery(conn, [src], browser, DiscoveryConfig(albums_per_artist=1)) + assert _artist_rows(conn) == [] + + def test_no_popularity_provider_yields_newest_only(conn): insert_watched_artist(conn, mbid="s1", name="Seed") src = FakeSimilaritySource(similar={"s1": [SimilarArtist("c1", "Cand", 0.9)]})