mirror of https://github.com/supabase/supabase.git synced 2026-05-07 01:10:15 -04:00

Files

T

Sean Oliver 2d4c562462 chore: split Copilot review guidelines into topic-specific files (#43926 )

## Context

Noticed while working on #43913 that `copilot-instructions.md` is
currently at ~4,600 characters. Per [GitHub's docs on Copilot code
review](https://docs.github.com/en/copilot/how-tos/use-copilot-agents/request-a-code-review/use-code-review):

> Copilot code review only reads the first 4,000 characters of any
custom instruction file. Any instructions beyond this limit will not
affect the reviews generated by Copilot code review.

This means the testing section at the bottom of our current file isn't
being read during reviews.

## Proposal

Split the single file into path-specific instruction files under
`.github/instructions/`, following [GitHub's recommended pattern for
repository custom
instructions](https://docs.github.com/en/copilot/how-tos/configure-custom-instructions/add-repository-instructions):

> These are specified in one or more `NAME.instructions.md` files within
or below the `.github/instructions` directory.

> If the path you specify matches a file that Copilot is working on, and
a repository-wide custom instructions file also exists, then the
instructions from both files are used.

This gives us separate files that each stay under the 4K limit and get
combined automatically by Copilot during reviews:

| File | Size | Scope |
|------|------|-------|
| `copilot-instructions.md` | 929 chars | General repo context +
pointers |
| `instructions/studio-telemetry.instructions.md` | 3,570 chars |
Telemetry rules for `apps/studio/**` |
| `instructions/studio-testing.instructions.md` | 1,228 chars | Testing
rules for `apps/studio/**` |

Note: Copilot reads instructions from the **base branch** of a PR, not
the feature branch — so these won't take effect until merged to master.

### New telemetry guidance

The telemetry file adds guidance we've been missing — specifically
around feature-flagged rollouts:

- Flag PRs that use `usePHFlag`/`useFlag` to gate behavior but don't
capture the flag state in telemetry
- Flag rollouts that track flag state but not user response to the new
behavior
- Documents the raw flag pattern (read via `usePHFlag`, not coerced
wrapper hooks) to avoid the `undefined`→`false` data quality bug we hit
in #43913

### What didn't change

All existing telemetry and testing rules are preserved — nothing was
removed, just reorganized. The telemetry rules still reference
`.claude/skills/telemetry-standards/SKILL.md` as the authoritative
source.

## References

- [Adding repository custom instructions for GitHub
Copilot](https://docs.github.com/en/copilot/how-tos/configure-custom-instructions/add-repository-instructions)
— file structure, path-specific instructions, frontmatter format
- [Using Copilot code
review](https://docs.github.com/en/copilot/how-tos/use-copilot-agents/request-a-code-review/use-code-review)
— 4K character limit, base branch behavior

## Open questions

Would love the team's input on:
- Does the file split make sense, or would you prefer keeping everything
in one file (and trimming to fit)?
- Are there other topics that should get their own instruction file?
- Any concerns with the new feature flag telemetry guidance?

2026-03-18 12:59:27 -07:00

1.2 KiB

Raw Permalink Blame History

applyTo

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.

1.2 KiB Raw Permalink Blame History