# Tier 2 Papercuts Implementation Plan

> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax.

**Goal:** Clear a batch of small, well-specified correctness/observability/UX fixes from the issue tracker (#22, #18, #9, #4, #34, #31, #32, #37) — no new features.

**Architecture:** Independent small fixes grouped by area into four tasks: backend API behaviour (#22, #18), backend cleanup (#9, #4), frontend states/a11y (#34, #31, #32, #37), then verification.

**Tech Stack:** Rust (axum, sqlx, tracing), React + TS, TanStack Query, react-i18next, Vitest + RTL + MSW.

**Conventions (every task):** nightly `cargo +nightly fmt`; `cargo clippy`. Frontend: no `any`/`eslint-disable`/`@ts-ignore`; en/sv i18n parity; no codename "biggus"/"dickus". Test infra via compose: `DATABASE_URL=postgres://postgres:postgres@localhost:5442/cms_dev` (this machine's override port), `MEILI_URL=http://localhost:7700`, `MEILI_MASTER_KEY=masterKey`. cargo from repo root; web from `web/`.

---

## Task 1: Backend API — 404 for missing vocabulary (#22) + log public 500s (#18)

**Files:** Modify `crates/api/src/admin_vocab.rs`, `crates/api/src/public.rs`; Test in the existing `crates/api/tests/admin_catalog.rs` (vocab/authority harness).

### #22 — `add_term` returns 404 when the vocabulary doesn't exist
Today `db::vocab::add_term(...)` maps every error to 500; a well-formed `{id}` for a missing vocabulary triggers a foreign-key violation (SQLSTATE 23503) that should be **404**.

- [ ] **Step 1: Failing test** — add to `crates/api/tests/admin_catalog.rs` (mirror its existing seed-editor/login/oneshot harness). Read the file first to reuse its helpers:
```rust
#[sqlx::test(migrations = "../db/migrations")]
async fn add_term_to_missing_vocabulary_is_404(pool: PgPool) {
    // (use this file's existing migrate_sessions + seed editor + login helpers)
    let app = /* build_app with state */;
    let cookie = /* login as editor */;
    let resp = app
        .oneshot(
            Request::builder()
                .method("POST")
                .uri("/api/admin/vocabularies/00000000-0000-0000-0000-000000000000/terms")
                .header(header::COOKIE, &cookie)
                .header(header::CONTENT_TYPE, "application/json")
                .body(Body::from(r#"{"labels":[{"lang":"en","label":"X"}]}"#))
                .unwrap(),
        )
        .await
        .unwrap();
    assert_eq!(resp.status(), StatusCode::NOT_FOUND);
}
```
(Match the exact helper names/signatures already in `admin_catalog.rs`. If that file doesn't have a login helper, copy the pattern from `crates/api/tests/admin_fields.rs`.)

- [ ] **Step 2: Run → fails** (currently 500): `cargo test -p api --test admin_catalog add_term_to_missing_vocabulary`.

- [ ] **Step 3: Fix** — in `crates/api/src/admin_vocab.rs` `add_term`, replace:
```rust
    let term_id = db::vocab::add_term(&mut tx, &new)
        .await
        .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
```
with:
```rust
    let term_id = db::vocab::add_term(&mut tx, &new).await.map_err(|err| {
        // A well-formed id for a missing vocabulary hits the FK constraint (23503).
        if err.as_database_error().and_then(|e| e.code()).as_deref() == Some("23503") {
            StatusCode::NOT_FOUND
        } else {
            tracing::error!(?err, "adding term");
            StatusCode::INTERNAL_SERVER_ERROR
        }
    })?;
```

- [ ] **Step 4: Run → passes**, and confirm adding a term to an existing vocab still returns 201 (existing tests cover this).

### #18 — log the discarded `sqlx::Error` on public 500 paths
`crates/api/src/public.rs` discards errors via `.map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)` (lines ~74, ~78) and `Err(_) => StatusCode::INTERNAL_SERVER_ERROR.into_response()` (line ~109). `tracing` is already a dependency of the `api` crate — just log.

- [ ] **Step 5:** In `list_objects`, change both `.map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?` to:
```rust
        .map_err(|err| {
            tracing::error!(?err, "listing public objects");
            StatusCode::INTERNAL_SERVER_ERROR
        })?;
```
(use a message specific to each call site — e.g. "listing public objects" and "counting public objects" — match what each query does).

- [ ] **Step 6:** In `get_object`, change the `Err(_) => StatusCode::INTERNAL_SERVER_ERROR.into_response()` arm to bind and log the error:
```rust
        Err(err) => {
            tracing::error!(?err, "fetching public object");
            StatusCode::INTERNAL_SERVER_ERROR.into_response()
        }
```

- [ ] **Step 7: Verify** — `cargo +nightly fmt`, `cargo clippy -p api --all-targets`, `cargo test -p api`. Commit:
```bash
git add crates/api
git commit -m "fix(api): 404 when adding a term to a missing vocabulary (#22); log public 500s (#18)"
```

---

## Task 2: Backend cleanup — enum/CHECK cross-refs (#9) + dead clone & test handle (#4)

**Files:** Modify `crates/domain/src/authority.rs`, `crates/domain/src/audit.rs`, `crates/server/src/lib.rs`, `crates/server/tests/serve.rs`.

> Do **not** edit any file under `crates/db/migrations/` — `sqlx::migrate!()` checksums applied migrations, so editing them (even a comment) breaks existing databases. The cross-reference comments go in the Rust enums only.

- [ ] **Step 1: #9 — cross-reference comments.**
  - In `crates/domain/src/authority.rs`, above `pub enum AuthorityKind`, add:
    ```rust
    /// Allowed kinds. NOTE: kept in sync by hand with the
    /// `CHECK (kind IN ('person','organisation','place'))` constraint in
    /// `crates/db/migrations/0002_vocabularies_authorities.sql` — update both together.
    ```
  - In `crates/domain/src/audit.rs`, above `pub enum AuditAction`, add an equivalent comment pointing at the `action` CHECK in `crates/db/migrations/0001_*.sql` (open the migration to name the exact file + values).

- [ ] **Step 2: #4 — remove the dead clone.** In `crates/server/src/lib.rs` `run`, the `AppState` is built with `app_name: config.app_name.clone()`. Since `config.app_name` is a `String` and the only later use of `config` is the disjoint field `config.bind_addr`, change it to a move:
```rust
        app_name: config.app_name,
```
Confirm it still compiles (partial move of one field; `&config.bind_addr` afterward is fine).

- [ ] **Step 3: #4 — smoke-test handle.** Open `crates/server/tests/serve.rs`. The spawned `serve(...)` task's `.unwrap()` swallows server errors as a task panic, surfacing as a confusing client error. Capture the `JoinHandle` and, after the assertions, either abort it cleanly or check it didn't error — make a server-start failure surface as a clear test failure rather than a `reqwest` error. Read the file and apply the minimal change that propagates/surfaces the server error (e.g. keep the handle, assert it hasn't finished-with-error, or `handle.abort()` at the end). Keep the test green.

- [ ] **Step 4: Verify** — `cargo +nightly fmt`, `cargo clippy --workspace --all-targets`, `cargo test -p server -p domain`. Commit:
```bash
git add crates/domain crates/server
git commit -m "chore: cross-ref enum/CHECK constraints (#9); drop dead clone + harden smoke test (#4)"
```

---

## Task 3: Frontend — search 503 (#34), list error states (#31), a11y + dead keys (#32), authority-kind test (#37)

**Files:** Modify `web/src/api/queries.ts`, `web/src/search/search-panel.tsx`, `web/src/vocab/vocabulary-terms.tsx`, `web/src/authorities/authorities-page.tsx`, `web/src/i18n/{en,sv}.json`, `web/src/fields/fields.test.tsx`; Tests in `web/src/search/search.test.tsx`, plus the vocab/authorities test files.

### #34 — distinguish search 503 ("unavailable") from a generic error
- [ ] **Step 1:** In `web/src/api/queries.ts`, add a tiny typed error and have `useSearch` throw it with the HTTP status (so the UI can branch without `any`). Near the top:
```ts
export class HttpError extends Error {
  constructor(public readonly status: number) {
    super(`HTTP ${status}`);
    this.name = "HttpError";
  }
}
```
In `useSearch`'s `queryFn`, replace `if (error || !data) throw new Error("search failed");` with:
```ts
      if (error || !data) throw new HttpError(response.status);
```
(`response` is already destructured from `api.GET`; if not, add it.)

- [ ] **Step 2: i18n** — add `search.unavailable` to BOTH `en.json` and `sv.json` (parity):
  - en: `"unavailable": "Search is not available on this server"`
  - sv: `"unavailable": "Sök är inte tillgängligt på den här servern"`

- [ ] **Step 3:** In `web/src/search/search-panel.tsx`, where `search.isError` renders `t("search.loadError")`, branch on a 503:
```tsx
        {hasQuery && search.isError && (
          <p className="p-4 text-sm text-red-600">
            {search.error instanceof HttpError && search.error.status === 503
              ? t("search.unavailable")
              : t("search.loadError")}
          </p>
        )}
```
Import `HttpError` from `../api/queries`.

- [ ] **Step 4: Tests** — in `web/src/search/search.test.tsx`, add: a `503` response → renders `search.unavailable`; a `500` response → renders `search.loadError`. (Use `server.use(http.get("/api/admin/search", () => new HttpResponse(null, { status: 503 })))` etc., then type a query and assert the text.)

### #31 — loading/error states on the terms + authorities lists
- [ ] **Step 5:** In `web/src/vocab/vocabulary-terms.tsx`, the terms list uses `useTerms(id)` but renders empty/data only. Add `isLoading` (skeleton or `…`) and `isError` (`t("vocab.loadError")`) branches before the empty/data render, mirroring `vocabulary-list.tsx`'s state ladder.
- [ ] **Step 6:** In `web/src/authorities/authorities-page.tsx`, the list uses `useAuthorities(kind)`; add an `isError` branch rendering `t("authorities.loadError")` (currently a dead key — this uses it) and a loading branch. Keep the existing empty/data render.
- [ ] **Step 7: Tests** — add an error-state test to the vocab and authorities test files: MSW returns 500 for the terms / authorities GET → the respective `loadError` text appears. (Override the default handler with `server.use(...)`.)

### #32 — ARIA tab semantics + remove dead i18n keys
- [ ] **Step 8:** In `web/src/authorities/authorities-page.tsx`, the kind tabs are `NavLink`s. Add tab semantics: wrap them in a container with `role="tablist"`, give each `role="tab"` and `aria-selected={isActive}` (the `NavLink` className callback already exposes `isActive` — use the render-prop form to set `aria-selected`). Keep the existing styling.
- [ ] **Step 9:** Remove the unused keys `vocab.title` and `authorities.title` from BOTH `en.json` and `sv.json` (grep first: `grep -rn "vocab.title\|authorities.title\|\.title" web/src` — confirm only the i18n definitions match; nothing references them).

### #37 — frontend authority-kind reveal test
- [ ] **Step 10:** In `web/src/fields/fields.test.tsx`, add a test mirroring the existing Term test: type a key + EN label, `selectOptions(type, "authority")`, assert the authority-kind `<select>` (label `/authority kind/i`) appears, `selectOptions` it to `"person"`, submit, and assert the POST body's `authority_kind === "person"` (use a `server.use` POST handler that captures the body, like the Term test does).

- [ ] **Step 11: Verify** — `cd web && pnpm test && pnpm typecheck && pnpm lint && pnpm build && pnpm check:size`. All green; bundle ≤150 KB. Commit:
```bash
cd /Users/olsson/Laboratory/biggus-dickus
git add web
git commit -m "fix(web): search 503 vs error (#34); terms/authorities list error states (#31); authority-tab a11y + dead keys (#32); authority-kind test (#37)"
```

---

## Task 4: Verification

- [ ] **Step 1: i18n parity** —
```bash
cd web
node -e "const a=require('./src/i18n/en.json'),b=require('./src/i18n/sv.json');const k=o=>Object.entries(o).flatMap(([K,v])=>typeof v==='object'?k(v).map(s=>K+'.'+s):[K]);const ka=k(a).sort(),kb=k(b).sort();console.log(JSON.stringify(ka)===JSON.stringify(kb)?'PARITY OK':'MISMATCH '+JSON.stringify({onlyEn:ka.filter(x=>!kb.includes(x)),onlySv:kb.filter(x=>!ka.includes(x))}))"
```
Expected `PARITY OK`.

- [ ] **Step 2: Frontend** — `pnpm typecheck && pnpm lint && pnpm test && pnpm build && pnpm check:size` (report bundle gz).

- [ ] **Step 3: Backend** —
```bash
cd /Users/olsson/Laboratory/biggus-dickus
DATABASE_URL=postgres://postgres:postgres@localhost:5442/cms_dev \
  MEILI_URL=http://localhost:7700 MEILI_MASTER_KEY=masterKey \
  cargo test -p api -p domain -p server
cargo clippy --workspace --all-targets
cargo +nightly fmt --check
```
All pass; clippy + fmt clean.

- [ ] **Step 4:** No codename: `git grep -in 'biggus\|dickus' -- crates web/src` → no matches.

---

## Self-Review (completed)
- **Spec coverage:** #22 (404), #18 (log 500s) → Task 1; #9 (Rust cross-ref comments), #4 (clone + smoke test) → Task 2; #34, #31, #32, #37 → Task 3; parity + suites → Task 4. ✓
- **Scope adjustments baked in:** #8 already closed (thiserror is used); #37 backend-403 omitted (no non-EditCatalogue role exists); #9 Rust-side only (migration checksums). ✓
- **Placeholder scan:** none — code is concrete; the "match the existing harness" notes are verification instructions against named files.
- **Type consistency:** `HttpError` defined in queries.ts and imported in search-panel; the 23503/FK pattern matches the field-def handler; `authorities.loadError` (existing key) now consumed; `search.unavailable` added at parity.
