Skip to content

Commit 62ef96f

Browse files
authored
v0.7.20: perf, ui improvements, fable 5, tables row size bump, blogs
2 parents 6e426f8 + b346088 commit 62ef96f

277 files changed

Lines changed: 7482 additions & 3069 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.claude/rules/sim-components.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,22 @@ Component authoring rules — structure order (refs → external hooks → store
1212
- `'use client'` only when using React hooks or browser-only APIs.
1313
- Prefer semantic HTML (`aside`, `nav`, `article`).
1414
- Optional-chain callbacks: `onAction?.(id)`.
15+
16+
## List-render performance
17+
18+
When rendering or sorting a list of rows against a lookup collection (members, folders, tags), keep the per-row work O(1):
19+
20+
- **Precompute a lookup `Map` once**, never `array.find(...)` per row. Build `const byId = useMemo(() => { const m = new Map<string, T>(); for (const x of items ?? []) m.set(x.id, x); return m }, [items])` and read `byId.get(id)` in the sort comparator, `.map(...)`, and cell builders. A `.find` inside a sort comparator is O(n²·log n) — the worst offender. Depend memos on the derived `Map`, not the raw array.
21+
- **Sort with `[...array].sort(cmp)` in client code — NOT `array.toSorted(cmp)`.** SWC (Next's compiler) transforms syntax but does not polyfill prototype methods, and the repo sets no core-js/browserslist target, so `toSorted`/`toReversed`/`toSpliced`/`Array.prototype.with` ship as-is and throw `TypeError` on Safari <16 / iOS 15 (they landed in Safari 16). `tsconfig` `lib: ES2023` only affects type-checking, never the runtime. The `[...arr].sort()` spread copy is the intended cost — it keeps the sort non-mutating without an unpolyfilled builtin. These methods are only safe in server-only modules (no `'use client'`, executed on Node ≥20). react-doctor's `js-tosorted-immutable` is therefore a won't-fix in client components.
22+
- **Partition in a single pass** — when splitting one collection into several (`fileIds`/`folderIds`), do one `for…of` pushing into each bucket and return `{ a, b }` from a single `useMemo`, not two memos that each `map→filter→map` the same source twice.
23+
24+
## react-doctor (`npx react-doctor`) — apply the wins, skip the false positives
25+
26+
react-doctor diagnostics are hypotheses, not verdicts — confirm against the code before acting, and preserve behavior. Known repo-specific false positives to NOT "fix":
27+
28+
- `no-barrel-import` — barrel imports are the repo convention (see sim-imports.md, "Barrel Exports"). Keep them.
29+
- `js-tosorted-immutable` — in `'use client'` code, keep `[...arr].sort(cmp)`; `toSorted` is unpolyfilled and crashes Safari <16 / iOS 15 (see "List-render performance" above). Only apply it in server-only modules.
30+
- `rerender-state-only-in-handlers` / "state set but never rendered" — a false positive when the `useState` is consumed by a `useEffect`/`useLayoutEffect` dependency (the effect must re-run on change). Only convert to a ref when nothing reads the value reactively.
31+
- `no-render-in-render` — a helper *called inline* (`{renderRow()}`) is reconciled by position and does **not** remount, so extracting it to a component is usually pure churn and can regress behavior (prop-drilling many closures, focus/scroll loss on the inner `<input>`). Apply it only when the helper is genuinely a *component defined during render*, or when the move is mechanical (a stateless, ref-free helper whose closures become a small, explicit prop set).
32+
- `async-await-in-loop` on an upload/progress loop where sequential execution is intentional (per-item progress, server backpressure) — leave it.
33+
- Broad refactors (`prefer-useReducer` for many `useState`, `no-giant-component` splits) — out of scope for a perf pass; note, don't churn.

.claude/rules/sim-hooks.md

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,16 +51,24 @@ export function useFeature({ id, onSelect }: UseFeatureProps) {
5151

5252
## State shape
5353

54-
Never mirror a prop into state with `useState(prop)` + a syncing `useEffect` — a prop change clobbers in-progress local edits. Use the prop directly, reset via a remount `key`, or — when you must seed local state from a prop only on a transition (e.g. a modal opening) — reset during render with the `prevX` ref idiom:
54+
Never mirror a prop into state with `useState(prop)` + a syncing `useEffect` — a prop change clobbers in-progress local edits. Use the prop directly, reset via a remount `key`, or — when you must seed local state from a prop only on a transition (e.g. a modal opening) — adjust it **during render** with a `prev`-value tracker held in `useState`:
5555

5656
```typescript
57-
const prevOpenRef = useRef(open)
58-
if (prevOpenRef.current !== open) {
59-
prevOpenRef.current = open
57+
const [prevOpen, setPrevOpen] = useState(open)
58+
if (prevOpen !== open) {
59+
setPrevOpen(open)
6060
if (open) setName(initialName) // closed → open only
6161
}
6262
```
6363

64+
React re-renders immediately with the corrected state without committing the stale value. Rules: the `if (prev !== current)` guard is mandatory (an unconditional `setState` in render loops forever), the tracker is set **inside** the guard, and you may only set the currently-rendering component's state this way. Hold the tracker in `useState`, **not a `useRef`** — React forbids reading/writing `ref.current` during render (react.dev, useRef → "Do not write _or read_ `ref.current` during rendering"; the `react-hooks` `refs` lint flags it), and a `useState` tracker is concurrent-safe where a mutated ref is not (a discarded render rolls state back, not a ref).
65+
66+
**The tracker's initial value decides mount behavior — choose it deliberately.** The example seeds `useState(open)` because the modal mounts closed, so the first render's guard is `false` and nothing resets on mount (correct — `name` is already at its initial value). When the effect you're replacing did real work **on mount** — opening a panel because a prop already matches, seeding editable state from an already-present value, or a component that can mount in the active state — seed a **sentinel** the live value can't equal (e.g. `useState<T | null>(null)`), otherwise the guard is `false` on the first render and that mount action is silently dropped. Place the block before any early `return`.
67+
68+
> Some existing components use a `useRef` prev-tracker (`if (prevRef.current !== x) { prevRef.current = x; … }`). It works but reads/writes a ref during render — prefer the `useState` form above for new code.
69+
70+
Only convert a `useState` to a `useRef` when the value is **never read during render/JSX and is never a hook dependency** — a value in a `useEffect`/`useMemo`/`useCallback` dep array must re-run the hook on change, so it stays state (see also `rerender-state-only-in-handlers` in `sim-components.md`). Convert only set-only values read solely inside handlers or effect bodies (e.g. a prompt-history index, a pending-upload URL). If a ref feeds render, mutating it won't re-render and the UI goes stale.
71+
6472
Model mutually-exclusive flags as ONE `status` enum, not several contradictory booleans. `isLoading`/`isVerified`/`isInvalidOtp` describing one machine collapse to `status: 'idle' | 'verifying' | 'verified' | 'error'` (+ `errorMessage`); derive any boolean a consumer still needs (`status === 'error'`).
6573

6674
Derive busy/success from the mutation object — never duplicate `mutation.isPending`/`mutation.isSuccess` into local `useState`. Read them directly (`mutation.isSuccess`) and reset with `mutation.reset()`. A distinct phase the mutation doesn't cover — e.g. a pre-submit captcha/Turnstile gate that runs before `mutate()` — is not a duplicate; keep that flag.

.claude/rules/sim-imports.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,20 @@ import { Dashboard, Sidebar } from '@/app/workspace/[workspaceId]/logs/component
3131
import { Dashboard } from '@/app/workspace/[workspaceId]/logs/components/dashboard/dashboard'
3232
```
3333

34+
## Code-splitting through barrels
35+
36+
When you `lazy(() => import(...))` a component to keep it out of a route's initial bundle, import the **deep module path** (`./components/foo/foo`), never the barrel — and **delete the now-dead barrel re-export** of that component. This app has no `"sideEffects": false` in `apps/sim/package.json`, so when any sibling still imports that barrel, webpack can conservatively keep the barrel's re-export edge to the heavy module. A leftover `export { Foo } from './foo'` line can therefore drag `Foo` (and its transitive deps) back into the initial chunk and silently defeat the split. Removing the dead re-export is the guaranteed fix; verify with a production bundle diff, not by eyeballing the `lazy()` call.
37+
38+
```typescript
39+
// ✓ Good — deep lazy import + no barrel edge left behind
40+
const MothershipView = lazy(() =>
41+
import('./components/mothership-view/mothership-view').then((m) => ({ default: m.MothershipView }))
42+
)
43+
// (and remove `export { MothershipView } from './mothership-view'` from components/index.ts)
44+
```
45+
46+
Wrap the lazy component in a **local `<Suspense>`** so its suspension resolves at the nearest boundary instead of bubbling to the page-level fallback (which would flash the whole route). `React.lazy(memo(forwardRef(...)))` forwards a DOM `ref` correctly in React 19 — but during the fallback window `ref.current` is `null`, so every consumer must guard it (`if (!el) return` / `el?.`).
47+
3448
## No Re-exports
3549

3650
Do not re-export from non-barrel files. Import directly from the source.
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
# React & Render Performance
2+
3+
Behavior-preserving performance idioms for components, hooks, and hot render paths. These are safe defaults — apply them freely. For the render-causing *effect/state* anti-patterns (derived state in effects, effect chains, state synced to a prop), use the dedicated skills: `/you-might-not-need-an-effect`, `/you-might-not-need-state`, `/you-might-not-need-a-memo`, `/you-might-not-need-a-callback`. Those refactors change render timing — verify them against the running UI, never mass-apply blind.
4+
5+
## Lazy-init refs that hold objects
6+
7+
`useRef(new Map())` / `useRef(new Set())` / `useRef({...})` allocates a fresh object on **every render** and throws it away — only the first is ever kept. Lazy-init instead so the allocation happens once.
8+
9+
```typescript
10+
// ✗ Bad — allocates a new Map each render, discards all but the first
11+
const cacheRef = useRef<Map<string, string>>(new Map())
12+
13+
// ✓ Good — allocated once, stable identity thereafter
14+
const cacheRef = useRef<Map<string, string> | null>(null)
15+
cacheRef.current ??= new Map()
16+
```
17+
18+
Read `cacheRef.current` directly inside effects/handlers — refs are stable and never belong in a dependency array. A cheap primitive (`useRef(0)`, `useRef('')`, `useRef(null)`) needs no lazy init.
19+
20+
## Hoist static values and closure-free functions to module scope
21+
22+
A value or function declared inside a component is rebuilt every render. If it captures **nothing** from component scope (no props/state/refs), move it above the component at module scope. This skips the per-render allocation and keeps a stable identity so memoized children don't re-render.
23+
24+
```typescript
25+
// ✗ Bad — rebuilt every render, new identity each time
26+
function Toolbar({ mode }: ToolbarProps) {
27+
const TITLES = { create: 'Add', edit: 'Configure' } as const
28+
const handleWheel = (e: React.WheelEvent) => e.currentTarget.scrollBy(e.deltaX, e.deltaY)
29+
// ...
30+
}
31+
32+
// ✓ Good — allocated once at module load
33+
const TITLES = { create: 'Add', edit: 'Configure' } as const
34+
function handleWheel(e: React.WheelEvent) {
35+
e.currentTarget.scrollBy(e.deltaX, e.deltaY)
36+
}
37+
function Toolbar({ mode }: ToolbarProps) { /* ... */ }
38+
```
39+
40+
A closure-free function that IS wired through a ref sink or intentionally kept for stable identity may stay inline — hoisting a one-line `preventDefault` handler is churn, not a win. Hoist when it removes a real per-render allocation or unblocks child memoization.
41+
42+
## Pre-index with Map/Set for repeated lookups
43+
44+
`array.find()` / `array.includes()` / `array.indexOf()` scan the whole list each call. Inside a loop or a hot render path over a non-trivial list, that is O(n·m). Build a `Map` (for lookup-by-key) or `Set` (for membership) **once before** the loop, then look up in O(1).
45+
46+
```typescript
47+
// ✗ Bad — find() re-scans outputs for every column
48+
for (const child of columns) {
49+
const output = group.outputs.find((o) => o.columnName === getColumnId(child))
50+
}
51+
52+
// ✓ Good — index once, then O(1) lookups
53+
const outputByName = new Map<string, Output>()
54+
for (const o of group.outputs) {
55+
if (!outputByName.has(o.columnName)) outputByName.set(o.columnName, o) // first wins, matches find()
56+
}
57+
for (const child of columns) {
58+
const output = outputByName.get(getColumnId(child))
59+
}
60+
```
61+
62+
Preserve `.find()`'s **first-match** semantics when duplicate keys are possible: `new Map(arr.map(...))` keeps the *last* entry, so guard with `if (!map.has(key))` when replacing a `.find()`. Skip this for tiny, cold arrays (a handful of items in an event handler) where the Map build costs more than it saves.
63+
64+
## Never mutate a shared array in place
65+
66+
The real bug to avoid is `array.sort()` / `array.reverse()` on an array you don't own — sorting a React Query cache array in place corrupts shared state. Always sort a copy:
67+
68+
```typescript
69+
// ✗ Bad — mutates the (possibly shared) source array in place
70+
return items.sort(compare)
71+
72+
// ✓ Good — sorts a throwaway copy, source untouched
73+
return [...items].sort(compare)
74+
```
75+
76+
**Do NOT reach for `toSorted()` / `toReversed()` / `with()` / `toSpliced()` on client render paths.** They are ES2023 *runtime* methods — and a tsconfig `"lib": ["ES2023"]` only makes them **type-check**, it does not make them **run**. Next/SWC compiles syntax but does **not** polyfill prototype methods, and the default browserslist still includes browsers without them (`toSorted` landed in Safari 16 / iOS 16, so any device capped at iOS 15 throws `TypeError: x.toSorted is not a function` and crashes the page). The perf difference vs `[...arr].sort()` is negligible (both allocate one array), so the copy-then-sort form is the correct default everywhere client code runs. Only consider the immutable methods in Node-only code (server routes, scripts) on Node ≥20, where the runtime is known.
77+
78+
## Run independent awaits in parallel
79+
80+
Sequential `await`s that don't consume each other's result serialize latency for nothing — in an async Server Component or a route handler this directly delays the response. Kick them off together with `Promise.all` and destructure.
81+
82+
```typescript
83+
// ✗ Bad — waits for params, then separately waits for searchParams
84+
const { id } = await params
85+
const { kbName } = await searchParams
86+
87+
// ✓ Good — one combined wait
88+
const [{ id }, { kbName }] = await Promise.all([params, searchParams])
89+
```
90+
91+
Only keep awaits sequential when a later call genuinely uses an earlier result, or when the ordering is deliberate (rate-limited batches, retry loops, write-then-read).
92+
93+
## Local feature barrels are the convention — do not "fix" them
94+
95+
Tooling (e.g. react-doctor's `no-barrel-import`) will flag imports from local `index.ts` barrels as a bundle cost. In this repo that is a **false positive**: barrel imports for 3+ export folders are mandated by `.claude/rules/sim-imports.md`. Leave them.

.github/workflows/ci.yml

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ jobs:
9292
ecr_repo_secret: ECR_PII
9393
steps:
9494
- name: Checkout code
95-
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
95+
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6
9696

9797
- name: Configure AWS credentials
9898
uses: aws-actions/configure-aws-credentials@e7f100cf4c008499ea8adda475de1042d6975c7b # v6
@@ -130,6 +130,49 @@ jobs:
130130
provenance: false
131131
sbom: false
132132

133+
# Dev: deploy Trigger.dev background tasks to the preview "dev-sim" branch.
134+
# Gated after migrate-dev for the same reason as build-dev — the new task
135+
# code runs against the dev DB, so the schema must be pushed first.
136+
deploy-trigger-dev:
137+
name: Deploy Trigger.dev (Dev)
138+
needs: [migrate-dev]
139+
if: github.event_name == 'push' && github.ref == 'refs/heads/dev'
140+
runs-on: blacksmith-4vcpu-ubuntu-2404
141+
steps:
142+
- name: Checkout code
143+
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6
144+
145+
- name: Setup Bun
146+
uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2
147+
with:
148+
bun-version: 1.3.13
149+
150+
- name: Cache Bun dependencies
151+
uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5
152+
with:
153+
path: |
154+
~/.bun/install/cache
155+
node_modules
156+
**/node_modules
157+
key: ${{ runner.os }}-bun-${{ hashFiles('**/bun.lock') }}
158+
restore-keys: |
159+
${{ runner.os }}-bun-
160+
161+
- name: Install dependencies
162+
run: bun install --frozen-lockfile
163+
164+
- name: Deploy to Trigger.dev
165+
working-directory: ./apps/sim
166+
env:
167+
TRIGGER_ACCESS_TOKEN: ${{ secrets.DEV_TRIGGER_ACCESS_TOKEN }}
168+
TRIGGER_PROJECT_ID: ${{ secrets.TRIGGER_PROJECT_ID }}
169+
run: |
170+
if [ -z "$TRIGGER_ACCESS_TOKEN" ] || [ -z "$TRIGGER_PROJECT_ID" ]; then
171+
echo "ERROR: DEV_TRIGGER_ACCESS_TOKEN and TRIGGER_PROJECT_ID repo secrets must both be set" >&2
172+
exit 1
173+
fi
174+
bunx trigger.dev@4.4.3 deploy --env preview --branch dev-sim
175+
133176
# Main/staging: build AMD64 images and push to ECR + GHCR
134177
build-amd64:
135178
name: Build AMD64
@@ -359,7 +402,7 @@ jobs:
359402
steps:
360403
- uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6
361404
with:
362-
fetch-depth: 2 # Need at least 2 commits to detect changes
405+
fetch-depth: 2 # Need at least 2 commits to detect changes
363406
- uses: dorny/paths-filter@fbd0ab8f3e69293af611ebaee6363fc25e6d187d # v4
364407
id: filter
365408
with:

.github/workflows/migrations.yml

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,16 @@ jobs:
6969
7070
if [ "${ENVIRONMENT}" = "dev" ]; then
7171
echo "Dev environment — pushing schema directly (db:push)"
72-
bun run db:push --force
72+
# drizzle-kit push needs a TTY to resolve ambiguous renames (--force only
73+
# covers data-loss). In CI it throws "Interactive prompts require a TTY
74+
# terminal" but still exits 0, so the job goes green without applying the
75+
# change. tee keeps the output live in the log; we then fail on drizzle's
76+
# own TTY error. A genuine non-zero exit already fails via `set -e`.
77+
bun run db:push --force < /dev/null 2>&1 | tee /tmp/db-push.log
78+
if grep -q "Interactive prompts require a TTY terminal" /tmp/db-push.log; then
79+
echo "ERROR: db:push needs an interactive rename decision; land it as a versioned migration instead of relying on push." >&2
80+
exit 1
81+
fi
7382
else
7483
echo "Applying versioned migrations (db:migrate)"
7584
bun run ./scripts/migrate.ts

.github/workflows/test-build.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,9 @@ jobs:
128128
- name: Bare-icon theme-safety audit
129129
run: bun run check:bare-icons
130130

131+
- name: Icon SVG path validity audit
132+
run: bun run check:icon-paths
133+
131134
- name: Verify realtime prune graph
132135
run: bun run check:realtime-prune
133136

CLAUDE.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,8 @@ export function Component({ requiredProp, optionalProp = false }: ComponentProps
127127

128128
Extract when: 50+ lines, used in 2+ files, or has own state/logic. Keep inline when: < 10 lines, single use, purely presentational.
129129

130+
Behavior-preserving render-performance idioms — lazy-init object refs, hoist closure-free values/functions to module scope, pre-index repeated lookups with `Map`/`Set`, and never mutating a shared array in place — are in `.claude/rules/sim-react-performance.md` (which also explains why `toSorted`/`toReversed` are unsafe on client render paths despite the ES2023 tsconfig lib — SWC does not polyfill prototype methods, so use `[...arr].sort()`). For the render-timing effect/state anti-patterns use the `/you-might-not-need-*` skills and verify against the running UI.
131+
130132
## API Contracts
131133

132134
Boundary HTTP request and response shapes for all routes under `apps/sim/app/api/**` live in `apps/sim/lib/api/contracts/**` (one file per resource family — `folders.ts`, `chats.ts`, `knowledge.ts`, etc.). Routes never define route-local boundary Zod schemas, and clients never define ad-hoc wire types — both sides consume the same contract.

0 commit comments

Comments
 (0)