From 53dd9ed732f89a126b6c54ed2fbf8f98b3786b03 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Tue, 20 Oct 2026 16:40:37 -0400 Subject: [PATCH 1/3] Bring agent guidance in line with make and CI PR #1373 made `make typecheck` cover app, website and calculator-app, made `make test` cover app and website, and added app's Stylelint to CI. Several agent commands and docs still described the older, app-only setup. - /typecheck, /eslint and /prettier run `make typecheck`, `make lint` and `make format` from the repository root instead of npm scripts in app/ only. /finalize-pr names Stylelint and no longer says the PR is ready without the tests and build. - The lint-format-validator agent covers Stylelint and checks Prettier in all three projects, as CI does. Both validator agents run from the repository root instead of a hard-coded /Users/administrator path. - testing.md says which directory each command runs in, points to `make test`, and says calculator-app has no suite of its own. Its coverage command fails because @vitest/coverage-v8 isn't installed; the doc now says so. `bun run check` doesn't run Stylelint or tests. - repository-guidance.md says CI runs the tests on Node 26 while `make test` uses the local Node (app/.nvmrc says v24.3.0), so a test of formatted numbers can differ locally. - /write-unit-tests and /next-phase pointed at app/src/tests/CLAUDE.md, which moved to docs/engineering/skills/testing.md on 2026-09-10. Co-Authored-By: Claude Opus 5.5 --- .claude/agents/lint-format-validator.md | 50 +++++++++++-------- .claude/agents/typecheck-validator.md | 8 ++- .claude/commands/eslint.md | 6 +-- .claude/commands/finalize-pr.md | 6 +-- .claude/commands/next-phase.md | 4 +- .claude/commands/prettier.md | 2 +- .claude/commands/typecheck.md | 2 +- .claude/commands/write-unit-tests.md | 2 +- .../engineering/skills/repository-guidance.md | 2 +- docs/engineering/skills/testing.md | 20 ++++++-- 10 files changed, 65 insertions(+), 37 deletions(-) diff --git a/.claude/agents/lint-format-validator.md b/.claude/agents/lint-format-validator.md index f1be7e2da..2759b9bb3 100644 --- a/.claude/agents/lint-format-validator.md +++ b/.claude/agents/lint-format-validator.md @@ -1,6 +1,6 @@ --- name: lint-format-validator -description: Run ESLint then Prettier via the Makefile, automatically fix any failures, and iterate until both pass cleanly +description: Run ESLint and Stylelint, then Prettier, via the Makefile, automatically fix any failures, and iterate until all pass cleanly tools: Bash, Read, Edit, Glob, Grep allowedTools: Bash, Read, Edit, Glob, Grep model: sonnet @@ -8,21 +8,25 @@ model: sonnet # Lint and format validator -You are a validation agent that runs ESLint and then Prettier, automatically fixes all failures, and iterates until both pass cleanly. ESLint runs first, then Prettier — this order matters because Prettier can undo ESLint autofixes if run in the wrong order. +You are a validation agent that runs the linters (ESLint and Stylelint) and then Prettier, automatically fixes all failures, and iterates until all of them pass cleanly. The linters run first, then Prettier — this order matters because Prettier can undo ESLint autofixes if run in the wrong order. + +Run every command below from the repository root. ## Workflow -### Phase 1: ESLint +### Phase 1: ESLint and Stylelint -#### 1a. Run ESLint via the Makefile +#### 1a. Run the linters via the Makefile ```bash -cd /Users/administrator/Documents/PolicyEngine/policyengine-app-v2 && make lint 2>&1 +make lint 2>&1 ``` +`make lint` runs ESLint in `app/`, then Stylelint on `app/`'s CSS (`stylelint '**/*.css'`, configured in `app/.stylelintrc.json`), then ESLint in `website/` and `calculator-app/`. These are the same linters as CI's Lint and format job. `make` stops at the first linter that fails, so the ones after it only run once it passes. + **IMPORTANT:** CI uses `eslint --max-warnings 0`, so warnings are failures too. Fix ALL warnings and errors. -#### 1b. If ESLint passes with zero errors and zero warnings, move to Phase 2. +#### 1b. If `make lint` passes with zero errors and zero warnings, move to Phase 2. #### 1c. If there are errors or warnings, fix them: @@ -40,46 +44,52 @@ For each ESLint issue: **Do NOT add blanket eslint-disable comments.** Fix the underlying issue. -#### 1d. Re-run ESLint +For each Stylelint issue: +1. Read the CSS file at the reported line +2. Fix the declaration or selector the named rule reports (for example, `property-no-unknown` means a misspelled or invalid property) + +**Do NOT add stylelint-disable comments or turn rules off in `app/.stylelintrc.json`.** Fix the CSS. + +#### 1d. Re-run the linters ```bash -cd /Users/administrator/Documents/PolicyEngine/policyengine-app-v2 && make lint 2>&1 +make lint 2>&1 ``` -#### 1e. Repeat 1c-1d until ESLint reports zero errors and zero warnings. +#### 1e. Repeat 1c-1d until `make lint` reports zero errors and zero warnings. ### Phase 2: Prettier #### 2a. Run Prettier in write mode via the Makefile ```bash -cd /Users/administrator/Documents/PolicyEngine/policyengine-app-v2 && make format 2>&1 +make format 2>&1 ``` -This automatically reformats all files. Prettier in write mode is idempotent — running it once should be sufficient. +This reformats the files CI checks: `.ts` and `.tsx` files in `app/`, and `.ts`, `.tsx`, `.mjs` and `.css` files in `website/` and `calculator-app/`. Prettier in write mode is idempotent — running it once should be sufficient. -#### 2b. Verify Prettier is clean by running the check +#### 2b. Verify Prettier is clean by running CI's three checks ```bash -cd /Users/administrator/Documents/PolicyEngine/policyengine-app-v2/app && bun run prettier 2>&1 +(cd app && bun run prettier && cd ../website && bun run format:check && cd ../calculator-app && bun run format:check) 2>&1 ``` If any files are listed as needing formatting, run `make format` again. -#### 2c. Repeat until the check reports no issues. +#### 2c. Repeat until the checks report no issues. ### Phase 3: Final verification -Run both checks one more time to confirm everything is clean: +Run the linters and the Prettier checks one more time to confirm everything is clean: ```bash -cd /Users/administrator/Documents/PolicyEngine/policyengine-app-v2 && make lint 2>&1 && make format 2>&1 +make lint 2>&1 && (cd app && bun run prettier && cd ../website && bun run format:check && cd ../calculator-app && bun run format:check) 2>&1 ``` ### Phase 4: Report -When both pass, report: -- How many ESLint iterations it took -- What ESLint issues were fixed (brief summary) +When all pass, report: +- How many lint iterations it took +- What ESLint and Stylelint issues were fixed (brief summary) - Whether Prettier made any changes -- Confirmation that both `make lint` and `make format` now pass cleanly +- Confirmation that `make lint` and the three Prettier checks now pass cleanly diff --git a/.claude/agents/typecheck-validator.md b/.claude/agents/typecheck-validator.md index 975404025..599631bbb 100644 --- a/.claude/agents/typecheck-validator.md +++ b/.claude/agents/typecheck-validator.md @@ -10,14 +10,18 @@ model: sonnet You are a validation agent that runs TypeScript type checking and automatically fixes all errors, iterating until the check passes cleanly. +Run every command below from the repository root. + ## Workflow ### 1. Run type checking via the Makefile ```bash -cd /Users/administrator/Documents/PolicyEngine/policyengine-app-v2 && make typecheck 2>&1 +make typecheck 2>&1 ``` +This runs `tsc --noEmit` in `app/`, `website/` and `calculator-app/`, the same three checks as CI's Type checking job. `make` stops at the first project with errors, so a project after it is only checked once the ones before it pass. + ### 2. If the check passes with no errors, report success and stop. ### 3. If there are errors, fix them: @@ -36,7 +40,7 @@ For each type error reported: ### 4. Re-run type checking ```bash -cd /Users/administrator/Documents/PolicyEngine/policyengine-app-v2 && make typecheck 2>&1 +make typecheck 2>&1 ``` ### 5. Repeat steps 3-4 until there are zero errors. diff --git a/.claude/commands/eslint.md b/.claude/commands/eslint.md index 8c3255e58..df42e273f 100644 --- a/.claude/commands/eslint.md +++ b/.claude/commands/eslint.md @@ -1,10 +1,10 @@ --- -description: Run ESLint and optionally fix errors +description: Run ESLint and Stylelint and optionally fix errors allowed-tools: Bash --- -Run `npm run eslint` from the app directory. +Run `make lint` from the repository root. It runs ESLint in `app/`, `website/` and `calculator-app/`, and Stylelint on `app/`'s CSS, the same linters as CI's Lint and format job. ESLint fails on warnings as well as errors (`--max-warnings 0`). `make` stops at the first linter that fails, so after fixing its errors, run it again to check the rest. -If there are ESLint errors, ask me if I should fix them. If I say yes, fix all the errors and keep running eslint until there are no more errors. If I say no, do nothing. +If there are ESLint or Stylelint errors, ask me if I should fix them. If I say yes, fix all the errors and keep running `make lint` until there are no more errors. If I say no, do nothing. After everything is done, return to the original directory. diff --git a/.claude/commands/finalize-pr.md b/.claude/commands/finalize-pr.md index 4c511e699..9a7d86a7f 100644 --- a/.claude/commands/finalize-pr.md +++ b/.claude/commands/finalize-pr.md @@ -1,5 +1,5 @@ --- -description: Run all code quality checks (typecheck, eslint, prettier) with progress tracking +description: Run all code quality checks (type checks, ESLint and Stylelint, Prettier) with progress tracking --- Run the following slash commands in order, showing progress after each: @@ -8,6 +8,6 @@ Run the following slash commands in order, showing progress after each: 2. Run `/typecheck` 3. Say "✅ Typecheck complete (1/3 tasks completed)" 4. Run `/eslint` -5. Say "✅ ESLint complete (2/3 tasks completed)" +5. Say "✅ ESLint and Stylelint complete (2/3 tasks completed)" 6. Run `/prettier` -7. Say "✅ All tasks complete (3/3) - PR is ready! 🎉" +7. Say "✅ All tasks complete (3/3). CI also runs the tests (`make test`) and the build (`make build`)." diff --git a/.claude/commands/next-phase.md b/.claude/commands/next-phase.md index 6e1bff0a3..e14316c7c 100644 --- a/.claude/commands/next-phase.md +++ b/.claude/commands/next-phase.md @@ -6,13 +6,13 @@ Read the current plan from `.claude-plans/calculation-system-refactor.md` and do 1. Identify the current phase and move to the next phase 2. Execute all tasks in the next phase -3. Write or update unit tests for the code changes made in this phase, following the conventions in `app/src/tests/CLAUDE.md`: +3. Write or update unit tests for the code changes made in this phase, following the conventions in `docs/engineering/skills/testing.md`: - Place tests in `app/src/tests/unit/` mirroring the source structure - Use Given-When-Then naming pattern - Import from `@test-utils` not `@testing-library/react` - Use accessibility selectors (getByRole, etc.) - Mock Plotly if needed -4. Ensure all mocks and test data have been moved to `app/src/tests/fixtures/` following the structure in CLAUDE.md +4. Ensure all mocks and test data have been moved to `app/src/tests/fixtures/` following the structure in `docs/engineering/skills/testing.md` 5. Update the plan file to mark this phase as complete 6. Provide a brief summary of: - What was completed in this phase diff --git a/.claude/commands/prettier.md b/.claude/commands/prettier.md index 6c8c6635a..039f5b14b 100644 --- a/.claude/commands/prettier.md +++ b/.claude/commands/prettier.md @@ -3,4 +3,4 @@ description: Run Prettier to format code allowed-tools: Bash --- -Run `npm run prettier:write` from the app directory, then return to the original directory. +Run `make format` from the repository root, then return to the original directory. It runs Prettier in write mode in `app/`, `website/` and `calculator-app/`, which CI's Lint and format job checks. diff --git a/.claude/commands/typecheck.md b/.claude/commands/typecheck.md index d8102643d..52135ac1d 100644 --- a/.claude/commands/typecheck.md +++ b/.claude/commands/typecheck.md @@ -3,7 +3,7 @@ description: Run TypeScript type checking and optionally fix errors allowed-tools: Bash --- -Run `npm run typecheck` from the app directory. +Run `make typecheck` from the repository root. It runs `bun run typecheck` (`tsc --noEmit`) in `app/`, `website/` and `calculator-app/`, the same three checks as CI's Type checking job. `make` stops at the first project with errors, so after fixing them, run it again to check the projects after it. If there are type errors, ask me if I should fix them. If I say yes, fix all the errors and keep running typecheck until there are no more errors. If I say no, do nothing. diff --git a/.claude/commands/write-unit-tests.md b/.claude/commands/write-unit-tests.md index 8fe17166e..6c6b0a91c 100644 --- a/.claude/commands/write-unit-tests.md +++ b/.claude/commands/write-unit-tests.md @@ -7,7 +7,7 @@ Write unit tests for the files or features described by the user. Follow these s ## Step 1: Read the test guidelines -Read `app/src/tests/CLAUDE.md` in full. All tests MUST follow these conventions: +Read `docs/engineering/skills/testing.md` in full. All tests MUST follow these conventions: - Import `render`, `screen`, `userEvent` from `@test-utils` (not `@testing-library/react`) - Import `describe`, `test`, `expect`, `vi` from `vitest` - Use Given-When-Then naming: `test('given X then Y', ...)` diff --git a/docs/engineering/skills/repository-guidance.md b/docs/engineering/skills/repository-guidance.md index 57a31ce4a..c219b3f5d 100644 --- a/docs/engineering/skills/repository-guidance.md +++ b/docs/engineering/skills/repository-guidance.md @@ -125,5 +125,5 @@ No route embeds a GitHub Pages site as a whole page any more: the 2025 year in r 1. Run `make format` to format `app/`, `website/` and `calculator-app/` with Prettier. `app/` has its own Prettier config; `website/` and `calculator-app/` use Prettier's defaults. 2. Run `make lint` to run ESLint (and Stylelint in `app/`) in all three. -3. Run `make typecheck` to type-check all three, and `make test` to run the `app/` and `website/` tests (`calculator-app/` has none). These match CI's type checking and tests jobs. +3. Run `make typecheck` to type-check all three, and `make test` to run the `app/` and `website/` tests (`calculator-app/` has none). These match CI's type checking and tests jobs. They don't match CI's Node version, though. vitest's launcher starts with `#!/usr/bin/env node`, so `bun run test` runs the tests on Node, not Bun. CI's Tests job pins Node 26 (in `pr.yaml` and `push.yaml`) because `Intl` number formatting differs between Node versions. `make test` uses whichever `node` is first on your PATH, and `app/.nvmrc` says v24.3.0, so a test that checks formatted numbers can pass locally and fail in CI, or the reverse. Run the tests on Node 26 to match CI. 4. CI fails on ESLint warnings as well as errors (`--max-warnings 0`), and on files Prettier would reformat, so fix all of them. diff --git a/docs/engineering/skills/testing.md b/docs/engineering/skills/testing.md index 2734ee7a5..8d03b8fa7 100644 --- a/docs/engineering/skills/testing.md +++ b/docs/engineering/skills/testing.md @@ -2,13 +2,27 @@ ## Test commands -- Run tests: `bun run vitest` +From the repository root, `make test` runs the `app/` tests and then the `website/` tests, the same two suites as CI's Tests job. `calculator-app/` has no test suite of its own; tests of its code run in the `app/` suite (for example, under `app/src/tests/unit/calculator/`). + +In `app/`: + +- Run tests: `bun run vitest` (or `bun run test`) +- Run specific files: `bun run vitest -- ` - Watch mode: `bun run vitest:watch` -- With coverage: `bun run vitest -- --coverage` -- Full check: `bun run check` (includes type checking, linting, and formatting verification) +- Coverage: `bun run vitest -- --coverage` fails with a missing dependency error, because `@vitest/coverage-v8` isn't installed. Add it as a dev dependency before measuring coverage. +- Full check: `bun run check` runs the type check, ESLint and the Prettier check. It doesn't run Stylelint or the tests; `make lint` and `make test` do. + +In `website/`: + +- Run tests: `bun run test` +- Watch mode: `bun run test:watch` + +vitest runs on Node, not Bun, so results can depend on your Node version. See step 3 of "Before committing" in `repository-guidance.md`. ## Writing tests +Unless an item says otherwise, these conventions are for `app/`. `website/` tests live in `website/src/__tests__/`, import from `@testing-library/react` directly (`website/` has no `@test-utils`) and share the setup in `website/src/__tests__/setup.ts`. + 1. **Always use @test-utils**: Import `render` and other utilities from `@test-utils`, not `@testing-library/react` ```typescript From 342c31ddabf8e54f4e579378dc942ed90247ea59 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Tue, 20 Oct 2026 16:52:14 -0400 Subject: [PATCH 2/3] Address review nits: README checks, bun in write-unit-tests - README's development workflow listed only `make test && make lint`; it now lists all four checks CI runs and points to "Before committing". The Testing section also shows `make typecheck`. - /write-unit-tests runs the modified tests with `bun run vitest --`, as testing.md does, instead of npx. - repository-guidance.md says `bun run test` runs vitest on the `node` first on PATH, rather than just "on Node". Co-Authored-By: Claude Opus 5.5 --- .claude/commands/write-unit-tests.md | 2 +- README.md | 8 +++++++- docs/engineering/skills/repository-guidance.md | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/.claude/commands/write-unit-tests.md b/.claude/commands/write-unit-tests.md index 6c6b0a91c..c6f0ba0ad 100644 --- a/.claude/commands/write-unit-tests.md +++ b/.claude/commands/write-unit-tests.md @@ -58,7 +58,7 @@ If any violations are found, fix them by moving the code to the appropriate fixt Run the specific test files that were created or modified using: ``` -cd app && npx vitest run +cd app && bun run vitest -- ``` Do NOT run the entire test suite. Only run the tests you wrote or modified. diff --git a/README.md b/README.md index 582272973..c6707ec96 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,12 @@ Run linters: make lint ``` +Type-check: + +```bash +make typecheck +``` + Format code: ```bash @@ -116,7 +122,7 @@ Vercel builds and deploys two projects from this repository: `policyengine-websi 1. Create a new branch from main 2. Make your changes -3. Run tests and linting: `make test && make lint` +3. Run `make format`, `make lint`, `make typecheck` and `make test`, the checks CI runs (see "Before committing" in `docs/engineering/skills/repository-guidance.md`) 4. Create a pull request 5. Once approved and merged, changes deploy automatically diff --git a/docs/engineering/skills/repository-guidance.md b/docs/engineering/skills/repository-guidance.md index c219b3f5d..1ce984502 100644 --- a/docs/engineering/skills/repository-guidance.md +++ b/docs/engineering/skills/repository-guidance.md @@ -125,5 +125,5 @@ No route embeds a GitHub Pages site as a whole page any more: the 2025 year in r 1. Run `make format` to format `app/`, `website/` and `calculator-app/` with Prettier. `app/` has its own Prettier config; `website/` and `calculator-app/` use Prettier's defaults. 2. Run `make lint` to run ESLint (and Stylelint in `app/`) in all three. -3. Run `make typecheck` to type-check all three, and `make test` to run the `app/` and `website/` tests (`calculator-app/` has none). These match CI's type checking and tests jobs. They don't match CI's Node version, though. vitest's launcher starts with `#!/usr/bin/env node`, so `bun run test` runs the tests on Node, not Bun. CI's Tests job pins Node 26 (in `pr.yaml` and `push.yaml`) because `Intl` number formatting differs between Node versions. `make test` uses whichever `node` is first on your PATH, and `app/.nvmrc` says v24.3.0, so a test that checks formatted numbers can pass locally and fail in CI, or the reverse. Run the tests on Node 26 to match CI. +3. Run `make typecheck` to type-check all three, and `make test` to run the `app/` and `website/` tests (`calculator-app/` has none). These match CI's type checking and tests jobs. They don't match CI's Node version, though. vitest's launcher starts with `#!/usr/bin/env node`, so `bun run test` runs the tests on the `node` first on your PATH, not on Bun. CI's Tests job pins Node 26 (in `pr.yaml` and `push.yaml`) because `Intl` number formatting differs between Node versions. `make test` uses your local Node, and `app/.nvmrc` says v24.3.0, so a test that checks formatted numbers can pass locally and fail in CI, or the reverse. Run the tests on Node 26 to match CI. 4. CI fails on ESLint warnings as well as errors (`--max-warnings 0`), and on files Prettier would reformat, so fix all of them. From d930354c57a592e6d5a7cc9ae153ed741d86d850 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Tue, 20 Oct 2026 16:53:24 -0400 Subject: [PATCH 3/3] Say make format writes and CI also builds in README workflow Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index c6707ec96..5f1fa64f2 100644 --- a/README.md +++ b/README.md @@ -122,7 +122,7 @@ Vercel builds and deploys two projects from this repository: `policyengine-websi 1. Create a new branch from main 2. Make your changes -3. Run `make format`, `make lint`, `make typecheck` and `make test`, the checks CI runs (see "Before committing" in `docs/engineering/skills/repository-guidance.md`) +3. Format with `make format`, then run `make lint`, `make typecheck` and `make test`. CI checks formatting and runs the same linters, type checks and tests, plus a build (see "Before committing" in `docs/engineering/skills/repository-guidance.md`) 4. Create a pull request 5. Once approved and merged, changes deploy automatically