From bdb19dfee47ab4b39eb8883c7b885e5cb8ef4369 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 18 Jun 2026 15:09:40 +0000 Subject: [PATCH 1/9] Add Copilot onboarding instructions --- .github/copilot-instructions.md | 242 ++++++++++++++++++++++++++++++++ 1 file changed, 242 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 000000000..c9cbdb2d3 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,242 @@ +# Copilot Instructions for `deephaven-plugins` + +## Repository overview + +- This repository is a mixed Python + JavaScript monorepo for Deephaven plugins. +- Most plugin work happens under `/home/runner/work/deephaven-plugins/deephaven-plugins/plugins//`. +- JavaScript plugin packages live in `plugins/*/src/js/` and are managed from the repo root with npm workspaces. +- Some plugins are Python-only, some are JS-only, and some have both. +- Architecture details for server-plugin / JS-plugin interaction are documented in `ARCHITECTURE.md`. + +## First steps in every terminal + +Always activate the repository virtual environment before doing anything else: + +```bash +source .venv/bin/activate +``` + +If `.venv` does not exist yet, create it from the repo root and install the repo requirements: + +```bash +python -m venv .venv +source .venv/bin/activate +pip install --upgrade -r requirements.txt +``` + +Node is managed from the repo root. The expected version is in `.nvmrc` (`v24.10.0`). + +## Repo structure that matters most + +- `plugins/` — plugin source code +- `tests/` — Playwright end-to-end tests +- `tools/plugin_builder.py` — the main helper for building, reinstalling, serving, and watching plugins +- `sphinx_ext/` — Sphinx extensions and doc build requirements +- `.github/workflows/` — the authoritative source for CI behavior +- `templates/` — cookiecutter plugin templates; treat them as templates, not active packages + +## Fast path for common tasks + +Run commands from `/home/runner/work/deephaven-plugins/deephaven-plugins` unless a plugin-specific path is called out. + +### Python environment and linting + +```bash +source .venv/bin/activate +pre-commit run --all-files +``` + +- `pre-commit` runs Black, blacken-docs, Pyright, and Ruff. +- This is the safest first validation for Python or Markdown changes. + +### JavaScript install, build, and test + +```bash +source .venv/bin/activate +npm ci --no-audit +npm run build +npm run test:ci +``` + +- `npm run build` builds all JS workspaces. +- `npm run test:ci` runs both JS unit tests and lint tests from the repo root. +- `npm run test:unit -- --testPathPattern="plugins/"` is the fastest targeted JS test command for one plugin. + +### Python plugin tests + +From a specific plugin directory: + +```bash +cd plugins/ +source ../../.venv/bin/activate +tox -e py3.12 +``` + +- CI runs Python plugin tox environments across Python 3.9 through 3.13. +- For local agent work, `tox -e py3.12` is a good default when the plugin supports tox. + +### Build and reinstall plugins + +Preferred helper: + +```bash +source .venv/bin/activate +python tools/plugin_builder.py --reinstall +``` + +Useful variants: + +```bash +python tools/plugin_builder.py --js --reinstall +python tools/plugin_builder.py --docs --install +python tools/plugin_builder.py --reinstall --server +``` + +- `plugin_builder.py` is the repo’s preferred automation entry point for plugin iteration. +- It also supports `--watch` for repeated rebuilds. + +### End-to-end tests + +Prefer the Docker-backed Playwright workflow because CI uses the same setup: + +```bash +source .venv/bin/activate +npm run e2e:docker -- ./tests/.spec.ts --reporter=list +``` + +Other useful commands: + +```bash +npm run e2e:ui +npm run e2e:update-snapshots -- ./tests/.spec.ts +``` + +### Docs work + +Preview docs from source: + +```bash +source .venv/bin/activate +npm run docs +``` + +Build plugin docs with API references: + +```bash +source .venv/bin/activate +pip install -r sphinx_ext/sphinx-requirements.txt +python tools/plugin_builder.py --docs --install +``` + +Important doc notes: + +- Only plugins with `make_docs.py` support generated docs builds. +- Built docs land in `plugins//docs/build/markdown/`. +- Do **not** commit generated files from `docs/build/markdown/`. +- For `plugins/ui` and `plugins/plotly-express`, prefer `dhautofunction`-based API reference docs instead of manually duplicating signatures. + +## How CI is organized + +- `pre-commit.yml` runs formatting/lint/type checks. +- `test-js-packages.yml` runs `npm run build`, `npm run types`, and `npm run test:ci`. +- `test-python-package.yml` runs `tox` per plugin across a Python version matrix. +- `modified-plugin.yml` detects changed plugins and fans out Python tests, JS tests, and docs builds. +- `build-docs.yml` only builds docs for plugins that contain `make_docs.py`. +- `e2e.yml` runs Playwright in Docker across Chromium, Firefox, and WebKit shards. + +When deciding what to validate: + +- Markdown-only changes: run `pre-commit run --all-files`. +- Python plugin changes: run `pre-commit run --all-files` and plugin `tox` if available. +- JS changes: run `npm run build` and `npm run test:ci`, or at least targeted `npm run test:unit -- --testPathPattern="plugins/"`. +- E2E-related changes: prefer `npm run e2e:docker`. +- Docs changes for doc-generating plugins: run `python tools/plugin_builder.py --docs --install ` when practical. + +## Repo-specific gotchas + +- Always activate `.venv` first; many repo commands assume it. +- JS builds and tests are root-level commands even when you only changed one plugin. +- `templates/` contains placeholder package names and can trigger tooling noise; do not “fix” template placeholders unless the task is specifically about templates. +- Some plugin docs are generated; output under `docs/build/markdown/` is build output, not source. +- `plugin_builder.py` ignores `dist/`, `build/`, `node_modules/`, hidden directories, and `plugins/python-remote-file-source/test-node-client/` while watching files. + +## Errors and workarounds seen while onboarding this repo + +### Missing virtual environment + +Error: + +```text +bash: .venv/bin/activate: No such file or directory +``` + +Workaround: + +- Create the venv from the repo root with `python -m venv .venv`. +- Re-run `source .venv/bin/activate`. +- Install requirements with `pip install --upgrade -r requirements.txt`. + +### `npm ci` deprecation noise + +Observed during `npm ci --no-audit`: + +- multiple dependency deprecation warnings from transitive packages + +Workaround: + +- Treat these as existing dependency noise unless the task is specifically dependency maintenance. +- `npm ci --no-audit` still completed successfully. + +### `npm run test:ci` template collision warning + +Observed warning: + +```text +jest-haste-map: Haste module naming collision: {{ cookiecutter.javascript_project_name }} +``` + +Cause: + +- Both template package trees under `templates/element/.../src/js/package.json` and `templates/widget/.../src/js/package.json` intentionally use the same cookiecutter placeholder names. + +Workaround: + +- Treat this as an existing repository warning when tests still pass. +- Do not rename template placeholders just to silence the warning unless the task is about the template or Jest configuration. + +### TypeScript support warning in lint tests + +Observed warning: + +```text +WARNING: You are currently running a version of TypeScript which is not officially supported by @typescript-eslint/typescript-estree. +SUPPORTED TYPESCRIPT VERSIONS: >=3.3.1 <5.2.0 +YOUR TYPESCRIPT VERSION: 5.9.3 +``` + +Workaround: + +- Treat this as existing tooling drift when tests pass. +- Do not change TypeScript or eslint stack versions unless dependency/tooling updates are part of the task. + +### Build warnings that are currently tolerated + +Observed during `npm run build`: + +- Vite CJS API deprecation warnings +- Sass deprecation warnings in the `ui` plugin +- warnings about mixed default + named exports +- warnings about `eval` in transitive dependencies + +Workaround: + +- These warnings are currently tolerated by the repository build. +- Only address them when the task is specifically about build tooling, Sass migration, or dependency cleanup. + +## Efficient agent behavior + +- Read the root `README.md`, relevant plugin `README.md`, and the matching workflow file before changing build/test behavior. +- Prefer small, plugin-scoped changes. +- Use `plugin_builder.py` instead of inventing custom build flows. +- Check whether the changed plugin has `tox.ini` or `make_docs.py` before assuming Python tests or doc generation exist. +- Avoid committing generated build output, `node_modules`, `.venv`, `.tox`, Playwright reports, or docs build artifacts. From 696687957243b578549af07406487d16cbec7b46 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 18 Jun 2026 15:10:31 +0000 Subject: [PATCH 2/9] Address review feedback in onboarding docs --- .github/copilot-instructions.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index c9cbdb2d3..adc0087c8 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -3,7 +3,7 @@ ## Repository overview - This repository is a mixed Python + JavaScript monorepo for Deephaven plugins. -- Most plugin work happens under `/home/runner/work/deephaven-plugins/deephaven-plugins/plugins//`. +- Most plugin work happens under `plugins//`. - JavaScript plugin packages live in `plugins/*/src/js/` and are managed from the repo root with npm workspaces. - Some plugins are Python-only, some are JS-only, and some have both. - Architecture details for server-plugin / JS-plugin interaction are documented in `ARCHITECTURE.md`. @@ -37,7 +37,7 @@ Node is managed from the repo root. The expected version is in `.nvmrc` (`v24.10 ## Fast path for common tasks -Run commands from `/home/runner/work/deephaven-plugins/deephaven-plugins` unless a plugin-specific path is called out. +Run commands from the repository root unless a plugin-specific path is called out. ### Python environment and linting From 7bb7bf95c94c5eb50f18c19a3ad93b972897c79d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 18 Jun 2026 15:11:13 +0000 Subject: [PATCH 3/9] Clarify dhautofunction reference --- .github/copilot-instructions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index adc0087c8..47304a432 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -133,7 +133,7 @@ Important doc notes: - Only plugins with `make_docs.py` support generated docs builds. - Built docs land in `plugins//docs/build/markdown/`. - Do **not** commit generated files from `docs/build/markdown/`. -- For `plugins/ui` and `plugins/plotly-express`, prefer `dhautofunction`-based API reference docs instead of manually duplicating signatures. +- For `plugins/ui` and `plugins/plotly-express`, prefer the project’s custom `dhautofunction` API reference directive instead of manually duplicating signatures. ## How CI is organized From e3d27df51c6d40244718ff13aef01c8742a7150d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 18 Jun 2026 15:42:02 +0000 Subject: [PATCH 4/9] Replace duplicate "First steps" section in copilot-instructions.md with reference to AGENTS.md --- .github/copilot-instructions.md | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 47304a432..7198291fd 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -10,19 +10,7 @@ ## First steps in every terminal -Always activate the repository virtual environment before doing anything else: - -```bash -source .venv/bin/activate -``` - -If `.venv` does not exist yet, create it from the repo root and install the repo requirements: - -```bash -python -m venv .venv -source .venv/bin/activate -pip install --upgrade -r requirements.txt -``` +See [`AGENTS.md`](../AGENTS.md) for the required venv activation step, available skills, and a quick-reference command table. Node is managed from the repo root. The expected version is in `.nvmrc` (`v24.10.0`). From 531c53146485da79dde9b90968e117640fe313cd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 18 Jun 2026 18:37:48 +0000 Subject: [PATCH 5/9] Slim copilot-instructions.md and add multi-agent copilot-review-instructions.md --- .github/copilot-instructions.md | 229 +------------------------ .github/copilot-review-instructions.md | 44 +++++ 2 files changed, 45 insertions(+), 228 deletions(-) create mode 100644 .github/copilot-review-instructions.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 7198291fd..ecd78c25b 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -1,230 +1,3 @@ # Copilot Instructions for `deephaven-plugins` -## Repository overview - -- This repository is a mixed Python + JavaScript monorepo for Deephaven plugins. -- Most plugin work happens under `plugins//`. -- JavaScript plugin packages live in `plugins/*/src/js/` and are managed from the repo root with npm workspaces. -- Some plugins are Python-only, some are JS-only, and some have both. -- Architecture details for server-plugin / JS-plugin interaction are documented in `ARCHITECTURE.md`. - -## First steps in every terminal - -See [`AGENTS.md`](../AGENTS.md) for the required venv activation step, available skills, and a quick-reference command table. - -Node is managed from the repo root. The expected version is in `.nvmrc` (`v24.10.0`). - -## Repo structure that matters most - -- `plugins/` — plugin source code -- `tests/` — Playwright end-to-end tests -- `tools/plugin_builder.py` — the main helper for building, reinstalling, serving, and watching plugins -- `sphinx_ext/` — Sphinx extensions and doc build requirements -- `.github/workflows/` — the authoritative source for CI behavior -- `templates/` — cookiecutter plugin templates; treat them as templates, not active packages - -## Fast path for common tasks - -Run commands from the repository root unless a plugin-specific path is called out. - -### Python environment and linting - -```bash -source .venv/bin/activate -pre-commit run --all-files -``` - -- `pre-commit` runs Black, blacken-docs, Pyright, and Ruff. -- This is the safest first validation for Python or Markdown changes. - -### JavaScript install, build, and test - -```bash -source .venv/bin/activate -npm ci --no-audit -npm run build -npm run test:ci -``` - -- `npm run build` builds all JS workspaces. -- `npm run test:ci` runs both JS unit tests and lint tests from the repo root. -- `npm run test:unit -- --testPathPattern="plugins/"` is the fastest targeted JS test command for one plugin. - -### Python plugin tests - -From a specific plugin directory: - -```bash -cd plugins/ -source ../../.venv/bin/activate -tox -e py3.12 -``` - -- CI runs Python plugin tox environments across Python 3.9 through 3.13. -- For local agent work, `tox -e py3.12` is a good default when the plugin supports tox. - -### Build and reinstall plugins - -Preferred helper: - -```bash -source .venv/bin/activate -python tools/plugin_builder.py --reinstall -``` - -Useful variants: - -```bash -python tools/plugin_builder.py --js --reinstall -python tools/plugin_builder.py --docs --install -python tools/plugin_builder.py --reinstall --server -``` - -- `plugin_builder.py` is the repo’s preferred automation entry point for plugin iteration. -- It also supports `--watch` for repeated rebuilds. - -### End-to-end tests - -Prefer the Docker-backed Playwright workflow because CI uses the same setup: - -```bash -source .venv/bin/activate -npm run e2e:docker -- ./tests/.spec.ts --reporter=list -``` - -Other useful commands: - -```bash -npm run e2e:ui -npm run e2e:update-snapshots -- ./tests/.spec.ts -``` - -### Docs work - -Preview docs from source: - -```bash -source .venv/bin/activate -npm run docs -``` - -Build plugin docs with API references: - -```bash -source .venv/bin/activate -pip install -r sphinx_ext/sphinx-requirements.txt -python tools/plugin_builder.py --docs --install -``` - -Important doc notes: - -- Only plugins with `make_docs.py` support generated docs builds. -- Built docs land in `plugins//docs/build/markdown/`. -- Do **not** commit generated files from `docs/build/markdown/`. -- For `plugins/ui` and `plugins/plotly-express`, prefer the project’s custom `dhautofunction` API reference directive instead of manually duplicating signatures. - -## How CI is organized - -- `pre-commit.yml` runs formatting/lint/type checks. -- `test-js-packages.yml` runs `npm run build`, `npm run types`, and `npm run test:ci`. -- `test-python-package.yml` runs `tox` per plugin across a Python version matrix. -- `modified-plugin.yml` detects changed plugins and fans out Python tests, JS tests, and docs builds. -- `build-docs.yml` only builds docs for plugins that contain `make_docs.py`. -- `e2e.yml` runs Playwright in Docker across Chromium, Firefox, and WebKit shards. - -When deciding what to validate: - -- Markdown-only changes: run `pre-commit run --all-files`. -- Python plugin changes: run `pre-commit run --all-files` and plugin `tox` if available. -- JS changes: run `npm run build` and `npm run test:ci`, or at least targeted `npm run test:unit -- --testPathPattern="plugins/"`. -- E2E-related changes: prefer `npm run e2e:docker`. -- Docs changes for doc-generating plugins: run `python tools/plugin_builder.py --docs --install ` when practical. - -## Repo-specific gotchas - -- Always activate `.venv` first; many repo commands assume it. -- JS builds and tests are root-level commands even when you only changed one plugin. -- `templates/` contains placeholder package names and can trigger tooling noise; do not “fix” template placeholders unless the task is specifically about templates. -- Some plugin docs are generated; output under `docs/build/markdown/` is build output, not source. -- `plugin_builder.py` ignores `dist/`, `build/`, `node_modules/`, hidden directories, and `plugins/python-remote-file-source/test-node-client/` while watching files. - -## Errors and workarounds seen while onboarding this repo - -### Missing virtual environment - -Error: - -```text -bash: .venv/bin/activate: No such file or directory -``` - -Workaround: - -- Create the venv from the repo root with `python -m venv .venv`. -- Re-run `source .venv/bin/activate`. -- Install requirements with `pip install --upgrade -r requirements.txt`. - -### `npm ci` deprecation noise - -Observed during `npm ci --no-audit`: - -- multiple dependency deprecation warnings from transitive packages - -Workaround: - -- Treat these as existing dependency noise unless the task is specifically dependency maintenance. -- `npm ci --no-audit` still completed successfully. - -### `npm run test:ci` template collision warning - -Observed warning: - -```text -jest-haste-map: Haste module naming collision: {{ cookiecutter.javascript_project_name }} -``` - -Cause: - -- Both template package trees under `templates/element/.../src/js/package.json` and `templates/widget/.../src/js/package.json` intentionally use the same cookiecutter placeholder names. - -Workaround: - -- Treat this as an existing repository warning when tests still pass. -- Do not rename template placeholders just to silence the warning unless the task is about the template or Jest configuration. - -### TypeScript support warning in lint tests - -Observed warning: - -```text -WARNING: You are currently running a version of TypeScript which is not officially supported by @typescript-eslint/typescript-estree. -SUPPORTED TYPESCRIPT VERSIONS: >=3.3.1 <5.2.0 -YOUR TYPESCRIPT VERSION: 5.9.3 -``` - -Workaround: - -- Treat this as existing tooling drift when tests pass. -- Do not change TypeScript or eslint stack versions unless dependency/tooling updates are part of the task. - -### Build warnings that are currently tolerated - -Observed during `npm run build`: - -- Vite CJS API deprecation warnings -- Sass deprecation warnings in the `ui` plugin -- warnings about mixed default + named exports -- warnings about `eval` in transitive dependencies - -Workaround: - -- These warnings are currently tolerated by the repository build. -- Only address them when the task is specifically about build tooling, Sass migration, or dependency cleanup. - -## Efficient agent behavior - -- Read the root `README.md`, relevant plugin `README.md`, and the matching workflow file before changing build/test behavior. -- Prefer small, plugin-scoped changes. -- Use `plugin_builder.py` instead of inventing custom build flows. -- Check whether the changed plugin has `tox.ini` or `make_docs.py` before assuming Python tests or doc generation exist. -- Avoid committing generated build output, `node_modules`, `.venv`, `.tox`, Playwright reports, or docs build artifacts. +See [`AGENTS.md`](../AGENTS.md) for environment setup, available skills, build/test commands, and repo conventions. diff --git a/.github/copilot-review-instructions.md b/.github/copilot-review-instructions.md new file mode 100644 index 000000000..865457b69 --- /dev/null +++ b/.github/copilot-review-instructions.md @@ -0,0 +1,44 @@ +# Copilot Code Review Instructions for `deephaven-plugins` + +Use a multi-agent review flow. Each agent below has a distinct focus — raise concerns only within that agent's scope. + +## Systems Architect + +Analyze the overall architecture of the PR. + +- Does the change fit naturally within the existing plugin architecture (Python server plugin ↔ JS client plugin boundary)? +- Are new abstractions justified, or does the change over-engineer a simple problem? +- Are cross-plugin or cross-layer dependencies introduced, and are they appropriate? +- Does the data flow make sense end-to-end? +- Are public APIs and extension points designed for future maintainability? + +## UI/UX Design + +Evaluate the user-facing experience. + +- Are buttons, labels, and interactive elements named clearly and consistently with the rest of the UI? +- Are colour variables used instead of hard-coded values in SCSS (e.g., `var(--dh-color-*)` tokens)? +- Is spacing, layout, and visual hierarchy consistent with existing components? +- Are loading, empty, and error states handled gracefully and communicated to the user? +- Is the interaction model intuitive — are affordances obvious, are destructive actions confirmed? + +## Code Quality + +Assess correctness, maintainability, and consistency. + +- Does the code follow established patterns in the affected plugin (naming, file structure, module boundaries)? +- Is the code readable and appropriately simple — no unnecessary complexity? +- Are edge cases and error paths handled? +- Is there any duplicated logic that should be extracted into a shared utility? +- Are TypeScript types correct and precise (no unnecessary `any` or unsafe casts)? +- Are Python types consistent with the rest of the codebase and validated with Pyright? + +## Test Reviewer + +Verify that the change is adequately tested. + +- Are there unit tests for any new functionality or bug fixes? +- Are there end-to-end (Playwright) tests where user-visible behaviour changes? +- Do the tests actually exercise the right behaviour — not just achieve coverage? +- Are edge cases and failure paths covered? +- Do all CI checks pass (pre-commit, `npm run test:ci`, tox, e2e)? From 2b969a40d65f8fb81a6895482316edd5119f4a3a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 18 Jun 2026 18:42:04 +0000 Subject: [PATCH 6/9] Split review instructions into per-reviewer files under .github/instructions/ --- .github/copilot-review-instructions.md | 51 +++++++------------ .../instructions/architecture.instructions.md | 17 +++++++ .../instructions/code-quality.instructions.md | 14 +++++ .github/instructions/tests.instructions.md | 24 +++++++++ .github/instructions/ui-ux.instructions.md | 14 +++++ 5 files changed, 86 insertions(+), 34 deletions(-) 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-review-instructions.md b/.github/copilot-review-instructions.md index 865457b69..1e4d714ed 100644 --- a/.github/copilot-review-instructions.md +++ b/.github/copilot-review-instructions.md @@ -1,44 +1,27 @@ # Copilot Code Review Instructions for `deephaven-plugins` -Use a multi-agent review flow. Each agent below has a distinct focus — raise concerns only within that agent's scope. +## Repository context -## Systems Architect +- Use `AGENTS.md` as the authoritative source for repository setup, build/test commands, and validation workflow. +- Do not duplicate `AGENTS.md` guidance in review comments; apply it. -Analyze the overall architecture of the PR. +## Code review workflow -- Does the change fit naturally within the existing plugin architecture (Python server plugin ↔ JS client plugin boundary)? -- Are new abstractions justified, or does the change over-engineer a simple problem? -- Are cross-plugin or cross-layer dependencies introduced, and are they appropriate? -- Does the data flow make sense end-to-end? -- Are public APIs and extension points designed for future maintainability? +Treat code review as a multi-pass workflow and only comment when you find a concrete, actionable issue. -## UI/UX Design +Run these specialist passes in order when they apply to the changed files: -Evaluate the user-facing experience. +1. **Systems architect**: check whether the PR fits the existing plugin architecture (Python server plugin ↔ JS client plugin boundary), abstraction level, and data flow. +2. **UI/UX reviewer**: for UI changes, check labels, colour variables, accessibility, and SCSS token usage. +3. **Code quality reviewer**: check maintainability, consistency with nearby patterns, error handling, and clean scoping. +4. **Test reviewer**: for behaviour changes, verify that the right unit and/or e2e coverage exists and that CI checks pass. -- Are buttons, labels, and interactive elements named clearly and consistently with the rest of the UI? -- Are colour variables used instead of hard-coded values in SCSS (e.g., `var(--dh-color-*)` tokens)? -- Is spacing, layout, and visual hierarchy consistent with existing components? -- Are loading, empty, and error states handled gracefully and communicated to the user? -- Is the interaction model intuitive — are affordances obvious, are destructive actions confirmed? +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. -## Code Quality +Use the specialized instruction files in `.github/instructions/` to deepen each pass: -Assess correctness, maintainability, and consistency. - -- Does the code follow established patterns in the affected plugin (naming, file structure, module boundaries)? -- Is the code readable and appropriately simple — no unnecessary complexity? -- Are edge cases and error paths handled? -- Is there any duplicated logic that should be extracted into a shared utility? -- Are TypeScript types correct and precise (no unnecessary `any` or unsafe casts)? -- Are Python types consistent with the rest of the codebase and validated with Pyright? - -## Test Reviewer - -Verify that the change is adequately tested. - -- Are there unit tests for any new functionality or bug fixes? -- Are there end-to-end (Playwright) tests where user-visible behaviour changes? -- Do the tests actually exercise the right behaviour — not just achieve coverage? -- Are edge cases and failure paths covered? -- Do all CI checks pass (pre-commit, `npm run test:ci`, tox, e2e)? +- `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 000000000..d48ee7bab --- /dev/null +++ b/.github/instructions/architecture.instructions.md @@ -0,0 +1,17 @@ +--- +applyTo: + - "plugins/**/*.{ts,tsx,js,jsx,py}" + - "tools/**/*.py" + - ".github/workflows/**/*.yml" +--- + +# Systems architect review + +- Review the PR as a systems architect before reviewing details. +- Check that changes fit the plugin model: Python server plugin (registered via `register_plugin` / `DhcPluginClient`) communicates with its JS counterpart over the standard message-passing boundary — do not bypass this boundary. +- Flag changes that introduce cross-plugin dependencies that should instead go through a shared utility or the established extension points. +- New abstractions should be justified; prefer extending existing patterns (hooks, utilities, plugin base classes) over adding new layers. +- For Python changes, ensure the public API surface is consistent with the rest of the plugin and that internal details are not unnecessarily exposed. +- For JS changes, ensure module boundaries within a plugin (`src/js/`) are clean and that nothing is re-exported that should stay internal. +- For workflow or tooling changes, verify that the validation order still makes sense: `source .venv/bin/activate`, `pre-commit run --all-files`, `npm run build`, `npm run test:ci`. +- Only raise architecture comments when the issue would make the design harder to extend, harder to reason about, or inconsistent with the existing plugin model. diff --git a/.github/instructions/code-quality.instructions.md b/.github/instructions/code-quality.instructions.md new file mode 100644 index 000000000..499e0dddb --- /dev/null +++ b/.github/instructions/code-quality.instructions.md @@ -0,0 +1,14 @@ +--- +applyTo: "plugins/**/*.{ts,tsx,js,jsx,py}" +--- + +# Code quality review + +- Review changed source as a code quality specialist. +- Check that the change matches established patterns in the same plugin 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, or component would be a better fit. +- Check for maintainable error handling and cleanup, especially around async code, event listeners, and subscriptions. +- In TypeScript/React code, ensure types are correct and precise — avoid unnecessary `any` or unsafe casts; prefer patterns already used nearby for hooks, props, and state ownership. +- In Python code, ensure types are consistent with the rest of the plugin and that Pyright would accept them (`pre-commit run --all-files` runs Pyright). +- 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 000000000..5fda6f909 --- /dev/null +++ b/.github/instructions/tests.instructions.md @@ -0,0 +1,24 @@ +--- +applyTo: + - "plugins/**/*.{ts,tsx,js,jsx,py}" + - "plugins/**/*.{test,spec}.{ts,tsx,js,jsx}" + - "plugins/**/test_*.py" + - "tests/**/*" + - "jest*.{js,cjs,mjs}" + - "playwright*.config.ts" +--- + +# Test review + +- Review the PR as a test specialist whenever behaviour 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, consider whether Playwright e2e coverage should also be added or updated (`tests/` directory). +- Check that tests exercise the intended behaviour, not just implementation details or coverage counts. +- Prefer targeted tests close to the changed plugin; only reach for e2e tests when the behaviour is truly end-to-end. +- Remember the repository validation order: + - `source .venv/bin/activate` + - `pre-commit run --all-files` (Black, Pyright, Ruff, blacken-docs) + - `npm run build && npm run test:ci` (JS unit + lint) + - `cd plugins/ && tox -e py3.12` (Python unit, when `tox.ini` exists) + - `npm run e2e:docker -- ./tests/.spec.ts --reporter=list` (e2e, requires Docker) +- Comment when coverage is missing, mis-scoped, or unvalidated — not simply because additional tests could theoretically exist. diff --git a/.github/instructions/ui-ux.instructions.md b/.github/instructions/ui-ux.instructions.md new file mode 100644 index 000000000..c88db9d8e --- /dev/null +++ b/.github/instructions/ui-ux.instructions.md @@ -0,0 +1,14 @@ +--- +applyTo: "plugins/**/*.{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, specific, and consistent with the rest of the UI. +- In SCSS, prefer existing colour variables/tokens (e.g., `var(--dh-color-*)`) over hard-coded colours, spacing, or typography values. +- Prefer existing shared components and patterns over introducing one-off UI elements. +- Ensure interactive elements are accessible: keyboard reachable, properly labelled (aria attributes where needed), and not dependent on colour alone. +- Are loading, empty, and error states handled gracefully and communicated to the user? +- Is the interaction model intuitive — are affordances obvious, are destructive actions confirmed? +- Do not leave generic style feedback; focus on usability, accessibility, design-system consistency, and maintainable styling. From e4eedeaf34f531ed2d06b81107e4101d36be52ed Mon Sep 17 00:00:00 2001 From: Mike Bender Date: Thu, 18 Jun 2026 20:30:23 -0400 Subject: [PATCH 7/9] Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../instructions/architecture.instructions.md | 8 ++++++-- .../instructions/code-quality.instructions.md | 2 +- .github/instructions/tests.instructions.md | 19 ++++++++++++++++--- .github/instructions/ui-ux.instructions.md | 2 +- 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/.github/instructions/architecture.instructions.md b/.github/instructions/architecture.instructions.md index d48ee7bab..f3990b599 100644 --- a/.github/instructions/architecture.instructions.md +++ b/.github/instructions/architecture.instructions.md @@ -1,6 +1,10 @@ --- applyTo: - - "plugins/**/*.{ts,tsx,js,jsx,py}" + - "plugins/**/*.ts" + - "plugins/**/*.tsx" + - "plugins/**/*.js" + - "plugins/**/*.jsx" + - "plugins/**/*.py" - "tools/**/*.py" - ".github/workflows/**/*.yml" --- @@ -8,7 +12,7 @@ applyTo: # Systems architect review - Review the PR as a systems architect before reviewing details. -- Check that changes fit the plugin model: Python server plugin (registered via `register_plugin` / `DhcPluginClient`) communicates with its JS counterpart over the standard message-passing boundary — do not bypass this boundary. +- Check that changes fit the plugin model: Python server plugins implement `Registration.register_into(...)` and register their JS counterparts via `create_js_plugin(...)` (often wrapped by `DheSafeCallbackWrapper`) — keep communication across the standard message-passing boundary and do not bypass it. - Flag changes that introduce cross-plugin dependencies that should instead go through a shared utility or the established extension points. - New abstractions should be justified; prefer extending existing patterns (hooks, utilities, plugin base classes) over adding new layers. - For Python changes, ensure the public API surface is consistent with the rest of the plugin and that internal details are not unnecessarily exposed. diff --git a/.github/instructions/code-quality.instructions.md b/.github/instructions/code-quality.instructions.md index 499e0dddb..64179b696 100644 --- a/.github/instructions/code-quality.instructions.md +++ b/.github/instructions/code-quality.instructions.md @@ -1,5 +1,5 @@ --- -applyTo: "plugins/**/*.{ts,tsx,js,jsx,py}" +applyTo: "plugins/**/*.ts, plugins/**/*.tsx, plugins/**/*.js, plugins/**/*.jsx, plugins/**/*.py" --- # Code quality review diff --git a/.github/instructions/tests.instructions.md b/.github/instructions/tests.instructions.md index 5fda6f909..a8ed34ac0 100644 --- a/.github/instructions/tests.instructions.md +++ b/.github/instructions/tests.instructions.md @@ -1,10 +1,23 @@ --- applyTo: - - "plugins/**/*.{ts,tsx,js,jsx,py}" - - "plugins/**/*.{test,spec}.{ts,tsx,js,jsx}" + - "plugins/**/*.ts" + - "plugins/**/*.tsx" + - "plugins/**/*.js" + - "plugins/**/*.jsx" + - "plugins/**/*.py" + - "plugins/**/*.test.ts" + - "plugins/**/*.test.tsx" + - "plugins/**/*.test.js" + - "plugins/**/*.test.jsx" + - "plugins/**/*.spec.ts" + - "plugins/**/*.spec.tsx" + - "plugins/**/*.spec.js" + - "plugins/**/*.spec.jsx" - "plugins/**/test_*.py" - "tests/**/*" - - "jest*.{js,cjs,mjs}" + - "jest*.js" + - "jest*.cjs" + - "jest*.mjs" - "playwright*.config.ts" --- diff --git a/.github/instructions/ui-ux.instructions.md b/.github/instructions/ui-ux.instructions.md index c88db9d8e..4dca38d0d 100644 --- a/.github/instructions/ui-ux.instructions.md +++ b/.github/instructions/ui-ux.instructions.md @@ -1,5 +1,5 @@ --- -applyTo: "plugins/**/*.{tsx,jsx,scss,css}" +applyTo: "plugins/**/*.tsx, plugins/**/*.jsx, plugins/**/*.scss, plugins/**/*.css" --- # UI/UX review From a4a11aaabb514d18607765c27c35d6cbb1d6e7bb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 19 Jun 2026 00:31:45 +0000 Subject: [PATCH 8/9] docs: clarify copilot review pass execution --- .github/copilot-review-instructions.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/copilot-review-instructions.md b/.github/copilot-review-instructions.md index 1e4d714ed..851f189af 100644 --- a/.github/copilot-review-instructions.md +++ b/.github/copilot-review-instructions.md @@ -8,6 +8,8 @@ ## Code review workflow Treat code review as a multi-pass workflow and only comment when you find a concrete, actionable issue. +These "passes" describe a review process (specialized lenses), not a hard requirement to spawn separate runtime agents for each pass. +When a PR opens, execute this checklist in order within the review session: load `AGENTS.md`, map changed files to relevant instruction files, run only applicable passes, then publish consolidated high-signal findings. Run these specialist passes in order when they apply to the changed files: From 19c31b3b78e94780b45f9c5ee59676736d29bfcd Mon Sep 17 00:00:00 2001 From: Mike Bender Date: Fri, 19 Jun 2026 15:21:39 -0400 Subject: [PATCH 9/9] Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .github/instructions/code-quality.instructions.md | 7 ++++++- .github/instructions/ui-ux.instructions.md | 6 +++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/.github/instructions/code-quality.instructions.md b/.github/instructions/code-quality.instructions.md index 64179b696..438b109d2 100644 --- a/.github/instructions/code-quality.instructions.md +++ b/.github/instructions/code-quality.instructions.md @@ -1,5 +1,10 @@ --- -applyTo: "plugins/**/*.ts, plugins/**/*.tsx, plugins/**/*.js, plugins/**/*.jsx, plugins/**/*.py" +applyTo: + - "plugins/**/*.ts" + - "plugins/**/*.tsx" + - "plugins/**/*.js" + - "plugins/**/*.jsx" + - "plugins/**/*.py" --- # Code quality review diff --git a/.github/instructions/ui-ux.instructions.md b/.github/instructions/ui-ux.instructions.md index 4dca38d0d..7ad1db4ff 100644 --- a/.github/instructions/ui-ux.instructions.md +++ b/.github/instructions/ui-ux.instructions.md @@ -1,5 +1,9 @@ --- -applyTo: "plugins/**/*.tsx, plugins/**/*.jsx, plugins/**/*.scss, plugins/**/*.css" +applyTo: + - "plugins/**/*.tsx" + - "plugins/**/*.jsx" + - "plugins/**/*.scss" + - "plugins/**/*.css" --- # UI/UX review