Files
biggus-dickus/docs/superpowers/plans/2026-06-04-tier2-papercuts.md
2026-06-04 17:14:39 +02:00

13 KiB

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:
#[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:

    let term_id = db::vocab::add_term(&mut tx, &new)
        .await
        .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;

with:

    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:
        .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:
        Err(err) => {
            tracing::error!(?err, "fetching public object");
            StatusCode::INTERNAL_SERVER_ERROR.into_response()
        }
  • Step 7: Verifycargo +nightly fmt, cargo clippy -p api --all-targets, cargo test -p api. Commit:
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:
      /// 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:

        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: Verifycargo +nightly fmt, cargo clippy --workspace --all-targets, cargo test -p server -p domain. Commit:

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:
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:

      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:

        {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 NavLinks. 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: Verifycd web && pnpm test && pnpm typecheck && pnpm lint && pnpm build && pnpm check:size. All green; bundle ≤150 KB. Commit:

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
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: Frontendpnpm typecheck && pnpm lint && pnpm test && pnpm build && pnpm check:size (report bundle gz).

  • Step 3: Backend

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.