Choose Containers for export and statistics through Container selection (ADR 0011)

AGENTS.md

@@ -1,19 +1,21 @@
+# 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 <number> --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
+## Running tests and checks
 
 - Web tests: `cd packages/web && bun run test -- <files>`. 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
 
-- 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`.

CODING_STANDARDS.md

@@ -2,58 +2,76 @@
 
 ## 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
 
 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`. 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.
+- 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.
 
 ### 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).
+- In a new write path, create Specimens and Containers with `registerSpecimens` from `src/lib/specimen-registration/` (ADR 0010). Bulk combined import, `/subjects/with-specimens` and `/specimens/bulk` have migrated, and every container write goes through `createContainerForSpecimen`, which wraps it. These paths still insert Specimens directly until their slice in ADR 0010 lands: `POST /specimens`, which validates with `validateSpecimenData` (`src/routes/specimens.ts`); derivations (`src/lib/derivations.ts`); and control batches (`src/lib/controls/batch-with-specimens.ts`).
+- In a new write path, validate a registration with `checkSpecimenRegistration`, or wrap the whole write in `withRolledBackTransaction` when the caller writes its own records first (ADR 0010).
+- Validate an inbound container with a `@sampledb/contract` schema (`containerWriteInputSchema`, `bulkCombinedContainerSchema` or `optionalContainerInputSchema`), not an endpoint-specific container Zod schema (ADR 0006).
+- Map a CSV row to `ContainerWriteInput` with `csvRowToContainerWriteInput` from `@sampledb/contract` (ADR 0006). The web bulk import and control-batch CSVs still map through `flatBulkContainerToWriteInput` and `flatControlBatchContainerToWriteInput` in `packages/web/src/lib/`.
+- In a `ContainerWriteInput`, place a paper container on a `collection` of type `sheet`, not on a box or bag `collection` with a sibling `sheetName` (ADR 0006).
+- Resolve a Specimen's Source with `resolveSpecimenSource` or `resolveSpecimenSources` from `src/lib/specimens/provenance.ts` (ADR 0007).
+- Return the full `SpecimenSource` shape to every caller, not a per-endpoint subset (ADR 0007).
+- Load a container read payload (placement, Tags, Specimen, Source) with `loadContainerReadViews` or `loadContainerReadViewsByIds` from `src/lib/container-read-view.ts` (ADR 0009).
+- Project the loaded view to its read path's type: `EnrichedContainerApi` for list, derivations and PATCH, `EnrichedStorageContainer` for collection detail, and CSV row fields for Container export (ADR 0009).
+- To choose which Containers an export or statistic covers, call `selectContainers` from `src/lib/container-selection/`, and filter a paginated Container query by Tags with `containerHasAllTags` (ADR 0011).
 
 ### 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).
+- In new or changed route code, report an error by throwing a typed error from `src/lib/error-handler.ts` or returning `handleRouteError(error, c)`, not with `return c.json({ error }, 4xx)` (ADR 0004).
+- Throw `ExpectedNotFoundError` for a 404 caused by user input, such as an export preflight miss or a user-supplied lookup, and `NotFoundError` for a missing resource on a direct GET or a stale link (ADR 0004).
+- Write no route-level `catch` block that calls `console.error`; return `handleRouteError(error, c)` from it instead (ADR 0004).
+- In new code, write log lines to stdout with `logDebug`, `logInfo`, `logWarn` or `logError` from `src/lib/observability.ts`, not `console.*`. Startup banners and schema-evolution CLI output stay plain `console.log` (ADR 0004). The `console.warn` in `logTemplateError` in `src/routes/qpcr-experiments.ts` predates this rule.
+- Persist a failure to the `error_logs` table with `src/lib/error-logger.ts`, not `observability.ts`. Both export `logError`, so check the import path (ADR 0004).
+- In a new or changed route handler, do not spread a Drizzle row into a `c.json` payload (ADR 0005). `...newSpecimen` in `src/routes/specimens.ts` and `...locationRecord` in `src/routes/locations.ts` predate this rule.
+- Map a response that has a schema in `@sampledb/contract/wire` (containers today) to that wire DTO before `c.json` (ADR 0005).
+- Leave null omission to `omitOnWireMiddleware`: do not call `toWireJson()` in a route handler (ADR 0005).
 
 ### 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`. 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
 
 Paths are relative to `packages/web/`.
 
 - Call the API through its domain module, `src/lib/api/<domain>`. 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).
+- Read a setting with `settingsApi.getValue(key)` or `settingsApi.get(key).value`, never `api.get` on `/settings/:key` (ADR 0002).
+- Write a setting with `settingsApi.putValue`, `settingsApi.update`, or the bulk wrappers `exportConfigurationsApi.update`, `scannerConfigurationsApi.update` and `tableViewConfigurationsApi.update`, never `api.put` on `/settings/:key` (ADR 0002).
+- In a Zod parser for a container response, mark an unset field `.optional()`, not `.nullish()` (ADR 0005). Other response parsers are out of scope, such as `parseSettingsEnvelope` in `src/lib/api/parse-response.ts`.
+- On a screen ADR 0001 lists as migrated, show a query's read error inline with `PageError` or `SectionMessage` from `src/ui/`, not a toast (ADR 0001). Workflow local state (CSV parse, server validate, submits) keeps its inline or toast errors.
+- Prefer to send a new mutation through a domain hook, such as `useCreateStudy`, that invalidates the queries the mutation changes, including `dashboardKeys` when it changes dashboard data (ADR 0001). Direct `*Api.create`, `update` and `delete` calls in `Locations.tsx`, `QpcrExperimentDetail.tsx`, `StudyDetail.tsx`, `SubjectForm.tsx` and `ControlDefinitionForm.tsx` predate this rule.
+- Put new Scan move behaviour in the `src/lib/scan-move/` core with plain-data unit tests, not in `ContainerMoveMicronix.tsx` or `ContainerMoveCryovial.tsx` (ADR 0008). The paper move page, `ContainerMovePapers.tsx`, is not a Scan move.
+- Pass a Scan move effect a `ScanMoveGateway` instead of importing `collectionsApi` into it (ADR 0008).
+- Test Scan move effects with plain stub gateway functions, not `vi.mock` (ADR 0008).
+- Open a modal with `Modal` from `src/ui/`, not `ModalPortal` (ADR 0001). `CommandPalette` and `SearchModal` still use `ModalPortal` directly.
 
 ### 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.
+- 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.

CONTEXT.md

@@ -137,6 +137,10 @@ _Avoid_: Well (as the domain term); treating a missing position as occupying a s
 
 ### Export
 
+**Container selection**:
+The set of **Containers**, with their **Specimens**, that a set of filters matches: a **Study**, a subject list, or a micronix barcode list, narrowed by specimen type, collection date, container type, **Tags**, **Location**, and when the Specimen or the Container was created. Selecting several Tags matches Containers carrying every one of them. A barcode list selects exactly those tubes and takes no other filters. **Container export** and the statistics dashboard both count and list what the selection matches.
+_Avoid_: Query (implementation term); treating Tag selection as "any of these Tags"; an unqualified "created" date (say Specimen created or Container created)
+
 **Container export**:
 Server-side export of operational **Container** records with specimen, source, and placement context, formatted using an **Export configuration**. Entry points include **Multi-Study Export** (page heading **Export Containers (Multi-Study)**), the Study export modal, and **Micronix Barcode Export**.
 _Avoid_: Using "bulk export" as the umbrella for all container exports; collection table snapshot export; using "sample" in export column headers
@@ -170,8 +174,9 @@ Server-side flat export of inventory summary data (command palette or inventory.
 _Avoid_: Container export; bulk export; collection table snapshot export
 
 **CSV download**:
-A comma-separated values file generated or downloaded from SampleDB — container/specimen/inventory exports, collection table snapshots, import templates, and other CSV outputs. All share the same wire conventions (UTF-8 with BOM, CRLF line endings, plain cell values per RFC 4180) so files behave consistently in Excel and in R/Python. **Container export** workflows additionally let lab staff override delimiter, BOM, and line ending before download.
-_Avoid_: Excel formula cell wrappers (`="..."`); using CSV as the Excel text-preservation format (use **XLSX download** for that); different wire conventions for templates vs exports
+A comma-separated values file generated or downloaded from SampleDB — container/specimen/inventory exports, collection table snapshots, import templates, and other CSV outputs. All share the same wire conventions (UTF-8 with BOM, CRLF line endings, cell values quoted per RFC 4180) so files behave consistently in Excel and in R/Python. **Container export** workflows additionally let lab staff override delimiter, BOM, and line ending before download.
+Server-side data exports (**Container export**, **Specimen export**, **Inventory export**) apply **formula neutralization**: a text cell that a spreadsheet would run as a formula (starting with `=`, `+`, `@`, tab, carriage return, or `-` unless the whole cell is a number) gets a leading `'`, so stored text such as `=HYPERLINK(...)` cannot execute when the file is opened. Numbers, including negative ones like `-131`, are unchanged. Import templates and other files meant to be re-imported are not neutralized, since the `'` would become part of the imported value.
+_Avoid_: Excel formula cell wrappers (`="..."`); using CSV as the Excel text-preservation format (use **XLSX download** for that); different wire conventions for templates vs exports; neutralizing import templates
 
 **XLSX download**:
 Excel workbook export from **Container export** workflows. Preserves identifier and code columns as text cells (leading zeros, long barcodes) without CSV workarounds.

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
 

docs/adr/0006-unified-container-inbound-write.md

@@ -64,6 +64,8 @@ All persistence funnels through `createContainerForSpecimen` (accepting `Contain
 
 Placement resolution (sheet find-or-create within box/bag, plate/box resolve) lives in one module — not duplicated across registration, control batch, and derivation CSV.
 
+> **Amended by [ADR 0010](./0010-specimen-registration.md):** that module is `lib/specimen-registration/collections.ts`, and `createContainerForSpecimen` now wraps `registerSpecimens`. It replaced `container-write-placement.ts`, whose lookup fell back from a missing collection barcode to the name; that file is deleted. The derivation CSV and control batch validators still look collections up themselves until their ADR 0010 slices replace them.
+
 ## CSV
 
 One contract mapper — `csvRowToContainerWriteInput` — maps flat CSV rows to `ContainerWriteInput`, then Zod validates.