test: reach 100% unit test coverage by deleting dead code, and enforce it

.agents/docs/architecture/testing-strategy.md

@@ -17,11 +17,11 @@ of operations.
 
 ## Tiers
 
-| Tier       | Runner     | Where                                                    | Command         |
-| ---------- | ---------- | -------------------------------------------------------- | --------------- |
-| Unit       | vitest     | `src/__tests__/` (or `test/`) in a participating package | `pnpm test`     |
-| End-to-end | Playwright | `testing/src/specs/`                                     | `pnpm test:e2e` |
-| Type-check | `tsc`      | —                                                        | `pnpm lint`     |
+| Tier       | Runner     | Where                                                    | Command              |
+| ---------- | ---------- | -------------------------------------------------------- | -------------------- |
+| Unit       | vitest     | `src/__tests__/` (or `test/`) in a participating package | `pnpm test:coverage` |
+| End-to-end | Playwright | `testing/src/specs/`                                     | `pnpm test:e2e`      |
+| Type-check | `tsc`      | —                                                        | `pnpm lint`          |
 
 There is no integration tier. `apps/api` unit tests mock the Prisma layer entirely; the only code
 path exercised against a real database is Playwright.
@@ -115,6 +115,25 @@ no database. Only `testing` owns a `test:e2e` script; only `apps/gateway` owns a
 `globalDependencies` are `.env`, `eslint.config.js`, `tsconfig.json` and `prettier.config.js`.
 Touching any of them invalidates the cache for every task in the repo.
 
+## Coverage
+
+`pnpm test:coverage` is the unit gate, locally and in CI. The root `vitest.config.ts` sets
+`coverage.thresholds: { 100: true }`, so the run fails when statements, branches, functions or lines
+drop below 100%. The scope is its `coverage.include` — `apps/{api,gateway,web}/src` and
+`packages/*/src`; `runtime/**` and `apps/playground` are outside it — minus `coverage.exclude`,
+which lists only non-code: test fixtures and helpers (`__tests__`, `__mocks__`,
+`apps/web/src/testing`), the generated `route-tree.ts`, and the bare entrypoints.
+
+- **A line no test can reach is dead code.** Prove it (the types, every caller, the library's
+  contract) and delete it, or restructure so the impossible state cannot be written — e.g. ts-pattern
+  `.exhaustive()` instead of `.otherwise(() => null)`. Never add an ignore comment or an exclude entry
+  to pass.
+- **A scoped run fails the threshold by construction**: `--project`, a file filter or a narrowed
+  `--coverage.include` measures a fraction of the scope. Add `--coverage.thresholds.100=false` to it
+  and read the per-file report.
+- `apps/api` confines its SWC transform to its own files (`apps/api/vitest.config.ts`); letting SWC
+  transform the workspace packages it imports makes the merged report count their functions twice.
+
 ## What CI gates
 
 `.github/workflows/ci.yaml`, one job (`lint-and-test`, ubuntu-latest), triggered on
@@ -125,7 +144,7 @@ Touching any of them invalidates the cache for every task in the repo.
 | 1   | Generate Environment | `./scripts/generate-env.sh`                            |
 | 2   | Install Dependencies | `pnpm install --frozen-lockfile`                       |
 | 3   | Lint                 | `pnpm lint`                                            |
-| 4   | Unit Tests           | `pnpm test`                                            |
+| 4   | Unit Tests           | `pnpm test:coverage`                                   |
 | 5   | Install Playwright   | chromium + firefox `--with-deps`, capped at 10 minutes |
 | 6   | End-to-End Tests     | `pnpm test:e2e`                                        |
 
@@ -135,8 +154,6 @@ does not hide their results.
 Not gated, despite existing in the repo:
 
 - **No separate type-check step** — it is folded into step 3.
-- **No coverage.** Nothing in CI runs `pnpm test:coverage`, and the root config declares no
-  `thresholds`. Its `include` is `apps/**/*` and `packages/**/*`, so `runtime/**` is outside it.
 - **No knip step**, although `knip.ts` and a `pnpm knip` script exist.
 - **No format check.** Prettier runs only in the `.husky/pre-commit` hook (`prettier-pre-commit`),
   which formats rather than verifies; the hook does not run lint or tests.

.agents/docs/playbooks/add-env-var.md

@@ -78,6 +78,6 @@ touched exactly two of these: `.env.template` and `$Env`, replacing a direct
 grep -n YOUR_KEY .env    # present after regeneration
 pnpm dev                 # $Env is validated at boot; an invalid value crashes here. Ctrl-C once up
 pnpm lint                # tsc + the no-process.env rule
-pnpm test
+pnpm test:coverage
 pnpm build               # only this catches a missing turbo.json `env` entry
 ```

.agents/docs/playbooks/add-instrument.md

@@ -82,7 +82,7 @@ advice does not apply here; everything it says about the definition object does.
 pnpm exec turbo run build --filter=@opendatacapture/instrument-library  # builds runtime/v1 first
 pnpm --filter @opendatacapture/instrument-library run available         # reads dist, not src
 pnpm lint
-pnpm test
+pnpm test:coverage
 pnpm test:e2e
 ```
 

.agents/docs/playbooks/add-vendor-package.md

@@ -73,5 +73,5 @@ pnpm --filter @opendatacapture/runtime-v1 build   # READ THE OUTPUT — it logs
 ls runtime/v1/dist/<name>@<range>                 # step 8 was silent if this is missing
 pnpm exec vitest --project runtime-v1             # peer pairing
 pnpm exec vitest --project runtime-bundler
-pnpm lint && pnpm test
+pnpm lint && pnpm test:coverage
 ```

.agents/docs/playbooks/add-web-data-hook.md

@@ -73,5 +73,5 @@ step type-checks and compiles, and fails at runtime or serves the wrong cached d
 ```sh
 pnpm exec vitest apps/web/src/hooks/__tests__/<hookName>.test.ts   # the new test alone
 pnpm exec vitest --project web                                     # every web test
-pnpm lint && pnpm test                                             # required before you are done
+pnpm lint && pnpm test:coverage                                    # required before you are done
 ```

.agents/docs/playbooks/promote-to-react-core.md

@@ -104,7 +104,7 @@ pnpm --filter @opendatacapture/react-core lint      # no app aliases in this pro
 pnpm --filter @opendatacapture/web lint
 pnpm --filter @opendatacapture/gateway lint         # meaningful only once step 9 wired the consumer
 pnpm exec vitest --project web
-pnpm lint && pnpm test
+pnpm lint && pnpm test:coverage
 pnpm test:e2e
 pnpm --filter @opendatacapture/storybook storybook  # port 6006 — listed under React Core, not Web
 ```

.agents/skills/odc-done/SKILL.md

@@ -37,14 +37,15 @@ exception rather than editing the tree yourself.
 ## 2. Test, and make the run name your file
 
 ```sh
-pnpm test
+pnpm test:coverage                  # fails below 100% on any metric — see testing-strategy.md §Coverage
 pnpm exec vitest list --filesOnly   # a green run prints counts only; this prints the collected files
 ```
 
 A test file the listing never names was collected by nothing —
 `.agents/skills/odc-testing/SKILL.md` opens on that failure and owns both the diagnosis and the fix.
 
-**Done when** the run is green _and_ `list --filesOnly` names every test file this change added.
+**Done when** the run is green at 100% coverage _and_ `list --filesOnly` names every test file this
+change added.
 
 ## 3. Run the end-to-end suite, or raise the exception
 

.agents/skills/odc-testing/SKILL.md

@@ -27,11 +27,11 @@ and the e2e test carries the change, or this change adds its `vitest.config.ts`.
 
 ## Choose the tier before you choose the file
 
-| What you changed                                        | The tier that can prove it                 | Runner          |
-| ------------------------------------------------------- | ------------------------------------------ | --------------- |
-| A pure function, a schema, a hook, a rendered component | Unit, in that package's own project        | `pnpm test`     |
-| Who can see or do what; anything reading real rows      | End-to-end only                            | `pnpm test:e2e` |
-| A type contract — inference, a discriminated union      | The type-check, which is `tsc` inside lint | `pnpm lint`     |
+| What you changed                                        | The tier that can prove it                 | Runner               |
+| ------------------------------------------------------- | ------------------------------------------ | -------------------- |
+| A pure function, a schema, a hook, a rendered component | Unit, in that package's own project        | `pnpm test:coverage` |
+| Who can see or do what; anything reading real rows      | End-to-end only                            | `pnpm test:e2e`      |
+| A type contract — inference, a discriminated union      | The type-check, which is `tsc` inside lint | `pnpm lint`          |
 
 **There is no integration tier, and the middle row is why.** `apps/api` unit tests mock the Prisma
 layer entirely (`apps/api/src/groups/__tests__/groups.service.spec.ts`), so the only code path

.github/workflows/ci.yaml

@@ -31,7 +31,7 @@ jobs:
       - name: Lint
         run: pnpm lint
       - name: Unit Tests
-        run: pnpm test
+        run: pnpm test:coverage
       - name: Install Playwright
         # Cap the step so a hang fails fast instead of hitting the 6h job limit, skip the interactive
         # apt prompt, and install only the browsers the e2e suite uses.

AGENTS.md

@@ -21,17 +21,20 @@ pnpm build                  # turbo build (all packages/apps)
 pnpm lint                   # turbo lint — `tsc && eslint --fix src` per package, plus lint:root for root-level scripts and configs
 pnpm format                 # turbo format (prettier per package)
 pnpm test                   # vitest across the workspace
+pnpm test:coverage          # vitest with coverage — fails below 100%, the gate CI and "Before you are done" run
 pnpm test:e2e               # playwright, in testing/
 ```
 
 Scope a run to one package with `pnpm --filter @opendatacapture/<pkg> <script>` or turbo's
 `--filter=<pkg>`. Scope vitest to one project with `pnpm exec vitest --project <name>` (names come
 from the `name` field in each package's `vitest.config.ts`, e.g. `api`, `web`, `schemas`). Run a
-single file with `pnpm exec vitest path/to/file.test.ts`.
+single file with `pnpm exec vitest path/to/file.test.ts`. A scoped run with `--coverage` measures
+only what it ran against the whole coverage scope, so add `--coverage.thresholds.100=false` to it.
 
-`pnpm lint` and `pnpm test` both need `.env` to exist. `pnpm lint` also needs a generated Prisma
-client, so a cold run is not fast. A fresh clone needs more than `pnpm install` before `pnpm dev`
-serves anything — `.agents/docs/playbooks/run-locally.md` is the order of operations.
+`pnpm lint`, `pnpm test` and `pnpm test:coverage` all need `.env` to exist. `pnpm lint` also needs a
+generated Prisma client, so a cold run is not fast. A fresh clone needs more than `pnpm install`
+before `pnpm dev` serves anything — `.agents/docs/playbooks/run-locally.md` is the order of
+operations.
 
 ## Hard rules
 
@@ -46,6 +49,11 @@ not every tool loads nested files.
 - **Every change needs a unit test _and_ an end-to-end test in `testing/`.** See
   `.agents/docs/playbooks/add-e2e-test.md`. Utility scripts under `scripts/` are the one exception
   and need no tests.
+- **Unit test coverage stays at 100%.** `pnpm test:coverage` fails below 100% statements, branches,
+  functions or lines, and CI runs it. Code no test can reach is dead code: prove it dead (the types,
+  the callers, the library contract) and delete it. Never add a coverage-ignore comment or widen
+  `coverage.exclude` to get past the gate; that list is for non-code — fixtures, generated files and
+  bare entrypoints.
 - **Commit messages follow Conventional Commits.** `type(scope): subject`, where `type` is one of the
   11 `@commitlint/config-conventional` types and `scope`, if present, is an `@opendatacapture/*`
   workspace name without the prefix. A `commit-msg` hook and the `Commitlint` CI job enforce it, and
@@ -64,7 +72,8 @@ not every tool loads nested files.
 
 ### Before you are done
 
-1. Run `pnpm lint`, `pnpm test`, and `pnpm test:e2e` from the repo root and fix every failure.
+1. Run `pnpm lint`, `pnpm test:coverage`, and `pnpm test:e2e` from the repo root and fix every
+   failure, a coverage shortfall included.
 2. **Re-read the `AGENTS.md` of every package or app you modified and confirm your change does not
    contradict it.** If it does, either revise the change or update that `AGENTS.md` in the same
    commit — never leave the two disagreeing.

apps/api/src/core/__tests__/prisma.spec.ts

@@ -25,8 +25,10 @@ vi.mock('@prisma/client', async (importOriginal) => ({
   }
 }));
 
-/** Only `getUri` is read from the replica set the factory starts. */
-const createReplSet = vi.hoisted(() => vi.fn<(options: unknown) => Promise<{ getUri: (db: string) => string }>>());
+/** Only `getUri` and `stop` are used on the replica set the factory starts. */
+const createReplSet = vi.hoisted(() =>
+  vi.fn<(options: unknown) => Promise<{ getUri: (db: string) => string; stop: () => Promise<boolean> }>>()
+);
 
 vi.mock('mongodb-memory-server', () => ({
   MongoMemoryReplSet: { create: createReplSet }
@@ -238,7 +240,8 @@ describe('PrismaModuleOptionsFactory', () => {
 
   it('should connect to a fresh in-memory replica set under test, so tests never touch a real database', async () => {
     createReplSet.mockResolvedValueOnce({
-      getUri: (db) => `mongodb://127.0.0.1:41234/${db}?replicaSet=rs0`
+      getUri: (db) => `mongodb://127.0.0.1:41234/${db}?replicaSet=rs0`,
+      stop: vi.fn()
     });
     await (await createFactory({ NODE_ENV: 'test' })).create();
     expect(createReplSet).toHaveBeenCalledExactlyOnceWith({ replSet: { count: 1, name: 'rs0' } });
@@ -269,6 +272,15 @@ describe('PrismaModuleOptionsFactory', () => {
     expect(options).toStrictEqual({ client: prismaClient });
   });
 
+  it('should stop the in-memory replica set on shutdown, so no mongod outlives the application', async () => {
+    const stop = vi.fn(() => Promise.resolve(true));
+    createReplSet.mockResolvedValueOnce({ getUri: (db) => `mongodb://127.0.0.1:41234/${db}`, stop });
+    const factory = await createFactory({ NODE_ENV: 'test' });
+    await factory.create();
+    await factory.onApplicationShutdown();
+    expect(stop).toHaveBeenCalledOnce();
+  });
+
   it('should shut down cleanly when it started no in-memory replica set', async () => {
     const factory = await createFactory(externalEnv);
     await factory.create();

apps/api/src/core/prisma.ts

@@ -194,8 +194,8 @@ export class PrismaModuleOptionsFactory implements OnApplicationShutdown {
   private async createMemoryConnection(): Promise<string> {
     // prevent mongodb-memory-server from being included in the production bundle
     const { MongoMemoryReplSet } = await import('mongodb-memory-server');
-    const replSet = await MongoMemoryReplSet.create({ replSet: { count: 1, name: 'rs0' } });
-    return new URL(replSet.getUri('test')).href;
+    this.memoryReplSet = await MongoMemoryReplSet.create({ replSet: { count: 1, name: 'rs0' } });
+    return new URL(this.memoryReplSet.getUri('test')).href;
   }
 
   private getExternalConnection(): string {

apps/api/src/instrument-records/__tests__/instrument-records.service.spec.ts

@@ -954,25 +954,6 @@ describe('InstrumentRecordsService', () => {
         await expect(result).rejects.toBe(error);
         expect(worker.terminate).toHaveBeenCalledOnce();
       });
-
-      // Pins a reported defect: a failed INIT neither rejects nor terminates. Expected to change when the source is fixed.
-      it('should leave the export pending without sending a chunk when a worker fails to initialize', async () => {
-        instrumentsService.findById.mockResolvedValueOnce({ id: 'instrument-1', internal: { edition: 1, name: 'X' } });
-        const worker = useFakeWorker();
-
-        const result = instrumentRecordsService.exportRecords({}, { ability });
-        await vi.waitFor(() => expect(worker.postMessage).toHaveBeenCalledOnce());
-        worker.emit('message', { success: false });
-
-        await expect(Promise.race([result, Promise.resolve('pending')])).resolves.toBe('pending');
-        expect(worker.postMessage).toHaveBeenCalledTimes(1);
-        expect(worker.postMessage).toHaveBeenCalledWith({
-          data: [{ edition: 1, id: 'instrument-1', name: 'X' }],
-          type: 'INIT'
-        });
-        worker.emit('error', new Error('settle'));
-        await expect(result).rejects.toThrow('settle');
-      });
     });
   });
 

apps/api/src/instrument-records/export-worker.js

@@ -16,28 +16,19 @@ try {
 }
 
 /**
- * @typedef {Object} SuccessExpand
+ * @typedef {Object} ExpandedMeasure
  * @property {string} measure
  * @property {any} measureValue
- * @property {true} success
  */
 
-/**
- * @typedef {Object} FailureExpand
- * @property {string} message
- * @property {false} success
- */
-
-/** @typedef {SuccessExpand | FailureExpand} ExpandDataType */
-
 /**
  * Flattens nested record array data into a list of expandable data objects.
  * @param {any[]} listEntry - The array of records to expand.
- * @returns {ExpandDataType[]} An array of expanded measure objects.
+ * @returns {ExpandedMeasure[]} An array of expanded measure objects.
  * @throws {Error} If the provided listEntry is empty.
  */
 function expandData(listEntry) {
-  /** @type {SuccessExpand[]} */
+  /** @type {ExpandedMeasure[]} */
   const validRecordArrayList = [];
   if (listEntry.length < 1) {
     throw new Error('Record Array is Empty');
@@ -46,8 +37,7 @@ function expandData(listEntry) {
     for (const [dataKey, dataValue] of Object.entries(objectEntry)) {
       validRecordArrayList.push({
         measure: dataKey,
-        measureValue: dataValue,
-        success: true
+        measureValue: dataValue
       });
     }
   }
@@ -123,9 +113,6 @@ function handleChunkComplete(_data) {
 
       const expanded = expandData(measureValue);
       for (const entry of expanded) {
-        if (!entry.success) {
-          throw new Error(`exportRecords: ${instrument.name}.${measureKey} — ${entry.message}`);
-        }
         rows.push({
           groupId: record.groupId ?? DEFAULT_GROUP_NAME,
           instrumentEdition: instrument.edition,

apps/api/src/instrument-records/instrument-records.service.ts

@@ -40,14 +40,7 @@ import { UsersService } from '@/users/users.service';
 
 import { InstrumentMeasuresService } from './instrument-measures.service';
 
-import type {
-  BeginChunkProcessingMessage,
-  InitData,
-  InitialMessage,
-  InitMessage,
-  RecordType,
-  WorkerMessage
-} from './thread-types';
+import type { BeginChunkProcessingMessage, InitData, InitMessage, RecordType, WorkerMessage } from './thread-types';
 
 @Injectable()
 export class InstrumentRecordsService {
@@ -235,18 +228,18 @@ export class InstrumentRecordsService {
         const worker = new Worker(join(import.meta.dirname, 'export-worker.js'));
         worker.postMessage({ data: availableInstrumentArray, type: 'INIT' } satisfies InitMessage);
 
-        worker.on('message', (message: InitialMessage) => {
-          if (message.success) {
-            worker.postMessage({ data: chunk, type: 'BEGIN_CHUNK_PROCESSING' } satisfies BeginChunkProcessingMessage);
-            worker.on('message', (message: WorkerMessage) => {
-              if (message.success) {
-                resolve(message.data);
-              } else {
-                reject(new Error(message.error));
-              }
-              void worker.terminate();
-            });
-          }
+        // The worker only ever acknowledges INIT with `{ success: true }`; a failure inside it arrives
+        // as an `error` event instead, handled below.
+        worker.on('message', () => {
+          worker.postMessage({ data: chunk, type: 'BEGIN_CHUNK_PROCESSING' } satisfies BeginChunkProcessingMessage);
+          worker.on('message', (message: WorkerMessage) => {
+            if (message.success) {
+              resolve(message.data);
+            } else {
+              reject(new Error(message.error));
+            }
+            void worker.terminate();
+          });
         });
 
         worker.on('error', (error) => {

apps/api/src/instrument-records/thread-types.ts

@@ -43,5 +43,3 @@ export type BeginChunkProcessingMessage = {
 export type ParentMessage = BeginChunkProcessingMessage | InitMessage;
 
 export type WorkerMessage = { data: InstrumentRecordsExport; success: true } | { error: string; success: false };
-
-export type InitialMessage = { success: true };