diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 0000000000..183276c99c --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,21 @@ +# Copilot Cloud Agent Instructions for `deephaven/web-client-ui` + +## Repository context +- Use `AGENTS.md` as the authoritative source for repository setup, architecture, build/test commands, constraints, and validation workflow. +- Do not duplicate AGENTS.md guidance in review comments; apply it. +- Only search for additional context when AGENTS.md or the path-specific instruction files are incomplete for the files being reviewed. + +## Copilot code review workflow +- Treat code review as a multi-pass workflow and only comment when you find a concrete, actionable issue. +- Run these specialist passes in order when they apply to the changed files: + 1. **Systems architect**: check whether the PR fits the existing package boundaries, plugin/layout/redux/jsapi architecture, and dependency direction. + 2. **UI/UX reviewer**: for UI changes, check labels, accessibility, Spectrum/component usage, and SCSS token/color-variable usage. + 3. **Code quality reviewer**: check maintainability, consistency with nearby patterns, error handling, and whether the change is scoped cleanly. + 4. **Test reviewer**: for behavior changes, verify that the right unit and/or e2e coverage exists and that validation steps are appropriate for the risk. +- Skip passes that do not apply to the touched files rather than forcing generic feedback. +- Prefer a few high-signal findings over many style comments. +- Use the specialized instruction files in `.github/instructions/` to deepen each pass: + - `architecture.instructions.md` + - `ui-ux.instructions.md` + - `code-quality.instructions.md` + - `tests.instructions.md` diff --git a/.github/instructions/architecture.instructions.md b/.github/instructions/architecture.instructions.md new file mode 100644 index 0000000000..66297cc7bf --- /dev/null +++ b/.github/instructions/architecture.instructions.md @@ -0,0 +1,24 @@ +--- +applyTo: + - "packages/**/*.{ts,tsx,js,jsx}" + - "package.json" + - "tsconfig.json" + - "lerna.json" + - "nx.json" + - ".github/workflows/**/*.yml" +--- + +# Systems architect review + +- Review the PR as a systems architect before reviewing details. +- Check that changes fit the monorepo package boundaries instead of introducing cross-package leakage. +- Preserve the main layering: + - `golden-layout` -> `dashboard` -> `dashboard-core-plugins` -> app packages + - `grid` -> `iris-grid` + - `jsapi-types` / `jsapi-shim` / `jsapi-bootstrap` / `jsapi-components` / `jsapi-utils` +- Flag changes that bypass established extension points such as plugin registration, reducer registration, or existing hooks/contexts. +- Prefer changes that reuse existing workspace packages over duplicating logic in app packages. +- Watch for dependency mistakes that break repository rules, especially direct `@adobe/react-spectrum` imports outside `packages/components/src/spectrum/**` and `packages/components/src/theme/**`. +- Watch for imports of a package's own `@deephaven/` alias from within that same package; relative imports should be used instead. +- For build or workflow changes, check that the required validation order still makes sense: Node 24/npm 11, `npm ci --no-audit`, `npm run build:necessary`, then targeted lint/tests or full build. +- Only raise architecture comments when the issue would make the design harder to extend, harder to reason about, or inconsistent with the existing package model. diff --git a/.github/instructions/code-quality.instructions.md b/.github/instructions/code-quality.instructions.md new file mode 100644 index 0000000000..90c054daaf --- /dev/null +++ b/.github/instructions/code-quality.instructions.md @@ -0,0 +1,14 @@ +--- +applyTo: "packages/**/*.{ts,tsx,js,jsx}" +--- + +# Code quality review + +- Review changed source as a code quality specialist. +- Check that the change matches established patterns in the same package before recommending a new abstraction. +- Prefer focused functions/components, descriptive names, and clear data flow over clever or overly dense logic. +- Flag copy/pasted logic when an existing utility, hook, model, or component would be a better fit. +- Check for maintainable error handling and cleanup, especially around async code, event listeners, and subscriptions. +- In React code, prefer patterns already used nearby for hooks, props, memoization, and state ownership. +- In stateful cross-cutting code, ensure reducers, selectors, and plugin registration remain easy to follow. +- Avoid nitpicks about formatting or trivial style unless they hide a real maintenance problem. diff --git a/.github/instructions/tests.instructions.md b/.github/instructions/tests.instructions.md new file mode 100644 index 0000000000..50113ccf41 --- /dev/null +++ b/.github/instructions/tests.instructions.md @@ -0,0 +1,26 @@ +--- +applyTo: + - "packages/**/*.{ts,tsx,js,jsx}" + - "packages/**/*.{test,spec}.{ts,tsx,js,jsx}" + - "tests/**/*" + - "jest*.cjs" + - "playwright*.config.ts" +--- + +# Test review + +- Review the PR as a test specialist whenever behavior changes, bugs are fixed, or logic is added. +- Expect unit tests for new logic and bug fixes unless the change is documentation-only or otherwise non-executable. +- For user-visible workflow changes in the main apps, consider whether e2e coverage should also be added or updated. +- Check that tests exercise the intended behavior, not just implementation details. +- Prefer targeted tests close to the changed package unless the behavior is truly cross-package. +- Remember repository validation order: + - `nvm install 24 && nvm use 24` + - `npm ci --no-audit` + - `npm run build:necessary` + - `git fetch origin main:refs/remotes/origin/main` before `--changedSince origin/main` commands if needed + - `npm run test:lint -- --changedSince origin/main` + - `npm run test:unit -- --changedSince origin/main` +- `npm run e2e` requires `localhost:4000/ide/` plus a `deephaven-core` backend on port `10000` with anonymous auth and `-Ddeephaven.application.dir=tests/docker-scripts/data/app.d`. +- Treat repeated `jest-haste-map: duplicate manual mock found` output as known noise unless accompanied by real test failures. +- Comment when coverage is missing, mis-scoped, or unvalidated—not simply because additional tests could exist. diff --git a/.github/instructions/ui-ux.instructions.md b/.github/instructions/ui-ux.instructions.md new file mode 100644 index 0000000000..6576e61250 --- /dev/null +++ b/.github/instructions/ui-ux.instructions.md @@ -0,0 +1,14 @@ +--- +applyTo: "packages/**/*.{tsx,jsx,scss,css}" +--- + +# UI/UX review + +- Review UI-facing changes as a UI/UX specialist. +- Check that button labels, menu items, empty states, and dialogs are clear and specific. +- Prefer existing components from `packages/components` and existing app patterns over introducing one-off UI. +- Ensure interactive elements are accessible: keyboard reachable, properly labeled, and not dependent on color alone. +- In SCSS, prefer existing variables/tokens and shared patterns over hard-coded colors, spacing, or typography. +- For Spectrum usage, ensure imports go through the repository's `components` package conventions instead of direct `@adobe/react-spectrum` imports outside the allowed directories. +- Flag visual or interaction changes that feel inconsistent with nearby screens, especially in `code-studio`, `embed-widget`, `embed-chart`, and `embed-grid`. +- Do not leave generic style feedback; focus on usability, accessibility, design-system consistency, and maintainable styling.