diff --git a/CONVENTIONS.md b/CONVENTIONS.md index 160e81b8..e129f3fd 100644 --- a/CONVENTIONS.md +++ b/CONVENTIONS.md @@ -129,22 +129,34 @@ export const useFeatureManager = () => { ## React Patterns -### No useMemo or useCallback (React 19) +### Reach for useMemo / useCallback only when they do work -**NEVER** use `useMemo` or `useCallback`. React 19's compiler handles memoization automatically. Importing and using these hooks is strictly prohibited. +Default to writing the code plainly. Most derived values are cheap and re-computing them per render costs +less than the memo that guards them. ```tsx -// ✅ Good - just write the code naturally +// ✅ Fine — cheap, so just write it const filteredItems = items.filter((item) => item.active); const handleClick = () => doSomething(); -const stats = computeStats(data); - -// ❌ Never do this - remove all useMemo/useCallback -const filteredItems = useMemo(() => items.filter((item) => item.active), [items]); -const handleClick = useCallback(() => doSomething(), []); ``` -**Rationale:** React 19's compiler optimizes re-renders automatically. Manual memoization adds complexity without benefit and can actually prevent optimizations. The compiler is smarter than manual memoization. +But they are ordinary tools, not forbidden ones. Use them where they earn it: + +- a value or callback in a **dependency array**, where an unstable identity re-runs an effect or + re-subscribes a socket every render; +- a genuinely **expensive** computation over a large list; +- a prop passed to a **memoized** child. + +```tsx +// ✅ Earns it — an unstable callback here would re-subscribe on every render +const onConnectionChange = useCallback((state) => setConnState(state), [setConnState]); +``` + +**This section used to say "NEVER — React 19's compiler handles memoization automatically", which was +wrong twice.** The React Compiler is a separate, opt-in build plugin and it is **not installed here** +(there is no `babel-plugin-react-compiler` in `package.json`); React 19 on its own memoizes nothing. And +the codebase never followed the rule — some 40 files use each hook. A convention that is both false and +universally ignored is worse than none, because it makes every other rule in this file look optional. ### Computation Functions Outside Components diff --git a/src/apps/CLAUDE.md b/src/apps/CLAUDE.md index 4adc430f..e8b179ea 100644 --- a/src/apps/CLAUDE.md +++ b/src/apps/CLAUDE.md @@ -38,9 +38,9 @@ A complete data table solution with sorting, filtering, and pagination. ```tsx import { DataTable, useDataControl } from '@/components/DataTable'; -function ExperimentsLibrary() { - const { experiments } = useExperimentsList(); - const dataController = useDataControl(experiments || []); +function JobsLibrary() { + const { jobs } = useJobs(); + const dataController = useDataControl(jobs || []); return ( <> @@ -50,12 +50,12 @@ function ExperimentsLibrary() { handleSearch={dataController.setSearchQuery} /> - + dataController={dataController} pageSize={10} columns={[ { field: 'id', label: 'ID', sortKey: 'id' }, - { field: 'name', label: 'Name', sortKey: 'name' }, + { field: 'type', label: 'Type', sortKey: 'type' }, { field: 'status', label: 'Status', @@ -64,7 +64,7 @@ function ExperimentsLibrary() { }, { label: 'Actions', - format: ({ item }) => + format: ({ item }) => }, ]} /> @@ -143,7 +143,10 @@ Features: - `usePopover` - Popover state management - `useTimeout` - Timeout management - `useTimer` - Interval-based timer -- `useWebsockets` - WebSocket connection +- `useChatWebSocket` - the chat socket, with reconnect + replay cursor +- `usePanelChannel` - panel-to-panel signals (refresh buses, NOT selection — selection is the URL) +- `useJobs` - background jobs +- `useCustomSorter`, `useImageLoader`, `useIsProduction`, `usePhotoEditor` ## State Management @@ -187,15 +190,15 @@ Features: Domain-specific hooks in `state/` directories wrap React Query: ```tsx -// src/apps/dashboard/state/experiments/useExperiment.ts -const { data, isLoading } = useExperiment(experimentId); +// src/workspaces/hooks/src/useJobs.ts +const { jobs, isLoading } = useJobs(); ``` ### When to Use What | Scenario | Hook | |----------|------| -| API data | Domain hooks (`useExperiment`, etc.) | +| API data | Domain hooks (`useJobs`, `useModels`, …) | | Shared UI state | `useGlobal` | | URL-driven state (filters, pagination) | `useQueryState` | | Component-only state | `useState` | @@ -239,10 +242,10 @@ type PanelProps = { **Hook return types**: Use `ReturnType` for typing hook returns in props: ```tsx -import { useExperimentsList } from '@/state/experiments/useExperimentsList'; +import { useJobs } from 'hooks/useJobs'; type TopHeaderProps = { - manager: ReturnType; + manager: ReturnType; }; ``` @@ -258,8 +261,11 @@ toast.success("Saved successfully"); API errors automatically trigger via `useClient.config.onError`. -## App-Specific Docs +## Further Reading -- [Dashboard](./dashboard/CLAUDE.md) - Admin UI specifics -- [Editor](./editor/CLAUDE.md) - Visual editor specifics -- [Runtime](./runtime/CLAUDE.md) - Injected scripts specifics +- `../../CLAUDE.md` — architecture and the frontend route conventions +- `../../docs/navigation-audit.md` — **authoritative** on routing and selection. The short version: + addressable state lives in the URL (`useParams` / `?selected=`), never in a channel or a global; rows + and nav items are real ``/``s. Read it before building a screen that selects things. +- `../../CONVENTIONS.md` — component organisation and React patterns +- `../workspaces/officerdev/APP_CONVENTIONS.md` — panel apps and the AppRegistry diff --git a/src/databases/CLAUDE.md b/src/databases/CLAUDE.md index b5412b4c..6ac33e41 100644 --- a/src/databases/CLAUDE.md +++ b/src/databases/CLAUDE.md @@ -2,99 +2,103 @@ ## Overview -Three PostgreSQL databases managed with Drizzle ORM: +**One PostgreSQL database**, `officer_db`, exposed as the workspace package `officerdb`. Managed with +Drizzle ORM. There is no second database and no cache database — anything that used to say otherwise was +describing a different codebase. -| Database | Purpose | Package | -|----------|---------|---------| -| `officer_db` | Main app data | `officerdb` | -| `ephemeral_db` | Cache & temporary data | `ephemeraldb` | - -## Type System - -### File Structure - -Each database package has: ``` -db_name/ +src/databases/officer_db/ ├── src/ -│ ├── index.ts # DB connection, exports schema + drizzle helpers -│ ├── types.ts # All type exports (Select, Insert, extended) +│ ├── db.ts # the connection +│ ├── index.ts # public surface: re-exports queries, schema and drizzle helpers +│ ├── types.ts # every type export (Select / Insert / extended) │ └── schema/ -│ ├── index.ts # Re-exports all schema files -│ └── *.ts # Table definitions -└── package.json # Exports: "." and "./types" +│ ├── index.ts # re-exports all schema files +│ └── *.ts # table definitions, grouped by domain +└── package.json # exports "." and "./types" ``` -### Type Naming Convention +Schema files are grouped by domain, not by table: `auth`, `chat-events`, `dashboards`, `email`, +`headscale`, `music`, `operations`, `pipeline-jobs`, `server`, `soulseek`, `user-data`, `vault`, +`wallet`. + +## Schema changes use `push`, not migrations + +`bun db:push` diffs the schema code against the live database and alters it directly. **`drizzle-kit +migrate` has never been run here** — there is no `__drizzle_migrations` table. Change the schema, run +push, done. `bun db:gen` writes files to `migrations/`, but nothing applies them; treat the schema code +as the source of truth, never those files. + +**The schema is the source of truth for what the database may contain**, not just for its shape. Where a +column has a known set of legal values, say so with a `check()` rather than leaving it free `text` — +otherwise a value the code stopped supporting sits there unnoticed. `serverIntegrations.provider` is the +worked example: rows for deleted chat integrations kept their bot tokens long after the code that read +them was gone, because nothing structural said they had become illegal. ```ts -// Pattern 1: Simple table (no relations needed in API) -export type Screenshot = typeof Schema.Screenshots.$inferSelect; -export type ScreenshotInsert = typeof Schema.Screenshots.$inferInsert; +const SERVER_PROVIDERS = ['google', 'apify'] as const; -// Pattern 2: Table with relations (for hydrated API responses) -export type UserSelect = typeof Schema.Users.$inferSelect; -export type UserInsert = typeof Schema.Users.$inferInsert; +check( + 'ck_server_integrations_provider', + // sql.raw, not sql`${p}` — an interpolated JS string binds as a parameter, so the constraint is + // emitted as `IN ($1, $2)` and Postgres refuses it, breaking push for the WHOLE schema. + sql`${table.provider} IN (${sql.join(SERVER_PROVIDERS.map((p) => sql.raw(`'${p}'`)), sql`, `)})`, +) +``` + +Adding a constraint fails while existing rows violate it. That is the point: push refusing tells you the +database has drifted, instead of quietly accepting it. Clean the rows, then push. + +## Type naming + +```ts +// types.ts — inferred from the schema, never hand-written +export type UserSelect = typeof Schema.users.$inferSelect; +export type UserInsert = typeof Schema.users.$inferInsert; + +// The bare name is the hydrated shape, when a table has relations worth carrying export type User = UserSelect & { - company: Company; - passkeys: Passkey[]; - // computed fields - passkeyCount: number; + passkeys: PasskeySelect[]; }; ``` -### Type Organization +Organise `types.ts` by domain with section comments, mirroring the schema files. -Organize types by domain with comments: +## Queries + +Hand-written, one file per domain under `src/queries/`, importing tables from `../schema` and types from +`../types`: ```ts -// officerdb/types.ts +import { eq, and } from 'drizzle-orm'; +import { db } from '../db'; +import { users, passkeys } from '../schema'; +import type { UserSelect } from '../types'; -// Auth -export type PasskeySelect = ... -export type Passkey = PasskeySelect & { user: User }; - -// Companies & Websites -export type CompanySelect = ... -export type Company = CompanySelect & { ... }; - -// Experiments -export type ExperimentSelect = ... -export type Experiment = ExperimentSelect & { ... }; +export async function getUsers(): Promise { … } ``` -## Importing Types +Every query is exported from `src/index.ts`, which is the only surface callers use. + +## Importing ```ts -// ✅ Good - import from types subpath +// ✅ types from the types subpath import type { User } from 'officerdb/types'; +// ✅ queries, schema and drizzle helpers from the package root +import { getUserById, eq } from 'officerdb'; -// ✅ Good - import schema/connection from main -import { officerdb, eq, Users } from 'officerdb'; - -// ❌ Bad - don't define manual types in schema files -// ❌ Bad - don't import types from schema directly +// ❌ never reach into src/schema or src/queries directly +// ❌ never hand-write a type a table can infer ``` -## Null Safety +In app code import from `'types'`, which re-exports the database types. Only server and database code +imports `officerdb/types` directly. -Drizzle-inferred types correctly reflect nullable columns. Add guards when needed: +## Conventions -```ts -export async function handleOauthAccount(account: GanOauth) { - // Guard for nullable fields - if (!account.refreshToken || !account.companyId) { - return; - } - // Now TypeScript knows these are non-null - const token = await refreshAccessToken(account.refreshToken); -} -``` - -## Schema Best Practices - -- Use `bigserial` with `mode: 'number'` for IDs -- Use `bigint` with `mode: 'number'` for foreign keys -- Always add indexes for frequently queried columns -- Use `varchar` with explicit length limits -- Timestamps: `timestamp('...', { withTimezone: true })` +- `serial` for ids on small tables; `bigserial` with `mode: 'number'` where the row count is unbounded + (`chat_session_events` is the example — its id is also the replay cursor). +- `timestamp('…', { withTimezone: true })`, always. +- Index anything you filter on. Drizzle-inferred types reflect nullability correctly, so guard rather + than cast.