Done cleanup

backlog/completed/ml-10 - Break-Records-→-Artists-→-Collection-→-Records-static-alias-cycle.md

@@ -0,0 +1,84 @@
+---
+id: ML-10
+title: Break Records → Artists → Collection → Records static alias cycle
+status: Done
+assignee: []
+created_date: '2026-04-20 08:49'
+updated_date: '2026-04-24 09:49'
+labels: []
+dependencies: []
+references:
+  - 'https://github.com/cloud8421/music_library/issues/173'
+priority: low
+ordinal: 1000
+---
+
+## Description
+
+<!-- SECTION:DESCRIPTION:BEGIN -->
+<!-- SECTION:DESCRIPTION:BEGIN -->
+_GitHub: created 2026-04-16 · updated 2026-04-16_
+
+## Summary
+
+A three-way compile-connected alias cycle exists between the three context facades. `mix xref graph --format cycles` surfaces it. No runtime cycle (the Records → Artists edge is Oban-async).
+
+## Evidence
+
+```
+Records → Artists:  lib/music_library/records.ex:10 (alias)
+  Artists.refresh_artist_info_async/1   records.ex:387
+  Artists.prune_artist_info_async/1     records.ex:417
+
+Artists → Collection: lib/music_library/artists.ex:10 (alias)
+  Collection.collected_artist_ids/0     artists.ex:29 (used inside get_similar_artists/1)
+
+Collection → Records: lib/music_library/collection.ex:9 (alias) + line 7 (import)
+  Records.search_records/4, Records.search_records_count/2, Records.essential_fields/0
+  import MusicLibrary.Records, only: [order_alphabetically: 0]
+```
+
+The compile-time edge in the cycle is `Collection → Records` via `import ..., only: [order_alphabetically: 0]`. `order_alphabetically` is a **macro** (`records.ex:56`), so the import is unavoidably compile-time. `mix xref graph --format cycles --label compile` reports this as a cycle of 15 nodes (3 contexts + `Records.Similarity` + 11 workers), meaning any change to any of those 15 modules triggers recompilation of the entire cycle.
+
+Breaking any other edge in the triangle (including `Artists → Collection`) removes the cycle without needing to eliminate the macro import.
+
+## Relation to #112
+
+#112 fixed the earlier `Records ↔ Artists` bidirectional dependency by moving `collected_artist_ids/0` from `Artists` to `Collection`. That fix made the shape `Records → Artists → Collection → Records` — unidirectional in hops, but still a compile-connected cycle.
+
+## Fix
+
+Parametrize `Artists.get_similar_artists/1` to receive `collected_artist_ids` instead of fetching inline:
+
+```elixir
+# in Artists
+def get_similar_artists(artist, collected_artist_ids) do
+  # ... use the set passed in
+end
+
+# in the caller (ArtistLive.Show)
+collected = Collection.collected_artist_ids()
+similar = Artists.get_similar_artists(artist, collected)
+```
+
+This removes the `Artists → Collection` edge without moving any behaviour. Within `lib/music_library/`, only `Artists` aliases `Collection` — everything else referencing `Collection` lives in `lib/music_library_web/` (outside the cycle), so this single change is sufficient to break the cycle.
+<!-- SECTION:DESCRIPTION:END -->
+
+## Acceptance Criteria
+<!-- AC:BEGIN -->
+- `mix xref graph --format cycles --label compile` reports no cycle involving Records/Artists/Collection
+- `ArtistLive.Show` tests still pass
+
+- [x] #1 `mix xref graph --format cycles --label compile` reports no cycle involving Records/Artists/Collection
+- [x] #2 `ArtistLive.Show` tests still pass
+<!-- AC:END -->
+
+## Final Summary
+
+<!-- SECTION:FINAL_SUMMARY:BEGIN -->
+Removed `alias MusicLibrary.Collection` from `MusicLibrary.Artists` and changed `get_similar_artists/1` to `get_similar_artists/2`, taking the collected artist id set as an argument instead of fetching it inline. The only caller (`ArtistLive.Show`) now computes the set via `Collection.collected_artist_ids()` and passes it in.
+
+`mix xref graph --format cycles --label compile` now reports **no cycles**. The 3-way cycle is gone, and the 15-module compile-connected cycle that previously forced recompilation cascades is broken. A 14-module runtime-only cycle (Records ↔ Artists via Oban-async edges + workers) remains as expected and does not trigger recompilation.
+
+Full test suite (836 tests including 10 in `ArtistLive.ShowTest`) passes. Format and Credo clean.
+<!-- SECTION:FINAL_SUMMARY:END -->

backlog/completed/ml-11 - Telemetry.Storage-GenServer-funnels-synchronous-SQLite-writes.md

@@ -0,0 +1,54 @@
+---
+id: ML-11
+title: Telemetry.Storage GenServer funnels synchronous SQLite writes
+status: Done
+assignee: []
+created_date: '2026-04-20 08:49'
+updated_date: '2026-04-24 09:29'
+labels: []
+dependencies: []
+references:
+  - 'https://github.com/cloud8421/music_library/issues/172'
+priority: medium
+---
+
+## Description
+
+<!-- SECTION:DESCRIPTION:BEGIN -->
+_GitHub: created 2026-04-16 · updated 2026-04-16_
+
+## Summary
+
+`MusicLibraryWeb.Telemetry.Storage` is a single named GenServer that receives a cast for every telemetry event and performs two synchronous SQLite writes per event. A page with 10 DB queries plus a render can push 30+ messages into this one mailbox, all queued behind serial disk I/O.
+
+## Evidence
+
+- `lib/music_library_web/telemetry/storage.ex:51` — `GenServer.cast(__MODULE__, {:store, ...})`
+- `lib/music_library_web/telemetry/storage.ex:56-69` — `handle_cast/2` runs `insert_and_prune/4` synchronously
+- `lib/music_library_web/telemetry/storage.ex:89-107` — two SQLite queries per event: `INSERT` + `DELETE`-preserving-last-N subquery
+- `TelemetryRepo` pool is 2 per `docs/production-infrastructure.md`, but the GenServer writes serially so pool size is irrelevant.
+- `lib/music_library_web/telemetry/storage.ex:108-109, 125-126` — bare `catch _, _ -> :ok` swallows every error class including EXIT signals.
+
+## Relation to #105
+
+Issue #105 was closed as NOT_PLANNED with the rationale that bare catches are "reasonable for telemetry resilience". New framing: the catches mask a more fundamental design: the mailbox + synchronous-write pattern itself.
+
+## Fix (options)
+
+1. **Buffer-and-flush**: accumulate datapoints in-memory, flush batch every N seconds in a single transaction.
+2. **Task.Supervisor per cast**: drain mailbox immediately, do I/O in a supervised task.
+3. **ETS ring buffer + periodic flusher**: read path hits ETS; single-writer flusher persists to SQLite at a low cadence.
+
+Whichever option is chosen, replace the bare `catch _, _` with either a targeted `rescue` or `Logger.debug` so failures become observable.
+
+## Acceptance Criteria
+<!-- AC:BEGIN -->
+- Telemetry write path does not block on the GenServer mailbox
+- Error path surfaces at least once per distinct failure (not silently swallowed)
+- Existing dashboard queries continue to work
+<!-- SECTION:DESCRIPTION:END -->
+
+- [x] #1 Telemetry write path does not block on the GenServer mailbox
+- [x] #2 Error path surfaces at least once per distinct failure (not silently swallowed)
+- [x] #3 Existing dashboard queries continue to work
+<!-- AC:END -->

backlog/completed/ml-13 - Tighten-ecto_sqlite3-version-constraint-in-mix.exs.md

@@ -0,0 +1,44 @@
+---
+id: ML-13
+title: Tighten ecto_sqlite3 version constraint in mix.exs
+status: Done
+assignee: []
+created_date: '2026-04-20 08:50'
+updated_date: '2026-04-24 08:53'
+labels: []
+dependencies: []
+references:
+  - 'https://github.com/cloud8421/music_library/issues/170'
+priority: low
+---
+
+## Description
+
+<!-- SECTION:DESCRIPTION:BEGIN -->
+_GitHub: created 2026-04-16 · updated 2026-04-16_
+
+## Summary
+
+`mix.exs:52` declares `{:ecto_sqlite3, ">= 0.0.0"}` — the loosest possible constraint. The lockfile pins 0.22.0.
+
+## Evidence
+
+```elixir
+{:ecto_sqlite3, ">= 0.0.0"}
+```
+
+All other production deps use `~>` with at least a major pin (e.g. `ecto_sql, "~> 3.10"`, `oban, "~> 2.21"`).
+
+## Fix
+
+Change to `{:ecto_sqlite3, "~> 0.22"}`. Matches the installed major; prevents accidental upgrade on a fresh `mix deps.get`.
+
+## Acceptance Criteria
+<!-- AC:BEGIN -->
+- `mix deps.get` still resolves cleanly
+- `mix test` passes
+<!-- SECTION:DESCRIPTION:END -->
+
+- [ ] #1 `mix deps.get` still resolves cleanly
+- [ ] #2 `mix test` passes
+<!-- AC:END -->

backlog/completed/ml-145 - Restructure-scrobble-routes-reuse-Release-component-on-scrobble-page.md

@@ -0,0 +1,157 @@
+---
+id: ML-145
+title: Restructure /scrobble routes; reuse Release component on scrobble page
+status: Done
+assignee: []
+created_date: '2026-04-23 06:11'
+updated_date: '2026-04-23 14:15'
+labels:
+  - ui
+  - scrobble
+  - refactor
+  - liveview
+dependencies: []
+references:
+  - lib/music_library_web/components/release.ex
+  - lib/music_library_web/live/scrobble_live/index.ex
+  - lib/music_library_web/live/scrobble_live/show.ex
+  - lib/music_library/records/tracklist_pdf.ex
+  - backlog/ml-142/plan.md
+  - >-
+    backlog/tasks/ml-142.1 -
+    Port-Finished-at-picker-and-selection-bar-to-ScrobbleLive.Show.md
+documentation:
+  - .claude/plans/scrobble-route-restructure/design.md
+priority: medium
+---
+
+## Description
+
+<!-- SECTION:DESCRIPTION:BEGIN -->
+## Why
+
+`/scrobble/:release_id` (`MusicLibraryWeb.ScrobbleLive.Show`) re-implements scrobbling UI that already exists as a LiveComponent used by collection/wishlist show pages (`MusicLibraryWeb.Components.Release`). Today the scrobble page imports `medium/1` and `scrobble_button_label/1` from that component but reimplements `scrobble_release` / `scrobble_medium` / `scrobble_selected_tracks` with hard-coded `DateTime.utc_now/0`, no `Finished at` picker, and no selection bar. ML-142.1 proposed porting those affordances into `ScrobbleLive.Show` in parallel — which would leave two drifting implementations.
+
+This task takes the opposite approach: make the Release LiveComponent reusable outside the sheet (currently tightly coupled to `%Records.Record{}` and wrapped in `<.sheet>`), restructure the scrobble routes into a bookmarkable hierarchy, and delete the custom scrobble UI in favour of reuse.
+
+**Supersedes ML-142.1** — the ML-142.1 ACs become free once the component is reused on the new scrobble page. ML-142.1 can be archived as superseded.
+
+## What
+
+Three routes:
+
+- `/scrobble` — search only. Release-group clicks `<.link navigate>` to `/scrobble/:rg_id` (no more inline release-loading state).
+- `/scrobble/:rg_id` (new) — release-group header (cover, title, primary artist, type badge, first-release date, release count) + full list of releases; each release links to `/scrobble/:rg_id/releases/:release_id`.
+- `/scrobble/:rg_id/releases/:release_id` (new, replaces `/scrobble/:release_id`) — the scrobble page itself, rendered by the same LiveComponent that powers the collection/wishlist show sheet.
+
+Old `/scrobble/:release_id` → 404 (personal app; bookmark churn is acceptable, not worth 301-redirect plumbing).
+
+## Key refactor decisions (working design doc: `.claude/plans/scrobble-route-restructure/design.md`, local-only)
+
+1. **Release LiveComponent input contract** — takes `release_id` (string) instead of `record`; adds `show_print?: boolean`. Header title/artists derive from the loaded `%MusicBrainz.Release{}`.
+2. **`<.sheet>` moves to callsites** — single render path for the component. Collection/wishlist show pages wrap in their own `<.sheet>`; the scrobble release page renders it directly under `<Layouts.app>`.
+3. **Selection bar stickiness** — switches to `position: sticky` so it works both inside a sheet (pins to sheet bottom) and on a page (pins to viewport bottom) with single markup, no mode flag.
+4. **`TracklistPdf` signatures** — `generate/1` and `generate_medium/2` take a `%MusicBrainz.Release{}`; no `%Records.Record{}` required (both fields used by the PDF — `title` and `artists` — already exist on the release struct).
+5. **Module names** — new `MusicLibraryWeb.ScrobbleLive.ReleaseGroupShow` at `/scrobble/:rg_id`; rename `MusicLibraryWeb.ScrobbleLive.Show` → `MusicLibraryWeb.ScrobbleLive.ReleaseShow` at `/scrobble/:rg_id/releases/:release_id`, removing its custom scrobble UI in favour of embedding the LiveComponent.
+6. **Unused after refactor** — `MusicLibraryWeb.Components.Release.scrobble_button_label/1` (delete).
+
+## Non-goals
+
+- Scrobble-rule picker on the scrobble page (fresh MB releases have no misrecognised tracks yet).
+- Cross-link to `/collection/:id` when the release is already collected.
+- 301-redirecting old `/scrobble/:release_id` URLs.
+- Breadcrumbs on the scrobble page.
+- Preserving `?query=X` across nested URLs (browser back button already works).
+- Splitting the Release LiveComponent into separate `Content` / `Sheet` modules.
+- Changes to `MusicLibrary.ScrobbleActivity`.
+<!-- SECTION:DESCRIPTION:END -->
+
+## Acceptance Criteria
+<!-- AC:BEGIN -->
+- [x] #1 /scrobble/:rg_id renders a release-group header (cover, release-group title, primary artist, type badge, first-release date, release count) and the full list of releases returned by MusicBrainz for that group.
+- [x] #2 Each release in the /scrobble/:rg_id list is a navigate link to /scrobble/:rg_id/releases/:release_id.
+- [x] #3 /scrobble/:rg_id shows an error toast and redirects to /scrobble if fetching the release group or its releases fails.
+- [x] #4 /scrobble/:rg_id/releases/:release_id renders a scrobble page with picker-driven `Finished at`, release-level scrobble, per-medium scrobble, track-selection + selection-bar scrobble, and Print tracklist — behaviour identical to the collection/wishlist show sheet.
+- [x] #5 /scrobble/:rg_id/releases/:release_id has a `Back to releases` link that navigates to /scrobble/:rg_id.
+- [x] #6 Old /scrobble/:release_id returns 404 (not redirected, not aliased).
+- [x] #7 /scrobble no longer renders releases inline; release-group list items are navigate links to /scrobble/:rg_id. The `?query=X` search param still works as before.
+- [x] #8 The Release LiveComponent on the scrobble page and on the collection/wishlist sheet share a single implementation — there is no parallel reimplementation of scrobble handlers, picker, or selection bar.
+- [x] #9 The selection bar stays visible at the bottom of the visible scroll region in both contexts (scrobble page viewport, collection/wishlist sheet inner scroll) while tracks scroll, using a single markup path.
+- [x] #10 MusicLibrary.Records.TracklistPdf generates tracklist PDFs from a MusicBrainz release struct alone (no Records.Record required). Output for collection/wishlist records is visually unchanged.
+- [x] #11 The Release LiveComponent supports suppressing both the release-level and per-medium Print tracklist dropdown entries via a single input (both hidden together); the suppression state is covered by a component test.
+- [x] #12 MusicLibraryWeb.ScrobbleLive.Show's custom scrobble UI and handlers (scrobble_release, scrobble_medium, scrobble_selected_tracks, validate, recover_form) are deleted. MusicLibraryWeb.Components.Release.scrobble_button_label/1 is deleted as unused.
+- [x] #13 All new user-facing strings are wrapped in gettext; .pot/.po files regenerated via `mix gettext.extract --merge`.
+- [x] #14 Tests updated: test/music_library_web/components/release_test.exs covers the new `release_id` input contract and both states of the print-suppression input; test/music_library/records/tracklist_pdf_test.exs covers the new generate/1 and generate_medium/2 signatures.
+- [x] #15 Tests added: test/music_library_web/live/scrobble_live/release_group_show_test.exs covers happy path (header fields + releases list + link targets) and fetch failure (toast + redirect); test/music_library_web/live/scrobble_live/release_show_test.exs (replacing show_test.exs) smoke-tests mount, component render, and back-link target.
+- [x] #16 Tests updated: test/music_library_web/live/scrobble_live/index_test.exs asserts release-group clicks navigate (no inline state); collection/wishlist show tests updated for the new component input shape and callsite sheet markup.
+- [x] #17 Manual UI verification via `iex -S mix phx.server`: sheet selection-bar still pins correctly on collection/wishlist show pages; page selection-bar pins to viewport bottom on scrobble page while tracks list scrolls; navigation loop /scrobble → /scrobble/:rg_id → /scrobble/:rg_id/releases/:release_id → back → back works, and `?query=X` survives the browser back button.
+<!-- AC:END -->
+
+## Final Summary
+
+<!-- SECTION:FINAL_SUMMARY:BEGIN -->
+## Summary
+
+Restructured `/scrobble` into a three-level URL hierarchy (`/scrobble` → `/scrobble/:rg_id` → `/scrobble/:rg_id/releases/:release_id`) and eliminated the duplicated scrobble UI by making the Release LiveComponent reusable on a standalone page.
+
+Supersedes ML-142.1 — picker and selection bar now live on the scrobble page via reuse, not via a parallel port.
+
+## Implementation
+
+Ten commits on branch `ml-145-scrobble-route-restructure`:
+
+- `8631b97b` Add joinphrase to Release.Artist; parse from MB
+- `0d1f5803` Refactor TracklistPdf to take release alone
+- `4c48bda7` Refactor Release component to take release_id
+- `d8041dc0` Add /scrobble/:rg_id release-group page
+- `0dcd6eab` Replace /scrobble/:release_id with nested route
+- `c7c7e1fe` Navigate from scrobble search to release-group page
+- `72818bfa` Validate release-group shape before parsing
+- `779708e6` Hide print tracklist on scrobble release page
+- `ca9cc768` Hide release-actions dropdown when empty
+- `62586c9a` Restore flex context for sheet selection bar
+
+Key changes:
+
+- `MusicBrainz.Release.Artist` gains `:joinphrase` so `TracklistPdf` can work from a release struct alone.
+- `MusicLibraryWeb.Components.Release` takes `release_id` + `show_print?` (instead of `record`); header title/artists derive from the loaded release; `<.sheet>` wrapper moved to callsites; release-actions dropdown auto-hides when both print + connect-last-fm are disabled.
+- `CollectionLive.Show` wraps the component in its own `<.sheet>` with `show_print?={true}`.
+- New `ScrobbleLive.ReleaseGroupShow` at `/scrobble/:rg_id` — async-fetches the release group + its releases, renders header (cover, title, primary artist, type badge, first-release date, release count) + list of releases.
+- New `ScrobbleLive.ReleaseShow` at `/scrobble/:rg_id/releases/:release_id` — thin LiveView embedding the Release LiveComponent directly (no sheet), passes `show_print?={false}`.
+- `ScrobbleLive.Index` loses inline release-loading state; release-group clicks navigate to `/scrobble/:rg_id`.
+- Old `ScrobbleLive.Show` module and test, plus `Components.Release.scrobble_button_label/1`, deleted.
+
+## Verification findings + fixes
+
+Two bugs surfaced during Phase 8 verification:
+
+1. **Error-path crash in `ReleaseGroupShow.load/1`.** When `MusicBrainz.get_release_group/1` returned an unexpected response shape, `ReleaseGroupSearchResult.from_api_response/1` crashed on `Enum.map_join(nil, …)`. The test passed because `handle_async({:exit, …}, …)` caught it, but the stack trace appeared in logs. Fixed with an explicit `parse_release_group/1` helper that pattern-guards the shape and returns `{:error, :invalid_release_group_response}` for bad input, routing cleanly through the graceful `{:error, reason}` handler.
+
+2. **Sheet layout regression.** The Phase-3 root-`<div>` replacement for `<.sheet>` did not establish a flex context, so the form's `flex-1` no longer expanded inside the sheet, which pushed the selection bar away from the bottom. Fixed by giving the root `<div>` classes `flex h-full min-h-0 flex-1 flex-col` — single markup that works in both sheet (flex-pinned last child) and page (sticky-pinned to viewport) contexts.
+
+## Tests
+
+- Full suite: 815 passed (43 doctests, 772 tests), 0 skipped.
+- `mix compile --warnings-as-errors`: clean.
+- `mise run dev:precommit`: clean (shellcheck, credo, sobelow, translations, formatting, unused deps, full test run).
+
+New coverage:
+- `test/music_library_web/live/scrobble_live/release_group_show_test.exs` — happy path (header + release links) and fetch failure (toast + redirect to `/scrobble`).
+- `test/music_library_web/live/scrobble_live/release_show_test.exs` — smoke (mount + component render + back link target).
+- `show_print?={true|false}` verified via `Phoenix.LiveViewTest.live_isolated/3` in `release_test.exs`.
+
+Deleted: `test/music_library_web/live/scrobble_live/show_test.exs` (replaced by the smoke test above).
+
+## Manual UI verification (Phase 8 AC#17)
+
+Confirmed by user in dev server:
+- Navigation loop `/scrobble` → `/scrobble/:rg_id` → `/scrobble/:rg_id/releases/:release_id` works; `?query=X` survives browser back.
+- Scrobble release page: picker, per-release/medium/track scrobble, and selection bar sticky at viewport bottom.
+- Collection show sheet: selection bar pins to sheet bottom while tracks scroll.
+- Print tracklist works on collection show; correctly hidden on scrobble page.
+- Release-actions dropdown button hides entirely on scrobble page (empty dropdown) when Last.fm is connected.
+
+## Follow-ups
+
+- **ML-142.1** is now fully superseded and can be archived (not done here — archive is a user decision).
+<!-- SECTION:FINAL_SUMMARY:END -->

backlog/completed/ml-21 - Classify-API-errors-as-transient-vs-permanent-in-workers.md

@@ -0,0 +1,130 @@
+---
+id: ML-21
+title: Classify API errors as transient vs permanent in workers
+status: Done
+assignee: []
+created_date: '2026-04-20 08:50'
+updated_date: '2026-04-24 11:59'
+labels: []
+dependencies: []
+references:
+  - 'https://github.com/cloud8421/music_library/issues/158'
+priority: medium
+ordinal: 1000
+---
+
+## Description
+
+<!-- SECTION:DESCRIPTION:BEGIN -->
+_GitHub: created 2026-04-05 · updated 2026-04-12 · re-scoped 2026-04-24 after per-API research_
+
+## Summary
+
+Workers that call MusicBrainz, Discogs, Wikipedia/Wikidata, Brave Search, and OpenAI bubble raw response bodies on error. They cannot distinguish "rate limited, snooze" from "bad request, cancel" from "server blip, retry", so Oban uses the generic `max_attempts: 3` backoff for every failure mode.
+
+## What the original framing got wrong
+
+The original issue pointed at `LastFm.API.ErrorResponse` as the pattern to copy across all integrations. After researching each API, that comparison doesn't hold:
+
+- **Last.fm is the odd one out**, not the reference. It returns HTTP 200 with `{"error": N, "message": "..."}` in the body, and its 14-value error taxonomy (`invalid_session_key`, `suspended_api_key`, `service_offline`, …) is a protocol unique to Last.fm. No other API we use has that shape.
+- **The REST APIs encode errors in HTTP status codes.** The only "classification" to recover is `status → transient | permanent`, plus two body-peek exceptions (see below). Copying `ErrorResponse` verbatim would produce four near-identical modules whose only job is to translate status codes.
+
+## What each API actually returns
+
+| API | Error channel | Rate limit status | Body shape | Retry hint |
+|-----|---------------|-------------------|------------|------------|
+| Last.fm | HTTP 200 + body | code `29` in body | `{"error": N, "message": "..."}` | none (already handled) |
+| MusicBrainz | HTTP status | **503** (not 429) | `{"error": "string"}` | `Retry-After` |
+| Discogs | HTTP status | 429 | `{"message": "..."}` | `X-Discogs-Ratelimit-*`, no `Retry-After` |
+| Wikipedia REST v1 | HTTP status | 429 | `{"httpCode", "messageTranslations"}` | `Retry-After` |
+| Wikipedia/Wikidata Action API | **HTTP 200 + body** (Last.fm-shaped) | 429 or 503 | `{"error": {"code", "info"}}` | `Retry-After` on 503 |
+| Brave Search | HTTP status | 429 | `{"type": "ErrorResponse", "error": {"status", "code", "detail"}}` | `X-RateLimit-Reset` (seconds) |
+| OpenAI | HTTP status + body `code` | 429 for **both** rate limit and quota | `{"error": {"type", "code", "message", "param"}}` | `x-ratelimit-reset-*` |
+
+## Real quirks to encode (not 14 error atoms)
+
+1. **MusicBrainz uses 503, not 429, for rate limits.** A generic "5xx = transient" rule already handles it; worth a comment so it isn't "fixed" by a future reader.
+2. **OpenAI 429 is ambiguous.** `code: "rate_limit_exceeded"` → retry. `code: "insufficient_quota"` → cancel (billing failure, not a retryable rate limit). Must inspect the body.
+3. **MediaWiki Action API returns errors inside HTTP 200 bodies.** Our current `Wikipedia.API.get_wikipedia_title/2` and `get_article_extract/2` would return `{:ok, body}` with an error payload — a silent bug. Needs a Last.fm-style `parse_error` step that converts `{"error": %{"code", "info"}}` in the body into `{:error, ...}`.
+
+## Suggested fix
+
+Per-API `classify/1` helpers (or a small shared module) that return `:retry | :cancel` given `{status, body}`. Workers then choose between `{:error, reason}` / `{:snooze, n}` / `{:cancel, reason}` based on that classifier.
+
+Out of scope (tracked separately): honouring `Retry-After` / `X-*-Reset` headers to compute precise snooze durations rather than using fixed defaults.
+
+## Evidence
+
+- `lib/last_fm/api/error_response.ex` — existing Last.fm model, keep as-is
+- `lib/music_brainz/api.ex:512-524` — `get_request/1` returns raw body on non-200
+- `lib/discogs/api.ex:47-59` — same
+- `lib/wikipedia/api.ex:86-97` — same, **plus** silently returns `{:ok, body}` when body is an Action API error payload
+- `lib/brave_search/api.ex:67-78` — same
+- `lib/open_ai/api.ex:30-31, 89-90` — same, never inspects `error.code` so treats `rate_limit_exceeded` and `insufficient_quota` identically
+- `lib/music_library/worker/refresh_scrobbles.ex:26-32` — example of snooze-vs-cancel logic driven by classified errors
+<!-- SECTION:DESCRIPTION:END -->
+
+- [ ] #1 Each API integration has structured error classification
+- [ ] #2 Workers can distinguish transient from permanent failures
+<!-- AC:END -->
+
+## Acceptance Criteria
+<!-- AC:BEGIN -->
+- [x] #1 MusicBrainz, Discogs, Wikipedia REST v1, Brave Search, and OpenAI API modules expose a classifier that maps {status, body} to :retry or :cancel
+- [x] #2 Wikipedia Action API responses (wbgetentities, prop=extracts) decode HTTP 200 bodies containing {"error": ...} into {:error, reason} instead of {:ok, body}
+- [x] #3 OpenAI classifier distinguishes rate_limit_exceeded (retry) from insufficient_quota (cancel) despite both being HTTP 429
+- [x] #4 MusicBrainz classifier treats 503 as the rate-limit signal (not 429) and is documented as such
+- [x] #5 Workers using these APIs use the classifier to return {:snooze, n} / {:error, reason} / {:cancel, reason} instead of bubbling raw bodies
+- [x] #6 Existing LastFm.API.ErrorResponse behaviour is preserved
+<!-- AC:END -->
+
+## Final Summary
+
+<!-- SECTION:FINAL_SUMMARY:BEGIN -->
+## Summary
+
+Introduced structured per-API `ErrorResponse` modules for MusicBrainz, Discogs, Wikipedia, Brave Search, and OpenAI, so workers can now distinguish transient failures (rate limit, 5xx, timeout) from permanent ones (4xx, not found, auth, quota). Workers emit `{:snooze, seconds}` / `{:cancel, reason}` / `{:error, reason}` instead of bubbling raw bodies. Preserved the existing `LastFm.API.ErrorResponse` behaviour and added struct-based `retryable?/1` / `retry_delay_seconds/1` helpers so Last.fm plugs into the same handler.
+
+## What changed
+
+**New shared modules**
+- `MusicLibrary.HttpError` — default HTTP status → kind mapping
+- `MusicLibrary.Worker.ErrorHandler.to_oban_result/1` — maps any known `ErrorResponse` struct to the correct Oban tuple; passes through `{:ok, _}`, `{:cancel, _}`, and atom-reason errors unchanged
+
+**New per-API `ErrorResponse` modules**
+- `MusicBrainz.API.ErrorResponse` — maps 503 to `:rate_limit` (MusicBrainz-specific; 429 is not used upstream)
+- `Discogs.API.ErrorResponse`
+- `Wikipedia.API.ErrorResponse` — has a dedicated `from_action_api_body/1` for HTTP 200 Action API error envelopes (AC2 silent-bug fix)
+- `BraveSearch.API.ErrorResponse`
+- `OpenAI.API.ErrorResponse` — disambiguates HTTP 429 between `rate_limit_exceeded` (retry) and `insufficient_quota` (cancel) via body `code` (AC3)
+
+**Updated API modules** — each now attaches a `parse_error/1` Req response step that halts with the appropriate struct on failure:
+- `MusicBrainz.API`, `Discogs.API`, `Wikipedia.API`, `BraveSearch.API`
+- `OpenAI.API` — `gpt/2`, `get_embeddings/2`, and `chat_stream/6` now return `{:error, %OpenAI.API.ErrorResponse{}}` instead of `{:error, "OpenAI API error: " <> inspect(body)}` strings
+
+**Updated workers** to route through `ErrorHandler.to_oban_result/1`, preserving existing atom-cancel branches (`:no_english_wikipedia`, `:cover_not_available`, `:image_not_found`, `:no_discogs_data`):
+- `ArtistRefreshMusicBrainzData`, `ArtistRefreshDiscogsData`, `ArtistRefreshWikipediaData`
+- `FetchArtistInfo`, `FetchArtistImage`, `FetchArtistLastFmData`
+- `RefreshCover`, `RecordRefreshMusicBrainzData`
+- `ImportFromMusicbrainzRelease`, `ImportFromMusicbrainzReleaseGroup`
+- `PopulateGenres`, `GenerateRecordEmbedding`
+
+`RefreshScrobbles` unchanged (already had its own `ErrorResponse` routing).
+
+## Notable fixes along the way
+
+- Wikipedia silent bug (AC2): Action API returned `{:ok, %{"error" => ...}}` to callers when the body contained an error envelope. Now properly surfaced as `{:error, %Wikipedia.API.ErrorResponse{}}`.
+- `lib/music_library_web/components/chat.ex`: Logger interpolation of the error reason (`"Chat streaming error: #{reason}"`) broke on struct errors. Switched to `inspect(reason)`.
+
+## Tests
+
+- New: `test/music_library/worker/error_handler_test.exs`, `test/music_brainz/api_test.exs`, `test/last_fm/api/error_response_test.exs`
+- Added per-API error-classification assertions to existing `test/open_ai/api_test.exs`, `test/brave_search/api_test.exs`, `test/discogs_test.exs`, `test/wikipedia_test.exs`
+- Updated callers that previously asserted on raw error bodies / string errors (`records_test.exs`, `similarity_test.exs`, worker tests)
+
+Full verification: `mise run dev:precommit` — shellcheck, credo --strict, sobelow, gettext, format, unused deps, 871 tests pass (43 doctests). Dialyzer shows 2 pre-existing warnings unrelated to this change.
+
+## Out of scope (ML-146)
+
+Honouring `Retry-After` / `X-*-Reset` headers to derive precise snooze durations — each ErrorResponse module currently returns fixed per-kind defaults (30–60 s).
+<!-- SECTION:FINAL_SUMMARY:END -->

backlog/completed/ml-5 - Document-Last.fm-OAuth-callback-trust-boundary.md

@@ -0,0 +1,86 @@
+---
+id: ML-5
+title: Document Last.fm OAuth callback trust boundary
+status: Done
+assignee:
+  - Claudio Ortolina
+created_date: '2026-04-20 08:48'
+updated_date: '2026-04-24 07:04'
+labels: []
+dependencies: []
+references:
+  - 'https://github.com/cloud8421/music_library/issues/178'
+priority: low
+---
+
+## Description
+
+<!-- SECTION:DESCRIPTION:BEGIN -->
+_GitHub: created 2026-04-16 · updated 2026-04-16_
+
+## Summary
+
+`GET /auth/last_fm/callback?token=X` is unauthenticated by design (OAuth flow requires it) but triggers storage of a session key in the encrypted secrets table. The trust model is not captured in comments, so future reviewers may re-flag it.
+
+## Evidence
+
+- `lib/music_library_web/controllers/last_fm_controller.ex:7`
+- `lib/music_library_web/router.ex:55` (route outside `:logged_in` pipe)
+
+Attack surface: cannot forge a valid token (Last.fm validates), but a malicious caller could consume `LastFm.get_session/1` rate-limit quota. Acceptable given the threat model.
+
+## Fix
+
+Add a short `@moduledoc` or inline comment in `last_fm_controller.ex` explaining:
+
+1. Why this endpoint is unauthenticated (OAuth flow cannot carry session cookies across the Last.fm redirect)
+2. What protects it (Last.fm-validated token, rate limiter, single-user scope)
+3. What the failure modes look like (invalid token → store nothing, error logged)
+
+Similarly in `router.ex:55`, a brief comment noting the deliberate exception from `:logged_in`.
+<!-- SECTION:DESCRIPTION:END -->
+
+## Acceptance Criteria
+<!-- AC:BEGIN -->
+- [x] #1 last_fm_controller.ex has a comment documenting the trust boundary
+- [x] #2 router.ex:55 notes the deliberate pipeline exemption
+<!-- AC:END -->
+
+## Implementation Plan
+
+<!-- SECTION:PLAN:BEGIN -->
+## Implementation Plan
+
+1. **`lib/music_library_web/controllers/last_fm_controller.ex`** — add `@moduledoc` covering:
+   - Why unauthenticated: OAuth redirect from Last.fm cannot carry session cookies across third-party redirect.
+   - What protects it: Last.fm validates the `token` via `LastFm.get_session/1`; the call is rate-limited at the `Req` layer (`Req.RateLimiter`, 500ms/req); this is a single-user deployment.
+   - Failure mode: invalid/forged tokens cause `get_session/1` to return `{:error, _}`; nothing is written to `secrets`, an error toast is shown, and the Req-layer error is logged.
+
+2. **`lib/music_library_web/router.ex:55`** — add a brief comment above the route noting deliberate exemption from the `:logged_in` pipeline (with pointer to the controller moduledoc for details).
+
+3. Run `mise run dev:precommit` to verify formatting, credo (including `ModuleDoc` rule), sobelow, and tests still pass.
+
+## Notes
+
+- Controllers are excluded from the `Credo.Check.Readability.ModuleDoc` regex in `.credo.exs`, so `@moduledoc` here is optional for lint purposes — using it is a deliberate choice for this security-sensitive module.
+- No test changes required (documentation-only).
+<!-- SECTION:PLAN:END -->
+
+## Final Summary
+
+<!-- SECTION:FINAL_SUMMARY:BEGIN -->
+## Summary
+
+Documented the Last.fm OAuth callback trust boundary in two places:
+
+- **`lib/music_library_web/controllers/last_fm_controller.ex`** — added `@moduledoc` explaining why the endpoint is unauthenticated (OAuth redirect from Last.fm cannot carry session cookies), what protects it (Last.fm-validated token, `Req.RateLimiter` bounding quota burn, single-user deployment), and the failure mode (invalid token → nothing stored, error toast shown).
+- **`lib/music_library_web/router.ex`** — added a comment above the `/auth/last_fm/callback` route noting the deliberate exemption from the `:logged_in` pipeline, with a pointer to the controller moduledoc for the full trust boundary.
+
+## Verification
+
+`mise run dev:precommit` passes: credo, sobelow, format, translations, unused deps, and tests (827 passed, 43 doctests).
+
+## Notes
+
+Controllers are excluded from the `Credo.Check.Readability.ModuleDoc` regex in `.credo.exs`, so the `@moduledoc` here is a deliberate choice for a security-sensitive module, not a lint requirement.
+<!-- SECTION:FINAL_SUMMARY:END -->

backlog/completed/ml-9 - Add-mix_audit-dependency-for-CVE-scanning.md

@@ -0,0 +1,65 @@
+---
+id: ML-9
+title: Add mix_audit dependency for CVE scanning
+status: Done
+assignee: []
+created_date: '2026-04-20 08:49'
+updated_date: '2026-04-24 07:13'
+labels: []
+dependencies: []
+references:
+  - 'https://github.com/cloud8421/music_library/issues/174'
+priority: low
+---
+
+## Description
+
+<!-- SECTION:DESCRIPTION:BEGIN -->
+_GitHub: created 2026-04-16 · updated 2026-04-16_
+
+## Summary
+
+The project has Sobelow (SAST) and `mix hex.audit` (retirement-only check) but no CVE database scan against the installed dep tree.
+
+## Evidence
+
+- `mix.exs` does not list `mix_audit`
+- `mise run dev:lint` pipeline does not include a CVE step
+- `.github/workflows/test_and_deploy.yml` lint job does not run any CVE scanner
+
+## Fix
+
+Add `{:mix_audit, "~> 2.1", only: :dev, runtime: false}` to `mix.exs` and wire into the lint pipeline:
+
+```toml
+# in mise.toml, under [tasks."dev:lint"] or as a new task
+mix deps.audit
+```
+
+And add a step in `.github/workflows/test_and_deploy.yml` lint job.
+
+## Acceptance Criteria
+<!-- AC:BEGIN -->
+- `mix deps.audit` runs in CI
+- CI fails (or warns, TBD) on advisories
+<!-- SECTION:DESCRIPTION:END -->
+
+- [x] #1 `mix deps.audit` runs in CI as a warn-only step (continue-on-error)
+- [x] #2 `mix deps.audit` runs in `mise run dev:lint` as a warn-only step
+<!-- AC:END -->
+
+## Implementation Notes
+
+<!-- SECTION:NOTES:BEGIN -->
+Decision on fail-vs-warn: **warn-only** on both CI and local lint. Rationale: CVE databases evolve independently of the project, so a newly published advisory should surface quickly (annotation in CI) without blocking unrelated PRs or breaking `dev:lint` for devs who may not be able to remediate immediately (e.g. waiting on upstream fix).
+
+Changes:
+- `mix.exs`: added `{:mix_audit, "~> 2.1", only: :dev, runtime: false}` alongside other dev-only quality tools (ex_slop, quokka, credo).
+- `scripts/dev/lint`: added `mix deps.audit || true` between credo and gettext steps.
+- `.github/workflows/test_and_deploy.yml`: added `🛡️ Audit dependencies for CVEs` step in the lint job with `continue-on-error: true`, placed after sobelow.
+
+Verification:
+- `mix deps.get` resolved mix_audit 2.1.x plus yaml_elixir/yamerl transitive deps.
+- `mix deps.audit` currently reports "No vulnerabilities found." — baseline clean.
+- `shellcheck` passes on the updated `scripts/dev/lint`.
+<!-- SECTION:NOTES:END -->