mirror of
https://github.com/supabase/supabase.git
synced 2026-10-05 09:25:06 +03:00
chore: make agent instructions agent-agnostic (#49941)
Makes the repo's AI-agent setup tool-agnostic: instructions live in
`AGENTS.md` files, skills live in `.agents/skills/`, and Claude Code,
Codex, Cursor, and Copilot all read the same sources. Also sweeps the
skills for stale and duplicated content while everything was being
moved.
**Changed:**
- Every `CLAUDE.md` (root, `apps/studio`, `apps/docs`, `apps/kb`) is now
a one-line `@AGENTS.md` import; the content moved verbatim into an
`AGENTS.md` beside it. The root one moved from `.claude/CLAUDE.md` to
the repo root for consistency.
- All skills now live in `.agents/skills/`; `.claude/skills` is a single
symlink to it (replacing the old mix of real dirs and per-skill
symlinks). Path references in `.coderabbit.yaml`, code comments, and
docs updated to match.
- `.github/copilot-instructions.md` keeps only the review policy and
points at `AGENTS.md` + `.agents/skills/`. Copilot code review reads
those natively now, so the per-topic
`.github/instructions/*.instructions.md` files were duplicates of the
skills.
- Stale skill content fixed: `studio-queries` imported a toast library
Studio doesn't use, `telemetry-standards` and `studio-testing` used
import paths that don't resolve, `safe-sql-execution` cited a boundary
test that doesn't exist, the ask-the-docs references described an
`AiPrompt` mechanism that was replaced by the ID-keyed registry, plus a
handful of wrong paths, a self-contradicting `waitForTimeout` rule, an
invalid Playwright signature, and a ConfigCat flag described as PostHog.
- `studio-error-handling` now explains when to use `AlertError` (the
default) vs `ErrorMatcher`.
**Added:**
- `apps/docs/AGENTS.md` (docs test requirements, from the old Cursor
rule)
- `studio-shortcuts` skill (from the old Copilot instruction file,
verified against the current registry)
- `ask-the-docs/reference/graphql-endpoint.md` and
`search-embeddings.md` (from the old Cursor rules, with the missing
resolver/registration/codegen steps filled in)
- Feature-flag measurement section in `telemetry-standards`
**Removed:**
- `.cursor/` (rules folded in as above; skill symlinks no longer needed)
and `.cursorignore`
- `.github/instructions/` (8 files)
- `vercel-composition-patterns/AGENTS.md` – a 946-line verbatim
concatenation of its own `rules/` directory, and a nested `AGENTS.md`
that agents could auto-load as repo instructions
- `edit-the-docs/reference/structure-and-flow.md` – word-for-word copy
of the skill's own Phase 2 text
## To test
- `readlink .claude/skills` → `../.agents/skills`, and `ls
.claude/skills/copywriting/SKILL.md` resolves
- Open a Claude Code session at the repo root and in `apps/studio` – the
imported `AGENTS.md` content should load as before
- `git diff master --stat -M` shows the skill moves as 100% renames
(content unchanged except the listed fixes)
- Spot-check a fixed claim, e.g. `import { toast } from 'sonner'` in
`studio-queries`, or the `logs.all` ESLint rule cited in
`clickhouse-logs-queries/references/codebase-integration.md`
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **Documentation**
- Expanded guidance for documentation workflows, GraphQL resources,
search, ClickHouse logs, React forms, Studio testing, shortcuts,
telemetry, accessibility, copywriting, and composition patterns.
- Clarified local testing, linting, build workflows, error handling, and
AI coding agent usage.
- Added contributor guidance for the knowledge base, documentation, and
Studio areas.
- **Chores**
- Consolidated agent instructions and skill references.
- Removed obsolete editor-specific guidance, duplicate links, and
superseded documentation.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Alaister Young <10985857+alaister@users.noreply.github.com>
This commit is contained in:
Alaister YoungandAlaister Young authored and GitHub committed
1 parent
6181e27b93
commit
f125126aec
75 files changed
+656
-1805
No files matched your search
@@ -19,7 +19,7 @@ Our CI pipeline already validates the following. **Never comment on these topics
|
||||
- **Missing tests for trivial changes** — Handled by topic-specific test instructions
|
||||
- **Import ordering or grouping** — Handled by linter
|
||||
- **Naming style preferences** (camelCase vs snake_case debates) — Follow existing file conventions
|
||||
- **Accessibility attributes on shadcn/Radix UI components** — See `studio-shadcn-components.instructions.md` for details
|
||||
- **Accessibility attributes on shadcn/Radix UI components** — handled by the primitives; see [shadcn/Radix accessibility](#shadcnradix-accessibility) below
|
||||
|
||||
### What TO Comment On (Priority Order)
|
||||
|
||||
@@ -47,15 +47,41 @@ This is a TypeScript/Next.js/React monorepo:
|
||||
|
||||
## Topic-Specific Guidelines
|
||||
|
||||
Path-specific rules in `.github/instructions/`:
|
||||
Coding conventions are not duplicated here. Read them from the shared agent instruction files, which apply to every AI tool working in this repo:
|
||||
|
||||
- **Telemetry**: `studio-telemetry.instructions.md` — event naming, property conventions, feature flag measurement
|
||||
- **Testing**: `studio-testing.instructions.md` — test strategy, extraction patterns, coverage expectations
|
||||
- **Error Handling**: `studio-error-handling.instructions.md` — error classification, `ErrorMatcher` usage
|
||||
- **E2E Tests**: `studio-e2e-tests.instructions.md` — selector priority, anti-patterns (`waitForTimeout`, `force: true`)
|
||||
- **Composition Patterns**: `studio-composition-patterns.instructions.md` — avoid boolean props, use compound components
|
||||
- **UI Copy**: `studio-copy.instructions.md` → `apps/design-system/content/docs/copywriting.mdx`
|
||||
- **shadcn/Radix Components**: `studio-shadcn-components.instructions.md` — accessibility handled by primitives, do not flag
|
||||
- **Keyboard Shortcuts**: `studio-shortcuts.instructions.md` — shortcut registry pattern, search-input escape handler, when to flag missing coverage
|
||||
- `AGENTS.md` (repo root) — monorepo structure, commands, CI, conventions
|
||||
- `apps/studio/AGENTS.md` — Studio-specific rules and the task → skill map
|
||||
- `.agents/skills/*/SKILL.md` — the source of truth per topic. Most relevant to review: `studio-testing`, `studio-mock-api-tests`, `studio-e2e-tests`, `studio-error-handling`, `studio-queries`, `studio-ui-patterns`, `react-hook-form`, `telemetry-standards`, `safe-sql-execution`, `vercel-composition-patterns`, `studio-shortcuts`, `copywriting`
|
||||
|
||||
These files are scoped to `apps/studio/` and applied automatically during reviews.
|
||||
When a skill says to flag something, treat it as **advisory** here — the confidence threshold and comment style above still apply.
|
||||
|
||||
## shadcn/Radix accessibility
|
||||
|
||||
Studio uses **shadcn/ui** components built on **Radix UI** primitives (from `packages/ui/`), which provide ARIA roles, keyboard navigation, focus management, and screen-reader support automatically. **Do not flag missing accessibility attributes that the underlying primitive already handles.**
|
||||
|
||||
| Component | What Radix handles |
|
||||
| ----------------------------- | -------------------------------------------------------------------------------- |
|
||||
| `Dialog`, `AlertDialog` | `role="dialog"`, `aria-modal`, focus trapping, ESC to close |
|
||||
| `DropdownMenu`, `ContextMenu` | `role="menu"` / `role="menuitem"`, arrow key navigation |
|
||||
| `Select` | `role="combobox"`, `aria-expanded`, keyboard selection |
|
||||
| `Tabs` | `role="tablist"` / `role="tab"` / `role="tabpanel"`, `aria-selected`, arrow keys |
|
||||
| `Checkbox` | `role="checkbox"`, `aria-checked`, Space to toggle |
|
||||
| `RadioGroup` | `role="radio"`, `aria-checked`, arrow key navigation |
|
||||
| `Switch` | `role="switch"`, `aria-checked`, keyboard toggle |
|
||||
| `Tooltip` | Trigger/content association, show/hide timing |
|
||||
| `Accordion`, `Collapsible` | `aria-expanded`, Enter/Space to toggle |
|
||||
| `Popover`, `HoverCard` | Focus management, dismiss on ESC |
|
||||
| `Slider` | `role="slider"`, `aria-valuemin/max/now`, arrow keys |
|
||||
| `Toggle`, `ToggleGroup` | `aria-pressed`, keyboard support |
|
||||
| `ScrollArea` | Accessible scrollbar replacement |
|
||||
| `NavigationMenu` | `role="navigation"`, keyboard navigation |
|
||||
|
||||
Specifically, never flag: missing `role` on Radix-based components; missing `aria-modal` on `Dialog`/`AlertDialog`; missing `aria-expanded` on `Accordion`, `Collapsible`, `Select`, or `DropdownMenu` triggers; missing `aria-selected` on `Tabs`; missing `aria-checked` on `Checkbox`, `RadioGroup`, or `Switch`; missing keyboard handlers on interactive Radix components; missing focus management in dialogs; missing `aria-label` on `DialogClose` (it renders `<span className="sr-only">Close</span>`).
|
||||
|
||||
**Do** flag accessibility issues on:
|
||||
|
||||
1. **Custom interactive elements** not using Radix primitives (e.g. a `<div onClick>` that should be a `<button>`)
|
||||
2. **Icon-only buttons** missing an accessible label — `<Button>` alone does not add one; use `aria-label` or `<span className="sr-only">`
|
||||
3. **Missing `Label` association** — form inputs should be paired with `<Label htmlFor="...">` or wrapped in a `<Field>` component
|
||||
4. **Images missing `alt` text**
|
||||
5. **Color-only state indicators**
|
||||
@@ -1,93 +0,0 @@
|
||||
---
|
||||
applyTo: "apps/studio/**"
|
||||
---
|
||||
|
||||
# React Composition Patterns Review Rules
|
||||
|
||||
All comments are **advisory**.
|
||||
|
||||
## Core Principle
|
||||
|
||||
Avoid boolean prop proliferation. Use composition (compound components, explicit variants, children) instead of boolean flags to customize behavior.
|
||||
|
||||
## When to Flag
|
||||
|
||||
### 1. Boolean Prop Proliferation (HIGH)
|
||||
|
||||
Flag components accumulating boolean props like `isThread`, `isEditing`, `showAttachments`. Each boolean doubles the state space.
|
||||
|
||||
```tsx
|
||||
// BAD — unclear intent, combinatorial explosion
|
||||
<Composer isThread isDMThread isEditing isForwarding={false} />
|
||||
|
||||
// GOOD — self-documenting variants
|
||||
<ThreadComposer channelId="abc" />
|
||||
<EditMessageComposer messageId="xyz" />
|
||||
```
|
||||
|
||||
### 2. Render Props Instead of Children (MEDIUM)
|
||||
|
||||
Flag `renderX` callback props when `children` composition would work.
|
||||
|
||||
```tsx
|
||||
// BAD — render prop for structure
|
||||
<Composer renderFooter={() => <F />} />
|
||||
|
||||
// GOOD — compound component
|
||||
<Composer.Footer>
|
||||
<Composer.Formatting />
|
||||
<Composer.Emojis />
|
||||
</Composer.Footer>
|
||||
```
|
||||
|
||||
### 3. UI Coupled to State Implementation (MEDIUM)
|
||||
|
||||
Flag UI components calling specific state hooks like `useGlobalChannelState()` directly. The provider should own the state implementation; UI should only use a generic context interface.
|
||||
|
||||
```tsx
|
||||
// BAD — UI knows HOW state is managed
|
||||
const state = useGlobalChannelState(channelId)
|
||||
|
||||
// GOOD — provider owns implementation, UI uses context
|
||||
<ChannelProvider channelId={channelId}>
|
||||
<Composer /> {/* reads from context */}
|
||||
</ChannelProvider>
|
||||
```
|
||||
|
||||
### 4. State Trapped in Child Components (MEDIUM)
|
||||
|
||||
Flag state that siblings or dialogs need but can't access without prop drilling or refs. Lift it into a provider.
|
||||
|
||||
```tsx
|
||||
// BAD — sibling can't access state
|
||||
function ForwardComposer() {
|
||||
const [state, setState] = useState(init)
|
||||
}
|
||||
// ForwardButton is a sibling and can't reach state
|
||||
|
||||
// GOOD — provider at shared ancestor
|
||||
<ForwardMessageProvider>
|
||||
<Composer /> {/* can access state */}
|
||||
<ForwardButton /> {/* can also access state */}
|
||||
</ForwardMessageProvider>
|
||||
```
|
||||
|
||||
### 5. React 19 API Updates
|
||||
|
||||
Flag `forwardRef` and `useContext` in new code — use `ref` as a regular prop and `use()` instead.
|
||||
|
||||
```tsx
|
||||
// BAD
|
||||
const Input = forwardRef((props, ref) => <input ref={ref} />)
|
||||
const value = useContext(MyContext)
|
||||
|
||||
// GOOD
|
||||
function Input({ ref, ...props }) { return <input ref={ref} /> }
|
||||
const value = use(MyContext)
|
||||
```
|
||||
|
||||
## Key Principle
|
||||
|
||||
Lift state → Compose UI → Inject via generic context → No boolean prop proliferation.
|
||||
|
||||
Canonical standard: `.claude/skills/vercel-composition-patterns/SKILL.md`
|
||||
@@ -1,13 +0,0 @@
|
||||
---
|
||||
applyTo: 'apps/studio/**'
|
||||
---
|
||||
|
||||
# Studio UI Copy
|
||||
|
||||
All comments are **advisory**.
|
||||
|
||||
**Source of truth:** `apps/design-system/content/docs/copywriting.mdx` — read it before writing or reviewing user-facing Studio strings.
|
||||
|
||||
## Agent checklist (not in the design doc)
|
||||
|
||||
- When changing visible copy, grep `e2e/studio/` and `.github/instructions/` for the old string.
|
||||
@@ -1,86 +0,0 @@
|
||||
---
|
||||
applyTo: 'e2e/studio/**,apps/studio/**'
|
||||
---
|
||||
|
||||
# Studio E2E Test Review Rules
|
||||
|
||||
All comments are **advisory**.
|
||||
|
||||
## Selector Priority (best to worst)
|
||||
|
||||
1. **`getByRole` with accessible name** — most robust, tests accessibility
|
||||
|
||||
```typescript
|
||||
page.getByRole('button', { name: 'Save' })
|
||||
```
|
||||
|
||||
2. **`getByTestId`** — stable, explicit test hooks
|
||||
|
||||
```typescript
|
||||
page.getByTestId('table-editor-side-panel')
|
||||
```
|
||||
|
||||
3. **`getByText` with exact match** — good for unique text
|
||||
|
||||
```typescript
|
||||
page.getByText('Data API access', { exact: true })
|
||||
```
|
||||
|
||||
4. **`locator` with CSS** — use sparingly, more fragile
|
||||
```typescript
|
||||
page.locator('[data-state="open"]')
|
||||
```
|
||||
|
||||
## Patterns to Flag
|
||||
|
||||
- **XPath selectors** — fragile to DOM changes
|
||||
|
||||
```typescript
|
||||
// BAD
|
||||
locator('xpath=ancestor::div[contains(@class, "space-y")]')
|
||||
```
|
||||
|
||||
- **Parent traversal with `locator('..')`** — breaks when structure changes
|
||||
|
||||
```typescript
|
||||
// BAD
|
||||
element.locator('..').getByRole('button')
|
||||
```
|
||||
|
||||
- **`waitForTimeout`** — never use; wait for something specific instead
|
||||
|
||||
```typescript
|
||||
// BAD
|
||||
await page.waitForTimeout(1000)
|
||||
|
||||
// GOOD — wait for UI element
|
||||
await expect(page.getByText('Success')).toBeVisible()
|
||||
|
||||
// GOOD — wait for API response
|
||||
const apiPromise = waitForApiResponse(page, 'pg-meta', ref, 'query?key=table-create')
|
||||
await saveButton.click()
|
||||
await apiPromise
|
||||
```
|
||||
|
||||
- **`force: true` on clicks** — make elements visible first instead
|
||||
|
||||
```typescript
|
||||
// BAD
|
||||
await menuButton.click({ force: true })
|
||||
|
||||
// GOOD — hover to reveal, then click
|
||||
await tableRow.hover()
|
||||
await expect(menuButton).toBeVisible()
|
||||
await menuButton.click()
|
||||
```
|
||||
|
||||
- **Broad `filter({ hasText })` on generic elements** — may match multiple elements; scope to specific containers instead
|
||||
|
||||
## Good Practices to Encourage
|
||||
|
||||
- Scope selectors to containers: `page.getByTestId('side-panel').getByRole('switch')`
|
||||
- Add `aria-label` to icon-only buttons in source code for better test selectors
|
||||
- Use `test.describe.configure({ mode: 'serial' })` for tests sharing database state
|
||||
- Add messages to expects: `await expect(locator, 'why').toBeVisible({ timeout: 30000 })`
|
||||
|
||||
Canonical standard: `.claude/skills/studio-e2e-tests/SKILL.md`
|
||||
@@ -1,43 +0,0 @@
|
||||
---
|
||||
applyTo: "apps/studio/**"
|
||||
---
|
||||
|
||||
# Studio Error Handling Review Rules
|
||||
|
||||
All comments are **advisory**.
|
||||
|
||||
## Architecture
|
||||
|
||||
Errors flow: `handleError()` → throws typed subclass → React Query catches → `ErrorMatcher` reads `errorType` → renders troubleshooting. The component does an O(1) lookup — it never does regex matching.
|
||||
|
||||
## When to Flag
|
||||
|
||||
- PR passes `error.message` instead of the full `error` object to `ErrorMatcher` — the class type is lost
|
||||
- PR puts regex patterns in `error-mappings.tsx` — they belong in `data/error-patterns.ts`
|
||||
- PR uses `Object.assign` to stamp `errorType` on an error — should throw a proper subclass instead
|
||||
- PR passes a raw URL string for support links — should use `supportFormParams={{ projectRef }}`
|
||||
- PR puts the page title inside the error mapping — it belongs on the `<ErrorMatcher>` caller
|
||||
- PR adds callback props (`onDebugWithAI`, `onRestartProject`) to troubleshooting components — use hooks inside them instead
|
||||
|
||||
## Correct Usage
|
||||
|
||||
```tsx
|
||||
{isError && (
|
||||
<ErrorMatcher
|
||||
title="Failed to load tables"
|
||||
error={error}
|
||||
supportFormParams={{ projectRef }}
|
||||
/>
|
||||
)}
|
||||
```
|
||||
|
||||
## Key Files
|
||||
|
||||
| File | Purpose |
|
||||
|------|---------|
|
||||
| `data/error-patterns.ts` | `{ pattern, ErrorClass }` array — regex lives here |
|
||||
| `types/api-errors.ts` | Error classes, `KnownErrorType` union |
|
||||
| `ErrorMatcher.tsx` | Reads `errorType`, looks up mapping, renders |
|
||||
| `error-mappings.tsx` | `Record<KnownErrorType, { id, Troubleshooting }>` |
|
||||
|
||||
Canonical standard: `.claude/skills/studio-error-handling/SKILL.md`
|
||||
@@ -1,53 +0,0 @@
|
||||
---
|
||||
applyTo: "apps/studio/**"
|
||||
---
|
||||
|
||||
# shadcn/Radix UI Component Review Rules
|
||||
|
||||
All comments are **advisory**.
|
||||
|
||||
## Core Principle
|
||||
|
||||
This project uses **shadcn/ui** components built on **Radix UI** primitives (from `packages/ui/`). These components provide comprehensive accessibility out-of-the-box. **Do not flag missing accessibility attributes that are already handled by the underlying Radix primitives.**
|
||||
|
||||
## Components with Built-In Accessibility — Do NOT Flag
|
||||
|
||||
The following components (imported from `ui`) already handle ARIA roles, keyboard navigation, focus management, and screen reader support automatically via Radix UI primitives:
|
||||
|
||||
| Component | What Radix Handles |
|
||||
|-----------|-------------------|
|
||||
| `Dialog`, `AlertDialog` | `role="dialog"`, `aria-modal`, focus trapping, ESC to close |
|
||||
| `DropdownMenu`, `ContextMenu` | `role="menu"` / `role="menuitem"`, arrow key navigation |
|
||||
| `Select` | `role="combobox"`, `aria-expanded`, keyboard selection |
|
||||
| `Tabs` | `role="tablist"` / `role="tab"` / `role="tabpanel"`, `aria-selected`, arrow keys |
|
||||
| `Checkbox` | `role="checkbox"`, `aria-checked`, Space to toggle |
|
||||
| `RadioGroup` | `role="radio"`, `aria-checked`, arrow key navigation |
|
||||
| `Switch` | `role="switch"`, `aria-checked`, keyboard toggle |
|
||||
| `Tooltip` | Trigger/content association, show/hide timing |
|
||||
| `Accordion`, `Collapsible` | `aria-expanded`, Enter/Space to toggle |
|
||||
| `Popover`, `HoverCard` | Focus management, dismiss on ESC |
|
||||
| `Slider` | `role="slider"`, `aria-valuemin/max/now`, arrow keys |
|
||||
| `Toggle`, `ToggleGroup` | `aria-pressed`, keyboard support |
|
||||
| `ScrollArea` | Accessible scrollbar replacement |
|
||||
| `NavigationMenu` | `role="navigation"`, keyboard navigation |
|
||||
|
||||
### Specifically, Never Flag These
|
||||
|
||||
- Missing `role` on `Dialog`, `AlertDialog`, `DropdownMenu`, `Select`, `Tabs`, `RadioGroup`, or other Radix-based components — roles are set by the primitive
|
||||
- Missing `aria-modal` on `Dialog` or `AlertDialog` — set automatically
|
||||
- Missing `aria-expanded` on `Accordion`, `Collapsible`, `Select`, or `DropdownMenu` triggers — managed by Radix state
|
||||
- Missing `aria-selected` on `Tabs` — managed by `TabsPrimitive`
|
||||
- Missing `aria-checked` on `Checkbox`, `RadioGroup`, or `Switch` — managed by Radix state
|
||||
- Missing keyboard event handlers (`onKeyDown`, `onKeyUp`) on interactive Radix components — keyboard support is built-in
|
||||
- Missing focus management in `Dialog` or `AlertDialog` — focus trapping is automatic
|
||||
- Missing `aria-label` on `DialogClose` or `AlertDialogCancel` — these render a visible `<span className="sr-only">Close</span>`
|
||||
|
||||
## What TO Flag
|
||||
|
||||
Only flag accessibility issues for:
|
||||
|
||||
1. **Custom interactive elements** not using Radix primitives (e.g., a `<div onClick>` that should be a `<button>`)
|
||||
2. **Icon-only buttons** missing an accessible label — `<Button>` alone does not add one; use `aria-label` or `<span className="sr-only">`
|
||||
3. **Missing `Label` association** — form inputs should be paired with `<Label htmlFor="...">` or wrapped in a `<Field>` component
|
||||
4. **Images missing `alt` text** — not handled by any component library
|
||||
5. **Color-only state indicators** — state changes should not rely solely on color
|
||||
@@ -1,56 +0,0 @@
|
||||
---
|
||||
applyTo: 'apps/studio/**'
|
||||
---
|
||||
|
||||
# Studio Shortcut Review Rules
|
||||
|
||||
All comments are **advisory**.
|
||||
|
||||
## Core Principle
|
||||
|
||||
When Studio UI changes introduce or materially alter repeated user actions, consider whether keyboard shortcut coverage should be added or updated. Shortcuts should use the shared Studio shortcut system and be discoverable from the visible UI.
|
||||
|
||||
## When to Flag
|
||||
|
||||
- PR adds a primary repeated action, toolbar action, list/table operation, or sub-page navigation without considering shortcut coverage.
|
||||
- PR adds a one-off `keydown` listener for a normal Studio action instead of using the shortcut registry and `useShortcut`.
|
||||
- PR registers a shortcut but does not expose it via `ShortcutTooltip`, `ShortcutBadge`, or command-menu badge where the action is visible.
|
||||
- PR uses `G then ...` for a non-navigation action.
|
||||
- PR adds a broad `Mod+letter` shortcut that overlaps common browser, editor, system, copy/save/search, or devtools behaviour.
|
||||
- PR adds a shortcut without checking existing registry and non-registry listeners for collisions.
|
||||
- PR adds a search/filter `<Input>` without `onKeyDown={onSearchInputEscape(...)}` — see **Search Inputs** below.
|
||||
|
||||
## Preferred Pattern
|
||||
|
||||
- Add definitions in `apps/studio/state/shortcuts/registry.ts` or `apps/studio/state/shortcuts/registry/*`.
|
||||
- Register with `useShortcut`.
|
||||
- Gate availability with `enabled`.
|
||||
- Surface visible actions with `ShortcutTooltip` or `ShortcutBadge`.
|
||||
- Prefer scoped, mnemonic sequential chords over global modifier chords.
|
||||
- Set `showInSettings: false` on contextual shortcuts (scoped to a specific page state, sheet, or panel).
|
||||
- When a shortcut group should appear in the reference sheet (`Mod+/`), add the group key to `SHORTCUT_REFERENCE_GROUP_ORDER` in `apps/studio/state/shortcuts/referenceGroups.ts` and a human label to `GROUP_LABELS` in `ShortcutsReferenceSheet.tsx`.
|
||||
- For sheet-scoped shortcuts (active only while a `<Sheet>` is open), mount `useShortcut` inside the sheet component gated by the `open` prop — see `apps/studio/components/interfaces/ConnectSheet/useConnectSheetShortcut.ts` as the canonical example.
|
||||
|
||||
## Search Inputs
|
||||
|
||||
Every `<Input>` used as a search or filter field must include the staged-Escape handler from `apps/studio/lib/keyboard.ts`:
|
||||
|
||||
```tsx
|
||||
import { onSearchInputEscape } from '@/lib/keyboard'
|
||||
|
||||
;<Input
|
||||
value={query}
|
||||
onChange={(e) => setQuery(e.target.value)}
|
||||
onKeyDown={onSearchInputEscape(query, setQuery)}
|
||||
/>
|
||||
```
|
||||
|
||||
Behaviour:
|
||||
|
||||
- **Escape while the input has a value** → clears the value, keeps focus (so a second Escape then blurs)
|
||||
- **Escape while the input is empty** → blurs the input
|
||||
- Stops propagation on Escape so the keystroke does not accidentally close a parent dialog or sheet
|
||||
|
||||
When pairing with `useShortcut(LIST_PAGE_FOCUS_SEARCH, ...)` to focus a search input via keyboard, always also add `onSearchInputEscape` on the same input — focus and escape-to-blur are always a pair.
|
||||
|
||||
Canonical implementation context: `apps/studio/state/shortcuts/registry.ts`, `apps/studio/state/shortcuts/useShortcut.tsx`, and `apps/studio/components/ui/Shortcut*.tsx`
|
||||
@@ -1,60 +0,0 @@
|
||||
---
|
||||
applyTo: 'apps/studio/**,packages/common/telemetry*'
|
||||
---
|
||||
|
||||
# Studio Telemetry Review Rules
|
||||
|
||||
All comments are **advisory** — suggest, do not request changes.
|
||||
|
||||
## When to Flag Missing Telemetry
|
||||
|
||||
Use judgment — not every PR needs telemetry. But **always flag** when:
|
||||
|
||||
1. **Changes to `packages/common/telemetry-constants.ts`** — validate event naming, property conventions, and JSDoc accuracy.
|
||||
2. **PostHog feature flags without measurement.** If a PR uses `usePHFlag` or PostHog-backed hooks like `useDataApiRevokeOnCreateDefaultEnabled` to gate behavior, the flag state should be captured in a telemetry event so the rollout can be measured. Flag if the flag value isn't included in a relevant `track()` call. (Note: `useFlag` from `common` reads ConfigCat flags, not PostHog — different system, different guidance.)
|
||||
3. **Feature-flagged rollouts without outcome tracking.** If a flag gates new behavior, there should be telemetry on both the flag state _and_ how users respond to the new behavior (e.g., toggle clicks, opt-in actions).
|
||||
4. **Growth-oriented components adding user interactions without tracking** — onboarding flows, setup wizards, upgrade CTAs, A/B experiment variants.
|
||||
|
||||
When tracking is missing, comment: _"This adds a user interaction (or feature flag) that may benefit from tracking."_ Then propose an event name and `useTrack()` call.
|
||||
|
||||
## Feature Flag Telemetry Pattern
|
||||
|
||||
When capturing a PostHog flag value for telemetry, read the raw flag via `usePHFlag('flagName')` — **not** through wrapper hooks that coerce `undefined` to `false`. Use conditional spread so the property is omitted (not false) when the flag store hasn't loaded:
|
||||
|
||||
```typescript
|
||||
const flagValue = usePHFlag<boolean>('myBooleanFlag') // for boolean flags
|
||||
track('event_name', {
|
||||
...(flagValue !== undefined && { myFlagEnabled: flagValue }),
|
||||
})
|
||||
```
|
||||
|
||||
For string-valued flags (e.g., experiment variants), use `usePHFlag<string>('flagName')` instead.
|
||||
|
||||
## Event Naming
|
||||
|
||||
Format: `[object]_[verb]` in snake_case.
|
||||
|
||||
Prefer verbs already in use in `packages/common/telemetry-constants.ts`: `opened`, `clicked`, `submitted`, `created`, `removed`, `updated`, `intended`, `evaluated`, `added`, `enabled`, `disabled`, `copied`, `exposed`, `failed`, `converted`, `closed`, `completed`, `applied`, `sent`, `moved`.
|
||||
|
||||
Flag: unapproved verbs (`saved`, `viewed`, `pressed`), wrong order (`click_product_card`), wrong casing (`productCardClicked`), passive view tracking on page load (exception: `_exposed` events for A/B experiments).
|
||||
|
||||
## Event Properties
|
||||
|
||||
- **camelCase** for new events; match existing convention when extending
|
||||
- Self-explanatory names — flag generic (`label`, `value`, `name`, `data`)
|
||||
- Check `telemetry-constants.ts` for consistency with similar events
|
||||
- Never track PII
|
||||
|
||||
## Event Implementation
|
||||
|
||||
- Use `useTrack` from `lib/telemetry/track` — avoid introducing new `useSendEventMutation` usage
|
||||
- New events need a TypeScript interface in `telemetry-constants.ts` with `@group Events` and `@source` JSDoc tags (add `@page` when applicable for page-specific events), added to the `TelemetryEvent` union
|
||||
|
||||
```typescript
|
||||
import { useTrack } from 'lib/telemetry/track'
|
||||
|
||||
const track = useTrack()
|
||||
track('product_card_clicked', { productType: 'database', planTier: 'pro' })
|
||||
```
|
||||
|
||||
Canonical standards: `.claude/skills/telemetry-standards/SKILL.md`
|
||||
@@ -1,29 +0,0 @@
|
||||
---
|
||||
applyTo: "apps/studio/**"
|
||||
---
|
||||
|
||||
# Studio Testing Review Rules
|
||||
|
||||
All comments are **advisory**.
|
||||
|
||||
## Core Principle
|
||||
|
||||
Push logic out of React components into pure `.utils.ts` functions, then test those functions exhaustively. Only use component tests for complex UI interactions.
|
||||
|
||||
## When to Comment
|
||||
|
||||
- PR adds **business logic inline in a component** that could be extracted to a `ComponentName.utils.ts` file next to the component and unit tested at `tests/components/.../ComponentName.utils.test.ts`
|
||||
- PR adds a **utility function without test coverage**
|
||||
- PR uses **component tests for pure logic** that should be a unit test on a pure function
|
||||
- PR adds a **feature used in both self-hosted and platform** without E2E test consideration
|
||||
|
||||
## Which Test Type to Suggest
|
||||
|
||||
- **Pure transformation** (parse, format, validate, compute) → extract to `.utils.ts` + unit test with vitest
|
||||
- **Complex UI interaction** → component test with `customRender` (or E2E if shared with self-hosted)
|
||||
- **E2E tests** should cover both click interactions AND keyboard shortcuts
|
||||
- **No tests at all** for non-trivial changes → nudge to add coverage
|
||||
|
||||
## Reference
|
||||
|
||||
See `.claude/skills/studio-testing/SKILL.md` for the full testing standard.
|
||||
Reference in new issue
Block a user