Add new skills based on project conventions

2026-05-04 22:10:04 +01:00
parent 65366738a7
commit 304f6c9dcc
6 changed files with 1211 additions and 0 deletions
@@ -0,0 +1,152 @@
+---
+name: error-investigation
+description: Use this skill when investigating, triaging, debugging, or responding to production errors. Use PROACTIVELY when the user mentions "error", "production error", "crash", "exception", "stacktrace", "error tracker", "ErrorTracker", "mute", "resolve", "triage", "investigate", "debug production", "what's happening in production", or asks about application health. Also use when fetch_production_errors or fetch_production_logs output is available and needs interpretation.
+---
+
+# Error Investigation
+
+Systematic workflow for investigating and triaging production errors. The project
+uses ErrorTracker with email notifications, mute support, and an ignorer for
+non-actionable errors.
+
+## Available Tools
+
+| Tool | Purpose | When to Use |
+|------|---------|------------|
+| `fetch_production_errors` | List/filter errors | Starting point — browse recent errors |
+| `fetch_production_error` | Full error details (stacktrace, occurrences, context) | Deep-dive into a specific error |
+| `fetch_production_logs` | Recent server logs | Correlate errors with server activity |
+
+## Investigation Workflow
+
+### Step 1: Get an Overview
+
+Start with `fetch_production_errors` with narrow filters:
+
+```
+fetch_production_errors(status: "unresolved", limit: 20)
+```
+
+Note the error IDs, reasons, and occurrence counts.
+
+### Step 2: Prioritize
+
+Rank errors by:
+
+1. **Frequency** — errors with hundreds of occurrences need attention
+2. **Recency** — is it still happening or was it a one-off?
+3. **Impact** — does it affect user-facing features or is it background noise?
+
+### Step 3: Deep Dive on Top Candidates
+
+For each candidate, fetch full details:
+
+```
+fetch_production_error(id: 42)
+```
+
+Look at:
+- **Stacktrace** — which module/function is failing?
+- **Occurrences** — pattern over time? Spikes correlate with deployments?
+- **Context** — what request params or user action triggered it?
+- **Breadcrumbs** — what happened before the error?
+
+### Step 4: Correlate with Logs
+
+If the error is infrastructure-related (timeouts, connection issues), check logs:
+
+```
+fetch_production_logs(grep: "timeout", tail: 50)
+```
+
+### Step 5: Determine Action
+
+| Error Type | Action |
+|-----------|--------|
+| Bot scanner (NoRouteError, wp-admin, .env, xmlrpc) | Already ignored by `ErrorIgnorer` — nothing to do |
+| Expected transient (rate limit, timeout) | Monitor occurrence rate; if spiking, check API health |
+| New bug | Create Backlog task, fix, deploy |
+| Recurring non-critical | Mute if understood and not actionable |
+| Resolved by recent deploy | Mark as resolved |
+
+## ErrorIgnorer — What's Already Filtered
+
+`MusicLibrary.ErrorIgnorer` implements `ErrorTracker.Ignorer` and filters non-actionable
+errors. These never appear in the error list:
+
+- `NoRouteError` from bot scanners (wp-admin, .env, xmlrpc, etc.)
+- Other noise classified as non-actionable
+
+If you see errors that look like bot traffic but aren't filtered, they may need to
+be added to `ErrorIgnorer`.
+
+## ErrorTracker.ErrorNotifier
+
+The notifier is a GenServer that:
+- Attaches to ErrorTracker telemetry events
+- **Skips muted errors** — no email sent
+- **Throttles repeated errors** — doesn't spam on recurring issues
+- **Dispatches email** via `Swoosh.Adapters.Mailgun` when an error meets thresholds
+
+### When to Mute
+
+Mute errors that are:
+- **Understood** — you know exactly what causes it
+- **Non-actionable** — can't fix (e.g., third-party API sporadic failures)
+- **Not impactful** — doesn't affect user experience
+- **Frequent enough to be noisy** — would otherwise spam email
+
+Muted errors still appear in the error list but are filtered by default and don't
+trigger email notifications.
+
+### When to Resolve
+
+Mark as resolved when:
+- A fix has been deployed and confirmed working
+- The error was a one-off that won't recur
+- The cause has been eliminated (e.g., config change, dependency update)
+
+## Common Error Patterns
+
+### External API Failures
+
+```
+%Req.TransportError{reason: :timeout}
+%Req.TransportError{reason: :econnrefused}
+```
+
+→ Check if the API is down. These are transient and Oban workers retry automatically.
+Monitor frequency — if spiking, the API may have an outage.
+
+### Rate Limiting
+
+```
+%MusicLibrary.LastFm.API.ErrorResponse{kind: :rate_limit}
+```
+
+→ Workers snooze and retry. Check if rate limit intervals need adjustment.
+
+### Database Issues
+
+```
+%SQLite3.Error{message: "database is locked"}
+```
+
+→ Check for concurrent write contention. `heavy_writes` queue has concurrency 1 to
+prevent this. If happening on `default` queue, consider moving the worker.
+
+### OpenAI Quota
+
+```
+%OpenAI.API.ErrorResponse{kind: :auth_error, body: %{"code" => "insufficient_quota"}}
+```
+
+→ This is a permanent error (`{:cancel, reason}`). Check OpenAI billing/quotas.
+
+## Post-Fix Verification
+
+After deploying a fix:
+1. Watch `fetch_production_errors` for new occurrences
+2. Once confirmed resolved, mark the error as resolved
+3. If the fix involved a worker, check Oban Web for any cancelled jobs related to
+   the error
@@ -0,0 +1,290 @@
+---
+name: external-api-integration
+description: Use this skill when adding, modifying, extending, or debugging ANY external API integration (MusicBrainz, Last.fm, Discogs, Wikipedia, OpenAI, BraveSearch, Mailgun). Use PROACTIVELY when the user mentions "API", "integration", "HTTP client", "rate limit", "Req", "MusicBrainz", "Last.fm", "Discogs", "Wikipedia", "OpenAI", "Brave", "webhook", "external service", "third-party API", "error response", or asks about API patterns. Also use when adding new API calls to existing integrations or creating worker modules for API operations.
+---
+
+# External API Integration
+
+Project-specific patterns for external API integrations. Every integration follows a
+consistent three-module architecture with rate limiting, structured error handling,
+and test stubbing.
+
+## Checklist Before Adding or Modifying an API Integration
+
+1. **Does an `ErrorResponse` module exist?** Every API MUST have a
+   `<API>.API.ErrorResponse` module implementing `MusicLibrary.ErrorResponse` behaviour.
+
+2. **Is rate limiting configured?** Every API uses `Req.RateLimiter` with a
+   per-API interval. Check the architecture doc for existing intervals.
+
+3. **Are test stubs in place?** All HTTP calls must be stubbed via `config/test.exs`
+   using `Req.Test`. Use fixture modules for response data.
+
+4. **Does the worker use `ErrorHandler`?** Workers that call APIs must route errors
+   through `MusicLibrary.Worker.ErrorHandler.to_oban_result/1`.
+
+## Three-Module Architecture
+
+Every external API integration follows this pattern:
+
+```
+lib/music_library/<service>/
+├── <service>.ex           # Facade: public API, delegates to API module
+├── api.ex                 # API: Req HTTP client, rate-limited
+├── api/
+│   ├── config.ex          # Config: NimbleOptions schema, reads from app env
+│   ├── error_response.ex  # ErrorResponse: implements MusicLibrary.ErrorResponse behaviour
+│   └── fixtures.ex        # Fixtures: test response data (for API-specific fixtures)
+```
+
+### Facade Module (`<Service>.ex`)
+
+The public interface. No HTTP logic here — delegates to the API module:
+
+```elixir
+defmodule MusicLibrary.LastFm do
+  @spec get_recent_tracks(keyword()) :: {:ok, [LastFm.Track.t()]} | {:error, term()}
+  def get_recent_tracks(opts \\ []) do
+    LastFm.API.get_recent_tracks(opts)
+  end
+end
+```
+
+### API Module (`api.ex`)
+
+Req-based HTTP client. Uses `Req.RateLimiter` via the request pipeline:
+
+```elixir
+defmodule MusicLibrary.LastFm.API do
+  @spec get_recent_tracks(keyword()) :: {:ok, [LastFm.Track.t()]} | {:error, term()}
+  def get_recent_tracks(opts) do
+    config = LastFm.API.Config.new!(opts)
+
+    Req.new(
+      base_url: "https://ws.audioscrobbler.com/2.0",
+      params: [api_key: config.api_key, format: "json"],
+      plugins: [
+        {Req.RateLimiter, id: :last_fm}
+      ]
+    )
+    |> Req.get(url: "/", params: [method: "user.getRecentTracks", user: config.user])
+    |> case do
+      {:ok, %{status: 200, body: body}} -> {:ok, parse_tracks(body)}
+      {:ok, %{status: status, body: body}} -> {:error, LastFm.API.ErrorResponse.new(status, body)}
+      {:error, error} -> {:error, error}
+    end
+  end
+end
+```
+
+Key patterns:
+- **Config goes first** — build it from opts, validate with NimbleOptions
+- **Rate limiter as Req plugin** — `{Req.RateLimiter, id: :service_name}`
+- **Non-200 status codes** are wrapped in `ErrorResponse` structs, not raw maps
+- **Req-level errors** (timeouts, DNS) are passed through as-is
+- **Successful responses** are parsed into domain structs
+
+### Config Module (`api/config.ex`)
+
+NimbleOptions schema, reads from application env:
+
+```elixir
+defmodule MusicLibrary.LastFm.API.Config do
+  @schema [
+    api_key: [type: :string, required: true],
+    shared_secret: [type: :string, required: false],
+    user: [type: :string, required: false]
+  ]
+
+  def new!(opts) do
+    env = Application.get_env(:music_library, :last_fm, [])
+
+    opts
+    |> Keyword.merge(env)
+    |> NimbleOptions.validate!(@schema)
+  end
+end
+```
+
+## ErrorResponse Behaviour
+
+Every API must have an `ErrorResponse` module implementing:
+
+```elixir
+defmodule MusicLibrary.ErrorResponse do
+  @type t :: struct()
+  @callback retryable?(t()) :: boolean()
+  @callback retry_delay_seconds(t()) :: non_neg_integer() | nil
+end
+```
+
+Example implementation:
+
+```elixir
+defmodule MusicLibrary.LastFm.API.ErrorResponse do
+  defstruct [:status, :body, :kind]
+
+  @behaviour MusicLibrary.ErrorResponse
+
+  def new(status, body) do
+    %__MODULE__{
+      status: status,
+      body: body,
+      kind: MusicLibrary.HttpError.kind(status)  # baseline mapping
+    }
+  end
+
+  @impl true
+  def retryable?(%{kind: :rate_limit}), do: true    # 429
+  def retryable?(%{kind: :server_error}), do: true   # 5xx
+  def retryable?(_), do: false
+
+  @impl true
+  def retry_delay_seconds(%{status: 429, body: %{"error" => _, "message" => msg}}) do
+    MusicLibrary.RetryDelay.parse(msg)  # parse Retry-After header
+  end
+
+  def retry_delay_seconds(_), do: nil
+end
+```
+
+### HTTP Error Kind Mapping (`MusicLibrary.HttpError`)
+
+Baseline mapping used by all APIs:
+
+| HTTP status | Kind |
+|-------------|------|
+| 429 | `:rate_limit` |
+| 5xx | `:server_error` |
+| Timeout | `:timeout` |
+| 401/403 | `:auth_error` |
+| 404 | `:not_found` |
+| 4xx | `:client_error` |
+| Other | `:unknown` |
+
+**Per-API overrides**: Some APIs have quirks that override the baseline:
+- **MusicBrainz**: HTTP 503 is their rate-limit signal (not a server error)
+- **OpenAI**: HTTP 429 with body `code: "insufficient_quota"` is `:auth_error` (permanent), not `:rate_limit`
+
+Override in the ErrorResponse module by pattern-matching on status + body before
+falling through to `MusicLibrary.HttpError.kind/1`.
+
+## Rate Limiting
+
+### Intervals (from architecture doc)
+
+| API | Interval |
+|-----|----------|
+| MusicBrainz | 1000 ms |
+| Last.fm | 500 ms |
+| Discogs | 2000 ms |
+| Wikipedia | 1000 ms |
+| BraveSearch | 1000 ms |
+| OpenAI | 250 ms |
+
+### Req.RateLimiter
+
+ETS-backed, per-API enforcement via Req request step. Configured per-request:
+
+```elixir
+Req.new(plugins: [{Req.RateLimiter, id: :music_brainz}])
+```
+
+The `:id` atom corresponds to the `Req.RateLimiter` ETS table entry. In tests,
+use `Req.RateLimiter.Clock` behaviour with `SystemClock` implementation.
+
+## Worker Error Handling
+
+Workers that call APIs follow this flow:
+
+```elixir
+defmodule MusicLibrary.Worker.FetchArtistInfo do
+  use Oban.Worker
+
+  @impl true
+  def perform(%{args: %{"musicbrainz_id" => mbid}}) do
+    case Artists.fetch_info(mbid) do
+      {:ok, _artist} ->
+        :ok
+
+      {:error, :no_english_wikipedia} ->
+        # App-layer permanent failures — match first
+        {:cancel, :no_english_wikipedia}
+
+      {:error, reason} ->
+        # Forward to ErrorHandler for structured HTTP error handling
+        MusicLibrary.Worker.ErrorHandler.to_oban_result(reason)
+    end
+  end
+end
+```
+
+`ErrorHandler.to_oban_result/1`:
+- `%ErrorResponse{retryable?: true}` → `{:snooze, seconds}` (Oban snooze + retry)
+- `%ErrorResponse{retryable?: false}` → `{:cancel, reason}` (permanent)
+- Other errors → `{:error, reason}` (Oban default retry)
+
+### Worker Return Values
+
+| Return | Meaning | Oban Behaviour |
+|--------|---------|----------------|
+| `:ok` | Success | Job completed |
+| `{:error, reason}` | Transient failure | Default retry with backoff |
+| `{:snooze, seconds}` | Retryable with delay | Retry after `seconds` |
+| `{:cancel, reason}` | Permanent failure | Job cancelled, never retried |
+
+**Never use `{:discard, reason}`** — it's deprecated. Use `{:cancel, reason}`.
+
+## Test Stubs (Req.Test)
+
+All API HTTP calls are stubbed in `config/test.exs`:
+
+```elixir
+# config/test.exs
+config :req, plug: {
+  Req.Test,
+  json_plug: &YourApp.JsonPlug.json/1
+}
+```
+
+Each API's fixture module provides realistic response data. When adding a new API call:
+
+1. **Add stub in `config/test.exs`** matching the request pattern
+2. **Use fixture data** from the appropriate fixtures module
+3. **Cover error cases**: rate limit (429), auth error (401/403), not found (404), server error (5xx)
+
+```elixir
+# Example: stubbing Last.fm with fixture
+Req.Test.stub(LastFm.API, fn conn ->
+  case conn.method do
+    "GET" -> Req.Test.json(conn, LastFm.Fixtures.RecentTracks.get())
+  end
+end)
+```
+
+### Available API Fixture Modules
+
+| Module | API |
+|--------|-----|
+| `MusicBrainz.ReleaseFixtures` | MusicBrainz release data |
+| `MusicBrainz.ReleaseGroupFixtures` | MusicBrainz release group data |
+| `MusicBrainz.ArtistFixtures` | MusicBrainz artist data |
+| `Discogs.ArtistFixtures` | Discogs artist profiles |
+| `LastFm.ArtistFixtures` | Last.fm artist info |
+| `LastFm.Fixtures.RecentTracks` | Last.fm scrobble history |
+| `Wikipedia.Fixtures` | Wikipedia extracts |
+
+## Adding a New API Integration
+
+Step-by-step:
+
+1. **Create directory** `lib/music_library/<service>/`
+2. **Create Config** (`api/config.ex`) — NimbleOptions schema
+3. **Create ErrorResponse** (`api/error_response.ex`) — implement behaviour, handle quirks
+4. **Create API** (`api.ex`) — Req client with rate limiter plugin
+5. **Create Facade** (`<service>.ex`) — public functions, delegates to API
+6. **Register rate limiter** in `config/config.exs` with interval
+7. **Add test stubs** in `config/test.exs`
+8. **Create fixture module** in `test/support/fixtures/`
+9. **Update architecture doc** — add to External API Integrations table
+10. **Add worker** (if needed) — thin wrapper with ErrorHandler
@@ -0,0 +1,130 @@
+---
+name: git-commit
+description: Use this skill when committing code, writing commit messages, staging changes, or preparing to push. Use PROACTIVELY when the user mentions "commit", "git commit", "commit message", "push", "stage", "what should I commit", or when code changes are complete and ready to be committed. Also use when the user asks about commit conventions or style.
+---
+
+# Git Commit Conventions
+
+Project-specific commit message rules and workflow. These conventions are enforced
+and commits that don't follow them will be rejected.
+
+## Checklist Before Committing
+
+1. **Is there a Backlog task?** If the work maps to a task, the task ID MUST prefix
+   the commit subject.
+
+2. **Is the subject under 60 characters?** Including the task ID prefix.
+
+3. **Is this a single logical change?** One change per commit. If a change spans
+   multiple tasks, split the commit.
+
+4. **Did you run precommit checks?** `mise run dev:precommit` before committing.
+
+## Commit Message Format
+
+```
+<task-id-prefix>: <imperative present tense summary, ≤60 chars>
+
+<optional body with details, wrapped at 72 chars>
+
+<version table for dependency updates>
+```
+
+### Subject Line Rules
+
+- **Imperative present tense**: "Add scrobble rule validation" NOT "Added..." or "Adds..."
+- **Describe intent/behavior**, not implementation details: "Fix record ordering on search page" NOT "Change ORDER BY clause in list_records"
+- **Single-line, under 60 characters**
+- **Task ID prefix when applicable**: `ML-3: fix scrobble rule ordering`
+- The prefix counts toward the 60-char limit
+
+### Examples
+
+```
+# With Backlog task
+ML-3: fix scrobble rule ordering
+
+# Without task (trivial fixes)
+Fix typo in artist biography template
+
+# Dependency updates
+Update dependencies
+
+mix:
+  bandit 1.6.10 => 1.6.11
+  phoenix_live_view 1.1.0 => 1.1.1
+
+# NPM dependency updates
+Update npm dependencies
+
+npm:
+  tailwindcss 4.1.4 => 4.1.5
+  sortablejs 1.15.3 => 1.15.6
+
+# Revert
+Revert "ML-3: fix scrobble rule ordering"
+```
+
+### Dependency Update Format
+
+The subject is **always** "Update dependencies" (Mix) or "Update npm dependencies" (npm).
+Never use "bump" or "upgrade" in the subject.
+
+The body MUST list each specific version change as `package_name from => to`:
+
+```
+mix:
+  bandit 1.6.10 => 1.6.11
+  phoenix_live_view 1.1.0 => 1.1.1
+```
+
+### What NOT to Include
+
+- ❌ "Co-Authored-By" references in the message body
+- ❌ Multi-line subjects
+- ❌ Past tense ("Added", "Fixed")
+- ❌ Implementation details ("Changed the SQL query")
+- ❌ "bump" or "upgrade" for dependency updates
+- ❌ Multiple unrelated changes in one commit
+
+## Workflow
+
+### Before Committing
+
+```bash
+# Run precommit checks
+mise run dev:precommit
+```
+
+This runs: format check, gettext check, credo, sobelow, mix_audit, shellcheck,
+Docker image validation, and asset build.
+
+### Staging
+
+Stage specific files, not `git add .`:
+
+```bash
+git add lib/music_library/records.ex test/music_library/records_test.exs
+```
+
+### Commit Message Validation
+
+After writing the commit message, verify:
+1. Subject is imperative present tense
+2. Subject is ≤60 characters (including task ID)
+3. Task ID references the correct Backlog task
+4. Body describes what changed (if non-trivial)
+
+### Backlog Task Integration
+
+- **One task ID per commit subject.** If work spans tasks, split the commit.
+- **Read the task's implementation plan** before committing to verify the plan
+  was followed.
+- **When finalizing a task**, the commit should be the last logical change
+  for that task.
+
+## Prohibited
+
+- **Never create, comment on, close, or reopen GitHub issues** — only the user does that. Use Backlog.md for task management.
+- **Never reference GitHub issue numbers** in commit messages. Use Backlog task IDs.
+- **Never use `{:discard, reason}`** — use `{:cancel, reason}` in Oban workers (related but common in commits).
@@ -0,0 +1,216 @@
+---
+name: oban-worker
+description: Use this skill when creating, modifying, debugging, or reviewing ANY Oban worker. Use PROACTIVELY when the user mentions "worker", "Oban", "background job", "job", "cron", "queue", "perform", "enqueue", "schedule", "async", "snooze", "cancel", "discard", or asks about running work in the background. Also use when adding Oban plugins, changing queue configuration, or writing worker tests.
+---
+
+# Oban Worker Development
+
+Project-specific patterns for Oban background job workers. Every worker follows
+consistent conventions for structure, error handling, queue assignment, and testing.
+
+## Checklist Before Creating or Modifying a Worker
+
+1. **Does the worker delegate to a context module?** `perform/1` must be a thin wrapper.
+   No business logic inline.
+
+2. **Is the queue correct?** Rate-limited APIs get dedicated queues with concurrency 1.
+   DB-intensive operations go to `heavy_writes`. General tasks go to `default`.
+
+3. **Does error handling use `ErrorHandler`?** Workers calling APIs route HTTP errors
+   through `MusicLibrary.Worker.ErrorHandler.to_oban_result/1`.
+
+4. **Is the worker registered under the correct Oban instance?** Production uses
+   `MusicLibrary.BackgroundRepo` as the Oban instance.
+
+## Worker Structure
+
+```elixir
+defmodule MusicLibrary.Worker.FetchArtistInfo do
+  use Oban.Worker,
+    queue: :default,           # Pick the right queue (see below)
+    max_attempts: 3,            # Oban default, override if needed
+    unique: [period: 60]        # Optional: prevent duplicate jobs
+
+  @impl true
+  def perform(%Oban.Job{args: %{"musicbrainz_id" => mbid}}) do
+    # Delegate to context — no business logic here
+    case Artists.fetch_info(mbid) do
+      {:ok, _artist} ->
+        :ok
+
+      # App-layer permanent failures matched FIRST
+      {:error, :no_english_wikipedia} ->
+        {:cancel, :no_english_wikipedia}
+
+      {:error, :cover_not_available} ->
+        {:cancel, :cover_not_available}
+
+      # Everything else routed through ErrorHandler
+      {:error, reason} ->
+        MusicLibrary.Worker.ErrorHandler.to_oban_result(reason)
+    end
+  end
+end
+```
+
+## Queues
+
+| Queue | Concurrency | Use For |
+|-------|-------------|---------|
+| `default` | 10 | General async tasks, non-API operations |
+| `heavy_writes` | 1 | DB-intensive or serialized operations |
+| `music_brainz` | 1 | MusicBrainz API calls |
+| `discogs` | 1 | Discogs API calls |
+| `wikipedia` | 1 | Wikipedia API calls |
+| `last_fm` | 1 | Last.fm API calls |
+
+**Rule**: Any worker that calls an external API goes on that API's dedicated queue.
+The concurrency of 1 combined with `Req.RateLimiter` ensures the API rate limit is
+never exceeded.
+
+## Return Values
+
+| Return | Meaning | Oban Behaviour |
+|--------|---------|----------------|
+| `:ok` | Success | Job marked complete |
+| `{:ok, result}` | Success with result | Job marked complete |
+| `{:error, reason}` | Transient failure | Oban retries with backoff |
+| `{:snooze, seconds}` | Retryable with delay | Oban retries after `seconds` |
+| `{:cancel, reason}` | Permanent failure | Job cancelled, never retried |
+
+**CRITICAL**: Never use `{:discard, reason}` — it's deprecated since Oban 2.14.
+Always use `{:cancel, reason}`.
+
+## Error Handling Flow
+
+```
+perform/1
+  ├── App-layer permanent failures (match FIRST)
+  │   ├── :no_english_wikipedia → {:cancel, :no_english_wikipedia}
+  │   ├── :cover_not_available → {:cancel, :cover_not_available}
+  │   └── ...other domain-specific permanent failures
+  │
+  └── Everything else → MusicLibrary.Worker.ErrorHandler.to_oban_result(reason)
+      ├── %ErrorResponse{retryable?: true}  → {:snooze, seconds}
+      ├── %ErrorResponse{retryable?: false} → {:cancel, reason}
+      └── Other errors                      → {:error, reason}
+```
+
+The order matters: **app-layer atoms first**, then fall through to `ErrorHandler`.
+This ensures domain-specific permanent failures (e.g., no Wikipedia article exists)
+are handled correctly before generic HTTP error classification kicks in.
+
+## Cron Workers
+
+Scheduled via `Oban.Plugins.Cron` in production config:
+
+```elixir
+# config/prod.exs
+config :music_library, Oban,
+  plugins: [
+    {Oban.Plugins.Cron,
+     crontab: [
+       {"@daily", MusicLibrary.Worker.SendRecordsOnThisDayEmail},
+       {"*/5 * * * *", MusicLibrary.Worker.RefreshScrobbles}
+     ]}
+  ]
+```
+
+Cron workers follow the same thin-wrapper pattern. The cron expression is the only
+difference from on-demand workers.
+
+### All Cron Workers
+
+| Schedule | Worker | Queue |
+|----------|--------|-------|
+| Every 12h | `ApplyScrobbleRules` | heavy_writes |
+| Every 12h | `PruneAssetCache` | default |
+| Daily 2 AM | `PruneAssets` | default |
+| Daily 3 AM | `RepoVacuum` | heavy_writes |
+| Daily 4 AM | `RepoOptimize` | heavy_writes |
+| Monthly 1st, 6 AM | `RecordRefreshAllMusicBrainzData` | music_brainz |
+| Monthly 1st, 7 AM | `RecordGenerateAllEmbeddings` | heavy_writes |
+| Monthly 1st, 8 AM | `ArtistRefreshAllMusicBrainzData` | music_brainz |
+| Monthly 1st, 9 AM | `ArtistRefreshAllDiscogsData` | discogs |
+| Monthly 1st, 10 AM | `ArtistRefreshAllWikipediaData` | wikipedia |
+| Daily 7 AM | `SendRecordsOnThisDayEmail` | default |
+| Every 5 min | `RefreshScrobbles` | last_fm |
+
+## Oban Plugins (Production)
+
+| Plugin | Config | Purpose |
+|--------|--------|---------|
+| `Pruner` | `max_age: 43200` (12h) | Clean up completed jobs |
+| `Reindexer` | `schedule: "@weekly"` | Reindex for query performance |
+| `Cron` | timezone: `"Europe/London"` | Schedule recurring jobs |
+
+## Batch Workers (Self-Chaining)
+
+Some workers process data in batches, enqueuing their next batch on completion:
+
+```elixir
+def perform(%{args: %{"page" => page}}) do
+  case process_page(page) do
+    {:ok, []} ->
+      :ok  # No more data, done
+
+    {:ok, _results} ->
+      # Enqueue next page
+      %{page: page + 1}
+      |> __MODULE__.new()
+      |> Oban.insert()
+
+      :ok
+  end
+end
+```
+
+The `Records.Batch` and `Artists.Batch` modules provide shared batch infrastructure
+(`stream + transaction + error accumulation`). Bulk workers should use these rather
+than implementing their own batching logic.
+
+## Testing Workers
+
+### Manual mode
+
+Oban runs in manual testing mode — jobs don't auto-execute. Use `perform_job/2`:
+
+```elixir
+import Oban.Testing, only: [perform_job: 2]
+
+test "fetches artist info" do
+  job = MyWorker.new(%{musicbrainz_id: "abc-123"})
+  assert :ok = perform_job(MyWorker, job.args)
+end
+```
+
+### Assert enqueued downstream jobs
+
+Workers that enqueue other jobs MUST verify with `assert_enqueued`:
+
+```elixir
+assert_enqueued(worker: FetchArtistImage, args: %{musicbrainz_id: "abc-123"})
+```
+
+`perform_job` returning `{:ok, []}` is NOT sufficient — verify the expected downstream
+workers were enqueued with correct args.
+
+### Testing all return states
+
+```elixir
+# Success
+assert :ok = perform_job(MyWorker, %{id: valid_id})
+
+# Transient (Oban retries)
+assert {:error, :rate_limit} = perform_job(MyWorker, %{id: rate_limited_id})
+
+# Permanent (job cancelled)
+assert {:cancel, :no_english_wikipedia} = perform_job(MyWorker, %{id: missing_id})
+```
+
+## Non-Fatal Enrichment
+
+Context functions that enrich records (colors, embeddings) use a private
+`best_effort_*` helper pattern — log a warning and return unchanged data rather
+than surfacing `{:error, reason}`. Workers calling these functions don't need
+error handling for enrichment failures.
@@ -0,0 +1,194 @@
+---
+name: sqlite-optimization
+description: Use this skill when writing, reviewing, or optimizing ANY Ecto query, SQL fragment, migration, or database index. Use PROACTIVELY when the user mentions "query", "SQL", "database", "index", "migration", "FTS", "full-text search", "json_extract", "json_each", "GROUP BY", "subquery", "performance", "slow query", "N+1", "Ecto query", "Repo.all", "Repo.one", or asks about database design. Also use when adding WHERE clauses, JOINs, or changing how data is fetched. The SQLite-specific patterns in this project are easy to miss and cause silent performance regressions.
+---
+
+# SQLite Optimization
+
+Project-specific SQLite query patterns, index strategies, and migration conventions.
+These rules are load-bearing — dev datasets are small enough to hide problems that
+degrade production performance.
+
+## Checklist Before Writing Any Database Query
+
+1. **Check for existing expression indexes** before writing a query. Your `WHERE` or
+   `GROUP BY` clause must match the index expression textually.
+
+2. **If no index exists for your query**, add one in a migration with an `up`/`down`
+   and a comment explaining which query it helps.
+
+3. **FTS5 tables are trigger-synced** — never INSERT/UPDATE/DELETE directly on
+   `records_search_index`. Write to `records` instead.
+
+4. **Config-driven constants.** Pagination defaults and magic numbers live in
+   `config/config.exs`, read via `Application.compile_env!/2`.
+
+## Expression Indexes & SQL Text Matching
+
+SQLite treats `json_extract(column, '$.path')` and `column ->> '$.path'` as
+**semantically equal but textually distinct**. If an expression index uses
+`json_extract`, your query MUST use the same text:
+
+```elixir
+# Index definition (in migration):
+# CREATE INDEX idx_records_format ON records(json_extract(data, '$.format'));
+
+# CORRECT — matches index text
+from(r in Record, where: fragment("json_extract(?, '$.format')", r.data) == "vinyl")
+
+# WRONG — won't use the index
+from(r in Record, where: fragment("? ->> '$.format'", r.data) == "vinyl")
+```
+
+Same applies to `GROUP BY` — match the expression index exactly:
+
+```elixir
+# Index exists on: json_extract(r.data, '$.artist_name')
+# CORRECT GROUP BY:
+from(r in Record,
+  group_by: fragment("json_extract(?, '$.artist_name')", r.data)
+)
+```
+
+## Subquery Materialization with `limit: -1`
+
+When a date-filtered subquery feeds an outer `GROUP BY`, SQLite may flatten the
+subquery and prefer the wrong composite index. **Force materialization with
+`limit: -1`**:
+
+```elixir
+filtered = from(t in Track,
+  where: t.scrobbled_at >= ^start_date,
+  limit: -1  # forces materialization, preserves range-scan index
+)
+
+from(t in subquery(filtered),
+  group_by: t.artist_name,
+  select: %{artist: t.artist_name, count: count()}
+)
+|> Repo.all()
+```
+
+## Correlated Scalar Subqueries
+
+For **small-LIMIT result enrichment** from large lookup tables, use correlated scalar
+subqueries instead of `LEFT JOIN`. Cost scales with the outer LIMIT, not the lookup
+table size:
+
+```elixir
+# CORRECT — cost proportional to LIMIT
+from(r in Record,
+  limit: 20,
+  select: %{
+    id: r.id,
+    title: r.title,
+    scrobble_count: fragment(
+      "(SELECT count(*) FROM scrobbled_tracks WHERE record_id = ?)", r.id
+    )
+  }
+)
+
+# WRONG — joins entire scrobbled_tracks table
+from(r in Record,
+  left_join: s in subquery(counts_query), on: s.record_id == r.id,
+  limit: 20
+)
+```
+
+## FTS5 Full-Text Search
+
+The `records_search_index` table is an FTS5 **virtual table auto-synced via database
+triggers**. Treat it as read-only:
+
+```elixir
+# CORRECT — insert into the source table, triggers sync FTS
+Repo.insert!(%Record{title: "Dark Side of the Moon", ...})
+
+# CORRECT — read from FTS index
+from(ri in "records_search_index",
+  where: fragment("records_search_index MATCH ?", ^query)
+)
+
+# WRONG — never write directly to FTS
+Repo.insert_all("records_search_index", [...])
+```
+
+## JSON Column Patterns
+
+### json_each() for expanding arrays
+
+```elixir
+from(r in Record,
+  cross_join: fragment("json_each(?)", r.genres),
+  where: fragment("value = ?", ^genre),
+  select: r
+)
+```
+
+### json_extract() for field access
+
+```elixir
+from(r in Record,
+  where: fragment("json_extract(?, '$.format') = ?", r.data, ^format)
+)
+```
+
+## Materialized Views via Triggers
+
+SQLite lacks native materialized views. Use explicit `up`/`down` in migrations for
+non-reversible DDL. Read-only schemas for materialized/view tables:
+
+```elixir
+# Read-only schema (no PK, no changeset, no timestamps)
+defmodule Records.RecordRelease do
+  use Ecto.Schema
+  @primary_key false
+  schema "record_releases" do
+    field :record_id, :binary_id
+    field :release_id, :string
+    field :cover_hash, :string
+    field :purchased_at, :utc_datetime
+  end
+end
+```
+
+## Migration Conventions
+
+Every migration must:
+- Provide **both `up` and `down`** for any `execute` block
+- Comment every index explaining which query it helps
+- Use explicit triggers for materialized views
+
+```elixir
+def change do
+  execute(
+    # UP
+    "CREATE INDEX idx_records_purchase_date ON records(purchased_at);",
+    # DOWN — always provided
+    "DROP INDEX IF EXISTS idx_records_purchase_date;"
+  )
+end
+```
+
+## Query Patterns to Avoid
+
+| Anti-pattern | Why | Fix |
+|-------------|-----|-----|
+| `Repo.all()` then `Enum.filter()` | Loads entire table into memory | Filter in SQL with `where:` |
+| `Enum.map()` with inline `Repo` calls | N+1 queries | Use preload or joins |
+| `preload: [:association]` on large result sets | Cartesian product in memory | Use `join:` and `select:` |
+| Dynamic `order_by` without corresponding index | Full table sort | Add composite index matching sort columns |
+| `distinct: true` without index | Temporary B-tree | Add index on distinct columns |
+
+## Using QueryReporter to Verify
+
+Before committing a query change, use the query-reporter skill to capture the actual
+SQL and verify:
+1. The query uses the expected indexes (check `EXPLAIN QUERY PLAN`)
+2. No unexpected full table scans
+3. Subqueries are materialized when needed
+
+```bash
+# In SQLite console (mise run dev:sqlite-console):
+EXPLAIN QUERY PLAN SELECT ...;
+```
@@ -0,0 +1,229 @@
+---
+name: testing
+description: Use this skill when writing, running, debugging, or fixing ANY tests. Also use when the user mentions "test", "testing", "spec", "ExUnit", "assert", "fixture", "test helper", "test setup", "run the tests", "test failure", "failing test", "add a test", "write a test", "test coverage", "LiveViewTest", "PhoenixTest", "sandbox", or asks about test conventions. Use PROACTIVELY whenever creating or modifying test files, test/support modules, or when a code change requires test updates.
+---
+
+# Testing
+
+Project-specific testing conventions, patterns, and pitfalls for the Music Library
+codebase. These rules override generic Elixir/Phoenix testing defaults.
+
+## Checklist Before Writing Any Test
+
+1. **Feature setup stays in the test module that needs it**, not in shared case templates
+   (`ConnCase`, `DataCase`). Only add to shared templates when EVERY test using that
+   template genuinely needs it.
+
+2. **Use fixture modules for test data.** Fixtures call through context functions (not
+   raw `Repo.insert`). Use `System.unique_integer([:positive])` for unique names.
+
+3. **Assert specific values, not just shapes.** Prefer `assert data == expected` over
+   `assert data != nil` or `assert {:ok, _} = result`. Wildcard matches (`_`) signal the
+   test is too vague.
+
+4. **Error assertions must match the specific error type.** `assert {:error, _reason}` is
+   too broad. Match the struct or atom: `%Req.TransportError{reason: :timeout}`, `:no_session_key`.
+
+5. **Verify outcomes through context modules**, not just UI assertions. Delete tests
+   should assert both `refute has_element?` AND `assert_raise Ecto.NoResultsError`.
+
+6. **No boilerplate-only tests.** Don't add test files that just verify Phoenix generator
+   output. Tests must exercise application behaviour.
+
+## Running Tests
+
+```bash
+# All tests (partitioned, like CI)
+mise run test
+
+# Specific file
+mix test test/music_library/records_test.exs
+
+# Specific test by line number
+mix test test/music_library/records_test.exs:123
+
+# With tags
+mix test --only tag_name
+mix test --only logged_out
+
+# Limit failures for faster feedback
+mix test --max-failures 5
+```
+
+## Test Styles — When to Use Each
+
+### PhoenixTest (`assert_has`, `click_button`, `click_link`, `visit`)
+
+Use for **page-level LiveView tests** where you're testing navigation and DOM presence:
+
+```elixir
+{:ok, view, _html} = live(conn, ~p"/collection")
+assert_has(view, ".record-card")
+```
+
+### Phoenix.LiveViewTest (`form/3`, `render_submit/1`, `element/2`, `render_click/1`)
+
+Use for **LiveComponent interactions** where `phx-target={@myself}` is involved:
+
+```elixir
+html = render_submit(form(view, "#record-form", record: attrs))
+```
+
+### Context Tests (standard ExUnit + DataCase)
+
+Use for **business logic** that doesn't involve the web layer:
+
+```elixir
+assert {:ok, %Record{}} = Records.create_record(valid_attrs)
+```
+
+## SQLite Test Gotchas
+
+### Timestamp precision
+
+SQLite `utc_datetime` has **second-level precision**. Rapid inserts get identical
+timestamps, breaking ordering assertions:
+
+```elixir
+# DON'T rely on auto-timestamps for ordering
+record1 = record_fixture()
+record2 = record_fixture()
+
+# DO manually set timestamps when order matters
+{:ok, record1} = Repo.update_all(Record, set: [inserted_at: ~N[2024-01-01 00:00:00]])
+{:ok, record2} = Repo.update_all(Record, set: [inserted_at: ~N[2024-01-02 00:00:00]])
+```
+
+### VACUUM / low-level ops
+
+Database operations like `VACUUM` cannot run in the test sandbox. **Do not write a test**
+that asserts the sandbox error message — delete or skip it.
+
+## Swoosh Email Testing
+
+Use `Swoosh.Adapters.Sandbox`, NOT `Swoosh.Adapters.Test`:
+
+```elixir
+setup do
+  SwooshSandbox.checkout()
+  on_exit(fn -> SwooshSandbox.checkin() end)
+end
+```
+
+When the mailer is invoked from a **separate process** (e.g., a GenServer started in
+the test), share the sandbox:
+
+```elixir
+SwooshSandbox.allow(self(), gen_server_pid)
+```
+
+## API Stubs (Req.Test)
+
+All external HTTP calls are stubbed in tests via `config/test.exs` using `Req.Test`.
+Each API has fixture modules that provide realistic response data.
+
+When adding a new API call, you must:
+1. Add a stub in `config/test.exs`
+2. Use fixture data from the appropriate fixtures module
+3. Ensure the stub covers error cases too (rate limiting, auth errors, 404s)
+
+## Worker Tests
+
+Workers that enqueue other jobs MUST use `assert_enqueued`:
+
+```elixir
+# DON'T just check perform_job return value
+assert {:ok, []} = perform_job(worker, args)
+
+# DO verify downstream enqueues
+assert_enqueued(worker: SomeDownstreamWorker, args: %{id: id})
+```
+
+Worker return value testing:
+```elixir
+# Success
+assert :ok = perform_job(MyWorker, %{id: id})
+
+# Transient error (Oban will retry)
+assert {:error, :rate_limit} = perform_job(MyWorker, %{id: id})
+
+# Permanent termination
+assert {:cancel, :no_english_wikipedia} = perform_job(MyWorker, %{id: id})
+```
+
+## Don't Duplicate Assertions
+
+### Don't test the same guard at every call site
+
+If a shared check (e.g., session key presence) is enforced in one place, test it once.
+Don't duplicate the same assertion across every function that calls the shared check.
+
+### Consolidate identical assertions across endpoints
+
+When multiple routes share the same plug/middleware behaviour (e.g., auth), test it
+once with a loop or parameterised approach, not N separate identical tests.
+
+## Test Organization
+
+### Tags
+
+```elixir
+@tag :logged_out     # Public endpoint tests (no auth)
+@tag :capture_log    # Tests with expected error log output
+
+# Run only tagged tests
+mix test --only logged_out
+```
+
+### Test modules structure
+
+```
+test/
+├── music_library/         # Context tests
+├── music_library_web/     # LiveView/controller tests
+└── support/
+    ├── conn_case.ex       # HTTP test setup
+    ├── data_case.ex       # Database sandbox
+    ├── live_test_helpers.ex
+    └── fixtures/          # Fixture modules
+```
+
+### Available fixture modules
+
+| Module | Creates |
+|--------|---------|
+| `RecordsFixtures` | Records with MusicBrainz data |
+| `RecordSetsFixtures` | Record sets with items |
+| `OnlineStoreTemplatesFixtures` | Store templates |
+| `ArtistInfoFixtures` | ArtistInfo records |
+| `ScrobbleRulesFixtures` | Scrobble rules |
+| `ScrobbledTracksFixtures` | Last.fm tracks |
+| `Discogs.ArtistFixtures` | Discogs API responses |
+| `LastFm.ArtistFixtures` | Last.fm API responses |
+| `MusicBrainz.*Fixtures` | MusicBrainz API responses |
+| `Wikipedia.Fixtures` | Wikipedia API responses |
+
+Always call through the fixture module — never `Repo.insert` directly.
+
+## Testing JS Hooks
+
+Use `render_hook/3` for testing JS hook interactions:
+
+```elixir
+assert render_hook(view, "music_library:clipcopy", %{text: "copy me"}) =~ "copy me"
+```
+
+## Testing `handle_async`
+
+Always handle all three cases:
+
+```elixir
+assert {:noreply, socket} = handle_async(socket, :task_ref, {:ok, {:ok, result}})
+assert {:noreply, socket} = handle_async(socket, :task_ref, {:ok, {:error, reason}})
+assert {:noreply, socket} = handle_async(socket, :task_ref, {:exit, reason})
+```
+
+## Oban in Tests
+
+Oban runs in **manual testing mode** — jobs don't auto-execute. Use `perform_job/2`
+from `Oban.Testing` to execute them explicitly.