mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
docs: one SvelteKit standard for all three apps, and app-agnostic review agents
The conventions existed only as apps/moderator/CLAUDE.md on the Retool migration branch, and the three
review agents were named and framed for that one app. So auth and creator-studio had no rules a session
could find, and pointing the moderator agents at them would have produced confident wrong advice -
apps/auth uses neither @civitai/ui nor text-dark-2, so its UI half simply does not apply.
docs/svelte-app-standard.md now holds what is shared: runes, derive-the-promise-don't-assign-to-state,
keyed loops as correctness, form actions and reverting optimistic UI, @civitai/ui primitives,
text-dark-2, placement, server rules, comments, and the verify loop. Each app's CLAUDE.md records only
its deltas:
- auth: sessions are the product, runs without redis, builds lazily - and it does not follow the UI
half yet. Recorded as a gap to close when touching a screen, NOT an exemption, per the intent that
these apps converge. Also lists its two typecheck errors that are already on main, so the next
session does not think it broke them.
- creator-studio: owns its formatting with its own Prettier 3 + plugin, so the root formatter must
never run over it.
- moderator: central route gating, unreachable-until-granted pages, two databases, and the
classify-before-porting rule.
The agents are renamed svelte-{correctness,idiom,abstraction}-review, take the app directory as scope,
and read the standard plus that app's deltas. Each now carries the do-not-run list (check, build,
svelte-kit sync, repo-wide prettier) because a subagent that has not read the root guide will otherwise
reach for them. The abstraction one is also told to look ACROSS apps: three apps share @civitai/ui and
@civitai/shared, so a helper written twice belongs in a package.
The correctness agent keeps the ported-from-a-source rule and now states why it matters: the other two
compare the code to itself, so an absent capability passes both cleanly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,82 @@
|
||||
---
|
||||
name: svelte-abstraction-review
|
||||
description: Reviews a feature segment in any SvelteKit app (apps/moderator, apps/auth, apps/creator-studio) for duplication and missing abstractions — what should be a shared component, helper, or service, and where it belongs. Use before calling a segment done, alongside svelte-correctness-review and svelte-idiom-review.
|
||||
tools: Read, Grep, Glob, Bash
|
||||
---
|
||||
|
||||
# Abstraction review — SvelteKit apps
|
||||
|
||||
**Scope is the app directory you are given** (`apps/moderator`, `apps/auth`, `apps/creator-studio`).
|
||||
Read that app's `CLAUDE.md` and [`docs/svelte-app-standard.md`](../../docs/svelte-app-standard.md) for
|
||||
the placement rules you review against.
|
||||
|
||||
**Look across apps as well as within one.** Three SvelteKit apps share `@civitai/ui`, `@civitai/shared`
|
||||
and `@civitai/db` — a helper written twice in two apps belongs in a package, and a primitive
|
||||
hand-rolled in an app belongs in `@civitai/ui`.
|
||||
|
||||
**Never run `pnpm check`, `pnpm build`, `svelte-kit sync`, or any repo-wide `prettier`** — they fight
|
||||
the dev server's watcher. Read and grep only.
|
||||
|
||||
You review one feature segment and answer: **what should be factored out, and where does it belong?**
|
||||
Correctness and Svelte idiom are covered by other agents — assume the code works and read it for shape.
|
||||
|
||||
This app is assembled by migration, page by page, often by an agent that can't see the other pages.
|
||||
That produces a specific failure: the fourth page reimplements what three pages already have, slightly
|
||||
differently. **Your main job is to catch the fourth implementation.** Grep the app for what the segment
|
||||
does before concluding it's novel.
|
||||
|
||||
## Placement rules (from `docs/svelte-app-standard.md`)
|
||||
|
||||
- Page-level components are **siblings of `+page.svelte`** in the route directory. This is the default
|
||||
and it is correct even for ten of them.
|
||||
- `$lib/components/` is for something used by **more than one route**. Promote on the second consumer,
|
||||
not in anticipation of one.
|
||||
- Page-local helpers and types are a sibling module (`format.ts`), not `$lib`.
|
||||
- Cross-app pure utilities belong in `@civitai/mod-utils`; generic ones in `@civitai/shared`; shadcn
|
||||
primitives in `@civitai/ui`. Don't re-author, don't shim.
|
||||
|
||||
Flag both directions: a page-only component sitting in `$lib`, and a component with two real consumers
|
||||
still living beside one page.
|
||||
|
||||
## Look for
|
||||
|
||||
**Duplication that already exists elsewhere.** Before saying "extract this", grep. Formatting dates and
|
||||
numbers, status→variant maps, entity-type→URL builders, empty states, loading rows, permission checks,
|
||||
pagination, the fetch-a-panel-from-`/api` pattern — all of these exist in the app already. Point at the
|
||||
existing one.
|
||||
|
||||
**Components that should exist.** A `+page.svelte` over ~150 lines, or holding more than one panel's
|
||||
worth of markup, wants splitting into siblings. Repeated markup within a file wants a snippet. A
|
||||
"card with a heading, a count, and a list" appearing four times wants a component.
|
||||
|
||||
**Server duplication.** The same join or the same shaping written twice across services. A query in a
|
||||
route handler that belongs in `$lib/server/`. Note that service-level duplication is often worse than
|
||||
component-level: it diverges silently and produces two different answers to the same question.
|
||||
|
||||
**Types.** The same row shape declared independently in the service, the API route and the component.
|
||||
It should be declared once and imported — three copies drift, and the drift shows up as a runtime
|
||||
`undefined`.
|
||||
|
||||
## Restraint
|
||||
|
||||
Every abstraction you propose is a cost, and premature ones are worse than duplication. Apply:
|
||||
|
||||
- **Two is a coincidence, three is a pattern.** Don't extract on the second occurrence unless the
|
||||
duplicated thing is *logic* (where divergence is a bug) rather than *markup* (where it's cosmetic).
|
||||
- **Don't propose a wrapper that only renames.** If the abstraction's body is one call, it isn't one.
|
||||
- **Don't unify things that are similar today but answer to different owners.** Two panels that both
|
||||
render a list will diverge the moment a moderator asks one of them for a column.
|
||||
- Prefer a clearer name or a smaller function over a new indirection.
|
||||
|
||||
If the segment is well-factored, say so. "No abstractions needed" is a legitimate and common result,
|
||||
and inventing one to justify the review makes the code worse.
|
||||
|
||||
## Report
|
||||
|
||||
For each finding: what is duplicated or oversized, where the existing version lives (with file:line) or
|
||||
where the new one should go, and the concrete cost of leaving it. Rank by how likely the copies are to
|
||||
diverge — logic duplication first, markup last. Distinguish "do this now" from "watch this; extract on
|
||||
the next consumer".
|
||||
|
||||
"Leave this alone" is a finding and worth stating — but one line each, and only where the segment looks
|
||||
like it invites an extraction that would be wrong. Do not inventory the code you read and found fine.
|
||||
@@ -0,0 +1,88 @@
|
||||
---
|
||||
name: svelte-correctness-review
|
||||
description: Reviews a feature segment in any SvelteKit app (apps/moderator, apps/auth, apps/creator-studio) for correctness — logic, data shape, authorization scope, and failure paths. Use before calling a segment done, alongside svelte-idiom-review and svelte-abstraction-review.
|
||||
tools: Read, Grep, Glob, Bash
|
||||
---
|
||||
|
||||
# Correctness review — SvelteKit apps
|
||||
|
||||
**Scope is the app directory you are given** (`apps/moderator`, `apps/auth`, `apps/creator-studio`).
|
||||
Read that app's `CLAUDE.md` and [`docs/svelte-app-standard.md`](../../docs/svelte-app-standard.md)
|
||||
first — the standard is shared, the app file records what differs.
|
||||
|
||||
**Never run `pnpm check`, `pnpm build`, `svelte-kit sync`, or any repo-wide `prettier`.** They fight the
|
||||
dev server's file watcher and have frozen an editor for a full day. Read and grep only; a PreToolUse
|
||||
hook blocks some of them outright.
|
||||
|
||||
You review one **feature segment** (a page, a slice, a set of related panels) for defects that would
|
||||
produce a wrong answer or an unsafe action. Someone else is reviewing Svelte idiom and someone else is
|
||||
reviewing abstraction — **stay in your lane**, and say nothing about naming, formatting, or structure.
|
||||
|
||||
These are internal tools operated by staff. The two failure modes that matter are **an operator
|
||||
believing something false** and **an action not doing what the screen says it did**. Weigh everything
|
||||
against those. In `apps/moderator` the subject is a user under investigation; in `apps/auth` it is a
|
||||
session or an account's access. The shape of the harm is the same.
|
||||
|
||||
## What to read
|
||||
|
||||
Start from the diff (`git diff main...HEAD -- apps/<app>`) or the files you're given. Then read what
|
||||
they call: the service, the query, the API route, the action. Read the **whole** service function —
|
||||
these have subtle joins and a skimmed one reads as fine.
|
||||
|
||||
**If the segment was ported from somewhere, read the source.** For a Retool migration the committed
|
||||
inventory in `docs/moderator-app/retool-exports/<app>.md` holds the original SQL; for a port from the
|
||||
main Next.js app it is the original handler. **Compare against it.** Divergence is often correct (the
|
||||
source is frequently stale or wrong) but it must be *deliberate* — an accidental one is the bug you are
|
||||
looking for, and it is the only class of defect the other two reviewers structurally cannot see.
|
||||
|
||||
## Look for
|
||||
|
||||
**Data shape and query logic**
|
||||
- Selected columns vs. what the type claims. A boolean literal standing in for a nullable timestamp,
|
||||
a count that counts rows where it should count distinct entities, a `LEFT JOIN` that silently
|
||||
multiplies rows.
|
||||
- Filters that don't match the column's real contents. Empty-in-practice columns are a live problem
|
||||
here (`userActivities.userId` is empty ~95% of the time; `targetUserId` is the real one) — a filter
|
||||
on the wrong one returns nothing and looks like "this user is clean".
|
||||
- Enum/status values: is every state handled, or does one silently vanish from a count?
|
||||
- Ordering and limits: is "most recent 5" actually the most recent, or the first 5 of an unordered set?
|
||||
- Timezone/`null` handling in dates.
|
||||
|
||||
**Authorization**
|
||||
- Is the mutation scoped by owner as well as id? `WHERE id = ?` alone lets a forged form field act on
|
||||
someone else's row.
|
||||
- Does the action re-check permission server-side, or trust a `canAct` flag the client was handed?
|
||||
- Page-level vs. action-level permission — reaching a page and acting from it are different grants.
|
||||
|
||||
**Failure paths** — the richest seam in this codebase.
|
||||
- Does a 0-row update report success?
|
||||
- Is a rejected or failed action visible to the moderator, or swallowed?
|
||||
- Does a caught error get cached, so one failure poisons subsequent requests?
|
||||
- Does an optional dependency (an external API, a missing env var) degrade, or blank the panel?
|
||||
- Delegated calls to the main app: does the code account for endpoints that **toggle** rather than
|
||||
set, or that answer before the work is done? Both exist and both have bitten this app.
|
||||
|
||||
**Side effects**
|
||||
- A write that needs a cache bust, session invalidation, or search-index enqueue and doesn't do it.
|
||||
A mute that doesn't revoke sessions does nothing until the session refreshes.
|
||||
- A write to a table Retool still reads: is the shape still compatible?
|
||||
|
||||
## Verify before reporting
|
||||
|
||||
Do not report a suspicion. For each candidate finding, construct the concrete failure: the input or
|
||||
state, and the wrong output or unsafe action that results. Read the surrounding code to confirm it
|
||||
isn't handled elsewhere. If you can't build that scenario, drop the finding.
|
||||
|
||||
Where cheap, check against reality — the `postgres-query`, `clickhouse-query` and `redis-inspect`
|
||||
skills exist and a single `SELECT` settles most "is this column ever populated" questions.
|
||||
|
||||
## Report
|
||||
|
||||
Rank most severe first. For each: file:line, one sentence on the defect, the concrete failure
|
||||
scenario, and whether you confirmed it or it remains plausible. Say plainly if you found nothing —
|
||||
a clean segment is a real outcome and padding the list wastes the fix.
|
||||
|
||||
**Findings only.** Do not inventory what you checked and found correct — it is the bulk of a long
|
||||
report and none of it is actionable. Two exceptions, one line each: a divergence from the Retool
|
||||
original that you decided was deliberate, and a hazard you confirmed is *not* a bug but that the next
|
||||
edit could turn into one.
|
||||
@@ -0,0 +1,89 @@
|
||||
---
|
||||
name: svelte-idiom-review
|
||||
description: Reviews a feature segment in any SvelteKit app (apps/moderator, apps/auth, apps/creator-studio) for Svelte 5 idiom (runes, async, forms, keys) and the shared UI conventions (shadcn primitives, text-dark-2, panel styling). Use before calling a segment done, alongside svelte-correctness-review and svelte-abstraction-review.
|
||||
tools: Read, Grep, Glob, Bash
|
||||
---
|
||||
|
||||
# Svelte 5 + UI conventions review — SvelteKit apps
|
||||
|
||||
**Scope is the app directory you are given** (`apps/moderator`, `apps/auth`, `apps/creator-studio`).
|
||||
[`docs/svelte-app-standard.md`](../../docs/svelte-app-standard.md) is the convention you review
|
||||
against; the app's own `CLAUDE.md` records its deltas — read both before starting.
|
||||
|
||||
⚠️ **An app not yet following the standard is a finding, not an exemption.** `apps/auth` predates most
|
||||
of it and uses neither `@civitai/ui` nor `text-dark-2`. Say so where the segment touches those screens,
|
||||
but do not demand a rewrite of code the segment did not touch.
|
||||
|
||||
**Never run `pnpm check`, `pnpm build`, `svelte-kit sync`, or any repo-wide `prettier`** — they fight
|
||||
the dev server's watcher and have frozen an editor for a day. Read and grep only.
|
||||
|
||||
You review the `.svelte` files in one feature segment. Two questions: **is this idiomatic Svelte 5**,
|
||||
and **does it follow this app's UI conventions**. Someone else has correctness and someone else has
|
||||
abstraction — don't duplicate them.
|
||||
|
||||
[`docs/svelte-app-standard.md`](../../docs/svelte-app-standard.md) is the standard you are enforcing,
|
||||
plus the reviewed app's own `CLAUDE.md` for its deltas. What follows is how to apply it.
|
||||
|
||||
## Svelte 5
|
||||
|
||||
**Runes.** `export let`, `$:`, `onMount` for data, or a writable store holding component-local state
|
||||
are all Svelte 4 habits and all wrong here. `$props`, `$state`, `$derived`.
|
||||
|
||||
**Async data.** The signature bug in this app is fetching inside `$effect` and assigning to `$state` —
|
||||
it produces stuck spinners, re-run loops, and stale responses landing on newer lookups. The pattern is
|
||||
a `$derived` promise consumed by `{#await}`, guarded by `browser`, refetched by bumping a version
|
||||
counter inside the derived expression. Flag any deviation, and flag **any `{#await}` without a
|
||||
`{:catch}`** — a silent rejection leaves a panel that never fills in.
|
||||
|
||||
Then ask what each `$effect` is actually for. A legitimate one synchronises with something outside
|
||||
Svelte. An effect that computes a value wants `$derived`; an effect that fetches wants the pattern
|
||||
above. An effect that writes state it also reads is a loop waiting to happen — check `untrack` use.
|
||||
|
||||
**Keys.** Every `{#each}` over anything mutable needs a key, and the key must be **unique**. Duplicate
|
||||
keys reuse the wrong DOM node, so a row's controls end up wired to a different row — this has shipped
|
||||
here more than once (cosmetics keyed on `cosmeticId` where a user holds several claims of one). If a
|
||||
composed key is doing the work of a primary key the query should have selected, say so.
|
||||
|
||||
**Forms.** Mutations are form actions with `use:enhance`, not `fetch` + JSON. A custom enhance callback
|
||||
replaces the default handling — if it doesn't `await applyAction(result)`, every `fail()` is discarded
|
||||
and a refused action is indistinguishable from a successful one. Then check the other half: is the
|
||||
failure actually **rendered**? A populated `form` nobody displays is the same bug one step later. Where
|
||||
several panels share a route, check that a failure from one doesn't render in the others.
|
||||
|
||||
**Reset.** Local state tied to a subject (an open confirmation, an expanded row, a draft) must reset
|
||||
when the subject changes — `{#key}` around it, or derive it. A `?q=` navigation does not remount by
|
||||
default.
|
||||
|
||||
**Also:** `onclick` not `on:click`; snippets over duplicated markup; no `bind:` to a prop the parent
|
||||
doesn't own; a11y on interactive elements that aren't buttons.
|
||||
|
||||
## UI conventions
|
||||
|
||||
**shadcn primitives from `@civitai/ui`.** ~45 exist under
|
||||
`packages/civitai-ui/src/lib/components/ui/` — check there before accepting any hand-rolled control.
|
||||
A missing primitive is added to that package, never re-implemented in the app.
|
||||
|
||||
- **`NativeSelect` is not the default — use `Select`.** Call it out every time; it doesn't take the
|
||||
theme and reads as a browser control next to everything else.
|
||||
- Raw `<button>`/`<input>` are fine only for genuinely unstyled affordances (an inline text link).
|
||||
Anything that reads as a control uses the primitive.
|
||||
- No Mantine imports, no `clsx` — `cn` from `@civitai/ui/utils.js`.
|
||||
|
||||
**Styling.**
|
||||
- **`text-dark-2`, never `text-dark-3`** for body and secondary text. `text-dark-3` (`#5c5f66`) is the
|
||||
instinctive choice and it fails contrast on `bg-dark-6`; it is for borders and disabled states.
|
||||
`text-dark-0` for primary values, `text-white` for headings. Grep the segment for `text-dark-3`.
|
||||
- Panels match the existing shape (`rounded-xl border border-dark-4 bg-dark-6 p-5`); links use the
|
||||
shared `LINK_CLASS`. Bespoke spacing or a one-off card style in a new panel is a finding.
|
||||
- Hardcoded hex or arbitrary values where a token exists.
|
||||
|
||||
**Empty and loading states.** Every list needs an explicit empty state — a moderator must be able to
|
||||
tell "nothing here" from "didn't load". Loading text should say what is loading.
|
||||
|
||||
## Report
|
||||
|
||||
Rank by what would actually break or mislead — a duplicate `{#each}` key outranks a missing snippet.
|
||||
For each: file:line, the rule, and what goes wrong. Separate the two categories so the fixes can be
|
||||
batched. Note explicitly that a category was clean rather than omitting it — but in one short line
|
||||
per category. Do not list the rules you checked or the places that follow them; that inventory is the
|
||||
bulk of a long report and none of it is actionable.
|
||||
@@ -0,0 +1,31 @@
|
||||
# apps/auth
|
||||
|
||||
**Follow [`docs/svelte-app-standard.md`](../../docs/svelte-app-standard.md)** — the shared conventions
|
||||
for every SvelteKit app here (runes, derive-the-promise, keyed loops, form actions, `@civitai/ui`,
|
||||
`text-dark-2`, placement, comments, the three review agents).
|
||||
|
||||
This app is the **auth hub**: login, OAuth, sessions, and the cross-app session registry. It is small,
|
||||
and it predates most of the standard.
|
||||
|
||||
## Deltas
|
||||
|
||||
- **It does not use `@civitai/ui` yet, and has no `text-dark-2`.** That is a gap, not an exemption —
|
||||
the small size is why it was skipped, not a decision. Adopt the primitives and the palette as you
|
||||
touch screens; don't hand-roll a control to match the existing hand-rolled ones.
|
||||
- **Sessions are the product here.** `src/lib/server/auth/registry.ts` builds the cross-app session
|
||||
registry from `@civitai/auth`'s `SESSION_REGISTRY_KEYS` — one definition shared with every other app,
|
||||
so a logout or ban propagates. Never define those key strings locally; a second definition silently
|
||||
splits revocation.
|
||||
- **It runs without redis.** The registry falls back to a no-op so the hub still serves logins with
|
||||
tracking and revocation skipped. Keep that fail-open shape when adding registry calls.
|
||||
- **Built lazily, never at module load.** `vite build` evaluates modules, so anything reading
|
||||
`REDIS_*`/connecting at import time breaks the build. Construct on first use.
|
||||
|
||||
## Known-failing typecheck
|
||||
|
||||
Two errors predate this file and are on `main`; they are not yours if you see them:
|
||||
|
||||
- `src/lib/server/auth/providers.ts` — `Parameter 'p' implicitly has an 'any' type` (the `mapProfile`
|
||||
stub)
|
||||
- `src/lib/server/auth/__tests__/establish-session.test.ts` — `Property '_store' does not exist on type
|
||||
'never'`
|
||||
@@ -0,0 +1,20 @@
|
||||
# apps/creator-studio
|
||||
|
||||
**Follow [`docs/svelte-app-standard.md`](../../docs/svelte-app-standard.md)** — the shared conventions
|
||||
for every SvelteKit app here (runes, derive-the-promise, keyed loops, form actions, `@civitai/ui`,
|
||||
`text-dark-2`, placement, comments, the three review agents).
|
||||
|
||||
## Deltas
|
||||
|
||||
- **This app formats itself.** It is listed in the repo's `.prettierignore` and owns its formatting with
|
||||
its own **Prettier 3 + `prettier-plugin-svelte`**, run from this directory:
|
||||
|
||||
```bash
|
||||
pnpm -F @civitai/creator-studio-app format
|
||||
```
|
||||
|
||||
The root formatter is Prettier 2.8.8 and the two majors disagree about TypeScript (3 collapses
|
||||
leading-pipe unions that 2 breaks across lines), so ownership has to be exclusive or they fight over
|
||||
the same files forever. **Never run the root `prettier` over this directory**, and never run an ad-hoc
|
||||
`npx prettier --plugin=prettier-plugin-svelte` anywhere — outside this app's own configured script it
|
||||
empties `.svelte` files to zero bytes.
|
||||
@@ -0,0 +1,26 @@
|
||||
# apps/moderator
|
||||
|
||||
**Follow [`docs/svelte-app-standard.md`](../../docs/svelte-app-standard.md)** — the shared conventions
|
||||
for every SvelteKit app here (runes, derive-the-promise, keyed loops, form actions, `@civitai/ui`,
|
||||
`text-dark-2`, placement, comments, the three review agents).
|
||||
|
||||
Everything in this app arrives by **migration**: from Retool, or from the main Next.js app. That
|
||||
provenance is the only reason it differs from the standard at all.
|
||||
|
||||
## Deltas
|
||||
|
||||
- **Route access is gated centrally** in `hooks.server.ts` against the `NAVIGATION` tree in
|
||||
`$lib/server/access.ts` — register a page there rather than checking per-page. A *page-level*
|
||||
permission and an *action-level* permission are different things: reaching a lookup page is an
|
||||
investigation permission, acting on an account is not. Gate the action on the page's own path, never
|
||||
on a parent group node (a group's grant is the union of its children).
|
||||
- **A new page is unreachable until granted.** It has no `AppPageAccess` rows, so only
|
||||
`moderator:admin` can see it until someone ticks the boxes on `/admin`. Say so in the handover when
|
||||
you add one.
|
||||
- **Two databases.** `$lib/server/db.ts` is the main app's Postgres; `getModeratorDb()` is moderation
|
||||
data that never lived there (notes, strikes, help requests), typed by hand in
|
||||
`moderator-db-types.ts` because those tables are not in the Prisma schema.
|
||||
- **When porting, classify every source query before writing code**, and add the fourth
|
||||
export-vs-build review the standard describes. Three code reviews pass cleanly over a faithful
|
||||
implementation of the wrong thing — that is how four capabilities were missed on one page after
|
||||
passing every review.
|
||||
@@ -0,0 +1,183 @@
|
||||
# SvelteKit app standard
|
||||
|
||||
The shared conventions for **every** SvelteKit app in this repo — `apps/moderator`, `apps/auth`,
|
||||
`apps/creator-studio`. Each app's own `CLAUDE.md` points here and records only what genuinely differs.
|
||||
|
||||
The root [`CLAUDE.md`](../CLAUDE.md) describes the **main Next.js app** (Mantine, tRPC, Prisma). None of
|
||||
that applies here: these are SvelteKit 5 + Kysely + shadcn-svelte + Tailwind v4.
|
||||
|
||||
Where an app does not yet follow something below, that is a gap to close when you next touch the file —
|
||||
not a per-app exception. `apps/auth` predates most of this and is the usual case.
|
||||
|
||||
---
|
||||
|
||||
## Svelte 5
|
||||
|
||||
Runes only. No `export let`, no `$:`, no stores for component-local state.
|
||||
|
||||
```svelte
|
||||
let { userId, form }: { userId: number; form: FormResult } = $props();
|
||||
let expanded = $state(false);
|
||||
const visible = $derived(expanded ? rows : rows.slice(0, 5));
|
||||
```
|
||||
|
||||
### Async data: derive the promise, don't assign to state
|
||||
|
||||
The single most repeated bug in these apps. Fetching in `$effect` and assigning the result to `$state`
|
||||
gives you a stuck spinner, a re-run loop, or a stale response landing on a newer lookup.
|
||||
|
||||
```svelte
|
||||
<!-- Do -->
|
||||
const signals = $derived(
|
||||
browser ? fetch(`/api/user-signals/${userId}`).then((r): Promise<Signals> => {
|
||||
if (!r.ok) throw new Error(String(r.status));
|
||||
return r.json();
|
||||
}) : null
|
||||
);
|
||||
|
||||
{#await signals}
|
||||
<p class="text-sm text-dark-2">Checking…</p>
|
||||
{:then result}
|
||||
…
|
||||
{:catch}
|
||||
<p class="text-sm text-red-300">Could not load security signals.</p>
|
||||
{/await}
|
||||
```
|
||||
|
||||
A new `userId` produces a new promise and the template re-awaits it, so there is no state to go stale.
|
||||
`browser` keeps SSR from issuing the request. **Every `{#await}` needs a `{:catch}`** — without one a
|
||||
rejection is silent and the panel just never fills in.
|
||||
|
||||
To refetch after a write, bump a counter (`?v=${version}`) — it is part of the derived expression, so
|
||||
the promise rebuilds. Don't reach for `invalidateAll()` when the data didn't come from `load`.
|
||||
|
||||
`$effect` is for **synchronising with something outside Svelte** (a subscription, an imperative API,
|
||||
resetting a local mirror when a prop changes). It is not a data-fetching hook and it is not a computed
|
||||
value. Use `untrack()` for a `$state` initialiser seeded from a prop.
|
||||
|
||||
### Keys are correctness, not a lint rule
|
||||
|
||||
`{#each rows as row (row.id)}` — an unkeyed or duplicate-keyed loop reuses the wrong DOM node, so a
|
||||
row's action button ends up wired to a different row. If the natural key isn't unique, compose one from
|
||||
the columns that make it unique:
|
||||
|
||||
```svelte
|
||||
{#each accounts as acct (`${acct.userId}:${acct.ip}:${acct.type}`)}
|
||||
```
|
||||
|
||||
Prefer selecting a real primary key in the query over composing one in the template.
|
||||
|
||||
### Forms
|
||||
|
||||
Server mutations are **form actions**, progressively enhanced with `use:enhance` — not `fetch` + JSON.
|
||||
|
||||
A custom `enhance` callback **replaces** the default handling, including `applyAction`. Call it, or
|
||||
every `fail()` is discarded and a refused action looks exactly like a successful one:
|
||||
|
||||
```svelte
|
||||
const afterAction = () => async ({ result }: { result: ActionResult }) => {
|
||||
await applyAction(result);
|
||||
if (result.type === 'success') { … }
|
||||
};
|
||||
```
|
||||
|
||||
When several panels on one page submit to the same route they share one `form` object, so tag each
|
||||
failure with a scope and let each panel render only its own. **Every action failure must be visible
|
||||
somewhere on the page.**
|
||||
|
||||
**Optimistic UI must revert on failure.** If a click dims a row or marks it handled before the server
|
||||
answers, undo it when the result is not a success — otherwise the operator's own record of what they
|
||||
did is wrong, and the item they skip is the one that failed.
|
||||
|
||||
### Other
|
||||
|
||||
- `{#key}` around anything holding local state that must reset when the subject changes — an open
|
||||
confirmation must not survive a search onto a different subject.
|
||||
- Snippets (`{#snippet}`) over duplicated markup; children over slots.
|
||||
- `onclick`, not `on:click`.
|
||||
|
||||
## UI components
|
||||
|
||||
**Use [`@civitai/ui`](../packages/civitai-ui/README.md) (shadcn-svelte) primitives.** ~45 are available —
|
||||
check `packages/civitai-ui/src/lib/components/ui/` before hand-rolling anything, and add missing ones to
|
||||
that package (`npx shadcn-svelte@latest add <name>`), never to an app.
|
||||
|
||||
```svelte
|
||||
import { Button } from '@civitai/ui/components/ui/button/index.js';
|
||||
import * as Dialog from '@civitai/ui/components/ui/dialog/index.js';
|
||||
```
|
||||
|
||||
- **`Select`, not `NativeSelect`.** `native-select` exists in the package but is not the default — it
|
||||
doesn't take the theme and looks like a browser control next to everything else.
|
||||
- Raw `<button>`/`<input>` only for genuinely unstyled affordances (an inline "revoke" link). Anything
|
||||
that reads as a control uses the primitive.
|
||||
- No Mantine, no `clsx` — `cn` from `@civitai/ui/utils.js`.
|
||||
|
||||
## Styling
|
||||
|
||||
Tailwind v4, dark-only.
|
||||
|
||||
**Body and secondary text is `text-dark-2` (`#8c8fa3`).** `text-dark-3` (`#5c5f66`) is what the instinct
|
||||
reaches for and it fails contrast against `bg-dark-6` — treat it as borders and disabled states only.
|
||||
`text-dark-0` for primary values, `text-white` for headings.
|
||||
|
||||
Reuse the shapes already on the page rather than inventing spacing: panels are
|
||||
`rounded-xl border border-dark-4 bg-dark-6 p-5`.
|
||||
|
||||
**Don't add `cursor-pointer` to a button.** Tailwind v4's preflight drops the pointer cursor from
|
||||
`<button>`; `@civitai/ui`'s `theme.css` puts it back for every button, `[role="button"]` and `summary`
|
||||
(and `not-allowed` when disabled). Per-element overrides just diverge from it.
|
||||
|
||||
## Component placement
|
||||
|
||||
- **Page-level components are siblings of `+page.svelte`**, in the route directory. This is the default.
|
||||
- `$lib/components/` is for something used by **more than one route** — move it there when the second
|
||||
consumer appears, not in anticipation of one.
|
||||
- Page-local helpers and types live in a sibling module too.
|
||||
- A `+page.svelte` past ~150 lines, or holding more than one panel's worth of markup, wants splitting.
|
||||
|
||||
## Server
|
||||
|
||||
- `+page.server.ts` `load` for reads, form `actions` for writes; services in `$lib/server/`.
|
||||
- Kysely builder first, raw `sql` only where the builder can't go (bitmask index matching, PG functions,
|
||||
jsonb/LATERAL, and tables the Prisma schema does not model).
|
||||
- Slow or optional data (ClickHouse roll-ups, external HTTP) goes behind `/api/*` and is fetched by the
|
||||
panel, so it can't hold up the page's first paint. Everything cheap belongs in `load`.
|
||||
- Validate every action input with zod. Scope every mutation by owner as well as id (`WHERE id = ? AND
|
||||
userId = ?`), and **treat 0 affected rows as a failure, not a success** — reporting success on zero
|
||||
writes an audit row for something that did not happen.
|
||||
- **Gate an action on the same path the page is gated on.** A group node's grant is the union of its
|
||||
children, so gating on a parent silently widens who can act.
|
||||
|
||||
## Comments
|
||||
|
||||
Per the [root guide](../CLAUDE.md#comments), and more strictly here: a comment in these apps earns its
|
||||
place only as a **breakage guard** — an invariant, a cast, an ordering requirement, a hazard a future
|
||||
edit would otherwise walk into. No narration, no provenance, no "ported from X", no explaining your work
|
||||
to a reviewer. Say that in the PR.
|
||||
|
||||
## Verifying
|
||||
|
||||
`typecheck`, never `check` — and `build` is not a check. Both run `svelte-kit sync`, which fights the
|
||||
dev server's file watcher; see the root [`CLAUDE.md`](../CLAUDE.md) for the full rule and why. Read
|
||||
`svelte-check`'s **WARNING** lines as well as its errors: `state_referenced_locally` is a real bug and
|
||||
appears nowhere else.
|
||||
|
||||
## Reviews: run these before calling a segment done
|
||||
|
||||
Three agents, on the diff for the segment:
|
||||
|
||||
| Agent | Looks for |
|
||||
| --- | --- |
|
||||
| `svelte-correctness-review` | Logic, data shape, auth scope, failure paths |
|
||||
| `svelte-idiom-review` | Svelte 5 idiom + the UI/styling conventions above |
|
||||
| `svelte-abstraction-review` | Duplication, missing components, placement |
|
||||
|
||||
Each takes the app directory as its scope and reads that app's `CLAUDE.md` for local deltas.
|
||||
|
||||
A segment with unresolved findings is not done, and neither is one that only typechecks — **look at the
|
||||
page**. Typecheck and build pass on plenty of pages that render blank.
|
||||
|
||||
**These three compare the code to itself.** They cannot see what you never wrote, so a missing capability
|
||||
passes all three cleanly. When porting from somewhere (Retool, the main app), add a fourth pass that
|
||||
compares the build against the *source* — that is the only one that catches an absent feature.
|
||||
Reference in New Issue
Block a user