Jonathan
248 lines
13 KiB
Markdown
248 lines
13 KiB
Markdown
# 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. ✓
|