diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 000000000..ecd78c25b --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,3 @@ +# Copilot Instructions for `deephaven-plugins` + +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..851f189af --- /dev/null +++ b/.github/copilot-review-instructions.md @@ -0,0 +1,29 @@ +# Copilot Code Review Instructions for `deephaven-plugins` + +## Repository context + +- 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. + +## 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: + +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. + +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 000000000..f3990b599 --- /dev/null +++ b/.github/instructions/architecture.instructions.md @@ -0,0 +1,21 @@ +--- +applyTo: + - "plugins/**/*.ts" + - "plugins/**/*.tsx" + - "plugins/**/*.js" + - "plugins/**/*.jsx" + - "plugins/**/*.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 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. +- 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..438b109d2 --- /dev/null +++ b/.github/instructions/code-quality.instructions.md @@ -0,0 +1,19 @@ +--- +applyTo: + - "plugins/**/*.ts" + - "plugins/**/*.tsx" + - "plugins/**/*.js" + - "plugins/**/*.jsx" + - "plugins/**/*.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..a8ed34ac0 --- /dev/null +++ b/.github/instructions/tests.instructions.md @@ -0,0 +1,37 @@ +--- +applyTo: + - "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" + - "jest*.cjs" + - "jest*.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..7ad1db4ff --- /dev/null +++ b/.github/instructions/ui-ux.instructions.md @@ -0,0 +1,18 @@ +--- +applyTo: + - "plugins/**/*.tsx" + - "plugins/**/*.jsx" + - "plugins/**/*.scss" + - "plugins/**/*.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.