From 6662a620528ac716439fed5ea5a498aa0ba29bef Mon Sep 17 00:00:00 2001 From: Maxwell Murphy Date: Tue, 6 Oct 2026 15:46:32 -0700 Subject: [PATCH 01/21] docs(agents): drop no-ops from AGENTS.md and start CODING_STANDARDS.md. - issue-tracker.md: drop the generic gh command list and the git remote note; gh does both by default. Keep the skill-term mappings. - triage-labels.md: the table mapped every role to itself. One line now says the labels match the role names. - domain.md: rewrite as one trigger per doc. The glossary-vocabulary and ADR-conflict rules move to CODING_STANDARDS.md as checkable rules, so code review enforces them. - AGENTS.md: drop the repo-root test row (package.json has it), the ~1s example run, and the CI step list (ci:verify has it). Keep the bun test hang, the clean lint baseline, and what pre-commit skips. Pointers now lead with when to read them; add one to the standards. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 33 ++++++++++----------------------- CODING_STANDARDS.md | 10 ++++++++++ docs/agents/domain.md | 20 +++++--------------- docs/agents/issue-tracker.md | 22 +++------------------- docs/agents/triage-labels.md | 16 ++-------------- 5 files changed, 30 insertions(+), 71 deletions(-) create mode 100644 CODING_STANDARDS.md diff --git a/AGENTS.md b/AGENTS.md index 0ae94ecc..40c71550 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,15 +2,19 @@ ### Issue tracker -GitHub Issues on eppicenter/sampledb3 (via `gh` CLI). See `docs/agents/issue-tracker.md`. +GitHub Issues on `eppicenter/sampledb3`. When a skill publishes or fetches an issue, see `docs/agents/issue-tracker.md`. ### Triage labels -Canonical triage roles mapped to GitHub labels; see `docs/agents/triage-labels.md`. +When a skill applies or reads a triage role, see `docs/agents/triage-labels.md`. ### Domain docs -`CONTEXT.md` (glossary) and `docs/adr/` (decisions) at repo root. See `docs/agents/domain.md`. +`CONTEXT.md` (glossary) and `docs/adr/` (decisions). When naming a domain concept or changing an area an ADR covers, see `docs/agents/domain.md`. + +### Coding standards + +When writing code, tests, issues, or proposals, or reviewing a change, follow `CODING_STANDARDS.md`. ### Package docs @@ -25,28 +29,11 @@ Canonical triage roles mapped to GitHub labels; see `docs/agents/triage-labels.m |---------|---------|--------| | `@sampledb/web` | `cd packages/web && bun run test -- ` | **Vitest** (jsdom, `vi.mock`) | | `@sampledb/api` | `cd packages/api && bun test src/...` | **Bun** (needs `bun:sqlite`) | -| repo root | `bun run test` | both, via package scripts | - -**Run web tests with `bun run test` (Vitest).** It runs `vitest run`, which exits on its own, so piping to `tail` or `grep` is safe. Bun's own runner (`bun test`) hangs on React component tests. - -Example (completes in ~1s): -```bash -cd packages/web && bun run test -- src/components/__tests__/BulkImportFlow.test.tsx -``` +Run web tests with `bun run test`, never `bun test`: Bun's own runner hangs on React component tests. `vitest run` exits on its own, so piping it to `tail` or `grep` is safe. ### Pre-commit hooks and CI -The `Checks` workflow runs `bun run ci:verify` on every PR: typecheck, lint, build, unit tests, and the docs stale-claims check. Lint is clean, so any lint error is yours. - -Install repo hooks once per clone: - -```bash -bun run setup:hooks -``` - -The pre-commit hook runs `typecheck` on `@sampledb/contract`, `@sampledb/api`, and/or `@sampledb/web` when staged files touch those packages (or Docker/build config), and lints the staged API/web source files. To mirror CI fully before pushing: +The `Checks` workflow runs `bun run ci:verify` on every PR. Lint is clean, so any lint error is yours. -```bash -bun run ci:verify -``` +Install the repo hooks once per clone with `bun run setup:hooks`. The pre-commit hook only typechecks the packages your staged files touch and lints the staged API/web files; it runs no tests or build. Run `bun run ci:verify` before pushing to mirror CI. diff --git a/CODING_STANDARDS.md b/CODING_STANDARDS.md new file mode 100644 index 00000000..fc1115ab --- /dev/null +++ b/CODING_STANDARDS.md @@ -0,0 +1,10 @@ +# Coding standards + +## Domain vocabulary + +- Name a domain concept with the term `CONTEXT.md` defines for it, in code, test names, issue titles, and refactor proposals. Terms the glossary lists under _Avoid_ are synonyms to replace with the defined term. +- When a concept has no glossary entry, say so in the issue, PR, or proposal before introducing a term for it. + +## ADRs + +- A change that contradicts an ADR in `docs/adr/` names the ADR and gives the reason to reopen it, in the PR or proposal. For example: _Contradicts ADR-0006 (unified container inbound write), but worth reopening because…_ diff --git a/docs/agents/domain.md b/docs/agents/domain.md index 1005bb42..bc356991 100644 --- a/docs/agents/domain.md +++ b/docs/agents/domain.md @@ -1,17 +1,7 @@ -# Domain Docs +# Domain docs -Read these before naming or changing domain concepts: +- **Naming a domain concept**: read `CONTEXT.md`, the glossary of entity names, hierarchies, and scope. +- **Changing an area a design decision covers**: read the ADRs in `docs/adr/` that touch it. +- **Describing a user-facing workflow**: read the user guide in `packages/docs/`. -- **`CONTEXT.md`** is the domain glossary: entity names, hierarchies, and scope. -- **`docs/adr/`** records design decisions. Read the ADRs that touch the area you're changing. -- **`packages/docs/`** holds the user guide, for user-facing workflow detail. - -## Use the glossary's vocabulary - -When your output names a domain concept (an issue title, a test name, a refactor proposal), use the term as `CONTEXT.md` defines it, not a synonym it lists under _Avoid_. If the concept isn't in the glossary, say so rather than inventing a term. - -## Flag ADR conflicts - -If your change contradicts an ADR, say so explicitly instead of silently overriding it: - -> _Contradicts ADR-0006 (unified container inbound write), but worth reopening because…_ +`CODING_STANDARDS.md` has the rules for using the glossary's terms and for contradicting an ADR. diff --git a/docs/agents/issue-tracker.md b/docs/agents/issue-tracker.md index cce77ecb..41b01aa0 100644 --- a/docs/agents/issue-tracker.md +++ b/docs/agents/issue-tracker.md @@ -1,22 +1,6 @@ # Issue tracker: GitHub -Issues and PRDs for this repo live as GitHub issues. Use the `gh` CLI for all operations. +Issues and PRDs for this repo are GitHub issues on `eppicenter/sampledb3`, worked with the `gh` CLI. -## Conventions - -- **Create an issue**: `gh issue create --title "..." --body "..."`. Use a heredoc for multi-line bodies. -- **Read an issue**: `gh issue view --comments`, filtering comments by `jq` and also fetching labels. -- **List issues**: `gh issue list --state open --json number,title,body,labels,comments --jq '[.[] | {number, title, body, labels: [.labels[].name], comments: [.comments[].body]}]'` with appropriate `--label` and `--state` filters. -- **Comment on an issue**: `gh issue comment --body "..."` -- **Apply / remove labels**: `gh issue edit --add-label "..."` / `--remove-label "..."` -- **Close**: `gh issue close --comment "..."` - -Infer the repo from `git remote -v` — `gh` does this automatically when run inside a clone. - -## When a skill says "publish to the issue tracker" - -Create a GitHub issue. - -## When a skill says "fetch the relevant ticket" - -Run `gh issue view --comments`. +- A skill's "publish to the issue tracker" means `gh issue create`. +- A skill's "fetch the relevant ticket" means `gh issue view --comments`, so the comments come with it. diff --git a/docs/agents/triage-labels.md b/docs/agents/triage-labels.md index b716855d..caec6c7a 100644 --- a/docs/agents/triage-labels.md +++ b/docs/agents/triage-labels.md @@ -1,15 +1,3 @@ -# Triage Labels +# Triage labels -The skills speak in terms of five canonical triage roles. This file maps those roles to the actual label strings used in this repo's issue tracker. - -| Label in mattpocock/skills | Label in our tracker | Meaning | -| -------------------------- | -------------------- | ---------------------------------------- | -| `needs-triage` | `needs-triage` | Maintainer needs to evaluate this issue | -| `needs-info` | `needs-info` | Waiting on reporter for more information | -| `ready-for-agent` | `ready-for-agent` | Fully specified, ready for an AFK agent | -| `ready-for-human` | `ready-for-human` | Requires human implementation | -| `wontfix` | `wontfix` | Will not be actioned | - -When a skill mentions a role (e.g. "apply the AFK-ready triage label"), use the corresponding label string from this table. - -Edit the right-hand column to match whatever vocabulary you actually use. +Each of the five canonical triage roles is a GitHub label with the same name: `needs-triage`, `needs-info`, `ready-for-agent`, `ready-for-human`, `wontfix`. Apply the role's name as the label. From 3efb4941a3f3209bf55e2a3ce7ad888c5ad0e557 Mon Sep 17 00:00:00 2001 From: Maxwell Murphy Date: Tue, 6 Oct 2026 15:49:21 -0700 Subject: [PATCH 02/21] docs: move test and database rules into CODING_STANDARDS.md. - CODING_STANDARDS.md gains Database, Tests, API tests, and Web tests sections: withWriteTransaction, DDL only in migrations, one database entry point, keep assertions strict, setupTestDatabase/factories/ setupAuthenticatedRouteTest, checkInUse cases, awaited custom render, domain-module mocks, semantic queries, invalidation asserts. The READMEs keep the helpers and examples and point at the rules. - Test READMEs: cut the stale counts (431 tests, a 60% threshold bunfig does not set), the directory listings, generic best practices, the repo-root and coverage sections that package.json already holds, and the route-handler example (no route has a default export). Fix the setupTestDatabase description (it opens through openOperationalDatabase), show render as async with all its providers, and note that API tests import 'vitest' under Bun. - Web README and vitest.config.ts: drop the Vitest stack overflow note. A full run passes 909 tests and exits without it. - AGENTS.md: the bun test hang lives only here; runner details move behind the README pointers. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 14 +- CODING_STANDARDS.md | 24 ++++ packages/api/src/__tests__/README.md | 192 ++++---------------------- packages/api/src/db/README.md | 6 +- packages/web/src/__tests__/README.md | 193 ++++----------------------- packages/web/vitest.config.ts | 15 --- 6 files changed, 80 insertions(+), 364 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 40c71550..f0d03ea6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,19 +18,15 @@ When writing code, tests, issues, or proposals, or reviewing a change, follow `C ### Package docs -- Writing an API test (test database, factories, authenticated routes): `packages/api/src/__tests__/README.md` -- Writing a web test (render helper, API mocks, React Query hooks): `packages/web/src/__tests__/README.md` -- Changing the schema, a migration, or a transaction: `packages/api/src/db/README.md` +- Writing an API test (runner, test database, factories, authenticated routes): `packages/api/src/__tests__/README.md` +- Writing a web test (render helper, API mocks, response shapes, React Query hooks): `packages/web/src/__tests__/README.md` +- Changing the schema or adding a migration: `packages/api/src/db/README.md` - Finding the code behind a UI label: `packages/web/src/components/Sidebar.tsx` (label → path) → `packages/web/src/App.tsx` (path → page) → the page's hooks and `lib/` → `packages/api/src/routes` → `packages/api/src/lib` ### Tests -| Package | Command | Runner | -|---------|---------|--------| -| `@sampledb/web` | `cd packages/web && bun run test -- ` | **Vitest** (jsdom, `vi.mock`) | -| `@sampledb/api` | `cd packages/api && bun test src/...` | **Bun** (needs `bun:sqlite`) | - -Run web tests with `bun run test`, never `bun test`: Bun's own runner hangs on React component tests. `vitest run` exits on its own, so piping it to `tail` or `grep` is safe. +- Web: `cd packages/web && bun run test -- `. Use `bun run test`, never `bun test`: Bun's own runner hangs on React component tests. Vitest exits on its own, so piping it to `tail` or `grep` is safe. +- API: `cd packages/api && bun test src/...`. ### Pre-commit hooks and CI diff --git a/CODING_STANDARDS.md b/CODING_STANDARDS.md index fc1115ab..8b1db1fd 100644 --- a/CODING_STANDARDS.md +++ b/CODING_STANDARDS.md @@ -8,3 +8,27 @@ ## ADRs - A change that contradicts an ADR in `docs/adr/` names the ADR and gives the reason to reopen it, in the PR or proposal. For example: _Contradicts ADR-0006 (unified container inbound write), but worth reopening because…_ + +## Database + +- A write of several statements that `await`s runs inside `withWriteTransaction` from `packages/api/src/db/write-transaction.ts`. Drizzle's bun-sqlite `db.transaction` commits at the callback's first `await`, so its callback stays synchronous. +- Every schema change is a numbered migration plus a refreshed `initial_schema.sql`, made with the recipe in `packages/api/src/db/README.md`. DDL lives only in migrations and the snapshot. +- Code and tests open a database through `openOperationalDatabase`. In tests that means `setupTestDatabase()`. + +## Tests + +- A test asserts the expected behaviour. When it fails, fix the code under test and keep the assertion as strict as before. A new test may land failing if it states the expected behaviour. + +### API tests + +- Each test gets its own database from `setupTestDatabase()` and closes it with `cleanupTestDatabase()` in `afterEach`. +- Seed rows with the factories in `packages/api/src/__tests__/helpers/factories.ts`. +- A route behind session auth is tested through `setupAuthenticatedRouteTest`. +- A CRUD route with `checkInUse` has a delete test for both the in-use and the free case. + +### Web tests + +- Components render through `await render(...)` from `packages/web/src/__tests__/helpers/render`. +- A test mocks the domain module the code imports (`lib/api/`) with `createMockedDomainModule`, and an override replaces a whole `*Api` object. +- Query elements by role or label (`getByRole`, `getByLabelText`), and drive interactions with `@testing-library/user-event`. +- A mutation hook's test asserts which queries it invalidates. diff --git a/packages/api/src/__tests__/README.md b/packages/api/src/__tests__/README.md index 39dd4f8e..b64ba76e 100644 --- a/packages/api/src/__tests__/README.md +++ b/packages/api/src/__tests__/README.md @@ -1,50 +1,21 @@ -# Backend Testing Guide +# Backend testing guide -## Setup +The rules for API tests are under Tests in `CODING_STANDARDS.md`. This guide covers the helpers. -Tests use **Bun's test runner** (not Vitest) so that route and integration tests can use `bun:sqlite` in db-setup. Coverage uses **Bun's built-in coverage** (`bun test --coverage`) so all 431 tests are included in the report. Run tests with: +## Runner -```bash -bun test src # Run once (431 tests) -bun test --watch src # Watch mode -bun test:ui # Vitest UI (for lib-only tests; route tests require Bun) -bun test:coverage # Run tests with coverage (Bun; report in coverage/) -``` - -Coverage is configured in `packages/api/bunfig.toml` (reporters: text, lcov). Baseline has been raised with lib and route tests; the target is 90% lines. Optional `coverageThreshold` in bunfig.toml can be enabled and raised in steps (e.g. 60 → 75 → 90) as coverage improves. - -## Test Structure - -- `__tests__/helpers/` - Test utilities - - `db-setup.ts`: In-memory database setup and cleanup - - `factories.ts`: Test data factories for creating test entities - - `authenticated-route-test.ts`: Authenticated route test adapter (DB, login, `createRequestApp`, `request`) - - `test-client.ts`: Hono test client utilities and assertion helpers -- `__tests__/fixtures/` - Test data fixtures -- `routes/__tests__/` - Route handler tests -- `lib/__tests__/` - Library/utility function tests (e.g. plate-csv, plate-scan-validation, defaults, container-move, derivations, error-utils, logger, cache, openapi) -- `lib/controls/__tests__/` - Control domain libs (batch-with-specimens, batch-csv-validate, definition-summary, batch-delete, strain-composition, control-read) -- `middleware/__tests__/` - Middleware tests (e.g. auth, rate-limit) - -## Test Database - -Tests use an in-memory SQLite database created in `db-setup.ts`. The `setupTestDatabase()` function: -- Creates a fresh in-memory database for each test suite -- Applies `packages/api/initial_schema.sql` (same Drizzle-generated snapshot production uses via `applyInitialSchema`) -- Returns both the Drizzle database instance and SQLite instance for cleanup - -Regenerate the snapshot after schema changes: `bunx drizzle-kit generate` in `packages/api`, then copy the generated SQL to `initial_schema.sql` (see `drizzle.config.ts`). +Tests run on Bun's runner because route and integration tests need `bun:sqlite`. Test files import `describe`, `it`, and `expect` from `'vitest'`; Bun maps that import to `bun:test`. `bun run test:ui` starts Vitest, which only runs tests that never touch `bun:sqlite`. -Integrity tests that seed orphan rows use `withForeignKeysDisabled` from `src/db/apply-initial-schema.ts` so fixtures match production constraints while allowing deliberate corruption. +## Test database -Example usage: +`setupTestDatabase()` in `helpers/db-setup.ts` opens an in-memory database through `openOperationalDatabase(':memory:')`, the production path, so it gets `initial_schema.sql` plus any pending migrations. To change the schema, follow `packages/api/src/db/README.md`. ```typescript import { setupTestDatabase, cleanupTestDatabase } from '../../__tests__/helpers/db-setup' describe('My API', () => { let testDb: Database - let sqlite: any + let sqlite: SQLiteDatabase beforeEach(async () => { const setup = await setupTestDatabase() @@ -52,40 +23,25 @@ describe('My API', () => { sqlite = setup.sqlite }) - afterEach(() => { - if (sqlite) { - cleanupTestDatabase(sqlite) - } - }) + afterEach(() => cleanupTestDatabase(sqlite)) }) ``` -## Test Data Factories +Integrity tests that seed orphan rows wrap the inserts in `withForeignKeysDisabled` from `src/db/apply-initial-schema.ts`. + +## Factories -Use factories from `factories.ts` to create test data: +`helpers/factories.ts` exports a `createTest*` function per entity, taking the database and the fields to set: ```typescript -import { createTestState, createTestStorageContainer } from '../../__tests__/helpers/factories' +import { createTestStudy } from '../../__tests__/helpers/factories' -const testState = await createTestState(testDb, { name: 'Test State' }) +const study = await createTestStudy(testDb, { title: 'Test Study', shortCode: 'TS' }) ``` -Available factories: -- `createTestTag` -- `createTestStorageType` -- `createTestSpecimenType` -- `createTestStrain` -- `createTestStorageContainer` -- `createTestLocation` -- `createTestControlDefinition` -- `createTestSpecimen` -- `createTestUnit` -- `createTestStudy` -- `createTestStudySubject` - ## Authenticated route tests -For route handlers behind session auth, use `setupAuthenticatedRouteTest` instead of duplicating DB setup, settings, user creation, and login: +`setupAuthenticatedRouteTest` creates the database, auth settings, users, and a logged-in session, then sends requests with that session's cookie: ```typescript import { setupAuthenticatedRouteTest, type AuthenticatedRouteTestContext } from '../../__tests__/helpers/authenticated-route-test' @@ -100,7 +56,7 @@ describe('Search API', () => { settings: { pagination: true }, // when list routes use validateLimit additionalUsers: [{ key: 'member', email: 'member@test.com', role: 'member' }], seed: async ({ db }) => { /* optional fixtures before login */ }, - mount: (app, { db, sqlite }) => { + mount: (app, { db }) => { app.route('/api/search', createSearchRoutes(db)) }, }) @@ -114,120 +70,22 @@ describe('Search API', () => { }) it('returns 401 without auth', async () => { - const res = await authenticatedRequest(ctx.createRequestApp(), '/api/search?q=test', { method: 'GET' }) + const res = await ctx.request('/api/search?q=test', { cookie: null }) expect(res.status).toBe(401) }) }) ``` -Per-test routes (e.g. CRUD factory): omit `mount` and use `ctx.createRequestApp((app) => app.route('/api/tags', routes))`. - -### Integration workflows - -`__tests__/integration/` uses the same adapter for cross-route HTTP workflows (e.g. derivation create + list, collection move). Lib tests in `lib/__tests__/` still own pure logic; integration tests assert the full HTTP seam. - -## Writing Tests - -### Testing Routes with CRUD Factory - -For routes using `createCrudRoutes`, create the routes dynamically in tests with the test database: - -```typescript -import { createCrudRoutes } from '../../lib/crud-routes' -import { z } from 'zod' - -describe('States API', () => { - let statesRoutes: Hono - - beforeEach(async () => { - const { db } = await setupTestDatabase() - - const createSchema = z.object({ - name: z.string().min(1), - }) - - statesRoutes = createCrudRoutes({ - table: state, - database: db, - entityName: 'State', - pluralName: 'states', - singularName: 'state', - createSchema, - }) - }) -}) -``` - -### Testing Route Handlers - -```typescript -import { describe, it, expect, beforeEach } from 'vitest' -import { testClient } from 'hono/testing' -import { Hono } from 'hono' -import myRoutes from '../my-routes' - -describe('My API', () => { - let app: Hono - - beforeEach(() => { - app = new Hono() - app.route('/api/my-route', myRoutes) - }) - - it('should do something', async () => { - const client = testClient(app) - const res = await client.api['my-route'].$get() - expect(res.status).toBe(200) - const data = await res.json() - expect(data).toHaveProperty('items') - }) -}) -``` - -## Coverage - -Coverage thresholds are set to 60% for: -- Statements -- Branches -- Functions -- Lines - -Run coverage reports: - -```bash -bun test:coverage -``` - -Coverage reports are generated in: -- `coverage/` directory (HTML, JSON, text formats) - -## Test Utilities - -### `test-client.ts` Helpers - -- `createTestClient(app)`: Creates a Hono test client -- `getResponseData(response)`: Unpacks `{ data }` from standardized API responses -- `authenticatedRequest(app, path, options)`: HTTP request with session cookie -- `loginAndGetCookie` / `createAuthenticatedClientWrapper`: Session helpers for route tests - -## Running Tests From Repo Root - -From the repository root you can run all package tests and coverage: - -```bash -bun run test # Run API and web tests -bun run test:coverage # Run API and web tests with coverage -``` +`ctx.cookies[key]` holds each additional user's session cookie. To mount routes per test, as the `createCrudRoutes` tests do, omit `mount` and call `ctx.createRequestApp((app) => app.route('/api/tags', routes))`. -## New Tests and Failing Behavior +`__tests__/integration/` uses the same adapter for workflows that cross routes, such as derivation create then list. Pure logic stays in `lib/__tests__/`. -New tests are allowed to **fail** initially if they correctly express expected behavior. Follow-up work will update the implementation to satisfy the tests. Do not remove or relax assertions solely to make tests pass; fix the code under test instead. +## Other helpers -## Best Practices +`helpers/test-client.ts`: -1. **Always use test database**: Use `setupTestDatabase()` to get a fresh database for each test -2. **Clean up**: Always call `cleanupTestDatabase()` in `afterEach` -3. **Use factories**: Use test data factories instead of manual inserts -4. **Test edge cases**: Include tests for validation errors, 404s, duplicates, etc. -5. **Test "in use" scenarios**: For routes with `checkInUse`, test both in-use and not-in-use cases +- `getResponseData(response)` unpacks `{ data }` from a standard API response. +- `authenticatedRequest(app, path, options)` sends a request with a session cookie. +- `loginAndGetCookie` and `createAuthenticatedClientWrapper` manage sessions by hand. +`helpers/auth-helpers.ts` creates users and sessions and seeds the password, session, and pagination settings. diff --git a/packages/api/src/db/README.md b/packages/api/src/db/README.md index bbefac77..8d77afc6 100644 --- a/packages/api/src/db/README.md +++ b/packages/api/src/db/README.md @@ -8,9 +8,7 @@ See [ADR-0003: SQLite schema evolution](../../../docs/adr/0003-sqlite-schema-evo Use `openOperationalDatabase(optionalPath?)` from the client module. It resolves the file path, enables WAL, turns on foreign keys, runs schema evolution, and returns Drizzle + raw `bun:sqlite` handles. Importing the client module does **not** open a connection. -Multi-statement writes that `await` must use `withWriteTransaction` from `write-transaction.ts`. Drizzle's bun-sqlite `db.transaction(async () => …)` commits when the callback first yields; it does not await the returned Promise. ESLint rejects an `async` callback passed to `.transaction(...)`. - -Tests should use the same entry point (e.g. `openOperationalDatabase(':memory:')` via `setupTestDatabase()`). +The rules for transactions, DDL, and which entry point to open a database with are under Database in `CODING_STANDARDS.md`. ## Schema version @@ -38,8 +36,6 @@ Migration `001` is **only** for legacy files; fresh installs from the snapshot s 6. Refresh `initial_schema.sql` so **new** empty databases match the latest shape (merge generated SQL; keep `INSERT INTO schema_version` at `CURRENT_SCHEMA_VERSION`). `__tests__/initial-schema-snapshot.test.ts` fails until the snapshot holds every table and column in `schema.ts`. 7. Extend the legacy fixtures if the migration touches a table they don't create. Older-version tests in `__tests__/schema-evolution.test.ts` build minimal databases through `createTablesForLaterMigrations`, and `__tests__/client.test.ts` creates its own tables inline; both must contain every table and column a later migration alters. -Do **not** add inline DDL to `open.ts` or connection code. - ## Failures Each migration runs in a transaction. On failure, the transaction rolls back, `schema_version` is unchanged, and startup throws `SchemaMigrationError` with the migration number. Restore from backup or fix the database manually; there are no down-migrations. diff --git a/packages/web/src/__tests__/README.md b/packages/web/src/__tests__/README.md index 747bc19f..9138155e 100644 --- a/packages/web/src/__tests__/README.md +++ b/packages/web/src/__tests__/README.md @@ -1,76 +1,31 @@ -# Frontend Testing Guide +# Frontend testing guide -## Important: Known Vitest Issue +Tests run on Vitest with jsdom and React Testing Library. The rules for web tests are under Tests in `CODING_STANDARDS.md`. This guide covers the helpers. -**Stack Overflow During Worker Cleanup (Non-Blocking)** +`__tests__/setup.ts` runs before every file. It stubs `IntersectionObserver` and `localStorage` and mocks the `auth` and `settings` domain modules; override those per file as needed. -When running tests, you may see a `RangeError: Maximum call stack size exceeded` error after all tests complete. This is a **known non-blocking issue** with Vitest/tinypool during worker cleanup. +## Rendering -- **Impact**: None - all tests pass successfully before the error occurs -- **When it happens**: Only during worker cleanup/teardown, after all tests complete -- **Action required**: None - this can be safely ignored -- **Status**: All frontend tests pass; this is a cosmetic cleanup issue - -**Do not attempt to "fix" this error** - it's a known Vitest/tinypool bug that doesn't affect test results. - -## Setup - -Tests use Vitest + React Testing Library. **Do not use `bun test`** for this package — that invokes Bun's native runner, which lacks Vitest's `vi.importActual` / jsdom setup and can hang indefinitely on component tests. Use the npm scripts (Vitest) instead: - -```bash -bun run test # Run once (vitest run) -bun run test:watch # Watch mode -bun run test:ui # UI mode -bun run test:coverage # With coverage -``` - -## HTTP client mocks - -See [ADR 0002: HTTP client returns response bodies](../../../docs/adr/0002-http-client-response-unwrap.md). - -`lib/api/client` `api` methods resolve to the **response body**, not `AxiosResponse`. Domain mocks return body shapes directly (`{ user }`, `{ studies: [] }`), not `{ data: { … }, status, headers }`. - -Reference-data list helpers still return `{ data: T[], meta? }` — keep that envelope in mocks for `specimenTypesApi.list()` and similar. - -## Test Structure - -- `__tests__/helpers/` - Test utilities - - `render.tsx`: Custom render function with providers (QueryClient, BrowserRouter, ToastProvider) - - `mock-api.ts`: `createMockedDomainModule()` — `importActual` per `lib/api/` + per-test overrides - - `mock-api-templates.ts`: domain presets (`statisticsPageMock`, `studiesPageMock`, etc.) for common page/hook tests - - `setup.ts`: Global test setup (IntersectionObserver, in-memory localStorage; default mocks for `auth` and `settings`) -- `__tests__/fixtures/` - Test data fixtures -- `lib/__tests__/` - Lib unit tests (e.g. commands, constants, plate-filename-match, container-types, localUserHistory, hotkeys, error-logger) -- `components/__tests__/` - Component tests (e.g. BulkImportFlow, ContainerDerivationModal) -- `components/forms/__tests__/` - Form tests (SubjectForm, StudyForm, ControlDefinitionForm, SpecimenForm) -- `components/wizards/__tests__/` - Wizard step tests (e.g. BatchInfoStep) -- `hooks/__tests__/` - Custom hook tests -- `pages/__tests__/` - Page smoke tests - -## Test Utilities - -### Custom Render Function - -The `render` function from `helpers/render.tsx` automatically wraps components with: -- `QueryClientProvider` (React Query) -- `BrowserRouter` (React Router) +`render` from `helpers/render.tsx` wraps the component in a fresh `QueryClient` (no retries), a router, and the theme, user, and toast providers. It is async because it flushes the user provider's first fetch. Pass `initialEntries` to use a `MemoryRouter` at a given URL. ```typescript import { render, screen } from '../../__tests__/helpers/render' +import userEvent from '@testing-library/user-event' import MyComponent from '../MyComponent' -it('renders component', () => { - render() - expect(screen.getByText('Hello')).toBeInTheDocument() +it('handles a click', async () => { + const user = userEvent.setup() + await render() + await user.click(screen.getByRole('button', { name: 'Save' })) + expect(screen.getByText('Saved')).toBeInTheDocument() }) ``` -### Mocking API Calls +## Mocking the API -Mock the **domain module** your component imports (`lib/api/studies`, `lib/api/client`, etc.). `setup.ts` provides default `auth` and `settings` (table view) mocks; override per file as needed. +Production code and tests import from the domain modules under `src/lib/api/`; there is no barrel. `createMockedDomainModule(domain, overrides)` in `helpers/mock-api.ts` loads the real module and swaps in the `*Api` objects you pass: ```typescript -import { vi } from 'vitest' import { studiesApi } from '../../lib/api/studies' vi.mock('../../lib/api/studies', async () => { @@ -86,18 +41,11 @@ vi.mocked(studiesApi.list).mockResolvedValue({ }) ``` -Replace whole `*Api` objects in overrides (do not patch single methods on the real object). For raw axios usage, mock `lib/api/client` and pass `{ default: { get: vi.fn() } }`: +Import `createMockedDomainModule` with a dynamic `import()` inside the factory. Vitest hoists `vi.mock` above the file's imports, so a top-level import hits a TDZ error when the file also imports `helpers/render`. -```typescript -vi.mock('../../lib/api/client', async () => { - const { createMockedDomainModule } = await import('../../__tests__/helpers/mock-api') - return createMockedDomainModule('client', { default: { get: vi.fn() } }) -}) -``` - -Use dynamic `import()` inside the factory (not a top-level import of `createMockedDomainModule`) when the test file also imports `helpers/render` — Vitest hoists `vi.mock` and causes a TDZ error otherwise. +For raw axios calls, mock `lib/api/client` with `createMockedDomainModule('client', { default: { get: vi.fn() } })`. -Prefer `mock-api-templates` for repeated page setups (one `vi.mock` per affected domain): +`helpers/mock-api-templates.ts` has presets for common pages and hooks (`statisticsPageMock`, `studiesHooksMock`, and others). Use one `vi.mock` per affected domain: ```typescript vi.mock('../../lib/api/statistics', async () => { @@ -107,117 +55,26 @@ vi.mock('../../lib/api/statistics', async () => { }) ``` -Production and tests both import from domain modules under `src/lib/api/`. There is no aggregate barrel. - -## Writing Tests +`helpers/settings-mocks.ts` builds settings values and `settingsApi.getValue` mocks. -### Component Tests +### Response shapes -```typescript -import { describe, it, expect } from 'vitest' -import { render, screen } from '../../__tests__/helpers/render' -import userEvent from '@testing-library/user-event' -import MyComponent from '../MyComponent' +`api` methods resolve to the response body, not an `AxiosResponse` ([ADR 0002](../../../docs/adr/0002-http-client-response-unwrap.md)). Mocks return body shapes directly, such as `{ user }` or `{ studies: [] }`. Reference-data list helpers such as `specimenTypesApi.list()` are the exception: they return `{ data: T[], meta? }`, so their mocks keep that envelope. -describe('MyComponent', () => { - it('renders correctly', () => { - render() - expect(screen.getByText('Hello')).toBeInTheDocument() - }) - - it('handles user interaction', async () => { - const user = userEvent.setup() - render() - - const button = screen.getByRole('button') - await user.click(button) - - expect(screen.getByText('Clicked')).toBeInTheDocument() - }) -}) -``` +## React Query hooks -### React Query Hook Tests +Hook tests use `renderHook` with a wrapper that provides a fresh `QueryClient` with retries off, as in `hooks/__tests__/useStudies.test.tsx`: ```typescript -import { describe, it, expect, vi, beforeEach } from 'vitest' -import { renderHook, waitFor } from '@testing-library/react' -import { QueryClient, QueryClientProvider } from '@tanstack/react-query' -import { ReactNode } from 'react' -import { useStudies } from '../useStudies' -import { studiesApi } from '../../lib/api/studies' - -vi.mock('../../lib/api/studies', async () => { - const { createMockedDomainModule } = await import('../../__tests__/helpers/mock-api') - return createMockedDomainModule('studies', { studiesApi: { list: vi.fn() } }) -}) - function createWrapper() { const queryClient = new QueryClient({ - defaultOptions: { - queries: { retry: false, gcTime: 0 }, - mutations: { retry: false }, - }, + defaultOptions: { queries: { retry: false, gcTime: 0 }, mutations: { retry: false } }, }) - return ({ children }: { children: ReactNode }) => ( - - {children} - + {children} ) } -describe('useStudies', () => { - it('should fetch studies', async () => { - vi.mocked(studiesApi.list).mockResolvedValue({ - studies: [], - pagination: { total: 0, page: 1, limit: 10, totalPages: 0 }, - }) - - const { result } = renderHook(() => useStudies(), { - wrapper: createWrapper(), - }) - - await waitFor(() => { - expect(result.current.isSuccess).toBe(true) - }) - }) -}) -``` - -## Coverage - -Coverage excludes `src/**/*.css` and `src/lib/api.ts` (api.ts is exercised via hooks and page tests). Thresholds are set to a baseline and can be raised toward 90% as more tests are added. - -Run coverage reports: - -```bash -bun test:coverage -``` - -Coverage reports are generated in: -- `coverage/` directory (HTML, JSON, text formats) - -## Running Tests From Repo Root - -From the repository root you can run all package tests and coverage: - -```bash -bun run test # Run API and web tests -bun run test:coverage # Run API and web tests with coverage +const { result } = renderHook(() => useStudies(), { wrapper: createWrapper() }) +await waitFor(() => expect(result.current.isSuccess).toBe(true)) ``` - -## New Tests and Failing Behavior - -New tests are allowed to **fail** initially if they correctly express expected behavior. Follow-up work will update the implementation to satisfy the tests. Do not remove or relax assertions solely to make tests pass; fix the code under test instead. - -## Best Practices - -1. **Use custom render**: Always use `render` from `helpers/render.tsx` for components -2. **Mock API calls**: Use `vi.mock` to mock API modules in hook tests -3. **Test user interactions**: Use `@testing-library/user-event` for user interactions -4. **Test loading/error states**: Include tests for loading and error states -5. **Test cache invalidation**: For mutations, verify that queries are invalidated -6. **Use waitFor**: For async operations, use `waitFor` or `findBy` queries -7. **Test accessibility**: Use semantic queries (`getByRole`, `getByLabelText`) when possible - diff --git a/packages/web/vitest.config.ts b/packages/web/vitest.config.ts index d703b145..cf77864d 100644 --- a/packages/web/vitest.config.ts +++ b/packages/web/vitest.config.ts @@ -8,21 +8,6 @@ const appBuildId = process.env.VITE_APP_BUILD_ID || process.env.APP_BUILD_ID || // fails, wherever CI runs. Test workers inherit this. process.env.TZ = 'America/Los_Angeles' -/** - * Vitest Configuration - * - * NOTE: Known Issue - Stack Overflow During Cleanup - * - * You may see a "Maximum call stack size exceeded" error after tests complete. - * This is a known Vitest/tinypool issue with worker cleanup. - * - * - All tests pass successfully before the error - * - Error only occurs during worker cleanup/teardown - * - This is NON-BLOCKING and can be safely ignored - * - Do not attempt to "fix" this - it's a Vitest/tinypool bug - * - * See: packages/web/src/__tests__/README.md for more details - */ export default defineConfig({ define: { 'import.meta.env.VITE_APP_BUILD_ID': JSON.stringify(appBuildId), From bf1cbda68319e18b46225cf015f6a767cef9a97a Mon Sep 17 00:00:00 2001 From: Maxwell Murphy Date: Tue, 6 Oct 2026 15:54:20 -0700 Subject: [PATCH 03/21] docs(agents): turn AGENTS.md into a router and fold docs/agents into it. - AGENTS.md: three sections. Agent skills holds the tracker, label, and domain-doc facts inline; Checks holds what every task needs (the bun test hang, clean lint, ci:verify); Where to look has one pointer per branch. The UI-label trail stays as a pointer line, since that branch starts before the agent is in any package. - docs/agents/: deleted. Each file was 3-7 lines, about the cost of the pointer to it. - CODING_STANDARDS.md: grouped by package. Add one-line rules from ADRs 0001, 0002, 0004-0010 that bind everyday code. Drop the synchronous transaction callback rule, which ESLint enforces, and fold the mock response shapes into the web rules. - Package READMEs lead with what the branch needs first: the runner, the setup file, the schema recipe. - Config comments: drop the better-sqlite3 stack overflow note, the ~55% baseline, and the api.ts and coverage-plan mentions. - README.md: ci:verify runs more than typecheck and build. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 43 ++++++++--------------- CODING_STANDARDS.md | 51 +++++++++++++++++++++------- README.md | 6 ++-- docs/agents/domain.md | 7 ---- docs/agents/issue-tracker.md | 6 ---- docs/agents/triage-labels.md | 3 -- packages/api/bunfig.toml | 4 +-- packages/api/src/__tests__/README.md | 6 ++-- packages/api/src/db/README.md | 35 ++++++++++--------- packages/api/vitest.config.ts | 20 ++--------- packages/web/src/__tests__/README.md | 10 ++---- packages/web/vitest.config.ts | 2 +- 12 files changed, 82 insertions(+), 111 deletions(-) delete mode 100644 docs/agents/domain.md delete mode 100644 docs/agents/issue-tracker.md delete mode 100644 docs/agents/triage-labels.md diff --git a/AGENTS.md b/AGENTS.md index f0d03ea6..2af56e1d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,35 +1,20 @@ ## Agent skills -### Issue tracker +- **Issue tracker**: GitHub Issues on `eppicenter/sampledb3`. "Publish to the issue tracker" is `gh issue create`; "fetch the relevant ticket" is `gh issue view --comments`. +- **Triage labels**: each triage role is a GitHub label with the role's name. +- **Domain docs**: `CONTEXT.md` is the glossary, `docs/adr/` holds the decisions, and `packages/docs/` is the user guide. -GitHub Issues on `eppicenter/sampledb3`. When a skill publishes or fetches an issue, see `docs/agents/issue-tracker.md`. +## Checks -### Triage labels +- Web tests: `cd packages/web && bun run test -- `. Bun's own runner (`bun test`) hangs on React component tests; Vitest exits on its own, so piping it to `tail` or `grep` is safe. +- API tests: `cd packages/api && bun test src/...`. +- Lint is clean, so any lint error is yours. +- The pre-commit hook only typechecks and lints. Run `bun run ci:verify`, which CI runs on every PR, before pushing. -When a skill applies or reads a triage role, see `docs/agents/triage-labels.md`. +## Where to look -### Domain docs - -`CONTEXT.md` (glossary) and `docs/adr/` (decisions). When naming a domain concept or changing an area an ADR covers, see `docs/agents/domain.md`. - -### Coding standards - -When writing code, tests, issues, or proposals, or reviewing a change, follow `CODING_STANDARDS.md`. - -### Package docs - -- Writing an API test (runner, test database, factories, authenticated routes): `packages/api/src/__tests__/README.md` -- Writing a web test (render helper, API mocks, response shapes, React Query hooks): `packages/web/src/__tests__/README.md` -- Changing the schema or adding a migration: `packages/api/src/db/README.md` -- Finding the code behind a UI label: `packages/web/src/components/Sidebar.tsx` (label → path) → `packages/web/src/App.tsx` (path → page) → the page's hooks and `lib/` → `packages/api/src/routes` → `packages/api/src/lib` - -### Tests - -- Web: `cd packages/web && bun run test -- `. Use `bun run test`, never `bun test`: Bun's own runner hangs on React component tests. Vitest exits on its own, so piping it to `tail` or `grep` is safe. -- API: `cd packages/api && bun test src/...`. - -### Pre-commit hooks and CI - -The `Checks` workflow runs `bun run ci:verify` on every PR. Lint is clean, so any lint error is yours. - -Install the repo hooks once per clone with `bun run setup:hooks`. The pre-commit hook only typechecks the packages your staged files touch and lints the staged API/web files; it runs no tests or build. Run `bun run ci:verify` before pushing to mirror CI. +- Writing code, tests, issues, or proposals, or reviewing a change: `CODING_STANDARDS.md`. +- Writing an API test: `packages/api/src/__tests__/README.md`. +- Writing a web test: `packages/web/src/__tests__/README.md`. +- Changing the schema or adding a migration: `packages/api/src/db/README.md`. +- Tracing a UI label to code: `packages/web/src/components/Sidebar.tsx` maps the label to a path and `packages/web/src/App.tsx` maps the path to a page. From there follow the page's hooks and `lib/api/` into `packages/api/src/routes` and `packages/api/src/lib`. diff --git a/CODING_STANDARDS.md b/CODING_STANDARDS.md index 8b1db1fd..7d953e1d 100644 --- a/CODING_STANDARDS.md +++ b/CODING_STANDARDS.md @@ -1,34 +1,59 @@ # Coding standards -## Domain vocabulary +## Domain language -- Name a domain concept with the term `CONTEXT.md` defines for it, in code, test names, issue titles, and refactor proposals. Terms the glossary lists under _Avoid_ are synonyms to replace with the defined term. +- Name a domain concept with the term `CONTEXT.md` defines for it, in code, test names, issue titles, and proposals. A term the glossary lists under _Avoid_ is a synonym to replace with the defined term. - When a concept has no glossary entry, say so in the issue, PR, or proposal before introducing a term for it. +- A change that contradicts an ADR in `docs/adr/` names the ADR and gives the reason to reopen it, in the PR or proposal. For example: _Contradicts ADR-0006 (unified container inbound write), but worth reopening because…_ -## ADRs +## Tests -- A change that contradicts an ADR in `docs/adr/` names the ADR and gives the reason to reopen it, in the PR or proposal. For example: _Contradicts ADR-0006 (unified container inbound write), but worth reopening because…_ +- A test asserts the expected behaviour. When it fails, fix the code under test and keep the assertion as strict as before. A new test may land failing if it states the expected behaviour. -## Database +## API -- A write of several statements that `await`s runs inside `withWriteTransaction` from `packages/api/src/db/write-transaction.ts`. Drizzle's bun-sqlite `db.transaction` commits at the callback's first `await`, so its callback stays synchronous. -- Every schema change is a numbered migration plus a refreshed `initial_schema.sql`, made with the recipe in `packages/api/src/db/README.md`. DDL lives only in migrations and the snapshot. -- Code and tests open a database through `openOperationalDatabase`. In tests that means `setupTestDatabase()`. +Paths are relative to `packages/api/`. -## Tests +### Database -- A test asserts the expected behaviour. When it fails, fix the code under test and keep the assertion as strict as before. A new test may land failing if it states the expected behaviour. +- A write of several statements that `await`s runs inside `withWriteTransaction` from `src/db/write-transaction.ts`. +- Change the schema with a numbered migration, following the recipe in `src/db/README.md`. DDL lives only in the migrations and `initial_schema.sql`. +- Code opens a database through `openOperationalDatabase`. + +### Domain modules + +- Create a Specimen or Container through `registerSpecimens` in `src/lib/specimen-registration/`. Validate a request with `checkSpecimenRegistration`, which is the same write rolled back (ADR 0010). +- A request that carries a container validates it with `containerWriteInputSchema` from `@sampledb/contract` (ADR 0006). +- Resolve a Specimen's Source with `resolveSpecimenSource` or `resolveSpecimenSources` from `src/lib/specimens/provenance.ts`, and hand every caller the full `SpecimenSource` shape (ADR 0007). +- Build a container payload (placement, Tags, Specimen, Source) from `loadContainerReadViews` in `src/lib/container-read-view.ts`, then project it (ADR 0009). + +### Routes + +- A route maps Drizzle rows to a wire DTO before `c.json` wherever the shape matters. `omitOnWireMiddleware` drops null keys from every response, so null omission stays with it (ADR 0005). +- A route reports an error by throwing a typed error from `src/lib/error-handler.ts` or returning `handleRouteError(error, c)` (ADR 0004). +- Structured stdout goes through `src/lib/observability.ts`. A failure meant for Admin Error Logs goes through `src/lib/error-logger.ts` (ADR 0004). ### API tests - Each test gets its own database from `setupTestDatabase()` and closes it with `cleanupTestDatabase()` in `afterEach`. -- Seed rows with the factories in `packages/api/src/__tests__/helpers/factories.ts`. +- Seed rows with the factories in `src/__tests__/helpers/factories.ts`. - A route behind session auth is tested through `setupAuthenticatedRouteTest`. - A CRUD route with `checkInUse` has a delete test for both the in-use and the free case. +## Web + +Paths are relative to `packages/web/`. + +- Call the API through its domain module, `src/lib/api/`. Its methods resolve to the response body, so read `res.study`. Reference-data `list()` helpers are the exception: their body is `{ data, meta? }` (ADR 0002). +- Read and write settings with `settingsApi.getValue` and `settingsApi.putValue` (ADR 0002). +- A Zod schema for a response marks an unset field `.optional()`, because the wire omits it (ADR 0005). +- Show a query's read error inline with `PageError` or `SectionMessage` from `src/ui/`. Toasts are for mutation errors (ADR 0001). +- A mutation goes through a domain hook that invalidates every query it affects, `dashboardKeys` included (ADR 0001). +- Scan move behaviour lands in the `src/lib/scan-move/` core with plain-data unit tests. The move pages only wire it up (ADR 0008). + ### Web tests -- Components render through `await render(...)` from `packages/web/src/__tests__/helpers/render`. -- A test mocks the domain module the code imports (`lib/api/`) with `createMockedDomainModule`, and an override replaces a whole `*Api` object. +- Components render through `await render(...)` from `src/__tests__/helpers/render`. +- A test mocks the domain module the code imports with `createMockedDomainModule`. An override replaces a whole `*Api` object and resolves to the same body the real module returns. - Query elements by role or label (`getByRole`, `getByLabelText`), and drive interactions with `@testing-library/user-event`. - A mutation hook's test asserts which queries it invalidates. diff --git a/README.md b/README.md index fc499c7b..244a32f5 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,7 @@ bun run build bun run start ``` -Before pushing, run `bun run ci:verify` (typecheck + build) to match the Docker CI step. Install git pre-commit hooks with `bun run setup:hooks` to catch TypeScript errors on commit. +Before pushing, run `bun run ci:verify`, the same checks the `Checks` workflow runs on every PR. Install the pre-commit hook with `bun run setup:hooks`; it typechecks the packages your staged files touch and lints the staged API and web files. ## Project Structure @@ -72,8 +72,8 @@ Put local overrides in **`.env.local`** (gitignored) at the repo root. Bun’s a - **Unit tests (API + web)**: `bun run test` - **End-to-end (Playwright)**: `bun run test:e2e` -- **API**: `bun --filter @sampledb/api test` (Bun). Coverage: `bun --filter @sampledb/api test:coverage` (report in `packages/api/coverage/`). Target 90% lines; see `packages/api/src/__tests__/README.md`. -- **Web**: `bun --filter @sampledb/web test` (Vitest). Coverage: `bun --filter @sampledb/web test:coverage`. Coverage excludes `src/lib/api/**` and `src/**/*.css` so the 90% target applies to testable code. Thresholds in `packages/web/vitest.config.ts` can be raised in steps toward 90%. +- **API**: `bun --filter @sampledb/api test` (Bun). Coverage: `bun --filter @sampledb/api test:coverage` (report in `packages/api/coverage/`). Target 90% lines. Test helpers: `packages/api/src/__tests__/README.md`. +- **Web**: `bun --filter @sampledb/web test` (Vitest). Coverage: `bun --filter @sampledb/web test:coverage`. Coverage excludes `src/lib/api/**` and `src/**/*.css` so the 90% target applies to testable code. Thresholds in `packages/web/vitest.config.ts` can be raised in steps toward 90%. Test helpers: `packages/web/src/__tests__/README.md`. ## Database Configuration diff --git a/docs/agents/domain.md b/docs/agents/domain.md deleted file mode 100644 index bc356991..00000000 --- a/docs/agents/domain.md +++ /dev/null @@ -1,7 +0,0 @@ -# Domain docs - -- **Naming a domain concept**: read `CONTEXT.md`, the glossary of entity names, hierarchies, and scope. -- **Changing an area a design decision covers**: read the ADRs in `docs/adr/` that touch it. -- **Describing a user-facing workflow**: read the user guide in `packages/docs/`. - -`CODING_STANDARDS.md` has the rules for using the glossary's terms and for contradicting an ADR. diff --git a/docs/agents/issue-tracker.md b/docs/agents/issue-tracker.md deleted file mode 100644 index 41b01aa0..00000000 --- a/docs/agents/issue-tracker.md +++ /dev/null @@ -1,6 +0,0 @@ -# Issue tracker: GitHub - -Issues and PRDs for this repo are GitHub issues on `eppicenter/sampledb3`, worked with the `gh` CLI. - -- A skill's "publish to the issue tracker" means `gh issue create`. -- A skill's "fetch the relevant ticket" means `gh issue view --comments`, so the comments come with it. diff --git a/docs/agents/triage-labels.md b/docs/agents/triage-labels.md deleted file mode 100644 index caec6c7a..00000000 --- a/docs/agents/triage-labels.md +++ /dev/null @@ -1,3 +0,0 @@ -# Triage labels - -Each of the five canonical triage roles is a GitHub label with the same name: `needs-triage`, `needs-info`, `ready-for-agent`, `ready-for-human`, `wontfix`. Apply the role's name as the label. diff --git a/packages/api/bunfig.toml b/packages/api/bunfig.toml index 2c589c2c..1332dd59 100644 --- a/packages/api/bunfig.toml +++ b/packages/api/bunfig.toml @@ -1,10 +1,10 @@ [test] -# Only run tests from src directory, exclude dist (compiled files) +# Skip compiled output. The test scripts pass src. exclude = ["dist/**", "node_modules/**"] # Coverage: use Bun's built-in so all tests (including route tests using bun:sqlite) run coverage = true coverageReporter = ["text", "lcov"] coverageDir = "coverage" -# Optional: set to enforce minimum (e.g. 50). Current baseline ~55% lines; raise toward 60% as tests are added. +# Optional: set to fail the run below this line coverage. # coverageThreshold = 50 coveragePathIgnorePatterns = ["node_modules", "src/__tests__", "src/**/*.d.ts", "src/**/*.config.*", "dist"] diff --git a/packages/api/src/__tests__/README.md b/packages/api/src/__tests__/README.md index b64ba76e..bc11df66 100644 --- a/packages/api/src/__tests__/README.md +++ b/packages/api/src/__tests__/README.md @@ -1,10 +1,8 @@ # Backend testing guide -The rules for API tests are under Tests in `CODING_STANDARDS.md`. This guide covers the helpers. +API tests run on Bun's runner because route and integration tests need `bun:sqlite`. Test files import `describe`, `it`, and `expect` from `'vitest'`; Bun maps that import to `bun:test`. `bun run test:ui` starts Vitest, which only runs tests that never touch `bun:sqlite`. -## Runner - -Tests run on Bun's runner because route and integration tests need `bun:sqlite`. Test files import `describe`, `it`, and `expect` from `'vitest'`; Bun maps that import to `bun:test`. `bun run test:ui` starts Vitest, which only runs tests that never touch `bun:sqlite`. +This guide covers the helpers. The rules are under API tests in `CODING_STANDARDS.md`. ## Test database diff --git a/packages/api/src/db/README.md b/packages/api/src/db/README.md index 8d77afc6..eca6ea50 100644 --- a/packages/api/src/db/README.md +++ b/packages/api/src/db/README.md @@ -4,17 +4,15 @@ SampleDB stores **operational data** (studies, subjects, specimens, containers, See [ADR-0003: SQLite schema evolution](../../../docs/adr/0003-sqlite-schema-evolution.md) for rationale (snapshot + deltas, no drizzle-kit migrate, fail-hard on error). -## Opening a database - -Use `openOperationalDatabase(optionalPath?)` from the client module. It resolves the file path, enables WAL, turns on foreign keys, runs schema evolution, and returns Drizzle + raw `bun:sqlite` handles. Importing the client module does **not** open a connection. - -The rules for transactions, DDL, and which entry point to open a database with are under Database in `CODING_STANDARDS.md`. - -## Schema version +## Changing the schema -- Table: `schema_version` with a single row (`version INTEGER NOT NULL`). -- Constant: `CURRENT_SCHEMA_VERSION` in `schema-evolution.ts`. -- Inspect in support: `SELECT version FROM schema_version;` +1. Edit Drizzle definitions in `schema.ts`. +2. Generate SQL: `bunx drizzle-kit generate` in `packages/api` (see `drizzle.config.ts`). +3. Add `packages/api/src/db/migrations/00N_short_name.sql` (statements separated with `--> statement-breakpoint`). +4. Bump `CURRENT_SCHEMA_VERSION` in `schema-evolution.ts`. +5. Run `bun run build` in `packages/api` — `scripts/runtime-assets.ts` copies migrations into `dist/db/migrations` and verifies the dist layout. If you add other on-disk runtime assets, register them in `RUNTIME_ASSETS` in that script. +6. Refresh `initial_schema.sql` so **new** empty databases match the latest shape (merge generated SQL; keep `INSERT INTO schema_version` at `CURRENT_SCHEMA_VERSION`). `__tests__/initial-schema-snapshot.test.ts` fails until the snapshot holds every table and column in `schema.ts`. +7. Extend the legacy fixtures if the migration touches a table they don't create. Older-version tests in `__tests__/schema-evolution.test.ts` build minimal databases through `createTablesForLaterMigrations`, and `__tests__/client.test.ts` creates its own tables inline; both must contain every table and column a later migration alters. ## How evolution works @@ -26,15 +24,16 @@ The rules for transactions, DDL, and which entry point to open a database with a Migration `001` is **only** for legacy files; fresh installs from the snapshot skip it. -## Changing the schema +## Schema version -1. Edit Drizzle definitions in `schema.ts`. -2. Generate SQL: `bunx drizzle-kit generate` in `packages/api` (see `drizzle.config.ts`). -3. Add `packages/api/src/db/migrations/00N_short_name.sql` (statements separated with `--> statement-breakpoint`). -4. Bump `CURRENT_SCHEMA_VERSION` in `schema-evolution.ts`. -5. Run `bun run build` in `packages/api` — `scripts/runtime-assets.ts` copies migrations into `dist/db/migrations` and verifies the dist layout. If you add other on-disk runtime assets, register them in `RUNTIME_ASSETS` in that script. -6. Refresh `initial_schema.sql` so **new** empty databases match the latest shape (merge generated SQL; keep `INSERT INTO schema_version` at `CURRENT_SCHEMA_VERSION`). `__tests__/initial-schema-snapshot.test.ts` fails until the snapshot holds every table and column in `schema.ts`. -7. Extend the legacy fixtures if the migration touches a table they don't create. Older-version tests in `__tests__/schema-evolution.test.ts` build minimal databases through `createTablesForLaterMigrations`, and `__tests__/client.test.ts` creates its own tables inline; both must contain every table and column a later migration alters. +- Table: `schema_version` with a single row (`version INTEGER NOT NULL`). +- Inspect in support: `SELECT version FROM schema_version;` + +## Opening a database + +Use `openOperationalDatabase(optionalPath?)` from the client module. It resolves the file path, enables WAL, turns on foreign keys, runs schema evolution, and returns Drizzle + raw `bun:sqlite` handles. Importing the client module does **not** open a connection. + +The rules for transactions, DDL, and which entry point to open a database with are under Database in `CODING_STANDARDS.md`. ## Failures diff --git a/packages/api/vitest.config.ts b/packages/api/vitest.config.ts index 34a6e7ba..fb3e43de 100644 --- a/packages/api/vitest.config.ts +++ b/packages/api/vitest.config.ts @@ -1,30 +1,14 @@ import { defineConfig } from 'vitest/config' import path from 'path' -/** - * Vitest Configuration - * - * NOTE: Known Issue - Stack Overflow During Cleanup - * - * You may see a "Maximum call stack size exceeded" error after tests complete. - * This is a known Vitest/tinypool issue with native modules (better-sqlite3). - * - * - All tests pass successfully before the error - * - Error only occurs during worker cleanup/teardown - * - This is NON-BLOCKING and can be safely ignored - * - Do not attempt to "fix" this - it's a Vitest/tinypool bug - * - * See: packages/api/src/__tests__/README.md for more details - */ +// Vitest backs only `bun run test:ui`. The suite runs on Bun's runner (bunfig.toml), and +// Vitest can run only the tests that never touch bun:sqlite. export default defineConfig({ test: { globals: true, environment: 'node', include: ['src/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}'], exclude: ['node_modules', 'dist', '.idea', '.git', '.cache'], - // Use forks pool for native modules (better-sqlite3) - // This prevents segmentation faults and worker termination issues - // Note: This may cause a non-blocking stack overflow during cleanup pool: 'forks', coverage: { provider: 'v8', diff --git a/packages/web/src/__tests__/README.md b/packages/web/src/__tests__/README.md index 9138155e..012bbc75 100644 --- a/packages/web/src/__tests__/README.md +++ b/packages/web/src/__tests__/README.md @@ -1,8 +1,8 @@ # Frontend testing guide -Tests run on Vitest with jsdom and React Testing Library. The rules for web tests are under Tests in `CODING_STANDARDS.md`. This guide covers the helpers. +Web tests run on Vitest with jsdom and React Testing Library. `src/__tests__/setup.ts` runs before every file. It stubs `IntersectionObserver` and `localStorage` and mocks the `auth` and `settings` domain modules; override those per file as needed. -`__tests__/setup.ts` runs before every file. It stubs `IntersectionObserver` and `localStorage` and mocks the `auth` and `settings` domain modules; override those per file as needed. +This guide covers the helpers. The rules are under Web tests in `CODING_STANDARDS.md`. ## Rendering @@ -23,7 +23,7 @@ it('handles a click', async () => { ## Mocking the API -Production code and tests import from the domain modules under `src/lib/api/`; there is no barrel. `createMockedDomainModule(domain, overrides)` in `helpers/mock-api.ts` loads the real module and swaps in the `*Api` objects you pass: +Code calls the API through domain modules under `src/lib/api/`, so a test mocks the module the code imports. `createMockedDomainModule(domain, overrides)` in `helpers/mock-api.ts` loads the real module and swaps in the `*Api` objects you pass: ```typescript import { studiesApi } from '../../lib/api/studies' @@ -57,10 +57,6 @@ vi.mock('../../lib/api/statistics', async () => { `helpers/settings-mocks.ts` builds settings values and `settingsApi.getValue` mocks. -### Response shapes - -`api` methods resolve to the response body, not an `AxiosResponse` ([ADR 0002](../../../docs/adr/0002-http-client-response-unwrap.md)). Mocks return body shapes directly, such as `{ user }` or `{ studies: [] }`. Reference-data list helpers such as `specimenTypesApi.list()` are the exception: they return `{ data: T[], meta? }`, so their mocks keep that envelope. - ## React Query hooks Hook tests use `renderHook` with a wrapper that provides a fresh `QueryClient` with retries off, as in `hooks/__tests__/useStudies.test.tsx`: diff --git a/packages/web/vitest.config.ts b/packages/web/vitest.config.ts index cf77864d..0c645642 100644 --- a/packages/web/vitest.config.ts +++ b/packages/web/vitest.config.ts @@ -38,7 +38,7 @@ export default defineConfig({ 'src/lib/api/**', 'dist/**', ], - // Baseline; raise in steps (e.g. 50 → 65 → 80 → 90) as coverage improves (see test coverage plan). CSS and api.ts are excluded. + // Baseline; raise in steps toward 90% as coverage improves. thresholds: { statements: 35, branches: 26, From d4a8e9ab2df0dc0e2486a92f4fb8a48cd8948d7c Mon Sep 17 00:00:00 2001 From: Maxwell Murphy Date: Tue, 6 Oct 2026 16:11:21 -0700 Subject: [PATCH 04/21] docs: hold the ADR-derived rules back for their own review. The rules pass 3 drew from ADRs 0001 and 0004-0010 will be enforced by code review, so they move to a follow-up PR where each can be checked. The web response-body rule stays: it replaced the web test README's mock-shape note. Co-Authored-By: Claude Opus 5.5 (1M context) --- CODING_STANDARDS.md | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/CODING_STANDARDS.md b/CODING_STANDARDS.md index 7d953e1d..973bd163 100644 --- a/CODING_STANDARDS.md +++ b/CODING_STANDARDS.md @@ -20,19 +20,6 @@ Paths are relative to `packages/api/`. - Change the schema with a numbered migration, following the recipe in `src/db/README.md`. DDL lives only in the migrations and `initial_schema.sql`. - Code opens a database through `openOperationalDatabase`. -### Domain modules - -- Create a Specimen or Container through `registerSpecimens` in `src/lib/specimen-registration/`. Validate a request with `checkSpecimenRegistration`, which is the same write rolled back (ADR 0010). -- A request that carries a container validates it with `containerWriteInputSchema` from `@sampledb/contract` (ADR 0006). -- Resolve a Specimen's Source with `resolveSpecimenSource` or `resolveSpecimenSources` from `src/lib/specimens/provenance.ts`, and hand every caller the full `SpecimenSource` shape (ADR 0007). -- Build a container payload (placement, Tags, Specimen, Source) from `loadContainerReadViews` in `src/lib/container-read-view.ts`, then project it (ADR 0009). - -### Routes - -- A route maps Drizzle rows to a wire DTO before `c.json` wherever the shape matters. `omitOnWireMiddleware` drops null keys from every response, so null omission stays with it (ADR 0005). -- A route reports an error by throwing a typed error from `src/lib/error-handler.ts` or returning `handleRouteError(error, c)` (ADR 0004). -- Structured stdout goes through `src/lib/observability.ts`. A failure meant for Admin Error Logs goes through `src/lib/error-logger.ts` (ADR 0004). - ### API tests - Each test gets its own database from `setupTestDatabase()` and closes it with `cleanupTestDatabase()` in `afterEach`. @@ -45,11 +32,6 @@ Paths are relative to `packages/api/`. Paths are relative to `packages/web/`. - Call the API through its domain module, `src/lib/api/`. Its methods resolve to the response body, so read `res.study`. Reference-data `list()` helpers are the exception: their body is `{ data, meta? }` (ADR 0002). -- Read and write settings with `settingsApi.getValue` and `settingsApi.putValue` (ADR 0002). -- A Zod schema for a response marks an unset field `.optional()`, because the wire omits it (ADR 0005). -- Show a query's read error inline with `PageError` or `SectionMessage` from `src/ui/`. Toasts are for mutation errors (ADR 0001). -- A mutation goes through a domain hook that invalidates every query it affects, `dashboardKeys` included (ADR 0001). -- Scan move behaviour lands in the `src/lib/scan-move/` core with plain-data unit tests. The move pages only wire it up (ADR 0008). ### Web tests From dd642a4a6ada6185c4a4b7f44d1f415e0a55275f Mon Sep 17 00:00:00 2001 From: Maxwell Murphy Date: Tue, 6 Oct 2026 16:48:44 -0700 Subject: [PATCH 05/21] docs: cite ADRs as "ADR 0006" in the files this PR touches. Co-Authored-By: Claude Opus 5.5 (1M context) --- CODING_STANDARDS.md | 2 +- packages/api/src/db/README.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CODING_STANDARDS.md b/CODING_STANDARDS.md index 973bd163..9a577092 100644 --- a/CODING_STANDARDS.md +++ b/CODING_STANDARDS.md @@ -4,7 +4,7 @@ - Name a domain concept with the term `CONTEXT.md` defines for it, in code, test names, issue titles, and proposals. A term the glossary lists under _Avoid_ is a synonym to replace with the defined term. - When a concept has no glossary entry, say so in the issue, PR, or proposal before introducing a term for it. -- A change that contradicts an ADR in `docs/adr/` names the ADR and gives the reason to reopen it, in the PR or proposal. For example: _Contradicts ADR-0006 (unified container inbound write), but worth reopening because…_ +- A change that contradicts an ADR in `docs/adr/` names the ADR and gives the reason to reopen it, in the PR or proposal. For example: _Contradicts ADR 0006 (unified container inbound write), but worth reopening because…_ ## Tests diff --git a/packages/api/src/db/README.md b/packages/api/src/db/README.md index eca6ea50..b33f5597 100644 --- a/packages/api/src/db/README.md +++ b/packages/api/src/db/README.md @@ -2,7 +2,7 @@ SampleDB stores **operational data** (studies, subjects, specimens, containers, controls, and related records) in SQLite. This package owns how that file is opened, versioned, and evolved. -See [ADR-0003: SQLite schema evolution](../../../docs/adr/0003-sqlite-schema-evolution.md) for rationale (snapshot + deltas, no drizzle-kit migrate, fail-hard on error). +See [ADR 0003: SQLite schema evolution](../../../docs/adr/0003-sqlite-schema-evolution.md) for rationale (snapshot + deltas, no drizzle-kit migrate, fail-hard on error). ## Changing the schema From bfa8d4016a9fd4d27474423a3aedd294e30dd10e Mon Sep 17 00:00:00 2001 From: Maxwell Murphy Date: Tue, 6 Oct 2026 16:48:57 -0700 Subject: [PATCH 06/21] docs: write the coding standards as second-person imperatives. The repo's docs spell it "behaviour" (7 uses in 4 files against 1 "behavior"), which CODING_STANDARDS.md already follows. Co-Authored-By: Claude Opus 5.5 (1M context) --- CODING_STANDARDS.md | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/CODING_STANDARDS.md b/CODING_STANDARDS.md index 9a577092..c55d9f18 100644 --- a/CODING_STANDARDS.md +++ b/CODING_STANDARDS.md @@ -2,13 +2,13 @@ ## Domain language -- Name a domain concept with the term `CONTEXT.md` defines for it, in code, test names, issue titles, and proposals. A term the glossary lists under _Avoid_ is a synonym to replace with the defined term. -- When a concept has no glossary entry, say so in the issue, PR, or proposal before introducing a term for it. -- A change that contradicts an ADR in `docs/adr/` names the ADR and gives the reason to reopen it, in the PR or proposal. For example: _Contradicts ADR 0006 (unified container inbound write), but worth reopening because…_ +- Name a domain concept with the term `CONTEXT.md` defines for it, in code, test names, issue titles, and proposals. Treat a term the glossary lists under _Avoid_ as a synonym, and replace it with the defined term. +- When a concept has no glossary entry, say so in the issue, PR, or proposal before you introduce a term for it. +- When a change contradicts an ADR in `docs/adr/`, name the ADR and give the reason to reopen it, in the PR or proposal. For example: _Contradicts ADR 0006 (unified container inbound write), but worth reopening because…_ ## Tests -- A test asserts the expected behaviour. When it fails, fix the code under test and keep the assertion as strict as before. A new test may land failing if it states the expected behaviour. +- Assert the expected behaviour. When a test fails, fix the code under test and keep the assertion as strict as before. You may land a new test that fails, if it states the expected behaviour. ## API @@ -16,16 +16,16 @@ Paths are relative to `packages/api/`. ### Database -- A write of several statements that `await`s runs inside `withWriteTransaction` from `src/db/write-transaction.ts`. -- Change the schema with a numbered migration, following the recipe in `src/db/README.md`. DDL lives only in the migrations and `initial_schema.sql`. -- Code opens a database through `openOperationalDatabase`. +- Run a write of several statements that `await`s inside `withWriteTransaction` from `src/db/write-transaction.ts`. +- Change the schema with a numbered migration, following the recipe in `src/db/README.md`. Keep DDL in the migrations and `initial_schema.sql` only. +- Open a database through `openOperationalDatabase`. ### API tests -- Each test gets its own database from `setupTestDatabase()` and closes it with `cleanupTestDatabase()` in `afterEach`. +- Give each test its own database from `setupTestDatabase()`, and close it with `cleanupTestDatabase()` in `afterEach`. - Seed rows with the factories in `src/__tests__/helpers/factories.ts`. -- A route behind session auth is tested through `setupAuthenticatedRouteTest`. -- A CRUD route with `checkInUse` has a delete test for both the in-use and the free case. +- Test a route behind session auth through `setupAuthenticatedRouteTest`. +- For a CRUD route with `checkInUse`, write a delete test for both the in-use and the free case. ## Web @@ -35,7 +35,7 @@ Paths are relative to `packages/web/`. ### Web tests -- Components render through `await render(...)` from `src/__tests__/helpers/render`. -- A test mocks the domain module the code imports with `createMockedDomainModule`. An override replaces a whole `*Api` object and resolves to the same body the real module returns. +- Render components with `await render(...)` from `src/__tests__/helpers/render`. +- Mock the domain module the code imports with `createMockedDomainModule`. Make each override replace a whole `*Api` object and resolve to the same body the real module returns. - Query elements by role or label (`getByRole`, `getByLabelText`), and drive interactions with `@testing-library/user-event`. -- A mutation hook's test asserts which queries it invalidates. +- In a mutation hook's test, assert which queries it invalidates. From bf9746b38005f17d967f6bbb44dc60ba4c08b644 Mon Sep 17 00:00:00 2001 From: Maxwell Murphy Date: Tue, 6 Oct 2026 16:49:02 -0700 Subject: [PATCH 07/21] docs: allow read-only tools that must not migrate to skip openOperationalDatabase. check-integrity.ts opens a backup with new Database(..., { readonly: true }) on purpose, so the rule now names that exception and leaves room for others. Co-Authored-By: Claude Opus 5.5 (1M context) --- CODING_STANDARDS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CODING_STANDARDS.md b/CODING_STANDARDS.md index c55d9f18..042d2c9b 100644 --- a/CODING_STANDARDS.md +++ b/CODING_STANDARDS.md @@ -18,7 +18,7 @@ Paths are relative to `packages/api/`. - Run a write of several statements that `await`s inside `withWriteTransaction` from `src/db/write-transaction.ts`. - Change the schema with a numbered migration, following the recipe in `src/db/README.md`. Keep DDL in the migrations and `initial_schema.sql` only. -- Open a database through `openOperationalDatabase`. +- Open a database through `openOperationalDatabase`, which runs schema evolution. Open the file directly only when the code must not migrate it, and say why in a comment. For example, `src/scripts/check-integrity.ts` opens a backup with `new Database(path, { readonly: true })` so that checking it leaves it unchanged. ### API tests From d2797d82336343b020e2cbdceadc2e8dddc95153 Mon Sep 17 00:00:00 2001 From: Maxwell Murphy Date: Tue, 6 Oct 2026 16:49:09 -0700 Subject: [PATCH 08/21] docs: say when the hand-rolled session helpers are acceptable. The API test README offered loginAndGetCookie and friends with no caveat, against the rule to use setupAuthenticatedRouteTest. Logging in by hand is now limited to tests of the login flow, and the client wrapper is shown on top of the adapter's cookie, matching how the tests use them. Co-Authored-By: Claude Opus 5.5 (1M context) --- CODING_STANDARDS.md | 2 +- packages/api/src/__tests__/README.md | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CODING_STANDARDS.md b/CODING_STANDARDS.md index 042d2c9b..b5d718a2 100644 --- a/CODING_STANDARDS.md +++ b/CODING_STANDARDS.md @@ -24,7 +24,7 @@ Paths are relative to `packages/api/`. - Give each test its own database from `setupTestDatabase()`, and close it with `cleanupTestDatabase()` in `afterEach`. - Seed rows with the factories in `src/__tests__/helpers/factories.ts`. -- Test a route behind session auth through `setupAuthenticatedRouteTest`. +- Test a route behind session auth through `setupAuthenticatedRouteTest`. Log in by hand with `loginAndGetCookie` only when the login flow is under test, as in the auth route and middleware tests. - For a CRUD route with `checkInUse`, write a delete test for both the in-use and the free case. ## Web diff --git a/packages/api/src/__tests__/README.md b/packages/api/src/__tests__/README.md index bc11df66..2f63d038 100644 --- a/packages/api/src/__tests__/README.md +++ b/packages/api/src/__tests__/README.md @@ -83,7 +83,8 @@ describe('Search API', () => { `helpers/test-client.ts`: - `getResponseData(response)` unpacks `{ data }` from a standard API response. -- `authenticatedRequest(app, path, options)` sends a request with a session cookie. -- `loginAndGetCookie` and `createAuthenticatedClientWrapper` manage sessions by hand. +- `authenticatedRequest(app, path, options)` sends a request with a session cookie. `ctx.request` calls it for you. +- `createAuthenticatedClientWrapper(client, ctx.cookie)` adds the session cookie to a typed `hono/testing` client, for tests that build one on top of `setupAuthenticatedRouteTest`. +- `loginAndGetCookie` logs in and returns the session cookie. Call it directly only in tests of the login flow itself, such as `routes/__tests__/auth.test.ts` and `middleware/__tests__/auth.test.ts`. `helpers/auth-helpers.ts` creates users and sessions and seeds the password, session, and pagination settings. From 08e081cdb1ae034dd2008928822fe837c5fc0f4b Mon Sep 17 00:00:00 2001 From: Maxwell Murphy Date: Tue, 6 Oct 2026 16:49:17 -0700 Subject: [PATCH 09/21] docs: put the reason for withWriteTransaction back next to the rule. The db README defers to CODING_STANDARDS.md for the transaction rule, but the move dropped why: Drizzle's bun-sqlite transaction commits at the first await. Co-Authored-By: Claude Opus 5.5 (1M context) --- CODING_STANDARDS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CODING_STANDARDS.md b/CODING_STANDARDS.md index b5d718a2..ada7a33b 100644 --- a/CODING_STANDARDS.md +++ b/CODING_STANDARDS.md @@ -16,7 +16,7 @@ Paths are relative to `packages/api/`. ### Database -- Run a write of several statements that `await`s inside `withWriteTransaction` from `src/db/write-transaction.ts`. +- Run a write of several statements that `await`s inside `withWriteTransaction` from `src/db/write-transaction.ts`. Drizzle's bun-sqlite `db.transaction(async () => …)` commits at the callback's first `await` instead of awaiting the returned Promise, so the rest of the write runs outside the transaction. ESLint rejects an `async` callback passed to `.transaction(...)`. - Change the schema with a numbered migration, following the recipe in `src/db/README.md`. Keep DDL in the migrations and `initial_schema.sql` only. - Open a database through `openOperationalDatabase`, which runs schema evolution. Open the file directly only when the code must not migrate it, and say why in a comment. For example, `src/scripts/check-integrity.ts` opens a backup with `new Database(path, { readonly: true })` so that checking it leaves it unchanged. From 85cb068eed7f2c8efdacdf77dd8dc93d70e84518 Mon Sep 17 00:00:00 2001 From: Maxwell Murphy Date: Tue, 6 Oct 2026 16:49:24 -0700 Subject: [PATCH 10/21] docs: restore the test rules the README rewrite dropped. The old API and web test READMEs asked for edge-case tests, loading and error state tests, and waitFor or findBy for async work. The rewrite cut them instead of moving them. Co-Authored-By: Claude Opus 5.5 (1M context) --- CODING_STANDARDS.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CODING_STANDARDS.md b/CODING_STANDARDS.md index ada7a33b..607b49d9 100644 --- a/CODING_STANDARDS.md +++ b/CODING_STANDARDS.md @@ -26,6 +26,7 @@ Paths are relative to `packages/api/`. - Seed rows with the factories in `src/__tests__/helpers/factories.ts`. - Test a route behind session auth through `setupAuthenticatedRouteTest`. Log in by hand with `loginAndGetCookie` only when the login flow is under test, as in the auth route and middleware tests. - For a CRUD route with `checkInUse`, write a delete test for both the in-use and the free case. +- Test a route's error cases as well as its success: validation errors, 404s, and duplicates. ## Web @@ -38,4 +39,6 @@ Paths are relative to `packages/web/`. - Render components with `await render(...)` from `src/__tests__/helpers/render`. - Mock the domain module the code imports with `createMockedDomainModule`. Make each override replace a whole `*Api` object and resolve to the same body the real module returns. - Query elements by role or label (`getByRole`, `getByLabelText`), and drive interactions with `@testing-library/user-event`. +- Wait for async results with `findBy*` queries or `waitFor`. +- Test a component's loading and error states, not only its loaded state. - In a mutation hook's test, assert which queries it invalidates. From acb19ecbe6560f79db33cea31016e79731ccb615 Mon Sep 17 00:00:00 2001 From: Maxwell Murphy Date: Tue, 6 Oct 2026 16:49:28 -0700 Subject: [PATCH 11/21] docs: keep the whole-*Api override rule in CODING_STANDARDS.md only. The web test README restated it; it now shows how createMockedDomainModule works and points to the rule. Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/web/src/__tests__/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/web/src/__tests__/README.md b/packages/web/src/__tests__/README.md index 012bbc75..448cb198 100644 --- a/packages/web/src/__tests__/README.md +++ b/packages/web/src/__tests__/README.md @@ -23,7 +23,7 @@ it('handles a click', async () => { ## Mocking the API -Code calls the API through domain modules under `src/lib/api/`, so a test mocks the module the code imports. `createMockedDomainModule(domain, overrides)` in `helpers/mock-api.ts` loads the real module and swaps in the `*Api` objects you pass: +`createMockedDomainModule(domain, overrides)` in `helpers/mock-api.ts` loads the real module with `vi.importActual` and replaces each export named in `overrides`. What to pass is under Web tests in `CODING_STANDARDS.md`: ```typescript import { studiesApi } from '../../lib/api/studies' From 8e96e89a775b03cb2e56972a7afd98dca707c2ed Mon Sep 17 00:00:00 2001 From: Maxwell Murphy Date: Tue, 6 Oct 2026 16:49:40 -0700 Subject: [PATCH 12/21] docs: describe the pre-commit hook and ci:verify in README.md only. AGENTS.md now points to the paragraph instead of restating it. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 2af56e1d..a7ac2846 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -9,7 +9,7 @@ - Web tests: `cd packages/web && bun run test -- `. Bun's own runner (`bun test`) hangs on React component tests; Vitest exits on its own, so piping it to `tail` or `grep` is safe. - API tests: `cd packages/api && bun test src/...`. - Lint is clean, so any lint error is yours. -- The pre-commit hook only typechecks and lints. Run `bun run ci:verify`, which CI runs on every PR, before pushing. +- Run `bun run ci:verify` before pushing. The end of Build in `README.md` says what it and the pre-commit hook check. ## Where to look From be5a54714fc8085cab527f8383788ad6b788a2b5 Mon Sep 17 00:00:00 2001 From: Maxwell Murphy Date: Tue, 6 Oct 2026 16:49:45 -0700 Subject: [PATCH 13/21] docs: name the five triage labels in AGENTS.md. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index a7ac2846..e7ca05f8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,7 +1,7 @@ ## Agent skills - **Issue tracker**: GitHub Issues on `eppicenter/sampledb3`. "Publish to the issue tracker" is `gh issue create`; "fetch the relevant ticket" is `gh issue view --comments`. -- **Triage labels**: each triage role is a GitHub label with the role's name. +- **Triage labels**: each of the five triage roles is a GitHub label with the role's name: `needs-triage`, `needs-info`, `ready-for-agent`, `ready-for-human`, and `wontfix`. - **Domain docs**: `CONTEXT.md` is the glossary, `docs/adr/` holds the decisions, and `packages/docs/` is the user guide. ## Checks From 35f4f1d83ff71c81b9b3d2dfe6c44b42f19fd17f Mon Sep 17 00:00:00 2001 From: Maxwell Murphy Date: Tue, 6 Oct 2026 16:49:45 -0700 Subject: [PATCH 14/21] docs: rename the AGENTS.md "Checks" heading so it isn't confused with the Checks workflow. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index e7ca05f8..84d0d2ae 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -4,7 +4,7 @@ - **Triage labels**: each of the five triage roles is a GitHub label with the role's name: `needs-triage`, `needs-info`, `ready-for-agent`, `ready-for-human`, and `wontfix`. - **Domain docs**: `CONTEXT.md` is the glossary, `docs/adr/` holds the decisions, and `packages/docs/` is the user guide. -## Checks +## Running tests and checks - Web tests: `cd packages/web && bun run test -- `. Bun's own runner (`bun test`) hangs on React component tests; Vitest exits on its own, so piping it to `tail` or `grep` is safe. - API tests: `cd packages/api && bun test src/...`. From c0f3e5532f2c8fb4c65fb4fae5c2477643423a0c Mon Sep 17 00:00:00 2001 From: Maxwell Murphy Date: Tue, 6 Oct 2026 16:49:52 -0700 Subject: [PATCH 15/21] docs: say what CODING_STANDARDS.md holds in the AGENTS.md pointer. The old pointer covered nearly every task, so it routed nowhere. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 84d0d2ae..3e4c6247 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,7 +13,7 @@ ## Where to look -- Writing code, tests, issues, or proposals, or reviewing a change: `CODING_STANDARDS.md`. +- Rules for naming domain concepts, contradicting an ADR, failing tests, database access, calling the API from the web, and writing API and web tests: `CODING_STANDARDS.md`. The `code-review` skill checks changes against it. - Writing an API test: `packages/api/src/__tests__/README.md`. - Writing a web test: `packages/web/src/__tests__/README.md`. - Changing the schema or adding a migration: `packages/api/src/db/README.md`. From 063c188c7ee1998fa6a417a779d693fabf9f7ed7 Mon Sep 17 00:00:00 2001 From: Maxwell Murphy Date: Tue, 6 Oct 2026 16:49:52 -0700 Subject: [PATCH 16/21] docs: give AGENTS.md an H1 heading, as CODING_STANDARDS.md has. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 3e4c6247..d92a4430 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,3 +1,5 @@ +# Agent guide + ## Agent skills - **Issue tracker**: GitHub Issues on `eppicenter/sampledb3`. "Publish to the issue tracker" is `gh issue create`; "fetch the relevant ticket" is `gh issue view --comments`. From af31451606a9ad4bbe6417e89f86dd70b9cb3ff9 Mon Sep 17 00:00:00 2001 From: Maxwell Murphy Date: Tue, 6 Oct 2026 16:50:12 -0700 Subject: [PATCH 17/21] docs: make the root README the one place for coverage targets. The 90% target and what each threshold does were spread across README.md, packages/web/vitest.config.ts and the commented-out bunfig.toml threshold. The configs now point to a Coverage section; no config values change. Co-Authored-By: Claude Opus 5.5 (1M context) --- README.md | 11 +++++++++-- packages/api/bunfig.toml | 2 +- packages/api/vitest.config.ts | 1 + packages/web/vitest.config.ts | 2 +- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 244a32f5..0d793692 100644 --- a/README.md +++ b/README.md @@ -72,8 +72,15 @@ Put local overrides in **`.env.local`** (gitignored) at the repo root. Bun’s a - **Unit tests (API + web)**: `bun run test` - **End-to-end (Playwright)**: `bun run test:e2e` -- **API**: `bun --filter @sampledb/api test` (Bun). Coverage: `bun --filter @sampledb/api test:coverage` (report in `packages/api/coverage/`). Target 90% lines. Test helpers: `packages/api/src/__tests__/README.md`. -- **Web**: `bun --filter @sampledb/web test` (Vitest). Coverage: `bun --filter @sampledb/web test:coverage`. Coverage excludes `src/lib/api/**` and `src/**/*.css` so the 90% target applies to testable code. Thresholds in `packages/web/vitest.config.ts` can be raised in steps toward 90%. Test helpers: `packages/web/src/__tests__/README.md`. +- **API**: `bun --filter @sampledb/api test` (Bun). Coverage: `bun --filter @sampledb/api test:coverage` (report in `packages/api/coverage/`). Test helpers: `packages/api/src/__tests__/README.md`. +- **Web**: `bun --filter @sampledb/web test` (Vitest). Coverage: `bun --filter @sampledb/web test:coverage`. Test helpers: `packages/web/src/__tests__/README.md`. + +### Coverage + +This is the one place coverage targets are written down; the config files point here. Both packages target 90% line coverage. + +- **API**: no threshold is enforced. `coverageThreshold` in `packages/api/bunfig.toml` is commented out; set it to make `test:coverage` fail below that line coverage. The thresholds in `packages/api/vitest.config.ts` apply only to Vitest coverage runs, and Vitest cannot run the tests that touch `bun:sqlite`. +- **Web**: the thresholds in `packages/web/vitest.config.ts` are a floor that fails `test:coverage`. Raise them in steps toward 90%. Coverage excludes `src/lib/api/**` and `src/**/*.css`, so the target applies to testable code. ## Database Configuration diff --git a/packages/api/bunfig.toml b/packages/api/bunfig.toml index 1332dd59..630ecf32 100644 --- a/packages/api/bunfig.toml +++ b/packages/api/bunfig.toml @@ -5,6 +5,6 @@ exclude = ["dist/**", "node_modules/**"] coverage = true coverageReporter = ["text", "lcov"] coverageDir = "coverage" -# Optional: set to fail the run below this line coverage. +# Optional: set to fail the run below this line coverage. Targets: Coverage in the root README.md. # coverageThreshold = 50 coveragePathIgnorePatterns = ["node_modules", "src/__tests__", "src/**/*.d.ts", "src/**/*.config.*", "dist"] diff --git a/packages/api/vitest.config.ts b/packages/api/vitest.config.ts index fb3e43de..e91696eb 100644 --- a/packages/api/vitest.config.ts +++ b/packages/api/vitest.config.ts @@ -20,6 +20,7 @@ export default defineConfig({ 'src/**/__tests__/**', 'dist/**', ], + // Applies only to Vitest coverage runs. Targets: Coverage in the root README.md. thresholds: { statements: 60, branches: 60, diff --git a/packages/web/vitest.config.ts b/packages/web/vitest.config.ts index 0c645642..e238ca36 100644 --- a/packages/web/vitest.config.ts +++ b/packages/web/vitest.config.ts @@ -38,7 +38,7 @@ export default defineConfig({ 'src/lib/api/**', 'dist/**', ], - // Baseline; raise in steps toward 90% as coverage improves. + // A floor, not the target. Targets: Coverage in the root README.md. thresholds: { statements: 35, branches: 26, From d40df05a393259bf99d0636bf88d16ee39d8f8b2 Mon Sep 17 00:00:00 2001 From: Maxwell Murphy Date: Tue, 6 Oct 2026 16:50:25 -0700 Subject: [PATCH 18/21] docs: restore the CURRENT_SCHEMA_VERSION bullet in the db README. It is the only place the README defines the CURRENT that the evolution table compares against, and support needs it to read a schema_version value. Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/api/src/db/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/api/src/db/README.md b/packages/api/src/db/README.md index b33f5597..57b70a2c 100644 --- a/packages/api/src/db/README.md +++ b/packages/api/src/db/README.md @@ -27,6 +27,7 @@ Migration `001` is **only** for legacy files; fresh installs from the snapshot s ## Schema version - Table: `schema_version` with a single row (`version INTEGER NOT NULL`). +- Constant: `CURRENT_SCHEMA_VERSION` in `schema-evolution.ts`, the version the code expects. "How evolution works" calls it `CURRENT`. - Inspect in support: `SELECT version FROM schema_version;` ## Opening a database From f51b3521f9a9154fa8c28083731ebedd112a573b Mon Sep 17 00:00:00 2001 From: Maxwell Murphy Date: Thu, 29 Oct 2026 22:19:40 -0700 Subject: [PATCH 19/21] docs(standards): say which code may run DDL, and scope the web test rules to new and changed tests. Co-Authored-By: Claude Opus 5.5 --- CODING_STANDARDS.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/CODING_STANDARDS.md b/CODING_STANDARDS.md index 091d4e7f..97562f0b 100644 --- a/CODING_STANDARDS.md +++ b/CODING_STANDARDS.md @@ -17,7 +17,7 @@ Paths are relative to `packages/api/`. ### Database - Run a write of several statements that `await`s inside `withWriteTransaction` from `src/db/write-transaction.ts`. Drizzle's bun-sqlite `db.transaction(async () => …)` commits at the callback's first `await` instead of awaiting the returned Promise, so the rest of the write runs outside the transaction. ESLint rejects any callback passed to `.transaction(...)` that returns a Promise. -- Change the schema with a numbered migration, following the recipe in `src/db/README.md`. Keep DDL in the migrations and `initial_schema.sql` only. +- Change the schema with a numbered migration, following the recipe in `src/db/README.md`, and refresh `initial_schema.sql` to match. Keep DDL out of the connection code (`src/db/open.ts`, `src/db/client.ts`), routes, and `src/lib/`. The schema-evolution modules in `src/db/` (`migration-runner.ts`, `migration-preflight.ts`, `legacy-baseline.ts`, `apply-initial-schema.ts`) run DDL as part of evolution, and tests may create tables inline to build a legacy or partial database, as `src/db/__tests__/client.test.ts` and `src/lib/subjects/__tests__/subject-read.test.ts` do. - Open a database through `openOperationalDatabase`, which runs schema evolution. Open the file directly only when the code must not migrate it, and say why in a comment. For example, `src/scripts/check-integrity.ts` opens a backup with `new Database(path, { readonly: true })` so that checking it leaves it unchanged, and `src/db/__tests__/initial-schema-snapshot.test.ts` applies `initial_schema.sql` alone so that pending migrations can't hide a stale snapshot. ### API tests @@ -36,9 +36,8 @@ Paths are relative to `packages/web/`. ### Web tests -- Render components with `await render(...)` from `src/__tests__/helpers/render`. - Mock the domain module the code imports with `createMockedDomainModule`. Make each override replace a whole `*Api` object and resolve to the same body the real module returns. -- Query elements by role or label (`getByRole`, `getByLabelText`), and drive interactions with `@testing-library/user-event`. +- In a new or changed test, render with `await render(...)` from `src/__tests__/helpers/render`, query elements by role or label (`getByRole`, `getByLabelText`), and drive interactions with `@testing-library/user-event`. Older tests still render with `@testing-library/react`'s `render`, click with `fireEvent`, and query by text; convert a test when you change it. Tests of a provider itself, such as `src/contexts/__tests__/ThemeContext.test.tsx`, render without the helper because it would wrap them in a second copy of the provider. - Wait for async results with `findBy*` queries or `waitFor`. - Test a component's loading and error states, not only its loaded state. - In a mutation hook's test, assert which queries it invalidates. From c23c9f3a5fce48be381b470c4c3ae37dfc60eb99 Mon Sep 17 00:00:00 2001 From: Maxwell Murphy Date: Thu, 29 Oct 2026 22:19:40 -0700 Subject: [PATCH 20/21] docs(agents): give the API and contract test commands, setup:hooks, and a durable lint note. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index d92a4430..36ec53e8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -4,14 +4,15 @@ - **Issue tracker**: GitHub Issues on `eppicenter/sampledb3`. "Publish to the issue tracker" is `gh issue create`; "fetch the relevant ticket" is `gh issue view --comments`. - **Triage labels**: each of the five triage roles is a GitHub label with the role's name: `needs-triage`, `needs-info`, `ready-for-agent`, `ready-for-human`, and `wontfix`. -- **Domain docs**: `CONTEXT.md` is the glossary, `docs/adr/` holds the decisions, and `packages/docs/` is the user guide. +- **Domain docs**: `CONTEXT.md` is the glossary, `docs/adr/` holds the decisions, and `packages/docs/` is the user guide. Before you name or change a domain concept, read its `CONTEXT.md` entry and the ADRs that touch the area. ## Running tests and checks - Web tests: `cd packages/web && bun run test -- `. Bun's own runner (`bun test`) hangs on React component tests; Vitest exits on its own, so piping it to `tail` or `grep` is safe. -- API tests: `cd packages/api && bun test src/...`. -- Lint is clean, so any lint error is yours. -- Run `bun run ci:verify` before pushing. The end of Build in `README.md` says what it and the pre-commit hook check. +- API tests: `cd packages/api && NODE_ENV=test bun test src/...`. The package's `test` script sets `NODE_ENV=test`, and `src/db/legacy-baseline.ts` branches on it. +- Contract tests: `cd packages/contract && bun run test`. `bun run test` at the repo root runs the contract, API, and web suites. +- The `Checks` workflow lints every PR, so a lint error comes from your change. +- Run `bun run setup:hooks` once per clone to install the pre-commit hook, and `bun run ci:verify` before pushing. The end of Build in `README.md` says what each checks. ## Where to look From 2d7ce7098e7c27cac7751d71bf3fb92e2948fc0f Mon Sep 17 00:00:00 2001 From: Maxwell Murphy Date: Thu, 29 Oct 2026 22:19:40 -0700 Subject: [PATCH 21/21] docs(tests): make the README test snippets compile and follow the query rule. Co-Authored-By: Claude Opus 5.5 --- packages/api/src/__tests__/README.md | 3 +++ packages/web/src/__tests__/README.md | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/api/src/__tests__/README.md b/packages/api/src/__tests__/README.md index 2f63d038..6013ea73 100644 --- a/packages/api/src/__tests__/README.md +++ b/packages/api/src/__tests__/README.md @@ -9,6 +9,9 @@ This guide covers the helpers. The rules are under API tests in `CODING_STANDARD `setupTestDatabase()` in `helpers/db-setup.ts` opens an in-memory database through `openOperationalDatabase(':memory:')`, the production path, so it gets `initial_schema.sql` plus any pending migrations. To change the schema, follow `packages/api/src/db/README.md`. ```typescript +import { describe, beforeEach, afterEach } from 'vitest' +import type { Database as SQLiteDatabase } from 'bun:sqlite' +import type { Database } from '../../db/client' import { setupTestDatabase, cleanupTestDatabase } from '../../__tests__/helpers/db-setup' describe('My API', () => { diff --git a/packages/web/src/__tests__/README.md b/packages/web/src/__tests__/README.md index 008536c1..f73a6d38 100644 --- a/packages/web/src/__tests__/README.md +++ b/packages/web/src/__tests__/README.md @@ -17,7 +17,7 @@ it('handles a click', async () => { const user = userEvent.setup() await render() await user.click(screen.getByRole('button', { name: 'Save' })) - expect(screen.getByText('Saved')).toBeInTheDocument() + expect(await screen.findByRole('alert')).toHaveTextContent('Saved') }) ```