Lyra/docs/superpowers/specs/2026-07-13-cleanup-batch-design.md

# Cleanup batch — Tier A (loose ends) + Tier B (design tradeoffs)

**Date:** 2026-07-13
**Status:** design approved, pending spec review
**Branch:** `cleanup/tier-a-b`

## Goal

Close the deferred backlog down to zero *fixes* so the codebase has no known loose
ends before we brainstorm net-new features (Tier C). This batch covers the clear
hardening/polish items (Tier A) and the two benign-but-real design tradeoffs the
user chose to fix properly (Tier B). Tier C (Last.fm source, crash-resume
mid-pipeline, per-track duration matching, UI error states) is explicitly **out of
scope** here — it goes to the feature brainstorm next.

Non-goals: no behavior change to the acquisition pipeline, no new user-facing
features, no schema change beyond the two additive tables below.

---

## Tier A — loose ends (web, mechanical)

### A1. PATCH allowlist hardening
Both `web/src/app/api/monitor/config/route.ts` and
`web/src/app/api/discover/config/route.ts` PATCH handlers currently gate writes on
`k in DEFAULTS`, which walks the prototype chain. The DELETE `/api/config` handler
was already hardened to `Object.prototype.hasOwnProperty.call(DEFAULTS, k)`; apply
the same own-property check to both PATCH routes. This only ever prevented writing
harmless junk `Config` rows (neither PATCH deletes), so this is consistency
hardening, not a live vulnerability — but it removes the last prototype-chain gap.

### A2. a11y + copy
- Add an `aria-label` to the Monitor tab `enabled` toggle (it currently has none;
  every other control on the page is labelled and web tests key off labels).
- Update the Settings page eyebrow at `web/src/app/settings/settings-form.tsx:159`
  from `"Credentials · library · discovery"` to cover all five tabs, e.g.
  `"Credentials · library · monitor · discovery"`.

### A3. DELETE allowlist test
Add a `?field=constructor` case to the DELETE `/api/config` allowlist test
(`web/src/app/api/config/route.test.ts`) alongside the existing `__proto__` case, so
the own-property guard is covered against both prototype-chain keys. Structural fix
already in place; this is belt-and-suspenders coverage.

---

## Tier B1 — discovery scoring: per-seed contributions table (worker + migration)

### Problem
`_upsert_artist` / `_upsert_album` upsert with `score = EXCLUDED.score` (**replace**),
but the intended semantic is *score = sum of every seed's contribution*. Since
`discover.maxSeeds` was retired and chunking is now the default, a single sweep
partitions its seeds across many worker iterations. A candidate surfaced by seeds in
different chunks has its score **overwritten** by the last chunk instead of summed
(item 4), and the same replace-not-accumulate breaks across throttled sweeps
(item 5). The score a suggestion shows reflects only its most-recent chunk, not the
whole library's affinity.

### Design
Introduce a normalized contributions table so a suggestion's score is a derived
aggregate, independent of chunk and sweep boundaries.

```prisma
model DiscoverySeedContribution {
  id            String   @id @default(cuid())
  candidateMbid String
  candidateName String
  seedMbid      String
  score         Float
  sources       String[]
  updatedAt     DateTime @updatedAt

  @@unique([candidateMbid, seedMbid])
  @@index([candidateMbid])
  @@index([seedMbid])
}
```

`run_discovery`, per seed **S** processed in the chunk:

1. `affected := SELECT DISTINCT "candidateMbid" WHERE "seedMbid" = S` — the
   candidates S *used* to contribute to (so drop-offs are recomputed too).
2. `DELETE FROM "DiscoverySeedContribution" WHERE "seedMbid" = S`.
3. Build a per-seed aggregate over the live sources: for each similar candidate
   (skipping already-followed mbids), sum score across sources and union source
   names. `INSERT` one contribution row per (candidate, S).
4. `affected |= {candidate mbids newly inserted for S}`.

After the chunk's seeds are processed, recompute every candidate in the union of all
`affected` sets:

- `SELECT SUM(score), COUNT(*), <sources>, any candidateName
   FROM "DiscoverySeedContribution" WHERE "candidateMbid" = ANY(affected)
   GROUP BY "candidateMbid"`.
  `COUNT(*)` is the seedCount (unique on `(candidate, seed)` → one row per seed).
- For each with `score >= min_score`, upsert `DiscoverySuggestion` exactly as today,
  keeping the `WHERE status = 'pending'` guard (dismissed/wanted rows untouched) and
  the rowcount-drives-album-derivation logic. Album suggestions inherit the
  recomputed artist aggregate.

### Properties
- Score is now **exactly** the sum of all seeds' latest contributions — no chunk
  artifact, no partial-sweep reset. A sweep triggered by one newly-followed artist
  updates only that seed's rows and leaves every other seed's contribution intact.
- Deleting-then-reinserting a seed's rows self-heals stale entries: if S stops
  finding a candidate, S's row for it vanishes and the candidate is recomputed
  (seedCount drops) because it was in `affected`.
- A candidate whose contributions fall to zero: its pending suggestion is **left as
  is** (not auto-retracted), consistent with today's semantics — suggestions persist
  until dismissed/actioned. Documented, not a bug.

### Migration & cleanup
- Additive table only; no data backfill (existing suggestions keep their current
  score and are corrected on the next sweep that touches their seeds).
- Consider a one-time note: existing `DiscoverySuggestion` scores stay stale until
  re-swept — acceptable for an off-by-default feed.

---

## Tier B2 — scan worklist table (worker + migration)

### Problem
`scan_chunk` re-walks the whole `/music` tree every chunk and skips entries sorting
`<= cursor` — `O(N²/C)` directory stat ops over a full scan. The scan is
MB-rate-limited (~1 req/s), so this is ~2% overhead, but the only true `O(N)` fix is
a persisted worklist (a JSON-blob-in-Config would just move the cost to re-parsing
the blob each chunk).

### Design
Persist the album worklist once at scan start and pop it by indexed cursor.

```prisma
model ScanWorkItem {
  id      String  @id @default(cuid())
  scanId  String
  artist  String
  album   String
  path    String
  done    Boolean @default(false)

  @@unique([scanId, path])
  @@index([scanId, done, artist, album])
}
```

`scan.py`:
- `scan_build_worklist(conn, scan_id, dest_root)` — walk once via the existing
  `_iter_album_entries`, `INSERT` a row per album folder (`ON CONFLICT (scanId, path)
  DO NOTHING`, so a rebuild is idempotent). Store `artist`, `album` (= album folder),
  `path` so the chunk needs no re-derivation.
- `scan_chunk(conn, resolver, probe, browser, scan_id, limit)` — `SELECT` the next
  `limit` rows `WHERE scanId = %s AND NOT done ORDER BY artist, album`, process each
  via the existing `_process_album`, mark `done = true`. Returns
  `(imported, skipped, done)` where `done` is True when no undone rows remain.
- `scan_library(conn, resolver, probe, browser, dest_root)` (test-only unbounded
  path) — reimplemented on the worklist: build with a fixed scan id, drain fully,
  delete that scan's rows, return `ScanResult`. Keeps existing tests meaningful.

`main.maybe_run_scan`:
- On start (`scan.requested` and not in progress): set `scan.inProgress=true`,
  `scan.requested=false`, `scan.progress="0/0"`, generate a `scan_id`
  (uuid), store it in `scan.id`, and `scan_build_worklist`.
- Each iteration: read `scan.id`, pop one chunk, accumulate imported/skipped in
  `scan.progress`.
- On drain: write `scan.result` (unchanged `"imported X, skipped Y"` format), clear
  `scan.progress`/`scan.id`, set `scan.inProgress=false`, and
  `DELETE FROM "ScanWorkItem" WHERE scanId = %s`. Preserve the existing
  "rescan requested mid-scan triggers one more full pass" guarantee.
- `scan.cursor` is retired (worklist replaces the string cursor).

### Properties
- `O(log N + C)` per chunk via the `(scanId, done, artist, album)` index; one tree
  walk per scan total.
- Robust to folders appearing/disappearing mid-scan: the worklist is frozen at scan
  start, so the set processed is deterministic.
- Resumable/idempotent: `done` flag + `ON CONFLICT` inserts + `ON CONFLICT` record
  writes mean an interrupted worker resumes cleanly.

---

## Migration

One additive Prisma migration adds both tables (`DiscoverySeedContribution`,
`ScanWorkItem`). No column changes to existing tables, no backfill. Reaches live
`lyra` via the web entrypoint's `migrate deploy` on rebuild.

## Testing

- **Worker (`pytest`, against `lyra_test`):**
  - Contribution accumulation: a candidate surfaced by seeds split across two
    simulated chunks sums to the same score as a single-chunk sweep; a partial sweep
    (one seed) preserves other seeds' contributions; a seed dropping a candidate
    lowers its seedCount and recomputes the suggestion.
  - Scan worklist: build → pop chunks → drain marks all done; resume after a partial
    chunk; idempotent rebuild; `scan_library` unbounded path still returns the same
    `ScanResult`.
- **Web (`vitest`):**
  - PATCH `/api/monitor/config` and `/api/discover/config` reject a
    prototype-chain key (`__proto__`, `constructor`) — no `Config` write.
  - DELETE `/api/config` `?field=constructor` rejected (added to existing test).
- Full suites + `tsc` + `next build` green before merge.

## Delivery

Branch `cleanup/tier-a-b` → writing-plans → subagent-driven build (per-task review +
Opus whole-branch review) → ff-merge to `main` → `git push` →
`docker compose up -d --build web worker` (runs `migrate deploy`) → verify live
(web 200, worker healthy, migration applied). Then move to the Tier C feature
brainstorm.

## Out of scope (→ Tier C brainstorm)
Last.fm second `SimilaritySource`, crash-resume mid-pipeline, per-track duration
matching, UI error states on mutations.