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.

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.

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.