Skip to content

Coding rules

Generic engineering principles live in .claude/rules/principles.md; project invariants in CLAUDE.md. This page collects narrower conventions as they are formalised.

Documentation accuracy

Docs are treated as code: every concrete claim traces to a source, and a behaviour change updates its docs in the same pull request. Four rules:

  1. Ground every concrete claim in a source. Commands, paths, endpoints, env vars, field names, UI locations, and numbers are copied from verified code/config, never recalled. An untraceable claim does not ship.
  2. Accuracy is separate from rendering. A rendered page proves it displays, not that it is true. Confirm each checkable claim against HEAD as its own pass; a screenshot is not verification.
  3. Never describe a link target you have not opened. A page is "the reference for X" only if it actually contains X. Otherwise link the real source (code, deploy/README.md, .env.example) or mark it in-progress — never present a stub as complete.
  4. Behaviour change ⇒ doc change, same pull request. Moving a control, renaming a command, changing an endpoint / default / env var, or adding or removing a feature updates the affected page and any diagram with it (quality gate 3).

Broken and missing links fail mkdocs build --strict in CI; the rest is a review gate.

Config tables

Every operator-visible config table under docs/_generated/config/ is generated by bun run docs:gen from the live zod schema, and bun run docs:gen --check fails CI when a committed partial no longer matches. Four of the six columns — setting, what it is, accepted values, default — come straight from the schema, so they cannot drift.

The two guidance columns (when to change it, what to expect) are judgement the schema cannot express. They live in scripts/docs/config-notes/<schema>.ts, keyed by the field path the generator emits.

The generator refuses to run when those keys and the schema's leaves disagree. Adding a config field without a note fails; leaving a note behind for a field you removed also fails. So:

  • Adding a setting ⇒ add its note in the same pull request.
  • Renaming or moving a setting ⇒ move its note key with it.
  • Adding a new strategy ⇒ add scripts/docs/config-notes/<name>.ts and register it in STRATEGY_NOTES.

Write the guidance for an operator, not a reviewer: "when" names the situation that should make someone reach for the setting, "expect" names the observable consequence including the downside. A note that only restates the description is worse than none — it passes the gate while teaching nothing.

Environment variables

The env-var reference is generated the same way, from ENV_CATALOGUE in @app/core/env. That module owns the prose; the zod schemas in packages/core/src/env/schema.ts, apps/api/src/env.ts and apps/worker/src/env.ts own the defaults.

Two checks tie them together:

  • bun run docs:gen --check fails when a key declared in .env.example has no catalogue entry, when an entry is missing its description, values, when or expect, when a parsed entry carries a defNote without a defParsed, or when any entry carries a defParsed that nothing reads. A compose- or build-only entry (parsed: false) carries a defNote alone and passes.
  • env-docs.test.ts in apps/api and apps/worker parses an empty environment through the real loader and asserts every documented default is the default the process actually applies. Changing .default(9100) to .default(9200) fails there. A variable the process documents as required is asserted too: the test drops it from an otherwise-valid environment and requires the parse to fail.

So adding a variable means: declare it in .env.example, add its schema field, and add its catalogue entry — in the same pull request. Where a documented default legitimately differs from the schema's (a value .env.example ships, or one resolved at boot like STUDY_CPU_SHARE), set defNote to say why and defParsed to the exact value an empty environment parses to, or null when the schema yields no value at all. defNote redirects the assertion from def to defParsed; it never removes it. A parsed variable carrying a defNote without a defParsed fails docs:gen --check, and so does any entry carrying a defParsed nothing reads: one with no defNote to redirect the assertion onto it, or one on a variable no app parses. Compose- and build-only variables (parsed: false) have no schema to pin, so they take a defNote alone.

Screenshots

Every screenshot on this site is generated, never taken by hand:

bun run docs:screenshots

That one command creates a dedicated database (the dev one suffixed _docs) and Redis logical db, migrates and seeds them, boots the api and web dev server on their own ports, drives a headless browser over every documented screen, and writes the PNGs straight into docs/assets/screenshots/. Review the diff and commit it. No Binance credentials are involved, because the seeder writes the market data the UI would otherwise read from the exchange.

Prerequisites: Postgres and Redis up, a .env with AUTH_SECRET filled in (bun run setup), node on PATH, and Playwright's Chromium installed once with cd e2e && bunx playwright install chromium.

It never touches the database bun run dev uses. That isolation is the point: the seeder writes wallet balances and prices to the same Redis keys a running worker reads for sizing and exit decisions, so pointing it at a live account would have the worker act on holdings that do not exist. bun run seed:dev therefore refuses outright if the target database holds a live-mode account — only the docs pipeline, against its own disposable database, is allowed one.

What seed:dev does

On an empty database it bootstraps: it creates the operator through Better Auth (so the stored password hash is the one real sign-up would write), adds a placeholder Binance key row so the dashboard stops prompting for one, and creates one enabled profile per registered strategy. Defaults are docs@example.com / docs-screenshot-pw-1234, overridable with SEED_OPERATOR_EMAIL and SEED_OPERATOR_PASSWORD.

On a database that already has an operator it adopts it: existing accounts and profiles are reused and no profile is created, because adding an enabled, symbol-bound profile to someone's account is not adoption — the worker would pick it up and start trading coins nobody chose.

Either way it then seeds orders, positions and the Redis market data. Values are derived from a hash of each symbol, so coins differ from one another while a re-run reproduces the same account. A re-run replaces each profile's orders and positions wholesale.

Closed-trade archive rows are the exception: they are written, and wiped, only on the docs stack. trade_archive is the realised-P/L ledger and the only record of a completed cycle — the fee roll-up and decomposition are computed at archive time and cannot be re-derived from Binance later — and nothing distinguishes a seeded row from a real one afterwards. So the seeder never touches archive history on a dev database, and a dev database gets no realised-P/L data.

Other knobs: SEED_SYMBOLS_PER_PROFILE (default 4) and SEED_NOW_MS (the instant every seeded timestamp is measured back from — the capture freezes the browser clock to the same value, so ages render as a plausible spread instead of all "0s ago").

Two files own the set:

  • e2e/docs-screenshots.manifest.mjs — which screens exist, the route each one lives at, and the file(s) it writes. A screen documented on two pages is one capture with two destinations.
  • e2e/docs-screenshots.mjs — the capture itself: sizing, the frozen clock, and the handful of screens reached by an interaction rather than a URL.

Adding a screenshot means adding a manifest entry and embedding it from a page. scripts/ci/no-stale-screenshot.sh (part of bun run lint) fails if those two lists and the committed PNGs disagree — a page embedding a file nobody captures, a manifest entry nobody committed, or a committed PNG no page uses.

CI does not re-render the screenshots to compare them. PNG bytes are not reproducible across machines (Chromium build, font stack, rasteriser), so a regenerate-and-diff gate would be permanently red. Keeping the images current is a refresh run when the UI they show changes, under rule 4 above.

Dependency declaration

A workspace package's dependencies must not re-declare a third-party package it already receives transitively through another @app/* workspace dependency.

The test: if a third-party entry can be removed from a package's dependencies and it still resolves (because a workspace dependency pulls it in) and none of the package's own code — src/, scripts/, and every other entry point — has a direct import … from '<dep>' (import type counts), the entry is redundant; drop it.

Rationale: a re-declared transitive dependency is a second place a version can drift. For example, were Drizzle consumed only through @app/db, drizzle-orm would belong in packages/db alone — but apps/api imports it directly from scripts/reset-password.ts, so it stays declared there.

This rule covers runtime dependencies only. Build-time tooling in devDependencies — bundler plugins, the peers they require to build, and test libraries — is declared wherever the build needs it, regardless of whether the package's own code imports it directly.

Type-level guards

Type-level assertions live in <pkg>/__tests__/*.test-d.ts — their @ts-expect-error lines assert that a wrong-shape call fails to compile. Such a file protects nothing unless a tsc pass actually compiles it, and the default per-package tsconfig.json excludes __tests__ (or scopes include to src/), so an unwired guard is silently dead.

Each package holding a guard file needs a tsconfig.test-d.json extending the package config with include covering __tests__/**/*.test-d.ts (and noEmit, non-composite, so it only typechecks), and its typecheck script must append tsc -p tsconfig.test-d.json --noEmit. The no-unwired-test-d CI gate fails the build if any .test-d.ts is missing either wiring.

Loading branches must reserve height

apps/web/__tests__/loading-placeholder-gate.test.ts parses every apps/web/src/**/*.tsx with the TypeScript compiler API and fails listing each path:line:column that renders a bare-text loading state instead of a placeholder from @/shared/components/page-skeleton. It is a vitest test, not a shell gate: CI runs on bun:alpine, whose BusyBox grep silently no-ops GNU flags, so a shell version of this check would pass vacuously.

Two orthogonal detectors run and their results are unioned, deduped by source position. A is condition-side — a ?: / if / && whose condition is a loading test (.isLoading / .isPending / .isFetching, an identifier matching /^(is)?\w*[Ll]oading$/, or a || of those) and whose branch renders text with no placeholder tag under it (/(Skeleton|LoadingRows)$/, so LoadingRows clears it too). Negations and compound conditions are deliberately not loading tests. B is content-side — any element carrying loading-shaped copy (/^Loading\b/i, an ellipsis-terminated "loading"/"fetching" string, or a t('….loading') key) with no skeleton descendant, whatever guards it. B is what catches a placeholder behind a data test (data ? … : <p>Loading…</p>), which A structurally cannot see. A third rule flags a Skeleton/*Skeleton element whose static className carries no height token (h-, min-h-, aspect-, size-) — the right tag name is not the invariant, the reserved box is.

Exemptions are structural, never a path allow-list (an allow-list fails open the moment a file moves): a control tag as the nearest enclosing element, an sr-only element, and a value that cannot render. A render prop is not exempt — aside={<Panel/>} and pendingComponent={() => <p/>} are painted as page content.

Vacuity guards, because a gate that scans nothing is worse than no gate: the source root must exist, the walk must collect ≥150 files and find ≥30 loading branches (the denominator, not the offender count), parseDiagnostics is read off each SourceFile and throws rather than letting a truncated tree report clean, and each detector has a fixture only it can reach so neither can be neutered without a test going red.

Deadline-bounded writes take a thunk

A best-effort write whose reply you do not read — one you want attempted, not confirmed — goes through raceDeadline (apps/worker/src/lib/race-deadline.ts), which bounds it by a deadline and routes a stall, a rejection and a synchronous throw to a callback instead of propagating. Pass the write as a thunk (() => redis.set(...)), never as an already-created promise.

A bounded call whose reply is load-bearing wants the opposite shape and rejects instead, so the caller's error path gets a verdict it can act on; rejectOnDeadline in apps/worker/src/tick/override-settlement.ts is that counterpart. A bounded read that treats a fault and a stall identically has no use for the distinction either helper draws, and races inline.

The reason is where the argument is evaluated. A promise argument is built by the caller, so a client that throws before it can return one — a closed connection, an argument the command builder refuses — throws outside the helper. On the tick path that unwinds a tick which may have already placed or cancelled orders, and at a fire-and-forget call site it becomes an unhandled rejection. Passing a thunk moves the payload build and the call itself inside the guard, so the helper's "resolves, never rejects" contract holds by construction rather than by every caller remembering a .catch. The thunk must return the call: async () => { dep(); } compiles but resolves immediately, which makes the deadline vacuous.

apps/worker/__tests__/lib/race-deadline.test-d.ts is the compiler guard that keeps the signature honest — it asserts a bare promise argument fails to compile.

Error handling and logging

Log the raw caught binding under the err key: logger.error({ err }, '...'). pino's err serializer emits the error's type, message, and stack, and narrows non-Error throws safely. Passing a pre-stringified value under err (a .message, an instanceof-Error ternary, String(err), or a helper call like errorMessage(err)) fires before the serializer and strips the stack, so a log that should carry a trace carries only a line of text.

errorMessage(x) from @app/core/error is for operator-facing message strings only — return values, reason: fields, notification bodies, DB receipts — never under the err log key. Two CI gates hold the line: no-stripped-err-log fails on a stringified value under err, and no-error-cast fails on an (err as Error) cast.

Metric names

Every metric the worker emits is declared in apps/worker/src/metrics/catalog.ts: a member of the MetricName union, plus an entry in CATALOG giving its kind (counter, gauge, or histogram), its labelNames, and — for a histogram — its bucket bounds.

The rule is enforced by the type system, not by a CI script. MetricsSink.record takes name: MetricName, so an uncatalogued name is a compile error and shows up on bun run typecheck. There is no way around it: a non-null assertion, bracket access to the method, an aliased receiver, or a name computed into a string variable all fail the same way.

That strength is the point, because the runtime behaviour is silent. The prom-client adapter looks the name up and returns when it misses — no series, no log, no error. An uncatalogued metric would read zero forever, and a degrade counter stuck at zero is indistinguishable from a path that has never degraded, so the operator is reassured by a number that was never collected.

MetricName is spelled out as a union rather than derived with keyof typeof CATALOG, because isolatedDeclarations rejects an exported declaration whose type depends on an object literal. CATALOG is then annotated Readonly<Record<MetricName, MetricSpec>>, which keeps the two in step in both directions: a union member with no entry fails to compile, and an entry with no union member fails too. Do not relax that annotation to Record<string, MetricSpec> — it would widen MetricName to string and silently un-check every call site in the worker.

apps/worker/__tests__/metrics/catalog.test-d.ts pins this. It asserts the union is not string (a directive-free assertion, so it cannot drift), and carries a @ts-expect-error for each evasion above; no-unwired-test-d.sh fails the build if any of those directives stops suppressing a real error.

Pick the kind and labels from what the call site means, not from the name:

  • A level that is re-read each pass — a queue depth, a consumer backlog, a rolling usage window — is a gauge. Accumulating one reports a total that never existed.
  • A per-event tally is a counter.
  • A distribution of a per-pass magnitude is a histogram, with bounds chosen for that magnitude. Reusing the latency bounds for an entry count wastes most of them above the range the metric actually occupies.
  • A call site that passes no tags declares labelNames: []. Declaring a label the caller never supplies stamps it unknown on every series and invents a dimension the metric does not have.

Names are checked; labels are not. record() resolves each declared label as tags?.[key] ?? 'unknown', so renaming a tag key at a call site (stream → streamKey) still compiles: the metric exports audit_stream_length{stream="unknown"} and every profile collapses into one series. Nothing catches that today — check the labels by hand when you edit a call site.

Alert rules may only name metrics we emit

Every metric named in an expr: in deploy/observability/alerts.yml must be a series this repo actually emits. no-phantom-alert-metric.sh enforces it, and lint.sh runs it immediately before promtool-lint.sh.

The two gates catch different faults and the order matters. promtool check rules validates PromQL syntax; it has no view of which series exist, so a rule over a metric nothing writes passes it, then evaluates to an empty vector forever. Such a rule never fires and never errors, which looks exactly like a healthy rule that has not tripped — the operator believes they are covered and is not. Name existence therefore has to fail first, or the author reads a syntax verdict and stops.

The allowed set is assembled from the code rather than a list in the gate, so a new sink is covered the day it lands:

  • the worker catalogue — the MetricName union and the CATALOG keys, parsed separately and diffed against each other, since TypeScript already couples them and a divergence means the parser regressed;
  • every new Counter/Gauge/Histogram/Summary({ name: … }) under apps/** and packages/**, which picks up the api HTTP sink and the @app/observability registry without either path being named in the gate. The scan window closes at that call's own }), never at a character count: a fixed window can run past the call and adopt an unrelated name: literal as a declared metric, which is fail-open and would stop a real phantom being reported;
  • _count / _sum / _bucket off a name declared kind: 'histogram', and _count / _sum off a summary, because Prometheus derives those and only for those kinds. A counter or gauge derives nothing, so decision_count_sum is still a phantom even though decision_count is real;
  • two exact-match sets for series no repo code declares: Prometheus' synthesised up, and the process_* / nodejs_* metrics collectDefaultMetrics registers. Both match exactly and never by prefix, so up_wrong is still reported. nodejs_gc_duration_seconds carries a kind alongside its name because it is the one default metric prom-client registers as a Histogram, so its derived series are real.

Only expr: values are scanned — annotations legitimately carry {{ $labels.profileId }} and prose. Tokenising is subtractive (strip quoted strings, label matchers, range selectors, grouping label lists, offset/@ modifiers, then numeric literals, and read what survives), so a parse miss over-reports and fails the build instead of skipping a name.

Two YAML shapes are handled rather than skipped, because in both the subtractive order would erase the name before it was read and the rule would pass as if it named nothing: a fully quoted expr: scalar (unwrapped first, '' and \" escapes included) and a {__name__="x"} matcher (harvested before the brace strip). A regex or negated __name__ matcher cannot be resolved to a concrete series at all, so it is a hard error rather than a silent skip. Flow-style rule entries (- {alert: …, expr: …}) are rejected the same way: they match neither key pattern while keeping the head and expr counts balanced, so the consistency assert would not notice them.

Five vacuity floors fail the gate rather than pass it: zero catalogue names, zero constructed names, zero rules, zero exprs, and any single expr that names no metric. That last one is per-expr deliberately — a global count is satisfied by one healthy rule and lets a sibling parsed down to nothing through, which is the silent pass the gate exists to stop. A sixth assert requires every rule entry to have yielded an expr, so a block-scalar walk that over-consumed fails red instead of quietly checking less.

A rule that names no metric on purpose — a dead-man switch watchdog is expr: vector(1) by construction — opts out with a # names-no-metric: <reason> comment on the line directly above its expr: key. The opt-out is per-rule and has to be typed, so the floor stays fail-closed: an expression that silently parsed to nothing is still an error.

Rules are read as entries, starting at the dash that introduces each one, and sibling keys are matched at exactly that entry's key column. That is what makes key order inside a rule irrelevant (expr: may lead) while keeping a block-scalar annotation that happens to contain expr: or alert: from being read as a rule key.

no-phantom-alert-metric.selftest.sh runs the gate over twelve fixture trees under scripts/ci/__fixtures__/alert-metric/, and lint.sh runs it before the gate itself: set -e stops at the first failure, and on the run where both would fail it is the self-test that says whether the rules file is wrong or the parser regressed. Every rejecting case asserts its own diagnostic string, never a bare non-zero exit: the floors and the hard parse errors all exit 1, so a moved fixture would trip one of them and a non-zero-means-caught check would read that as a successful catch. The pass tree exercises every accepted syntax at once, since a false positive there would start rejecting valid rules, and the fail tree pins six distinct phantom shapes by name.

If a rule you want has no series behind it, emit the metric or drop the rule. Do not leave it in place: the file is operator-facing, and a rule structurally incapable of firing is worse than a documented gap. Record the gap in alerts.yml instead, as the "no alert coverage today" block does.

A lint rule carrying an invariant must stay armed

Some oxlint rules are not style — they are the only thing standing between the repo and a defect class. react/no-unstable-nested-components is one: a component declared inside another's render body is a new type every render, so React tears its subtree down instead of updating it, and on WebKit that clamps scrollTop and drags a reader off their place on every poll.

oxlint hard-fails on a misspelled rule or an unknown plugin prefix, so those need no gate. Two drift shapes are silent, and both exit 0:

  • A plugin removed from plugins. Setting that array overwrites oxlint's defaults, and react is not among them, so trimming the list makes every react/* rule vanish from the resolved config with no diagnostic.
  • A severity downgraded to warn. The rule still runs, but lint.sh invokes bunx oxlint without --deny-warnings, so a warning can never fail the build.

no-dropped-lint-rule.sh asserts against oxlint --print-config — the resolved config, not the source file — that each listed rule is present at the expected severity, and lint.sh runs it before bunx oxlint so a disarmed rule is reported before the lint pass reports clean. Add a rule to required_rules when its absence would retire an invariant rather than relax a preference.

no-dropped-lint-rule.selftest.sh drives the gate over mutated copies via the OXLINT_CONFIG seam, so the tracked .oxlintrc.json is never edited and a killed run cannot leave a disarmed rule behind. It fixtures only the two silent shapes: a fixture for a misspelling would prove nothing, because oxlint exits 1 before the gate can read anything. Each rejecting case asserts its own diagnostic string, and each fixture asserts that its own sed actually applied — a substitution that silently stopped matching would otherwise leave the case passing against an unmodified config.