docs: implementation plan for Discover unified rows

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-14 23:33:04 +02:00
parent 9dd235c2bc
commit e74d1d9c7b
1 changed files with 247 additions and 0 deletions
@@ -0,0 +1,247 @@
+# Discover Unified Rows + Newest/Most-Popular Albums — Implementation Plan
+
+> **For agentic workers:** REQUIRED SUB-SKILL: superpowers:subagent-driven-development or superpowers:executing-plans. Steps use checkbox (`- [ ]`) syntax.
+
+**Goal:** Collapse the Discover Artists|Albums tabs into one artist-centric row that shows each similar artist's newest AND most-popular album inline (album-click → tracklist), with the artist name linking to the full page.
+
+**Architecture:** Worker derives two albums per surfaced artist (newest = existing; most-popular = Last.fm `artist.getTopAlbums` matched into the browsed MB discography), tagging each `DiscoverySuggestion.albumReason`. The `/api/discover` route groups pending suggestions by artist into rows carrying `albums[]` + `seeds[]`. The client renders one row per artist with album thumbnails.
+
+**Tech Stack:** Python 3 + psycopg (worker), Next.js 15 App Router + Prisma + vitest (web), Playwright (verify).
+
+## Global Constraints
+
+- Popularity source is Last.fm `artist.getTopAlbums`; **graceful fallback to newest-only** when no `lastfm.api_key` or no match. No new required config.
+- `albumReason` values are exactly `"newest"`, `"popular"`, `"newest,popular"`; badge labels `Newest` / `Most played` / both; `null` → no badge.
+- Never break the sweep: any Last.fm/popularity error → treat as no popular pick.
+- Deploy is `docker compose up -d --build web worker` (never `down -v`). Tests: worker `lyra_test`, web `lyra_test`.
+- Reverts the D2 tab bar (intentional).
+
+---
+
+### Task 1: `albumReason` column + migration
+
+**Files:**
+- Modify: `web/prisma/schema.prisma` (DiscoverySuggestion model)
+- Create: `web/prisma/migrations/<ts>_add_discovery_album_reason/migration.sql` (via prisma)
+
+- [ ] **Step 1:** Add `albumReason String?` to `model DiscoverySuggestion` with a comment (nullable, backward compatible; null → no badge).
+- [ ] **Step 2:** `cd web && DATABASE_URL=postgresql://lyra:lyra@localhost:5432/lyra_test npx prisma migrate dev --name add_discovery_album_reason`. Expected: "Applying migration … add_discovery_album_reason".
+- [ ] **Step 3:** Commit (`feat(discover): add DiscoverySuggestion.albumReason`).
+
+---
+
+### Task 2: worker Last.fm album-popularity helper
+
+**Files:**
+- Create: `worker/lyra_worker/similarity/_lastfm_albums.py`
+- Modify: `worker/lyra_worker/registry.py` (add `build_album_popularity`)
+- Test: `worker/tests/test_lastfm_albums.py`
+
+**Interfaces:**
+- Produces: `LastfmAlbumPopularity(api_key, base_url=_BASE, timeout=15.0)` with
+  `top_albums(conn, artist_name) -> list[[name:str, mbid:str|None, playcount:int]]`
+  (playcount desc; cached in ApiCache key `lfm-artist-top-albums:{norm}`, 12h). Errors → `[]`.
+- Produces: `build_album_popularity(config, dsn) -> LastfmAlbumPopularity | None`
+  (None when `lastfm.api_key` unset).
+
+- [ ] **Step 1: failing test** — parse a canned `artist.gettopalbums` JSON body into
+  `[[name, mbid, playcount], …]` ordered by playcount desc; a `dict` (single album) is
+  wrapped; missing mbid → None; a body with `error` → `[]`. Inject the parse via a helper
+  `_parse_top_albums(data)` so no network is needed. Also assert `build_album_popularity({}, None) is None`.
+- [ ] **Step 2:** run → FAIL (module missing).
+- [ ] **Step 3: implement.**
+
+```python
+# _lastfm_albums.py
+import requests
+from lyra_worker.apicache import cached_api
+
+_BASE = "https://ws.audioscrobbler.com/2.0/"
+
+
+def _parse_top_albums(data) -> list[list]:
+    if isinstance(data.get("error"), (int, float)):
+        return []
+    rows = data.get("topalbums", {}).get("album", [])
+    if isinstance(rows, dict):
+        rows = [rows]
+    out = []
+    for r in rows:
+        name = r.get("name") or ""
+        if not name:
+            continue
+        out.append([name, r.get("mbid") or None, int(r.get("playcount") or 0)])
+    out.sort(key=lambda a: a[2], reverse=True)
+    return out
+
+
+class LastfmAlbumPopularity:
+    def __init__(self, api_key, base_url=_BASE, timeout: float = 15.0, limit: int = 25):
+        self._key, self._base, self._timeout, self._limit = api_key, base_url, timeout, limit
+
+    def _fetch(self, artist_name: str) -> list[list]:
+        try:
+            res = requests.get(self._base, params={
+                "method": "artist.gettopalbums", "artist": artist_name,
+                "api_key": self._key, "format": "json", "limit": str(self._limit),
+                "autocorrect": "1",
+            }, timeout=self._timeout)
+            return _parse_top_albums(res.json())
+        except Exception:
+            return []
+
+    def top_albums(self, conn, artist_name: str) -> list[list]:
+        norm = (artist_name or "").strip().lower()
+        if not norm:
+            return []
+        return cached_api(conn, f"lfm-artist-top-albums:{norm}", 43200,
+                          lambda: self._fetch(artist_name))
+```
+
+```python
+# registry.py — add
+from lyra_worker.similarity._lastfm_albums import LastfmAlbumPopularity
+
+def build_album_popularity(config: dict, dsn: str | None = None):
+    key = config.get("lastfm.api_key")
+    return LastfmAlbumPopularity(api_key=key) if key else None
+```
+
+- [ ] **Step 4:** run → PASS.
+- [ ] **Step 5:** commit (`feat(discover): Last.fm album-popularity helper`).
+
+---
+
+### Task 3: `_derive_albums` picks newest + most-popular
+
+**Files:**
+- Modify: `worker/lyra_worker/discovery.py` (`_derive_albums`, `_upsert_album`, `run_discovery`)
+- Test: `worker/tests/test_discovery_albums.py`
+
+**Interfaces:**
+- Consumes: `popularity.top_albums(conn, name)` from Task 2 (or None).
+- Produces: `_derive_albums(conn, browser, surfaced, cfg, popularity=None)`; `run_discovery(conn, sources, browser, cfg, popularity=None)`; `_upsert_album(conn, artist, rg, reason)`.
+
+- [ ] **Step 1: failing tests** (inject a fake popularity — no live Last.fm):
+  - newest + most-popular → two album rows, `albumReason` `"newest"` / `"popular"`.
+  - popular pick == newest rg → one row, `albumReason == "newest,popular"`.
+  - `popularity=None` → newest-only, `albumReason == "newest"`.
+  - popular match resolves by title when Last.fm mbid absent; skips owned/non-qualifying and falls to next-highest playcount; no match → newest-only.
+
+```python
+class FakePopularity:
+    def __init__(self, data): self._d = data          # {artist_name: [[name, mbid, plays], …]}
+    def top_albums(self, conn, name): return self._d.get(name, [])
+```
+
+- [ ] **Step 2:** run → FAIL.
+- [ ] **Step 3: implement.** In `_derive_albums`, after building `core` (qualifying un-owned, newest-first):
+
+```python
+def _norm(s): return (s or "").strip().lower()
+
+# newest picks (existing)
+picks = {}  # rg_mbid -> reason
+for rg in core[: cfg.albums_per_artist]:
+    picks[rg.rg_mbid] = "newest"
+
+# most-popular pick via Last.fm, matched into `core`
+if popularity is not None and core:
+    by_mbid = {g.rg_mbid: g for g in core}
+    by_title = {_norm(g.title): g for g in core}
+    for name, mbid, _plays in popularity.top_albums(conn, artist["name"]):
+        g = (by_mbid.get(mbid) if mbid else None) or by_title.get(_norm(name))
+        if g is None:
+            continue
+        picks[g.rg_mbid] = "newest,popular" if picks.get(g.rg_mbid) == "newest" else "popular"
+        break
+
+for rg in core:
+    if rg.rg_mbid in picks:
+        albums += _upsert_album(conn, artist, rg, picks[rg.rg_mbid])
+        have.add(rg.rg_mbid)
+```
+
+Add `reason` param to `_upsert_album` → include `"albumReason"` in the INSERT column list +
+`ON CONFLICT DO UPDATE SET … "albumReason" = EXCLUDED."albumReason"`. Thread `popularity`
+through `run_discovery` → `_derive_albums`.
+
+- [ ] **Step 4:** run → PASS; then full worker suite green.
+- [ ] **Step 5:** commit (`feat(discover): derive newest + most-popular album per artist`).
+
+---
+
+### Task 4: wire popularity into the worker loop
+
+**Files:**
+- Modify: `worker/lyra_worker/main.py` (`_run_discovery` builds + passes popularity)
+- Test: existing `test_discovery_trigger.py` still green (popularity defaults None there).
+
+- [ ] **Step 1:** In `main.py`, import `build_album_popularity`; in `_run_discovery` build it from `get_config(conn)` and pass to `run_discovery(conn, sources, browser, cfg, popularity=pop)`.
+- [ ] **Step 2:** run worker suite → PASS.
+- [ ] **Step 3:** commit (`feat(discover): supply Last.fm popularity to the sweep`).
+
+---
+
+### Task 5: `/api/discover` groups by artist
+
+**Files:**
+- Modify: `web/src/app/api/discover/route.ts`
+- Test: `web/src/app/api/discover/route.test.ts`
+
+**Interfaces:**
+- Produces: `GET` returns `{ rows: [{ id, artistMbid, artistName, score, seedCount, sources, seeds, albums: [{ id, rgMbid, album, primaryType, secondaryTypes, firstReleaseDate, albumReason }] }], search? }`. Rows ordered by score desc.
+
+- [ ] **Step 1: failing test** — seed pending artist + its two album suggestions (`albumReason` newest/popular) sharing `artistMbid`; assert one row with `albums.length === 2`, ordered; seeds present; an album whose artist has no pending artist-suggestion still yields a row.
+- [ ] **Step 2:** run → FAIL.
+- [ ] **Step 3: implement.** Fetch all pending suggestions (as today) + the seeds map (as today). Group by `artistMbid`: artist-kind rows seed the row header (id/score/seedCount/sources); album-kind rows push into `albums[]` (include `albumReason`). For an `artistMbid` with only album rows, synthesize a header from the album's `artistName`/`artistMbid` (no `id`, score = max album score). Sort rows by score desc; sort each `albums[]` by `firstReleaseDate` desc. Return `{ rows }`.
+- [ ] **Step 4:** run → PASS.
+- [ ] **Step 5:** commit (`feat(discover): group suggestions into artist rows`).
+
+---
+
+### Task 6: dismiss cascade
+
+**Files:**
+- Modify: `web/src/app/api/discover/[id]/route.ts`
+- Test: `web/src/app/api/discover/[id]/route.test.ts`
+
+- [ ] **Step 1: failing test** — dismissing an artist suggestion also marks its pending album suggestions (same `artistMbid`) dismissed; a dismissed album is not returned by `/api/discover`.
+- [ ] **Step 2:** run → FAIL.
+- [ ] **Step 3: implement.** In the dismiss branch, after dismissing the artist suggestion, `updateMany({ where: { kind: "album", artistMbid, status: "pending" }, data: { status: "dismissed" } })`. Follow/Want unchanged.
+- [ ] **Step 4:** run → PASS.
+- [ ] **Step 5:** commit (`fix(discover): dismissing an artist also dismisses its albums`).
+
+---
+
+### Task 7: unified-row client (drop tabs + ArtistModal)
+
+**Files:**
+- Modify: `web/src/app/discover/discover-client.tsx`
+- Modify: `web/src/app/design.css` (row + thumb strip + badge styles)
+
+- [ ] **Step 1:** Replace `Feed`/`tab` state with `rows` from `/api/discover` (`{ rows }`). Remove the tab bar, `ArtistModal` import + `artistModal` state + its JSX + the `isFullAlbum`/albums-tab code.
+- [ ] **Step 2:** Render one `<li class="list-row disco-row">` per row:
+  - artist block: `<a href={`/discover/artist/${row.artistMbid}`}>{row.artistName}</a>`, `score`, `similarTo(row.seeds)` (keep existing helper).
+  - album strip: `row.albums.map` → a `<button class="disco-thumb" onClick={() => setAlbumModal(album with artistMbid=row.artistMbid)}>`: `<CoverArt>` + title + badge (`badgeFor(albumReason)`) + a `Want` button (`act(album.id, "want")`).
+  - row actions: `Follow` (`act(row.id, "follow")` when `row.id`, else follow-by-mbid via `POST /api/artists`), `Dismiss` (`act(row.id, "dismiss")`, hidden when no `row.id`).
+  - `badgeFor(r)`: `newest`→`Newest`, `popular`→`Most played`, `newest,popular`→`Newest · Most played`, null→none.
+- [ ] **Step 3:** keep `AlbumModal` (thumb click) + `find-similar` search block (its result artist links become `<a href>` to the full page, no modal). Keep Discover-now/freshness header.
+- [ ] **Step 4:** CSS: `.disco-row` flex (artist block | thumb strip | actions), `.disco-thumb` (small cover + title + `.badge`), wraps under artist on narrow widths.
+- [ ] **Step 5:** `npx tsc --noEmit` + `npm test` + `npm run build` → all green.
+- [ ] **Step 6:** commit (`feat(discover): unified artist rows with inline album thumbnails`).
+
+---
+
+### Task 8: deploy + live verify
+
+- [ ] **Step 1:** `docker compose up -d --build web worker`; confirm migration applied + all healthy.
+- [ ] **Step 2:** Playwright on `/discover`: rows render; artist name navigates to `/discover/artist/[mbid]`; album thumbnail opens `AlbumModal` with a tracklist; badges show.
+- [ ] **Step 3:** Trigger a library **Scan** (populates artistMbid) then **Discover now**; confirm `albumReason` populates (`Most played` badges appear) once a fresh sweep runs. (If a full sweep is impractical to force cheaply, rely on the injected-data Playwright check + unit tests, and note it.)
+- [ ] **Step 4:** Update `MEMORY.md` / project-state with the shipped feature + commit range.
+
+## Self-Review
+
+- **Spec coverage:** Part A → Tasks 2–4; Part B → Task 5; Part C dismiss → Task 6, client/modal → Task 7; schema → Task 1; tests throughout; live verify → Task 8. ✓
+- **Placeholders:** none — code shown for each implementing step. ✓
+- **Type consistency:** `top_albums(conn, name) -> list[[name, mbid|None, plays]]` used identically in Tasks 2–3; `_upsert_album(…, reason)` and `_derive_albums(…, popularity=None)` consistent; route emits `rows[]` consumed by Task 7. ✓