From 098110253108002050b70213f93ccc26ba4a725c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 18 Jun 2026 15:05:21 +0000 Subject: [PATCH 1/4] docs: add copilot cloud agent onboarding instructions --- .github/copilot-instructions.md | 40 +++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 .github/copilot-instructions.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 0000000000..9b80b0736d --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,40 @@ +# Copilot Cloud Agent Instructions for `deephaven/web-client-ui` + +## Start here +- This is a Lerna + npm workspaces monorepo (`packages/*`) with Node.js **24.x** and npm **11.x** required. +- Before doing anything else: + 1. `nvm install 24 && nvm use 24` + 2. `npm ci --no-audit` +- Core apps: `packages/code-studio`, `packages/embed-widget`, `packages/embed-chart`, `packages/embed-grid`. + +## High-signal workflow for most tasks +1. Build generated prerequisites first: `npm run build:necessary` (builds `@deephaven/icons`). +2. Run targeted checks first when possible: + - Lint (changed files): `npm run test:lint -- --changedSince origin/main` + - Unit tests (changed files): `npm run test:unit -- --changedSince origin/main` +3. If task scope is broad, run full build: `npm run build`. + +## Important repository constraints +- Do not import `@adobe/react-spectrum` outside `packages/components/src/spectrum/**` and `packages/components/src/theme/**`. +- Inside a package, do not import that same package via its own `@deephaven/` alias; use relative imports. +- PR titles are enforced as Conventional Commits (`feat`, `fix`, `docs`, `chore`, etc.). + +## Known pitfalls and workarounds (encountered) +- **Error:** `npm ci` fails with `EBADENGINE` (`Required: node >=24, npm >=11`). + - **Workaround:** switch toolchain first: `nvm install 24 && nvm use 24`. +- **Error:** lint reports `import/no-unresolved` for `@deephaven/icons`. + - **Workaround:** run `npm run build:necessary` before lint/tests. +- **Error:** `fatal: bad revision 'origin/main...HEAD'` when using `--changedSince origin/main`. + - **Workaround:** fetch the branch first: `git fetch origin main:refs/remotes/origin/main`. +- **Noise:** repeated `jest-haste-map: duplicate manual mock found` warnings across packages. + - **Workaround:** expected in this monorepo; continue unless accompanied by actual failing suites. + +## Local run and test context +- Dev servers: `npm start` (code-studio on `:4000`, embed-widget on `:4010`). +- Web UI needs a running `deephaven-core` backend (default `http://localhost:10000`). +- If backend host/port differs, set `VITE_PROXY_URL` in package `.env.local` files. + +## E2E notes +- `npm run e2e` requires UI at `localhost:4000/ide/` and `deephaven-core` on port `10000` with anonymous auth plus `tests/docker-scripts/data/app.d` application directory. +- Run `npx playwright install` before first local e2e run. +- For CI-like e2e behavior and snapshot updates, use Docker scripts (`npm run e2e:docker`, `npm run e2e:update-ci-snapshots`). From f5453d1582d13224c950dac62eae8261f40d18f3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 18 Jun 2026 15:25:33 +0000 Subject: [PATCH 2/4] docs: add multi-agent review instructions --- .github/copilot-instructions.md | 10 +++++++++ .../instructions/architecture.instructions.md | 18 ++++++++++++++++ .../instructions/code-quality.instructions.md | 14 +++++++++++++ .github/instructions/tests.instructions.md | 21 +++++++++++++++++++ .github/instructions/ui-ux.instructions.md | 14 +++++++++++++ 5 files changed, 77 insertions(+) create mode 100644 .github/instructions/architecture.instructions.md create mode 100644 .github/instructions/code-quality.instructions.md create mode 100644 .github/instructions/tests.instructions.md create mode 100644 .github/instructions/ui-ux.instructions.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 9b80b0736d..d92d7d9671 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -19,6 +19,16 @@ - Inside a package, do not import that same package via its own `@deephaven/` alias; use relative imports. - PR titles are enforced as Conventional Commits (`feat`, `fix`, `docs`, `chore`, etc.). +## 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. + ## Known pitfalls and workarounds (encountered) - **Error:** `npm ci` fails with `EBADENGINE` (`Required: node >=24, npm >=11`). - **Workaround:** switch toolchain first: `nvm install 24 && nvm use 24`. diff --git a/.github/instructions/architecture.instructions.md b/.github/instructions/architecture.instructions.md new file mode 100644 index 0000000000..0e76eca2f6 --- /dev/null +++ b/.github/instructions/architecture.instructions.md @@ -0,0 +1,18 @@ +--- +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..6c9d4513bb --- /dev/null +++ b/.github/instructions/tests.instructions.md @@ -0,0 +1,21 @@ +--- +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 `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. From d5ce836b05b209a414981d2282b0141608406a53 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 18 Jun 2026 15:27:46 +0000 Subject: [PATCH 3/4] docs: refine Copilot review instructions --- .github/copilot-instructions.md | 47 +++++----------------- .github/instructions/tests.instructions.md | 2 +- 2 files changed, 10 insertions(+), 39 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index d92d7d9671..183276c99c 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -1,23 +1,9 @@ # Copilot Cloud Agent Instructions for `deephaven/web-client-ui` -## Start here -- This is a Lerna + npm workspaces monorepo (`packages/*`) with Node.js **24.x** and npm **11.x** required. -- Before doing anything else: - 1. `nvm install 24 && nvm use 24` - 2. `npm ci --no-audit` -- Core apps: `packages/code-studio`, `packages/embed-widget`, `packages/embed-chart`, `packages/embed-grid`. - -## High-signal workflow for most tasks -1. Build generated prerequisites first: `npm run build:necessary` (builds `@deephaven/icons`). -2. Run targeted checks first when possible: - - Lint (changed files): `npm run test:lint -- --changedSince origin/main` - - Unit tests (changed files): `npm run test:unit -- --changedSince origin/main` -3. If task scope is broad, run full build: `npm run build`. - -## Important repository constraints -- Do not import `@adobe/react-spectrum` outside `packages/components/src/spectrum/**` and `packages/components/src/theme/**`. -- Inside a package, do not import that same package via its own `@deephaven/` alias; use relative imports. -- PR titles are enforced as Conventional Commits (`feat`, `fix`, `docs`, `chore`, etc.). +## 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. @@ -28,23 +14,8 @@ 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. - -## Known pitfalls and workarounds (encountered) -- **Error:** `npm ci` fails with `EBADENGINE` (`Required: node >=24, npm >=11`). - - **Workaround:** switch toolchain first: `nvm install 24 && nvm use 24`. -- **Error:** lint reports `import/no-unresolved` for `@deephaven/icons`. - - **Workaround:** run `npm run build:necessary` before lint/tests. -- **Error:** `fatal: bad revision 'origin/main...HEAD'` when using `--changedSince origin/main`. - - **Workaround:** fetch the branch first: `git fetch origin main:refs/remotes/origin/main`. -- **Noise:** repeated `jest-haste-map: duplicate manual mock found` warnings across packages. - - **Workaround:** expected in this monorepo; continue unless accompanied by actual failing suites. - -## Local run and test context -- Dev servers: `npm start` (code-studio on `:4000`, embed-widget on `:4010`). -- Web UI needs a running `deephaven-core` backend (default `http://localhost:10000`). -- If backend host/port differs, set `VITE_PROXY_URL` in package `.env.local` files. - -## E2E notes -- `npm run e2e` requires UI at `localhost:4000/ide/` and `deephaven-core` on port `10000` with anonymous auth plus `tests/docker-scripts/data/app.d` application directory. -- Run `npx playwright install` before first local e2e run. -- For CI-like e2e behavior and snapshot updates, use Docker scripts (`npm run e2e:docker`, `npm run e2e:update-ci-snapshots`). +- 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/tests.instructions.md b/.github/instructions/tests.instructions.md index 6c9d4513bb..3852334cf6 100644 --- a/.github/instructions/tests.instructions.md +++ b/.github/instructions/tests.instructions.md @@ -16,6 +16,6 @@ applyTo: "packages/**/*.{ts,tsx,js,jsx},packages/**/*.{test,spec}.{ts,tsx,js,jsx - `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 `tests/docker-scripts/data/app.d`. +- `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. From 77922e6eca68eb1cbb49917abb53aa16c1d5f4ce Mon Sep 17 00:00:00 2001 From: Mike Bender Date: Thu, 18 Jun 2026 13:10:27 -0400 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .github/instructions/architecture.instructions.md | 8 +++++++- .github/instructions/tests.instructions.md | 7 ++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/.github/instructions/architecture.instructions.md b/.github/instructions/architecture.instructions.md index 0e76eca2f6..66297cc7bf 100644 --- a/.github/instructions/architecture.instructions.md +++ b/.github/instructions/architecture.instructions.md @@ -1,5 +1,11 @@ --- -applyTo: "packages/**/*.{ts,tsx,js,jsx},package.json,tsconfig.json,lerna.json,nx.json,.github/workflows/**/*.yml" +applyTo: + - "packages/**/*.{ts,tsx,js,jsx}" + - "package.json" + - "tsconfig.json" + - "lerna.json" + - "nx.json" + - ".github/workflows/**/*.yml" --- # Systems architect review diff --git a/.github/instructions/tests.instructions.md b/.github/instructions/tests.instructions.md index 3852334cf6..50113ccf41 100644 --- a/.github/instructions/tests.instructions.md +++ b/.github/instructions/tests.instructions.md @@ -1,5 +1,10 @@ --- -applyTo: "packages/**/*.{ts,tsx,js,jsx},packages/**/*.{test,spec}.{ts,tsx,js,jsx},tests/**/*,jest*.cjs,playwright*.config.ts" +applyTo: + - "packages/**/*.{ts,tsx,js,jsx}" + - "packages/**/*.{test,spec}.{ts,tsx,js,jsx}" + - "tests/**/*" + - "jest*.cjs" + - "playwright*.config.ts" --- # Test review