From bb6e826d9196aabb1a047d39c5fd3cb3b93385f5 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Thu, 19 Mar 2026 17:15:20 +0100 Subject: [PATCH] docs(site): add frontend agent guidelines from PR review analysis (#23299) --- site/AGENTS.md | 148 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 142 insertions(+), 6 deletions(-) diff --git a/site/AGENTS.md b/site/AGENTS.md index c7d6c4bf32..31790fb136 100644 --- a/site/AGENTS.md +++ b/site/AGENTS.md @@ -31,6 +31,14 @@ When investigating or editing TypeScript/React code, always use the TypeScript l - Do not use shadcn CLI - manually add components to maintain consistency - The modules folder should contain components with business logic specific to the codebase. - Create custom components only when shadcn alternatives don't exist +- **Before creating any new component**, search the codebase for existing + implementations. Check `site/src/components/` for shared primitives + (Table, Badge, icons, error handlers) and sibling files for local + helpers. Duplicating existing components wastes effort and creates + maintenance burden. +- Keep component files under ~500 lines. When a file grows beyond that, + extract logical sections into sub-components or a folder with an + index file. ## Styling @@ -52,7 +60,111 @@ When investigating or editing TypeScript/React code, always use the TypeScript l - Destructure imports when possible (eg. import { foo } from 'bar') - Prefer `for...of` over `forEach` for iteration - **Biome** handles both linting and formatting (not ESLint/Prettier) -- Always use react-query for data fetching. Do not attempt to manage any data life cycle manually. Do not ever call an `API` function directly within a component. +- Always use react-query for data fetching. Do not attempt to manage any + data life cycle manually. Do not ever call an `API` function directly + within a component. +- **Match existing patterns** in the same file before introducing new + conventions. For example, if sibling API methods use a shared helper + like `getURLWithSearchParams`, do not manually build `URLSearchParams`. + If sibling components initialize state with `useMemo`, don't switch to + `useState(initialFn)` in the same file without reason. +- Match errors by error code or HTTP status, never by comparing error + message strings. String matching is brittle — messages change, get + localized, or get reformatted. + +## TypeScript Type Safety + +- **Never use `as unknown as X`** double assertions. They bypass + TypeScript's type system entirely and hide real type incompatibilities. + If types don't align, fix the types at the source. +- **Prefer type annotations over `as` casts.** When narrowing is needed, + use type guards or conditional checks instead of assertions. +- **Avoid the non-null assertion operator (`!.`)**. If a value could be + null/undefined, add a proper guard or narrow the type. If it can never + be null, fix the upstream type definition to reflect that. +- **Use generated types from `api/typesGenerated.ts`** for all + API/server types. Never manually re-declare types that already exist in + generated code — duplicated types drift out of sync with the backend. +- If a component's implementation depends on a prop being present, make + that prop **required** in the type definition. Optional props that are + actually required create a false sense of flexibility and hide bugs. +- Avoid `// @ts-ignore` and `// eslint-disable`. If they seem necessary, + document why and seek a better-typed alternative first. + +## React Query Patterns + +- **Query keys must nest** under established parent key hierarchies. For + example, use `["chats", "costSummary", ...]` not `["chatCostSummary"]`. + Flat keys that break hierarchy prevent + `queryClient.invalidateQueries(parentKey)` from correctly invalidating + related queries. +- When you don't need to `await` a mutation result, use **`mutate()`** + with `onSuccess`/`onError` callbacks — not `mutateAsync()` wrapped in + `try/catch` with an empty catch block. Empty catch blocks silently + swallow errors. `mutate()` automatically surfaces errors through + react-query's error state. + +## Accessibility + +- Every `` / `
` must have an **`aria-label`** or + `
` so screen readers can distinguish between multiple tables + on a page. +- Every element with `tabIndex={0}` must have a semantic **`role`** + attribute (e.g., `role="button"`, `role="row"`) so assistive technology + can communicate what the element is. +- When hiding an interactive element visually (e.g., `opacity-0`, + `pointer-events-none`), you **must also** remove it from the keyboard + tab order and accessibility tree. Add `tabIndex={-1}` and + `aria-hidden="true"`, or better yet, conditionally render the element + so it's not in the DOM at all. `pointer-events: none` only suppresses + mouse/touch — keyboard and screen readers still reach the element. + +## Testing Patterns + +- **Assert observable behavior, not CSS class names.** In Storybook play + functions and tests, use queries like `queryByRole`, `toBeVisible()`, + or `not.toBeVisible()` — not assertions on class names like + `opacity-0`. Asserting class names couples tests to the specific + Tailwind/CSS technique and breaks when the styling mechanism changes + without user-visible regression. +- **Use `data-testid`** for test element lookup when an element has no + semantic role or accessible name (e.g., scroll containers, wrapper + divs). Never use CSS class substring matches like + `querySelector("[class*='flex-col-reverse']")` — these break silently + on class renames or Tailwind output changes. +- **Don't depend on `behavior: "smooth"` scroll** in tests. Smooth + scrolling is async and implementation-defined — in test environments, + `scrollTo` may not produce native scroll events at all. Use + `behavior: "instant"` in test contexts or mock the scroll position + directly. +- When modifying a component's visual appearance or behavior, **update or + add Storybook stories** to capture the change. Stories must stay + current as components evolve — stale stories hide regressions. + +## Robustness + +- When rendering user-facing text from nullable/optional data, always + provide a **visible fallback** (e.g., "Untitled", "N/A", em-dash). + Never render a blank cell or element. +- When converting strings to numbers (e.g., `Number(apiValue)`), **guard + against `NaN`** and non-finite results before formatting. For example, + `Number("abc").toFixed(2)` produces `"NaN"`. +- When using `toLocaleString()`, always pass an **explicit locale** + (e.g., `"en-US"`) for deterministic output across environments. Without + a locale, `1234` formats as `"1.234"` in `de-DE` but `"1,234"` in + `en-US`. + +## Performance + +- When adding state that changes frequently (scroll position, hover, + animation frame), **extract the state and its dependent UI into a child + component** rather than keeping it in a parent that renders a large + subtree. This prevents React from re-rendering the entire subtree on + every state change. +- **Throttle high-frequency event handlers** (scroll, resize, mousemove) + that call `setState`. Use `requestAnimationFrame` or a throttle + utility. Even when React skips re-renders for identical state, the + handler itself still runs on every frame (60Hz+). ## Workflow @@ -101,12 +213,12 @@ When investigating or editing TypeScript/React code, always use the TypeScript l ### 3. React orchestrates execution -- **Don’t call component functions directly; render them via JSX.** This keeps Hook rules intact and lets React optimize reconciliation. +- **Don't call component functions directly; render them via JSX.** This keeps Hook rules intact and lets React optimize reconciliation. - **Never pass Hooks around as values or mutate them dynamically.** Keep Hook usage static and local to each component. ### 4. State Management -- After calling a setter you’ll still read the **previous** state during the same event; updates are queued and batched. +- After calling a setter you'll still read the **previous** state during the same event; updates are queued and batched. - Use **functional updates** (setX(prev ⇒ …)) whenever next state depends on previous state. - Pass a function to useState(initialFn) for **lazy initialization**—it runs only on the first render. - If the next state is Object.is-equal to the current one, React skips the re-render. @@ -116,15 +228,39 @@ When investigating or editing TypeScript/React code, always use the TypeScript l - An Effect takes a **setup** function and optional **cleanup**; React runs setup after commit, cleanup before the next setup or on unmount. - The **dependency array must list every reactive value** referenced inside the Effect, and its length must stay constant. - Effects run **only on the client**, never during server rendering. -- Use Effects solely to **synchronize with external systems**; if you’re not “escaping React,” you probably don’t need one. +- Use Effects solely to **synchronize with external systems**; if you're not "escaping React," you probably don't need one. +- **Never use `useEffect` to derive state from props or other state.** If + a value can be computed during render, use `useMemo` or a plain + variable. A `useEffect` that reads state A and calls `setState(B)` on + every change is a code smell — it causes an extra render cycle and adds + unnecessary complexity. ### 6. Lists & Keys - Every sibling element in a list **needs a stable, unique key prop**. Never use array indexes or Math.random(); prefer data-driven IDs. -- Keys aren’t passed to children and **must not change between renders**; if you return multiple nodes per item, use `` +- Keys aren't passed to children and **must not change between renders**; if you return multiple nodes per item, use `` +- **Never use `key={String(booleanState)}`** to force remounts. When the + boolean flips, React unmounts and remounts the component synchronously, + killing exit animations (e.g., dialog close transitions) and wasting + renders. Use a monotonically increasing counter or avoid `key` for + this pattern entirely. ### 7. Refs & DOM Access - useRef stores a mutable .current **without causing re-renders**. -- **Don’t call Hooks (including useRef) inside loops, conditions, or map().** Extract a child component instead. +- **Don't call Hooks (including useRef) inside loops, conditions, or map().** Extract a child component instead. - **Avoid reading or mutating refs during render;** access them in event handlers or Effects after commit. + +### 8. Element IDs + +- **Use `React.useId()`** to generate unique IDs for form elements, + labels, and ARIA attributes. Never hard-code string IDs — they collide + when a component is rendered multiple times on the same page. + +### 9. Component Testability + +- When a component depends on a dynamic value like the current time or + date, **accept it as a prop** (or via context) rather than reading it + internally (e.g., `new Date()`, `Date.now()`). This makes the + component deterministic and testable in Storybook without mocking + globals.