Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
180 changes: 43 additions & 137 deletions .claude/commands/pr-review.md

Large diffs are not rendered by default.

103 changes: 103 additions & 0 deletions .claude/rules/code-quality.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
---
paths:
- "**/*.ts"
- "**/*.tsx"
---

# Code quality (and how it's enforced)

Quality rules are **mechanically enforced** by ESLint + Prettier + Husky — not just guidance. The build fails on `error`-level violations. Write to pass them the first time.

Reference: `eslint.config.mjs`, `tsconfig.json`, `.prettierrc`, `package.json` (husky / lint-staged).

## The rules → the enforcer

| Rule (what to do) | ESLint rule | Level |
| ----------------------------------------------- | -------------------------------------------------------------------------------------- | --------- |
| **File ≤ 500 LOC** (excl. blank/comment lines) | `max-lines` `{ max: 500, skipBlankLines, skipComments }` | **error** |
| Function complexity ≤ 10 | `complexity` | warn |
| Nesting depth ≤ 4 | `max-depth` | warn |
| ≤ 20 statements / function | `max-statements` | warn |
| ≤ 4 params / function | `max-params` | warn |
| No duplicate logic (DRY) | `sonarjs/no-identical-functions`, `no-identical-expressions`, `no-duplicated-branches` | warn |
| No repeated string literals (≥5×) | `sonarjs/no-duplicate-string` `{ threshold: 5 }` | warn |
| `const`/`let` only, no `var` | `no-var`, `prefer-const` | error |
| No duplicate imports | `no-duplicate-imports` | error |
| Unused vars (prefix `_` to allow) | `@typescript-eslint/no-unused-vars` | error |
| Escape entities in JSX | `react/no-unescaped-entities` | error |
| Only `console.warn` / `console.error` | `no-console` `{ allow: ["warn","error"] }` | warn |
| Hooks at top level, unconditional, stable order | `react-hooks/rules-of-hooks` (via next preset) | **error** |

Note: `react-hooks/exhaustive-deps` and `react-hooks/set-state-in-effect` are intentionally **off** — don't add deps comments to satisfy a rule that isn't running.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Disabling react-hooks/exhaustive-deps risks stale closures and missing dependencies.

Turning off exhaustive-deps removes the safety net that catches stale closures, infinite loops, and missing effect dependencies. These bugs are subtle, hard to debug, and commonly slip through code review. The rule exists specifically to prevent a class of runtime bugs that break user experience.

Consider re-enabling exhaustive-deps at warn level so developers see the signals without blocking the build.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.claude/rules/code-quality.md at line 31, Disabling the
react-hooks/exhaustive-deps rule risks stale closures and missing dependencies;
re-enable react-hooks/exhaustive-deps at "warn" in your ESLint config (set
"react-hooks/exhaustive-deps": "warn") and ensure
react-hooks/set-state-in-effect remains enabled or set to "warn" as well, then
update the documentation line that currently states they are "intentionally off"
to reflect the new warning-level configuration so developers see signals without
failing the build.


## SRP — Single Responsibility

Each component / hook / function / module does one thing. If a component fetches data **and** holds business logic **and** renders → split: data fetching → a hook (`app/hooks/`), business logic → a util (`app/lib/utils/`), keep the component presentational. The `complexity`/`max-depth`/`max-statements` warnings are the signal you've combined too much.

## No inline SVGs — icons live in the icons folder

Never inline an `<svg>` in feature code, pages, or components. Every icon is a hand-authored React component under `app/components/icons/` (in the matching domain subfolder: `common/`, `document/`, `evaluations/`, `guardrails/`, `prompt-editor/`, `sidebar/`), exported from `app/components/icons/index.tsx`, and imported from that barrel:

```tsx
import { CopyIcon, CheckIcon } from "@/app/components/icons";
```

Before authoring a new icon, check the existing ones — `common/` already covers arrows, check, chevron, copy, download, eye, gear, error-circle, etc. New icon → new `XxxIcon.tsx` in the right subfolder + barrel export, matching the existing shape:

```tsx
interface IconProps {
className?: string;
style?: React.CSSProperties;
}

export default function CopyIcon({ className, style }: IconProps) {
return (
<svg
className={className}
fill="none"
viewBox="0 0 24 24"
stroke="currentColor"
strokeWidth={2}
style={style}
>
{/* paths */}
</svg>
);
}
```
Comment on lines +48 to +67

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Icon component pattern is missing accessibility attributes.

The example CopyIcon component lacks aria-label, role, or <title> child for screen readers. Icons without accessible text are invisible to assistive technology, blocking users who rely on screen readers.

♿ Proposed fix to add accessibility support
 interface IconProps {
   className?: string;
   style?: React.CSSProperties;
+  "aria-label"?: string;
 }

-export default function CopyIcon({ className, style }: IconProps) {
+export default function CopyIcon({ className, style, "aria-label": ariaLabel }: IconProps) {
   return (
     <svg
       className={className}
       fill="none"
       viewBox="0 0 24 24"
       stroke="currentColor"
       strokeWidth={2}
       style={style}
+      aria-label={ariaLabel}
+      role="img"
     >
+      {ariaLabel && <title>{ariaLabel}</title>}
       {/* paths */}
     </svg>
   );
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
interface IconProps {
className?: string;
style?: React.CSSProperties;
}
export default function CopyIcon({ className, style }: IconProps) {
return (
<svg
className={className}
fill="none"
viewBox="0 0 24 24"
stroke="currentColor"
strokeWidth={2}
style={style}
>
{/* paths */}
</svg>
);
}
```
interface IconProps {
className?: string;
style?: React.CSSProperties;
"aria-label"?: string;
}
export default function CopyIcon({ className, style, "aria-label": ariaLabel }: IconProps) {
return (
<svg
className={className}
fill="none"
viewBox="0 0 24 24"
stroke="currentColor"
strokeWidth={2}
style={style}
aria-label={ariaLabel}
role="img"
>
{ariaLabel && <title>{ariaLabel}</title>}
{/* paths */}
</svg>
);
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.claude/rules/code-quality.md around lines 48 - 67, The CopyIcon component
is missing accessibility attributes for screen readers; update the IconProps and
CopyIcon to accept an optional title (string) and/or ariaLabel prop and spread
remaining SVG props, then render a <title> element inside the SVG when title is
provided and set role="img" plus aria-label (or aria-labelledby referencing the
title id) on the <svg> element so assistive tech can describe the icon; update
function signature (CopyIcon) to handle these props and ensure existing
className/style handling remains unchanged.


`stroke="currentColor"` so the icon inherits text color from Tailwind classes.

## DRY — Don't Repeat Yourself

Search before writing. Same pattern in 2+ places → extract to a shared helper/hook/component in `app/lib/` or `app/components/`. Repeated strings → constants in `app/lib/constants.ts`.

## Splitting a file approaching 500 LOC

Pick the natural seam:

- Component too big → extract sub-components into sibling files; move state logic into a hook.
- Hook too big → move pure/network logic into `app/lib/` fetchers/utils; keep the hook as orchestration (pattern: `useConfigs` ↔ `configFetchers`).
- Types crowding a file → move shapes into `app/lib/types/<domain>.ts`.

## Comments

- JSDoc block on non-trivial files and exported functions: purpose, usage, and non-obvious cost/behaviour notes (see `app/lib/configFetchers.ts`, `app/hooks/useInfiniteScroll.ts`).
- Sparse inline comments only for non-obvious logic / workarounds. Target ~5–10% density — explain **why**, not **what**.

## Formatting (Prettier — `.prettierrc`)

2-space indent · double quotes · semicolons · trailing commas (`all`) · `printWidth: 80` · `arrowParens: always` · LF line endings. Run `npm run format`. Husky + lint-staged run prettier + `eslint --fix` on staged `*.{js,ts,jsx,tsx,json,md,css}` at commit — match this up front to avoid churn.

## TypeScript & imports

- Strict mode is **off** in `tsconfig.json` (`"strict": false`), path alias `@/* → ./*`. Still write fully-typed code (explicit prop/return interfaces) — don't lean on the relaxed compiler to skip types.
- Imports: always the `@/` alias (e.g. `@/app/lib/apiClient`), never deep relative paths.

## Before "done"

```bash
npm run lint && npm run build
```

Both must pass. Fix all errors and address warnings introduced by your change.
80 changes: 80 additions & 0 deletions .claude/rules/components.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
---
paths:
- "app/components/**"
- "app/(main)/**/*.tsx"
- "app/(auth)/**/*.tsx"
---

# Components

App components live in `app/components/` (shared UI primitives in `app/components/ui/`, icons in `app/components/icons/`). Route-level pages live under `app/(auth)/` and `app/(main)/`.

## Skeleton

```tsx
"use client"; // only if the component uses state, effects, refs, or browser APIs

import { ReactNode, useState } from "react";

interface PageHeaderProps {
title?: string;
subtitle?: string;
children?: ReactNode;
actions?: ReactNode;
hidden?: boolean;
}

export default function PageHeader({
title,
subtitle,
children,
actions,
hidden = false,
}: PageHeaderProps) {
// ...
}
```

Reference: `app/components/PageHeader.tsx`, `app/components/Sidebar.tsx`, `app/components/ui/Button.tsx`.

## Rules

- **File name**: PascalCase, `.tsx` (e.g. `PageHeader.tsx`).
- **Export**: `export default function ComponentName(...)`. Function declaration, not `const X = () => {}`.
- **`"use client"`**: top of file, only when the component needs client-side React (state/effects/refs/handlers/browser APIs). Pure presentational server components omit it.
- **Props typing**: an `interface ComponentNameProps` immediately above the component. Optional props use `?`; defaults applied in the destructure (`hidden = false`).
- **Children/slots**: typed as `ReactNode` (e.g. `children`, `actions`).
- **HTML passthrough** (for primitives): extend the native attributes, e.g. `interface ButtonProps extends ButtonHTMLAttributes<HTMLButtonElement>`.
- **Imports**: use the `@/` alias for cross-folder imports; React imports first.

## Variant styling pattern (UI primitives)

Map variant/size to class strings via a typed `Record`, then compose with a template literal. No `cn()`/`clsx()` (see the styling rule).

```tsx
type ButtonVariant = "primary" | "outline" | "ghost";

const variantStyles: Record<ButtonVariant, { base: string; disabled: string }> =
{
primary: {
base: "bg-accent-primary text-white hover:bg-accent-hover",
disabled: "bg-neutral-200 text-text-secondary cursor-not-allowed",
},
outline: {
base: "bg-white text-text-primary border border-border hover:bg-neutral-50",
disabled: "border-border cursor-not-allowed opacity-50",
},
ghost: {
base: "bg-transparent text-text-secondary hover:bg-neutral-100",
disabled: "opacity-50",
},
};
```

## Reuse before creating

Check `app/components/ui/` for primitives (`Button`, `Modal`, `Field`, `MultiSelect`, `Select`, `RadioGroup`, `Toast`, `Loader`, `InfoTooltip`, `CodeBlock`, `ErrorModal`, `Tag`, `TabNavigation`, ...), `app/components/` for higher-level shared pieces (`PageHeader`, `Sidebar`, `GatePopover`, `ConfigCard`, and the `*Skeleton` loaders), and `app/components/icons/` first. Prefer composing existing primitives. New icons are hand-authored React components in `app/components/icons/` (domain subfolders, exported from its `index.tsx`) — never inline an SVG in feature code.

## Splitting

If a component file nears 500 LOC, extract sub-components into sibling files, move logic into a hook (`app/hooks/`), and pull types into `app/lib/types/`. See the code-quality rule.
94 changes: 94 additions & 0 deletions .claude/rules/data-fetching.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
---
paths:
- "app/api/**"
- "app/lib/apiClient.ts"
- "app/lib/guardrailsClient.ts"
- "app/lib/configFetchers.ts"
---

# API routes & data fetching (BFF layer)

The app uses a **BFF layer**: Next.js route handlers in `app/api/` proxy to the backend (`BACKEND_URL`, default `http://localhost:8000`). The client never calls the backend directly — it calls `/api/...`.

## Server-side: route handlers (`app/api/.../route.ts`)

Route handlers are **thin proxies**. Use `apiClient(request, endpoint, options)` (or `guardrailsClient` for guardrails). It relays `X-API-KEY` + `Cookie` automatically and returns `{ status, data, headers }`.

```ts
import { NextResponse } from "next/server";
import { apiClient } from "@/app/lib/apiClient";

export async function GET(request: Request) {
try {
const { searchParams } = new URL(request.url);
const queryString = searchParams.toString();
const endpoint = `/api/v1/configs${queryString ? `?${queryString}` : ""}`;
const { status, data } = await apiClient(request, endpoint);
return NextResponse.json(data, { status });
} catch (error) {
return NextResponse.json(
{
success: false,
error: error instanceof Error ? error.message : String(error),
data: null,
},
{ status: 500 },
);
}
}

export async function POST(request: Request) {
try {
const body = await request.json();
const { status, data } = await apiClient(request, "/api/v1/configs", {
method: "POST",
body: JSON.stringify(body),
});
return NextResponse.json(data, { status });
} catch (error) {
return NextResponse.json(
{
error: "Failed to forward request",
details: error instanceof Error ? error.message : String(error),
},
{ status: 500 },
);
}
}
```

Reference: `app/api/configs/route.ts`, `app/lib/apiClient.ts`, `app/lib/guardrailsClient.ts`.

### Rules

- One `route.ts` per endpoint, exporting named HTTP methods (`GET`, `POST`, ...).
- Always wrap in `try/catch`; on error return `NextResponse.json({...error}, { status: 500 })`. Error message via `error instanceof Error ? error.message : String(error)`.
- Forward backend `status` through (`NextResponse.json(data, { status })`) — don't hardcode 200 or swallow non-2xx.
- Build query strings from `new URL(request.url).searchParams`.
- Endpoint paths preserved verbatim — trailing-slash handling is a contract with the backend; don't drift.
- For guardrails endpoints use `guardrailsClient` (env-token auth) / `guardrailsUserClient` (user-key auth).
- Static/health routes set `export const dynamic = "force-dynamic"` and cache headers as needed.
- Don't log request bodies, tokens, or cookies — `console.error("Proxy error:", error)` is the convention.

## Client-side fetching

- Use `apiFetch<T>(url, apiKey, options)` from `app/lib/apiClient.ts` for browser calls. It handles 401 token refresh, dispatches `AUTH_EXPIRED_EVENT` on refresh failure (AuthContext logs out), and throws with a message from `error` / `message` / `detail` (via the internal `extractErrorMessage`). Raw `fetch("/api/...")` in a component bypasses this and is a bug.
- Overload signatures express empty-body handling: a variant returns `Promise<T | null>` with `{ acceptEmpty: true }`.
- File uploads with progress: `uploadWithProgress<T>(url, apiKey, body, onProgress)` returns `{ promise, abort }`.
- **Error extraction**: follow `extractErrorMessage(body, fallback)` → reads `body.error || body.message || body.detail`. Don't reinvent the parser.

## Fetchers are pure (no React)

Network/data logic lives in plain functions in `app/lib/` (e.g. `app/lib/configFetchers.ts`), **with no hooks or UI state**. Hooks (`app/hooks/`) call these fetchers and own the React state. Keep the boundary clean — it's what keeps both layers under 500 LOC and testable.

```ts
/**
* API fetch helpers for Config Management.
* Contains all network logic — no React, no UI state.
*/
export async function fetchAllConfigs(apiKey: string, pageSize?: number): Promise<FetchResult> { ... }
```

## SWR

SWR (2.3.6) is available and used **selectively** where its caching/revalidation helps (cached, revalidated reads — lists, dashboards). Default to native fetch + the fetchers/hooks above; one-shot mutations stay on `apiFetch`.
73 changes: 73 additions & 0 deletions .claude/rules/hooks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
---
paths:
- "app/hooks/**"
---

# Hooks

Custom hooks live in `app/hooks/` and are re-exported from `app/hooks/index.ts` (barrel) for clean imports.

## Skeleton

```ts
"use client";

import { useState } from "react";
import type { SavedConfig } from "@/app/lib/types/configs";

export interface UseConfigsResult {
configs: SavedConfig[];
isLoading: boolean;
error: string | null;
refetch: (force?: boolean) => Promise<void>;
}

export function useConfigs(options?: { pageSize?: number }): UseConfigsResult {
const [configs, setConfigs] = useState<SavedConfig[]>([]);
// ...
return { configs, isLoading, error, refetch };
}
```

Reference: `app/hooks/useConfigs.ts`, `app/hooks/usePaginatedList.ts`, `app/hooks/useInfiniteScroll.ts`, `app/hooks/useToast.ts`, `app/hooks/index.ts`.

## Rules

- **Name**: `use[Feature]` camelCase. File name matches the hook (`useConfigs.ts`).
- **`"use client"`** at the top.
- **Explicit return interface**: `export interface UseXxxResult { ... }`; the hook's signature is `): UseXxxResult`. Return a named object, not a tuple (except trivial cases).
- **Generics for reusable hooks**: `usePaginatedList<T>(...)`, `useInfiniteScroll<T>(...)` — parameterize the item type rather than hardcoding.
- **Refs for values that persist across renders without re-rendering**: `usePaginatedList` uses `skipRef`, `loadingMoreRef`. Use `useRef` for these, `useState` for render-affecting state.
- **No UI**: hooks hold state/effects only — no JSX. Network logic is delegated to pure fetchers in `app/lib/` (see the data-fetching rule); the hook orchestrates them.
- **Context-consumer hooks** throw if the provider is missing (e.g. `useToast`, `useAuth`).
- **Barrel**: add new hooks to `app/hooks/index.ts`.

## Hook placement & ordering (Rules of Hooks)

Every hook call — `useState`, `useRef`, `useEffect`, `use(params)`, and custom `useX()` — must sit at the **top level of the component/hook**, called unconditionally and in the same order on every render. `react-hooks/rules-of-hooks` is **on** and errors the build, but it only catches the hard violations — the ordering discipline below is on you.

- **Group all hook calls at the very top**, before deriving locals or any branching logic — context/data hooks and `useState`/`useRef` come first, then derived values:
```tsx
"use client";
export default function EvaluationsPage() {
const { activeKey } = useAuth(); // context/data hooks first
const toast = useToast();
const [items, setItems] = useState<string[]>([]);
const [error, setError] = useState<string | null>(null);
// only now derive + branch — every hook has already been called
const hasItems = items.length > 0;
// ...
}
```
For a Next 16 dynamic route, `params` is async (`params: Promise<{ id: string }>`) and unwrapped with React's `use(params)` — that's itself a hook, so it sits in the top block with the others, never below a `useState`.
- **No `return` (early-return / guard / `notFound()`) may appear before or between hook calls.** Guard _inside_ event handlers and effects instead — `if (!file) return;` at the top of `handleUpload` is fine; the same guard above a `useState` is a bug.
- **No hook inside a condition, loop, `&&`, ternary, or nested function.** Compute the condition into a variable and branch in the returned JSX, not around the hook.
- Order convention within the top block: context/data hooks (`useRouter`, `useAuth`, `useToast`) → `useState` → `useRef` → derived `useMemo`/`useCallback` → `useEffect`. See a real client page such as `app/(main)/evaluations/page.tsx`.

## Reuse before creating

`usePaginatedList`, `useInfiniteScroll`, `useConfigs`, `useToast`, `useCollections`, `useConfigPersistence` already cover common needs. Check `app/hooks/` (and `index.ts`) before writing a new one. For data fetching with pagination/search, reuse `usePaginatedList` rather than re-implementing skip/limit logic.

## Splitting

A hook near 500 LOC → extract pure helpers into `app/lib/` fetchers (`configFetchers.ts` pattern) or `app/lib/utils/<domain>/`, keep the hook as the orchestration layer.
Loading
Loading