skills/civitai__civitai
1
0
Fork 0
mirror of https://github.com/civitai/civitai.git synced 2026-09-20 22:08:18 +08:00
Code Issues Packages Projects Releases Wiki Activity

docs(claudedocs): give each doc a status line naming what it was true at (#4280)

Browse Source 
* style(claudedocs): apply Prettier to the existing docs

These files predate any formatting pass and none of them were Prettier-clean
(.md is not in .prettierignore, so the repo's own config already governs them).
Formatting is split out into its own commit so the status-line changes that
follow are readable as a diff rather than buried in reflowed emphasis markers
and realigned table separators.

No prose was changed. Reproduce with: npx prettier --write 'claudedocs/*.md'

* docs(claudedocs): give each doc a status line naming what it was true at

An audit of this directory got 6 of 11 statuses wrong, all in one direction: it
read each doc's self-reported status instead of re-checking the repo, so anything
that had moved since its doc was written stayed on the list unmoved.

Adds claudedocs/README.md stating the convention -- a doc asserting an outcome
names the commit or PR the claim was true at, so the status can be re-verified
rather than re-trusted -- and sweeps all 11 docs. Four statuses had changed:

- rca-appblocks-component-suite-flake: member #2 was fixed by #3654 the same day
  the doc was written and still said (NOT fixed)
- typecheck-tests-gap: merged by #3868/#4189, but NOT wired -- nothing invokes
  the gate, so it fires on nothing while reading as done
- turbopack-chunk-hash-collision: 'Next: 16.3.0' read as a blocker; the repo is
  already on ^16.3.1, which re-rolls the collision without fixing it
- rca-readtime-metric-privacy-cpu: fix shipped as #3331, never stated

Corrections are made in place rather than silently edited out. Formatting was
split into the preceding commit so this diff is only the substance.
This commit is contained in:
Zachary Lowden
2026-08-21 23:55:14 -05:00
committed by GitHub
parent ad8c7b56bf
commit 377d781f88
12 changed files with 383 additions and 278 deletions
Download Patch File Download Diff File Expand all files Collapse all files
claudedocs/README.md
+62
View File
@@ -0,0 +1,62 @@
# claudedocs/
In-depth technical documents: RCAs, audits, analyses, handovers, and investigations.
These are **not** auto-loaded by agents — they cost nothing until someone opens them.
They live here (not in `docs/`) because they contain operational detail, investigation
methodology, and measured evidence that would be noise in user-facing feature docs.
## Status-line convention
Every document that asserts an outcome (a fix landed, a proposal was merged, a
measurement holds, a recommendation stands) must carry a **status line** that names
the commit SHA or PR number the claim was true at. This makes the status a claim
about a point in history, not about the present, so it can be re-verified rather
than re-trusted.
### Format
The status line goes immediately after the document's `#` heading, as a bold
`**Status:**` paragraph. The first word after the colon is the status category,
followed by the evidence:
```markdown
# Document Title
**Status:** merged. PR #NNNN (merged YYYY-MM-DD). <one-sentence summary>.
```
### Categories
- **merged** — the change landed. Name the PR and merge date.
- **open** — the issue/PR is still open. Name the PR.
- **diagnosed, not fixed** — root cause known, no fix shipped. Say what would fix it.
- **historical snapshot** — the doc captures a point-in-time measurement or analysis.
Say what it was measured at (commit SHA, branch, date).
- **recommendation unchanged** — the doc's recommendation still holds. Say when it
was last checked.
### Corrections
When a status has changed since the doc was written, state the correction **in place**
rather than silently editing out the old claim. Use the pattern from
`runner-scouting-2026-08-15.md`:
```markdown
**Status (corrected YYYY-MM-DD):** <new status>. <evidence>.
```
The original text is not removed — the correction block sits above or replaces the
old status line, and the body of the document is left untouched.
### What NOT to put in a status line
- Internal infrastructure names, hostnames, or deployment details (this repo is public).
- Operational specifics that belong in the private infra repo.
- Claims that require running the codebase to verify (e.g., "coverage is still 32%").
### Verification
Re-derive a doc's status against the repository (`gh pr view`, `git log`), never
from the doc's own text. An audit that reads each doc's self-reported status instead
of re-checking the repo will get the same wrong answers the doc already has.
claudedocs/app-blocks-host-handler-parity-2026-06-29.md
+24 -22
View File
@@ -1,5 +1,7 @@
# App Blocks host↔SDK handler parity audit (2026-06-29)
**Status (added 2026-08-21):** Historical audit snapshot. Coverage matrix verified at the `@civitai/app-sdk@0.6.0` dist. PR #2799 (merged 2026-06-29) fixed `OPEN_CHECKPOINT_PICKER` on pages. The handler parity gap for remaining REQUEST-style messages remains open.
## The bug class (gotcha-#73 — "spins forever, no network call, no console error")
App Blocks has multiple host components that bridge block→host `postMessage`s, each
@@ -43,28 +45,28 @@ This exact gap bit `OPEN_CHECKPOINT_PICKER` on pages (a page block's `useCheckpo
`BlockToParentMessage` union? `pub` = yes (covered by the compile-time gate); **`ahead`** = NOT
yet published, forward-looking INVENTORY entry (runtime grep coverage only, not the type gate).
| # | Message (block→host) | Pub? | Style | Reply awaited | IframeHost | PageBlockHost | InlineHost |
|---|---|---|---|---|---|---|---|
| 1 | `BLOCK_READY` | pub | fire-forget | — | ✅ | ✅ | N/A (v1 stub) |
| 2 | `BLOCK_ERROR` | pub | fire-forget | — | ✅ | ✅ | N/A (v1 stub) |
| 3 | `RESIZE_IFRAME` | pub | fire-forget | — | ✅ | **N/A** (page iframe is full-viewport `height:100%` — no size-to-content; fire-forget so no hang) | N/A (v1 stub) |
| 4 | `NAVIGATE` | pub | fire-forget | — | **N/A** (model slot is an embedded panel; host-navigation out of remit) | ✅ | N/A (v1 stub) |
| 5 | `TRACK_EVENT` | pub | fire-forget | — | **N/A** (analytics; no host-side sink wired in EITHER host today — dropped, never hangs) | **N/A** (same) | N/A (v1 stub) |
| 6 | `REQUEST_SIGN_IN` | **ahead** | fire-forget | — | ✅ | ✅ | N/A (v1 stub) |
| 7 | `REQUEST_CONSENT` | **ahead** | fire-forget | — | ✅ | ✅ | N/A (v1 stub) |
| 8 | `REQUEST_TOKEN` | pub | **REQUEST** | `TOKEN_REFRESH_RESPONSE` | ✅ | ✅ | N/A (v1 stub) |
| 9 | `SUBMIT_WORKFLOW` | pub | **REQUEST** | `WORKFLOW_SUBMITTED` | ✅ | ✅ | N/A (v1 stub) |
| 10 | `ESTIMATE_WORKFLOW` | pub | **REQUEST** | `ESTIMATE_RESULT` | ✅ | ✅ | N/A (v1 stub) |
| 11 | `POLL_WORKFLOW` | pub | **REQUEST** | `WORKFLOW_STATUS` | ✅ | ✅ | N/A (v1 stub) |
| 12 | `CANCEL_WORKFLOW` | **ahead** | **REQUEST** | `WORKFLOW_CANCELED` | ✅ | ✅ | N/A (v1 stub) |
| 13 | `OPEN_BUZZ_PURCHASE` | pub | **REQUEST** | `BUZZ_PURCHASE_RESULT` | ✅ | ✅ | N/A (v1 stub) |
| 14 | `OPEN_CHECKPOINT_PICKER` | pub | **REQUEST** | `CHECKPOINT_PICKER_RESULT` | ✅ | ✅ (ported #2799) | N/A (v1 stub) |
| 15 | `SET_USER_CHECKPOINT` | pub | **REQUEST** | `USER_CHECKPOINT_SET` | ✅ | ❌→**fail-fast NACK (this PR)** — see OPEN DECISION | N/A (v1 stub) |
| 16 | `APP_STORAGE_GET` | pub | **REQUEST** | `APP_STORAGE_GET_RESULT` | ✅ | ✅ | N/A (v1 stub) |
| 17 | `APP_STORAGE_SET` | pub | **REQUEST** | `APP_STORAGE_SET_RESULT` | ✅ | ✅ | N/A (v1 stub) |
| 18 | `APP_STORAGE_DELETE` | pub | **REQUEST** | `APP_STORAGE_DELETE_RESULT` | ✅ | ✅ | N/A (v1 stub) |
| 19 | `APP_STORAGE_LIST` | pub | **REQUEST** | `APP_STORAGE_LIST_RESULT` | ✅ | ✅ | N/A (v1 stub) |
| 20 | `APP_STORAGE_QUOTA` | pub | **REQUEST** | `APP_STORAGE_QUOTA_RESULT` | ✅ | ✅ | N/A (v1 stub) |
| # | Message (block→host) | Pub? | Style | Reply awaited | IframeHost | PageBlockHost | InlineHost |
| --- | ------------------------ | --------- | ----------- | --------------------------- | ---------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------- | ------------- |
| 1 | `BLOCK_READY` | pub | fire-forget | — | ✅ | ✅ | N/A (v1 stub) |
| 2 | `BLOCK_ERROR` | pub | fire-forget | — | ✅ | ✅ | N/A (v1 stub) |
| 3 | `RESIZE_IFRAME` | pub | fire-forget | — | ✅ | **N/A** (page iframe is full-viewport `height:100%` — no size-to-content; fire-forget so no hang) | N/A (v1 stub) |
| 4 | `NAVIGATE` | pub | fire-forget | — | **N/A** (model slot is an embedded panel; host-navigation out of remit) | ✅ | N/A (v1 stub) |
| 5 | `TRACK_EVENT` | pub | fire-forget | — | **N/A** (analytics; no host-side sink wired in EITHER host today — dropped, never hangs) | **N/A** (same) | N/A (v1 stub) |
| 6 | `REQUEST_SIGN_IN` | **ahead** | fire-forget | — | ✅ | ✅ | N/A (v1 stub) |
| 7 | `REQUEST_CONSENT` | **ahead** | fire-forget | — | ✅ | ✅ | N/A (v1 stub) |
| 8 | `REQUEST_TOKEN` | pub | **REQUEST** | `TOKEN_REFRESH_RESPONSE` | ✅ | ✅ | N/A (v1 stub) |
| 9 | `SUBMIT_WORKFLOW` | pub | **REQUEST** | `WORKFLOW_SUBMITTED` | ✅ | ✅ | N/A (v1 stub) |
| 10 | `ESTIMATE_WORKFLOW` | pub | **REQUEST** | `ESTIMATE_RESULT` | ✅ | ✅ | N/A (v1 stub) |
| 11 | `POLL_WORKFLOW` | pub | **REQUEST** | `WORKFLOW_STATUS` | ✅ | ✅ | N/A (v1 stub) |
| 12 | `CANCEL_WORKFLOW` | **ahead** | **REQUEST** | `WORKFLOW_CANCELED` | ✅ | ✅ | N/A (v1 stub) |
| 13 | `OPEN_BUZZ_PURCHASE` | pub | **REQUEST** | `BUZZ_PURCHASE_RESULT` | ✅ | ✅ | N/A (v1 stub) |
| 14 | `OPEN_CHECKPOINT_PICKER` | pub | **REQUEST** | `CHECKPOINT_PICKER_RESULT` | ✅ | ✅ (ported #2799) | N/A (v1 stub) |
| 15 | `SET_USER_CHECKPOINT` | pub | **REQUEST** | `USER_CHECKPOINT_SET` | ✅ | ❌→**fail-fast NACK (this PR)** — see OPEN DECISION | N/A (v1 stub) |
| 16 | `APP_STORAGE_GET` | pub | **REQUEST** | `APP_STORAGE_GET_RESULT` | ✅ | ✅ | N/A (v1 stub) |
| 17 | `APP_STORAGE_SET` | pub | **REQUEST** | `APP_STORAGE_SET_RESULT` | ✅ | ✅ | N/A (v1 stub) |
| 18 | `APP_STORAGE_DELETE` | pub | **REQUEST** | `APP_STORAGE_DELETE_RESULT` | ✅ | ✅ | N/A (v1 stub) |
| 19 | `APP_STORAGE_LIST` | pub | **REQUEST** | `APP_STORAGE_LIST_RESULT` | ✅ | ✅ | N/A (v1 stub) |
| 20 | `APP_STORAGE_QUOTA` | pub | **REQUEST** | `APP_STORAGE_QUOTA_RESULT` | ✅ | ✅ | N/A (v1 stub) |
> The three **ahead** rows (#6, #7, #12) are NOT in the published `@civitai/app-sdk@0.6.0` dist
> union, so they are NOT enforced by the compile-time coverage gate — only by the runtime grep.
claudedocs/notifications-test-coverage-audit-2026-07-03.md
+33 -23
View File
@@ -1,66 +1,76 @@
# Notifications App — Test Coverage Audit (2026-07-03)
**Status (added 2026-08-21):** Historical snapshot. Coverage numbers measured at the stated commit; NOT VERIFIED against current `main`.
**Scope:** `apps/notifications` (`@civitai/notifications-app`) — the Fastify + raw-pg fan-out/producer service.
**Method:** ran `corepack pnpm exec vitest run --coverage` (v8 provider, worked) + static analysis of every source module and test. Read-only; no source or tests changed.
**Headline:** 25 tests pass across 5 files, but overall coverage is **32% statements / 27% branch / 23% functions / 34% lines**. The two highest-blast-radius modules are effectively untested behaviorally: the **fan-out worker `poll-loop.ts` (10% lines)** and the **producer write path `create.ts` (0%)**. Every existing test that touches SQL asserts on the *generated string*, never on the *behavior* of running it — there is **no integration / real-Postgres coverage anywhere**, including for the load-bearing claim query.
**Headline:** 25 tests pass across 5 files, but overall coverage is **32% statements / 27% branch / 23% functions / 34% lines**. The two highest-blast-radius modules are effectively untested behaviorally: the **fan-out worker `poll-loop.ts` (10% lines)** and the **producer write path `create.ts` (0%)**. Every existing test that touches SQL asserts on the _generated string_, never on the _behavior_ of running it — there is **no integration / real-Postgres coverage anywhere**, including for the load-bearing claim query.
---
## 1. Coverage map
| Module | Tests? | What's covered | v8 line % (branch) |
|---|---|---|---|
| `src/app.ts` | `__tests__/app.test.ts` | `/health`, `/metrics` (+ XFF 404 guard), one 400-validation path, RED-histogram scope guard | 45.1% (35.3%) |
| `src/env.ts` | — | none (`assertRequiredEnv` L30–40 unrun) | 53.8% (50%) |
| `src/server.ts` | — | none (entrypoint; not imported by any test) | 0% |
| `src/worker/poll-loop.ts` | `poll-loop.test.ts` | **only** the `PENDING_CLAIM_QUERY` *string* (structural asserts). `handleNormal`/`handleDebounce`/`create`/`run`/`startWorker` all unrun | **10.9% (0%)** |
| `src/lib/server/operations.ts` | `operations.test.ts` | `queryNotifications` + `countNotifications` SQL string + `$n` param indexing (fake pool) | 35.6% (20.8%) |
| `src/lib/server/create.ts` | — | **none** — producer create path entirely unrun (L24–80) | **0% (0%)** |
| `src/lib/server/cache.ts` | `cache.test.ts` | error-counter wrapper on `getUser`+`incrementUser` only | 35.6% (14.3%) |
| `src/lib/server/auth.ts` | `auth.test.ts` | 7 behavioral cases — bearer / x-webhook-token / wrong / len-mismatch / missing / non-bearer / disabled | 100% (87.5%) |
| `src/lib/server/lag.ts` | — | none (`getNotifDbWithoutLag`/`preventReplicationLag` L20–24 unrun) | 66.7% (25%) |
| `src/lib/server/metrics.ts` | (via app.test) | full — baseline series exercised | 100% (100%) |
| `src/lib/server/clients/db.ts` | (import side-effect) | pool construction only | 75% (42.9%) |
| `src/lib/server/clients/axiom.ts` | — | none | 33.3% (0%) |
| `src/lib/server/clients/redis.ts` | — | none | 0% (0%) |
| Module | Tests? | What's covered | v8 line % (branch) |
| --------------------------------- | ----------------------- | ---------------------------------------------------------------------------------------------------------------------------------------- | ------------------ |
| `src/app.ts` | `__tests__/app.test.ts` | `/health`, `/metrics` (+ XFF 404 guard), one 400-validation path, RED-histogram scope guard | 45.1% (35.3%) |
| `src/env.ts` | — | none (`assertRequiredEnv` L30–40 unrun) | 53.8% (50%) |
| `src/server.ts` | — | none (entrypoint; not imported by any test) | 0% |
| `src/worker/poll-loop.ts` | `poll-loop.test.ts` | **only** the `PENDING_CLAIM_QUERY` _string_ (structural asserts). `handleNormal`/`handleDebounce`/`create`/`run`/`startWorker` all unrun | **10.9% (0%)** |
| `src/lib/server/operations.ts` | `operations.test.ts` | `queryNotifications` + `countNotifications` SQL string + `$n` param indexing (fake pool) | 35.6% (20.8%) |
| `src/lib/server/create.ts` | — | **none** — producer create path entirely unrun (L24–80) | **0% (0%)** |
| `src/lib/server/cache.ts` | `cache.test.ts` | error-counter wrapper on `getUser`+`incrementUser` only | 35.6% (14.3%) |
| `src/lib/server/auth.ts` | `auth.test.ts` | 7 behavioral cases — bearer / x-webhook-token / wrong / len-mismatch / missing / non-bearer / disabled | 100% (87.5%) |
| `src/lib/server/lag.ts` | — | none (`getNotifDbWithoutLag`/`preventReplicationLag` L20–24 unrun) | 66.7% (25%) |
| `src/lib/server/metrics.ts` | (via app.test) | full — baseline series exercised | 100% (100%) |
| `src/lib/server/clients/db.ts` | (import side-effect) | pool construction only | 75% (42.9%) |
| `src/lib/server/clients/axiom.ts` | — | none | 33.3% (0%) |
| `src/lib/server/clients/redis.ts` | — | none | 0% (0%) |
---
## 2. Gaps ranked by risk (blast radius first)
### G1 — Fan-out worker `handleDebounce` / `handleNormal` behavior is 0% (`poll-loop.ts:96–195`) 🔴
**Untested:** the entire fan-out logic. Specifically the debounce **drop decision** (`poll-loop.ts:147–154`): `dayjs(lastTriggered).add(debounceSeconds,'seconds').isBefore(dayjs(nextSendAt))` → `DELETE` the pending row and return empty. Also the `ON CONFLICT (notificationId,userId) DO UPDATE SET createdAt=now(), viewed=FALSE` resurrection (L176–178), the reschedule `claimedAt=null, nextSendAt=now()+debounceSeconds` (L186–193), and `handleNormal`'s SELECT-before-INSERT + 23505 retry (L100–121).
**Failure it hides:** an inverted/off-by-one comparison or a swapped `lastTriggered`/`nextSendAt` silently either **drops every debounced notification** or **re-fans on every tick** (mass duplicate unread badges + signal spam). None of `poll-loop.test.ts`'s 5 assertions would change.
**Suggested approach:** unit-test `handleNormal`/`handleDebounce` with a **fake `PoolClient`** that records `client.query(...)` calls and returns canned rows (same recorder pattern as `operations.test.ts`). Export the two handlers (or test via `create` with a fake `notifDbWrite().connect()`). Assert: (a) debounce-drop path issues a `DELETE` and no `UserNotification` insert when `lastTriggered+debounce < nextSendAt`; (b) fan-out path issues the reschedule UPDATE; (c) `handleNormal` on a 23505 re-SELECTs instead of throwing.
### G2 — Producer write path `create.ts` is 0% (`create.ts:21–81`) 🔴
**Untested:** recipient dedup (`userIds` + `userId` → Set, L24–26), the **opt-out filter + `-1` sentinel drop** (L37: `id !== -1 && !disabled.has(id)`), UPDATE-first→INSERT-ON-CONFLICT fallback (L43–68), and the swallow-and-return-`{queued:0}` error contract (L71–81).
**Failure it hides:** a broken opt-out filter **delivers to users who disabled the type** (privacy/spam regression) or the `-1` sentinel leaks into `users[]`; a param/cast mistake (`$4::int[]`, `$5::jsonb`, `$3::"NotificationCategory"`) throws and is swallowed → producer silently queues nothing while returning 202. `createNotificationsBulk` (`operations.ts:23–66`) shares the same UPDATE-first/ON-CONFLICT shape and `pg-format` value building and is equally unrun.
**Suggested approach:** fake `mainDbRead`/`notifDbWrite` pools (as `operations.test.ts` mocks `./clients/db`). Assert the settings query filters out `disabled` ids + `-1`; assert `queued` counts targets; assert the INSERT fires only when the UPDATE returns 0 rows; assert a thrown pool error yields `{queued:0}` not a throw.
### G3 — `markNotificationsRead` serialization + retry is 0% (`operations.ts:172–289`) 🔴
**Untested:** the per-user promise-chain queue (L193–202, "never >1 concurrent `connect()` per user" — the rapid-click pool-starvation guard), `isTransientWriteError` matching (L184–186), the exponential backoff + `MARK_READ_MAX_ATTEMPTS` retry (L204–245), and `markReadImpl`'s all/category/single branches + cache decrement (L247–289).
**Failure it hides:** a break in the queue chaining reintroduces concurrent per-user writes (pool starvation under rapid clicks — the exact bug this code exists to prevent); a wrong transient-error substring makes every mark-read retry a permanent failure 4× or give up immediately. Pure in-process logic — cheap to test, high value.
**Suggested approach:** fake `notifDbWrite` whose `query` rejects N times with a transient message then resolves; use `vi.useFakeTimers()` to advance backoff; assert attempt count, that a non-transient error does **not** retry, and that two `markNotificationsRead` calls for the same user serialize (second `connect` starts only after the first settles).
### G4 — Replica-lag routing `lag.ts` untested + `countNotifications` write-pool branch unexercised 🟡
**Untested:** `getNotifDbWithoutLag` (`lag.ts:23–25`) — the `isStale → write : read` decision that gives read-your-writes after a mark-read. `operations.test.ts` mocks `./lag` with `isWritePool: () => false`, so `countNotifications`'s **write-pool cache-bust branch** (`operations.ts:114–115`) and the **cache-hit early return** (L117–118) are both never taken.
**Failure it hides:** a routing inversion serves **stale unread counts** right after the user marks read (the classic "badge won't clear" complaint) — invisible to current tests.
**Suggested approach:** unit `getNotifDbWithoutLag` with a fake tracker (`isStale` → true/false) asserting it returns the write vs read pool; add a `countNotifications` case with `isWritePool: () => true` asserting `bustUser` is called and the DB is queried.
### G5 — Cache semantics mostly untested (`cache.ts`) 🟡
**Untested:** `decrementUser` clamp-and-delete (L79–83 → `incrementUser` L72–76: `hDel` when value ≤ 0), `hasUser` gating, `setUser`/`clearCategory`/`bustUser` bodies, and the **null-redis degrade-to-no-op** path (every op's `if (!redis) return`). Only the error-counter wrapper on `getUser`+`incrementUser` is tested.
**Failure it hides:** a broken clamp lets the unread counter go **negative** (badge shows a bogus count); a regression in the null-redis guard throws when redis is unconfigured (dev/degraded prod) instead of no-op'ing.
**Suggested approach:** extend `cache.test.ts` (fake redis already present) — assert `decrementUser` calls `hDel` when `hGet` returns `"0"`, and that with `getRedis: () => null` every op resolves without throwing.
### G6 — SQL param-indexing untested for 5 of 7 operations 🟡
`operations.test.ts` guards `$n` indexing for `queryNotifications`/`countNotifications` only. `createNotificationsBulk`, `create.ts`, `notificationExists`, `cleanupNotifications`, and `markReadImpl` build SQL/params with no such guard — `create.ts`'s positional casts (`$1..$6`) and `createNotificationsBulk`'s `pg-format` value strings are the real off-by-one/injection-shape risk and are unrun. (Covered by adding G2/G3 tests.)
### G7 — `assertRequiredEnv` fail-fast boot check untested (`env.ts:29–42`) 🟢
The pod-won't-go-Ready-on-misconfig guard — including the **prod-only `NOTIFICATIONS_TOKEN` requirement** (L37, i.e. "don't ship an open producer API in prod"). A regression here silently reopens that gate. Cheap: stub env, assert it throws with the right missing-keys list; assert prod+no-token throws, dev+no-token doesn't.
### G8 — Authed-route 401 + success paths untested (`app.ts`) 🟢
`app.test.ts` runs with no token set, so `isAuthorized` always returns true → the **401 branch of `authedBody` (`app.ts:99–103`) is never hit end-to-end**, and no route is exercised past validation into a handler. The RED-outcome mapping (L61–70) is only verified for `rejected`. A route-level test with a mocked `operations`/`create` layer and a configured token would cover the 401, the 202-success, and the `success`/`error` outcome labels.
---
@@ -68,21 +78,21 @@ The pod-won't-go-Ready-on-misconfig guard — including the **prod-only `NOTIFIC
## 3. Quality assessment of existing tests
- **`auth.test.ts` — genuinely good.** 7 behavioral cases hitting every real branch (both header forms, wrong/short/missing/non-bearer, gate-disabled). The one true behavioral suite. Only gap: array-valued headers (`auth.ts:19,26`, branch 87.5%).
- **`operations.test.ts` — meaningful but narrow.** The fake-pool-records-SQL pattern correctly targets the real off-by-one risk (`$n` indexing) and is the right shape. But it covers 2 of 7 exported operations, asserts only on the *emitted string*, and its `./lag` + `./cache` mocks pin the branches so `countNotifications`'s write-pool/cache-hit paths never run. It verifies the SQL is *built* right, not that it *behaves* right.
- **`operations.test.ts` — meaningful but narrow.** The fake-pool-records-SQL pattern correctly targets the real off-by-one risk (`$n` indexing) and is the right shape. But it covers 2 of 7 exported operations, asserts only on the _emitted string_, and its `./lag` + `./cache` mocks pin the branches so `countNotifications`'s write-pool/cache-hit paths never run. It verifies the SQL is _built_ right, not that it _behaves_ right.
- **`poll-loop.test.ts` — superficial by design, and the riskiest gap.** All 5 assertions are `toContain`/regex over a whitespace-collapsed **static string**. They guard exactly one edit (deleting `FOR UPDATE SKIP LOCKED`) — valuable as a regression pin, honestly scoped in its own comment — but **would all still pass if `handleNormal`, `handleDebounce`, the debounce drop math, the txn wrapper, and the entire `run()` fan-out were deleted or inverted.** The module's actual logic has zero behavioral coverage.
- **`cache.test.ts` — thin.** Tests only that the error-counter wrapper increments + rethrows on 2 of 6 ops. The cache's actual read/write/decrement-clamp/null-degrade behavior is untested.
- **`app.test.ts` — solid for what it covers (real `inject`), blind past the gate.** Health/metrics/XFF-404/RED-scope are well done (the RED-scope regex belt-and-suspenders is a nice touch). But every authed route stops at the 400 validation; no 401, no handler success, no DB-touching path. Its own comment defers the create path to "an integration run" — **that integration run does not exist in the repo.**
**Integration coverage:** none. Every SQL assertion is against a fake pool or a static string. The load-bearing `PENDING_CLAIM_QUERY` (concurrency correctness under `FOR UPDATE SKIP LOCKED`) is only structurally asserted — its actual claim/lease semantics under concurrent workers are never executed against Postgres. Given replicas:1 + Recreate makes that clause a happy-path no-op today, the *behavioral* correctness of claim → fan-out → reschedule is the more urgent, and entirely absent, coverage.
**Integration coverage:** none. Every SQL assertion is against a fake pool or a static string. The load-bearing `PENDING_CLAIM_QUERY` (concurrency correctness under `FOR UPDATE SKIP LOCKED`) is only structurally asserted — its actual claim/lease semantics under concurrent workers are never executed against Postgres. Given replicas:1 + Recreate makes that clause a happy-path no-op today, the _behavioral_ correctness of claim → fan-out → reschedule is the more urgent, and entirely absent, coverage.
---
## 4. Top 5 prioritized recommendations (impact vs effort)
1. **★ HIGHEST VALUE — behaviorally test `handleDebounce` + `handleNormal` with a fake `PoolClient` (G1).** Highest blast radius (silent mass drop or duplicate of every debounced notification), pure logic, and the established fake-pool recorder pattern applies directly. Start with the debounce **drop-vs-fanout decision** (`poll-loop.ts:147–154`) and the reschedule UPDATE — the single most consequential untested branch in the app. *High impact / low-med effort.*
2. **Test the producer write path `create.ts` — opt-out filter + `-1` drop + UPDATE→INSERT fallback (G2).** Directly protects against delivering to opted-out users and against silent `{queued:0}` swallowing. Fake `mainDbRead`/`notifDbWrite`. *High impact / low effort.*
3. **Test `markNotificationsRead` retry + per-user serialization (G3).** Fake-timer unit test of the transient-retry/backoff and the pool-starvation guard the code exists for. *Med-high impact / low effort (no infra).*
4. **Cover `getNotifDbWithoutLag` routing + `countNotifications` write-pool/cache branches (G4/G5).** Guards read-your-writes staleness and counter-clamp correctness; small extensions to existing mocked suites. *Med impact / low effort.*
5. **Add route-level auth+success tests and an `assertRequiredEnv` test (G8/G7).** Close the 401 gate, the 202-success outcome labels, and the prod-token fail-fast — the "don't ship an open producer API" guard. *Med impact / low effort.*
1. **★ HIGHEST VALUE — behaviorally test `handleDebounce` + `handleNormal` with a fake `PoolClient` (G1).** Highest blast radius (silent mass drop or duplicate of every debounced notification), pure logic, and the established fake-pool recorder pattern applies directly. Start with the debounce **drop-vs-fanout decision** (`poll-loop.ts:147–154`) and the reschedule UPDATE — the single most consequential untested branch in the app. _High impact / low-med effort._
2. **Test the producer write path `create.ts` — opt-out filter + `-1` drop + UPDATE→INSERT fallback (G2).** Directly protects against delivering to opted-out users and against silent `{queued:0}` swallowing. Fake `mainDbRead`/`notifDbWrite`. _High impact / low effort._
3. **Test `markNotificationsRead` retry + per-user serialization (G3).** Fake-timer unit test of the transient-retry/backoff and the pool-starvation guard the code exists for. _Med-high impact / low effort (no infra)._
4. **Cover `getNotifDbWithoutLag` routing + `countNotifications` write-pool/cache branches (G4/G5).** Guards read-your-writes staleness and counter-clamp correctness; small extensions to existing mocked suites. _Med impact / low effort._
5. **Add route-level auth+success tests and an `assertRequiredEnv` test (G8/G7).** Close the 401 gate, the 202-success outcome labels, and the prod-token fail-fast — the "don't ship an open producer API" guard. _Med impact / low effort._
**Stretch (separate track):** one real-Postgres integration test (Testcontainers or a throwaway schema) that runs two concurrent `getPending()` claims and asserts no row is fanned twice — the only way to actually verify `FOR UPDATE SKIP LOCKED` rather than assert its presence in a string. Higher effort; do after 1–3.
claudedocs/rca-appblocks-component-suite-flake-2026-08-05.md
+49 -45
View File
@@ -1,6 +1,6 @@
# RCA — the App Blocks component-suite flake: an unwinnable race on a self-deleting state
**Date:** 2026-08-05 · **PR:** #3645 · **Status:** one defect fixed; the class is open.
**Date:** 2026-08-05 · **PR:** #3645 · **Status (corrected 2026-08-21):** two defects fixed; the class is open.
`preview / component-tests` had been intermittently red with no PR to blame — two pipeline
runs on **byte-identical trees** (the second an empty commit on the first) produced opposite
@@ -11,7 +11,7 @@ click through, and it had already pushed two merge decisions in opposite directi
## 1. The mechanism
`AppsSubmitEditView.browser.test.tsx` → *"retry RE-ARMS a fresh ceiling"* awaited a state
`AppsSubmitEditView.browser.test.tsx` → _"retry RE-ARMS a fresh ceiling"_ awaited a state
that **deletes itself** `loaderCeilingMs` after the click:
```tsx
@@ -23,7 +23,7 @@ await expect.element(page.getByTestId('apps-offsite-edit-loading')).toBeInTheDoc
> 🔴 **The invariant.** Waiting for a state to **ARRIVE** is safe: load only makes it arrive
> later and the matcher keeps polling. Waiting for a state that will **LEAVE** is a race the
> matcher cannot win — once it is gone it never comes back, so *every* remaining poll is also
> matcher cannot win — once it is gone it never comes back, so _every_ remaining poll is also
> too late. The failure is **unwinnable**, so it burns the **full 15s budget**.
### ⚠️ The ~15s wall is a CANDIDATE FILTER, not a diagnosis
@@ -37,12 +37,13 @@ this class. **That under-discriminates badly, and the correction is measured:**
(`IframeHostReadyTransition` and `PageBlockHostLaunchReveal` `no_token` terminals, which
`vi.waitFor` out a real `TOKEN_WAIT_TIMEOUT_MS = 15s`).
So the signature is **necessary but nowhere near sufficient**. It tells you only that *some*
So the signature is **necessary but nowhere near sufficient**. It tells you only that _some_
matcher was never satisfied. Use it to shortlist, never to conclude.
**To actually discriminate a self-deleting state from a never-arriving one:**
1. **Read the observable synchronously immediately after the action**, before the matcher
polls — *present-then-gone* is a self-deleting state; *never present* is a state that
polls — _present-then-gone_ is a self-deleting state; _never present_ is a state that
never arrived. (This is what `PROBE-T` did; see Appendix B for the shape.)
2. **Enlarge the component's own window** — the ceiling/debounce/timeout driving the state —
by orders of magnitude. If the failure disappears, the state was self-deleting; if it
@@ -69,9 +70,9 @@ costs 228s against 0.3s locally.
At `loaderCeilingMs={1}` the window measured **3.3ms** and the assertion missed it, failing at
14986–15021ms — byte-identical to the CI signature.
📌 **Correction (audit).** An earlier draft claimed the shipped `200` was itself a *previous
widening* of this window. **Git contradicts that**: `loaderCeilingMs={200}` was introduced
*with the test* at `eab627cdac` (#3432), commented "A comfortable ceiling…", and was never
📌 **Correction (audit).** An earlier draft claimed the shipped `200` was itself a _previous
widening_ of this window. **Git contradicts that**: `loaderCeilingMs={200}` was introduced
_with the test_ at `eab627cdac` (#3432), commented "A comfortable ceiling…", and was never
raised from a smaller value; the earlier flake fix `72b1485034` (#3544) **deleted** a racing
assertion rather than widening one. The "widening loses eventually" argument is still sound,
but it rests on the marketplace example in §4 — which genuinely was widened and lost again —
@@ -87,10 +88,10 @@ different hypothesis. Re-measured, `{1}` is **8 of 12 RED**.
The likely cause: **Vitest's summary lines are ANSI-prefixed**, so a `grep` for the verdict
silently matches nothing and a RED run reads as "no failures". This is written down in our own
gotchas and it still bit — and it bit *twice*, because the first re-measurement this session
gotchas and it still bit — and it bit _twice_, because the first re-measurement this session
made the same mistake before ANSI was stripped (`sed 's/\x1b\[[0-9;]*m//g'`).
**What actually settled it was instrumenting the component and reading a *timeline*, not
**What actually settled it was instrumenting the component and reading a _timeline_, not
re-running the knob.** A knob re-run only ever yields another pass/fail to misread; the
timeline shows the window directly.
@@ -105,7 +106,7 @@ click always lands. Container count was always 1, so it is not a cleanup/contain
Split into two tests so **every awaited state is absorbing**. Production source unchanged.
1. **`retry RE-ARMS the loader`** — widen the ceiling via `rerender` so the returning spinner
*cannot* delete itself, then assert it. Includes a **negative control**: after the widening
_cannot_ delete itself, then assert it. Includes a **negative control**: after the widening
the spinner must still be absent, so the spinner in the final step is attributable to the
retry and not to the prop change re-running the effect.
2. **`retry re-arms a FRESH ceiling`** — never assert the transient at all. Await the absorbing
@@ -121,41 +122,41 @@ one and leave the race unwinnable whenever the machine is slow enough — i.e. e
window is equivalent to lengthening the leg — deterministic instead of a load lottery. Every
ceiling in the file set to 1ms; **load 99–114 on both sides** (24-core box):
| variant | runs | result | wall | slowest test |
|---|---|---|---|---|
| before | 12 | **8 RED / 4 green** | 19–20s on reds | ~15.0s |
| after | 12 | **12 / 12 GREEN** | 4.8–6.1s | 0.9–1.6s |
| variant | runs | result | wall | slowest test |
| ------- | ---- | ------------------- | -------------- | ------------ |
| before | 12 | **8 RED / 4 green** | 19–20s on reds | ~15.0s |
| after | 12 | **12 / 12 GREEN** | 4.8–6.1s | 0.9–1.6s |
Stated in advance: *NEW going red would have refuted the fix.* It did not.
Stated in advance: _NEW going red would have refuted the fix._ It did not.
**Shipped settings under 48 synthetic CPU burners** (load 100–160): after = **11/11 GREEN**.
Before, at shipped settings, was **12/12 GREEN at load up to 121** — i.e. the natural red was
never reproduced locally; see §6.
**Mutation checks** — each guard dies for *its own* reason and is *not* killed by the other's,
**Mutation checks** — each guard dies for _its own_ reason and is _not_ killed by the other's,
so neither passes for the other's reason:
| mutation | test 1 | test 2 |
|---|---|---|
| M1 · delete `setLoaderExpired(false)` from `handleRetry` | **3/3 RED** — `Cannot find element … apps-offsite-edit-loading` | green (test 1 owns it) |
| M2 · drop `retryNonce` from the effect deps | green (correctly unaffected) | **3/3 RED** — `Cannot find element … apps-offsite-edit-not-found` |
| mutation | test 1 | test 2 |
| -------------------------------------------------------- | --------------------------------------------------------------- | ----------------------------------------------------------------- |
| M1 · delete `setLoaderExpired(false)` from `handleRetry` | **3/3 RED** — `Cannot find element … apps-offsite-edit-loading` | green (test 1 owns it) |
| M2 · drop `retryNonce` from the effect deps | green (correctly unaffected) | **3/3 RED** — `Cannot find element … apps-offsite-edit-not-found` |
Two further axes, constructed by an independent audit rather than by me — both reproduce the
separation and one closes a gap I had left open:
| mutation | test 1 | test 2 | what it proves |
|---|---|---|---|
| M3 · unconditional `setLoaderExpired(false)` at the top of the effect | **3/3 RED**, in the **negative control's own assertion** | green | the negative control in test 1 is load-bearing, not decorative |
| M4 · `setRetryNonce((n) => n)` — deps correct, value never changes | green | **3/3 RED** | test 2 pins the nonce's *value changing*, not merely its presence in the deps |
| mutation | test 1 | test 2 | what it proves |
| --------------------------------------------------------------------- | -------------------------------------------------------- | ----------- | ----------------------------------------------------------------------------- |
| M3 · unconditional `setLoaderExpired(false)` at the top of the effect | **3/3 RED**, in the **negative control's own assertion** | green | the negative control in test 1 is load-bearing, not decorative |
| M4 · `setRetryNonce((n) => n)` — deps correct, value never changes | green | **3/3 RED** | test 2 pins the nonce's _value changing_, not merely its presence in the deps |
M3 matters because a negative control is the easiest thing in a test to write and never
exercise; it now has a killing mutation of its own.
🔴 **I re-ran M3 and M4 myself rather than citing them, and M4's reported result was
incomplete.** M4 reds **two** tests, not one: test 2 *and* the pre-existing
incomplete.** M4 reds **two** tests, not one: test 2 _and_ the pre-existing
`error retry shows a disabled "Retrying…" state … (isFetching)`, which also depends on the
nonce bump forcing a re-render (with `(n) => n` React bails out of the re-render, so the
in-flight state never paints). The *separation* claim is unaffected — test 1 survives M4 —
in-flight state never paints). The _separation_ claim is unaffected — test 1 survives M4 —
but M4 is not a single-test discriminator, and a summary saying it "reds test 2" understates
its blast radius. Verified: M3 `1 failed | 9 passed` ×3; M4 `2 failed | 8 passed` ×3.
@@ -169,18 +170,18 @@ Independent verification also confirmed every awaited state in both tests as abs
## 4. 🔴 This is a CLASS, not one defect
Measured gate health: **3 fails / 19 recent PRs carrying a `component-tests` check (~16%)** —
a *lower bound*, since it reads each PR's latest run and a PR retried until green counts as a
a _lower bound_, since it reads each PR's latest run and a PR retried until green counts as a
pass.
### Confirmed member #2 — `AppListingsMarketplaceBody.browser.test.tsx:221` (NOT fixed)
### Confirmed member #2 — `AppListingsMarketplaceBody.browser.test.tsx:221` (fixed by PR #3654, merged 2026-08-05)
*"the search box does NOT write the URL per keystroke — only the debounced value"*
_"the search box does NOT write the URL per keystroke — only the debounced value"_
```
AssertionError: expected [ { query: 'mat' } ] to have a length of +0 but got 1
```
Wall **1521ms**, not ~15s — a different *shape*, but the **same family, inverted**: it asserts
Wall **1521ms**, not ~15s — a different _shape_, but the **same family, inverted**: it asserts
a 300ms debounce has **not yet** elapsed (racing an arrival) where ours asserted a state was
**still** present (racing a deletion). Both depend on wall-clock timing against a component
timer.
@@ -192,12 +193,13 @@ papering over this class.
**Follow-up plan (deliberately deferred — it cannot be verified to the N≥10 + mutation bar
without its own cycle):**
- The timing-independent intent is the END state: after the debounce settles there is exactly
one write carrying the final value — already asserted at lines 224–228.
- Line 221 is the racing assertion. Deleting it keeps the per-keystroke mutation killed:
straight-through wiring produces TWO writes, so `toHaveLength(1)` never passes and the
`vi.waitFor` times out red.
- ⚠️ **Close one vacuity first:** `toHaveLength(1)` can be satisfied *transiently* on the way
- ⚠️ **Close one vacuity first:** `toHaveLength(1)` can be satisfied _transiently_ on the way
to 2, so a "debounce fires twice" bug could slip through. Re-assert the count after the
`waitFor` settles, or drive the debounce with fake timers.
- Verify with the same instrument: shrink the debounce (accelerated control), N≥10 each side,
@@ -216,7 +218,7 @@ clobber a name the user already typed`, `selecting a category shows "Explore all
### 🔴 An independently-constructed sweep found no additional member either
My own sweep grepped for the *ceiling-prop spelling*, which finds only one shelf of the
My own sweep grepped for the _ceiling-prop spelling_, which finds only one shelf of the
hazard — a real weakness. An adversarial audit built the inverse instrument: enumerate
**167 timer-owning modules** → **18** browser tests whose component owns a wall-clock timer →
read every short-window candidate by hand (`ReportTabs.tsx:745`, the scan badge,
@@ -224,7 +226,7 @@ read every short-window candidate by hand (`ReportTabs.tsx:745`, the scan badge,
components), with a positive control proving the instrument could see a known member.
**Result: no additional live member.** A differently-constructed instrument reaching the same
answer is the strongest form this claim can take — but note it is a claim about *current*
answer is the strongest form this claim can take — but note it is a claim about _current_
members, not that the class is closed.
### ~~Same shape, but NOT at risk — and the discriminator is the MARGIN, not the shape~~ — 🔴 RETRACTED 2026-08-06, THE WRONG CONSTANT
@@ -241,8 +243,8 @@ members, not that the class is closed.
After `driveToFatal()` the host is in a **terminal** state, not `loading`. The timer armed there
is the auto-retry backoff — `AUTO_RETRY_BACKOFF_MS[0] = 2000ms` (`pageBlockHostLogic.ts:577`) —
not `BLOCK_READY_TIMEOUT_MS`. The margin is therefore **~26×**, not ~128×, and it is measured
against a **Playwright driver round-trip**, which this same RCA measured at 40–78ms *on an idle
box* and elsewhere describes as erased under CI load.
against a **Playwright driver round-trip**, which this same RCA measured at 40–78ms _on an idle
box_ and elsewhere describes as erased under CI load.
`PageBlockHostAutoRetry.browser.test.tsx:728` subsequently failed in CI on
`pr-preview-3679-lt4xt` (PR #3679) with:
@@ -257,7 +259,7 @@ moving it to the virtual clock + a DOM click.
🔴 **The four `PageBlockHost.browser.test.tsx` sites cleared above are still live members** and
are NOT fixed by that change. The cleanest of them is `error (mint failure): Retry calls
onRetryToken AND returns to loading` — an AUTH terminal, so the automatic attempt *re-mints*,
onRetryToken AND returns to loading` — an AUTH terminal, so the automatic attempt _re-mints_,
flipping `expect(onRetryToken).not.toHaveBeenCalled()` to 1 and
`toHaveBeenCalledTimes(1)` to 2.
@@ -272,7 +274,7 @@ assertion and the timer armed by THAT state — not the most prominent timeout i
**Conclusion: confirmed. The supporting numbers: do not quote them forward.**
A full-suite run at HEAD showed ~12 failures and briefly looked like collateral damage. It is
not — but my measurement of *how much* ambient flake there is did not hold up.
not — but my measurement of _how much_ ambient flake there is did not hold up.
I ran the whole 125-file suite at BASE and at HEAD back-to-back and counted 12 failures each,
9 of them the same tests, with neither rewritten test in either list. I noted at the time that
@@ -294,8 +296,8 @@ never a failure total at all.
real number. **The "~12 ambient flakes per run" figure is withdrawn — do not cite it.**
**The transferable lesson, which is stronger than the original one:** "count the tests, never
read the exit code" is necessary but not sufficient — *a **missing** summary line is itself
the signal that the run did not complete.* Treating its absence as a capture nuisance to be
read the exit code" is necessary but not sufficient — _a **missing** summary line is itself
the signal that the run did not complete._ Treating its absence as a capture nuisance to be
worked around, rather than as a failure, is what produced a confident comparison between two
partial runs. Assert the summary line exists **and** that the file count matches expectation
before believing any suite-level number.
@@ -357,27 +359,29 @@ suite.** The whole `component` project is green at HEAD.
## Appendix A — the `CLAUDE.md` addition (✅ APPLIED)
**Applied 2026-08-06** to the root `CLAUDE.md`, as the last `####` block of `### Testing`
(after *Convention guards run as tests*). The applied text is a condensed form of the draft
(after _Convention guards run as tests_). The applied text is a condensed form of the draft
below — `CLAUDE.md` is loaded every session, so it carries the imperative, the two fixes, the
anti-fix, and the ~15 s correction, and links back here for the evidence. The poll interval
(50 ms) and the 15 s browser-mode `testTimeout` default were re-derived from the installed
`vitest@4.0.18` before shipping. The draft is retained for the record: it is the source the
applied wording was cut down from, not a pending proposal.
````markdown
```markdown
#### Never `await` a browser-test state that DELETES ITSELF
`expect.element` polls (first attempt immediate, then every 50ms, 15s budget). Waiting for a state to **arrive** is safe — load only makes it arrive later and the matcher keeps polling. Waiting for a state that will **leave** is a race the matcher cannot win: once the state is gone it never comes back, so every remaining poll is also too late. Such a test is red on a busy box, green on a quiet one, and has no PR to blame.
Measured on `AppsSubmitEditView` (component instrumented with `performance.now()` timestamps): after an awaited `.click()`, a state the component deletes `N` ms later is observable for **exactly** `N` ms, and the matcher's first poll lands **40–78ms** into that window on an idle box. A test asserting it at `N=200` therefore ran on a ~150ms margin, which a saturated CI box erases. Shrinking `N` to 1ms is an accelerated equivalent (the failure condition is `RPC return leg > window`) and reproduced the CI failure exactly: **8 of 12 runs red at ~15.0s under contention (load 99–114)**, versus **12 of 12 green at ~1s** after the fix.
⚠️ **A ~15s failing test is a candidate filter, NOT a diagnosis.** It means only that some `expect.element` was never satisfied. Measured: four *non-race* mutations all failed at 14.97–15.09s, and two **healthy, passing** tests legitimately run 15.06s/15.26s waiting out a real 15s product timeout. To tell a self-deleting state from one that never arrived: read the observable **synchronously right after the action** (present-then-gone vs never-present), or **enlarge the component's own window** and see whether the failure disappears — as a diagnostic only, since shipping that widening is what this rule forbids.
⚠️ **A ~15s failing test is a candidate filter, NOT a diagnosis.** It means only that some `expect.element` was never satisfied. Measured: four _non-race_ mutations all failed at 14.97–15.09s, and two **healthy, passing** tests legitimately run 15.06s/15.26s waiting out a real 15s product timeout. To tell a self-deleting state from one that never arrived: read the observable **synchronously right after the action** (present-then-gone vs never-present), or **enlarge the component's own window** and see whether the failure disappears — as a diagnostic only, since shipping that widening is what this rule forbids.
Fixes, in order of preference:
1. **Make the state absorbing** — drive the component so nothing can take it away (e.g. `rerender` with a ceiling so large the timer can never fire), *then* assert it. Include a negative control proving the prop change alone did not produce the state.
1. **Make the state absorbing** — drive the component so nothing can take it away (e.g. `rerender` with a ceiling so large the timer can never fire), _then_ assert it. Include a negative control proving the prop change alone did not produce the state.
2. **Don't assert the transient at all** — await the absorbing end-state instead, and pin that the intermediate step happened via a non-DOM observable (a mock call count).
🔴 Do **not** widen the matcher budget, add a `retry`, or enlarge the component's timeout instead. Those convert a fast failure into a slow one and leave the race unwinnable whenever the machine is slow enough — which is exactly when CI runs. The two retry tests in `src/components/Apps/AppsSubmitEditView.browser.test.tsx` are worked examples of both fixes.
````
```
## Appendix B — probe instrumentation for continuing the `MySubmissionsList` hunt
claudedocs/rca-readtime-metric-privacy-cpu-2026-07-24.md
+17 -7
View File
@@ -1,11 +1,14 @@
# RCA — read-time model-metric-privacy CPU regression (`model-metric-privacy-readtime`)
**Status (added 2026-08-21):** Fix merged as PR #3331 (2026-07-24). The root cause and proposed fix in this doc describe the change that shipped. Production A/B verification (step 3 in the doc's verification plan) is NOT VERIFIED — see report.
Date: 2026-07-24
Author: perf investigation (static source + measured A/B evidence)
Repo: `civitai/civitai` @ `origin/main` (f5fe73fd5f)
Branch: `zach/readtime-metric-privacy-cpu-fix`
## Measured evidence (ground truth)
- `civitai-dp-prod-api-primary`: per-request server CPU ~+35% and event-loop **longtask**
time ~+71% higher with the Flipt flag `model-metric-privacy-readtime` **ON** vs **OFF**,
via a clean load-normalized bracketed A/B (ON→OFF→ON).
@@ -15,6 +18,7 @@ Branch: `zach/readtime-metric-privacy-cpu-fix`
- Longtask ⇒ the cost is **frequent synchronous event-loop blocking on a hot read path**.
## Root cause (1 paragraph)
The dominant flag-gated cost is **not** the membership lookup #3322 cached — it is an
**unconditional, uncached `dbRead.user.findMany({ select: { settings } })` that fetches and
synchronously deserializes every owner's FULL `settings` JSON blob, once per request, over
@@ -31,6 +35,7 @@ longtask. Critically, the codebase already has a batched, Redis-backed, bust-wir
a raw per-request DB read. This is a net-new query #3266 added and #3322 never removed.
## Why #3322 missed it (the key deliverable)
`#3322` optimized `getValidCreatorMembershipMap` (Redis-cached the `id→isValidMember` boolean,
skipping the `customerSubscription.findMany` + per-subscription
`subscriptionProductMetadataSchema.parse` on hits). But on the **feed** path the membership
@@ -48,6 +53,7 @@ know if a default hides. #3322 fixed the branch that the hot path skips and left
hot path always runs.
### Ruled out (with code)
- **Membership Zod parse per entity** (hint a): only runs on cache MISS inside
`queryValidCreatorMembership` (`creator-membership.service.ts:71`), and only for hide-owners;
not on the common hot path. Not the delta.
@@ -62,17 +68,19 @@ hot path always runs.
light boolean ORs; negligible per entity.
## Exact hot cost & fan-out
| Path | file:line | Gated by flag? | Per-request fan-out | Cost |
|---|---|---|---|---|
| Browse feed `model.getAll` → `getModelsRaw` | `model.service.ts:1428-1447` | **Yes** | 1 DB query + deserialize of full `settings` for **all N feed owners** (N up to ~100) | **Dominant** — highest volume on api-primary |
| v1 models list `getModelsWithVersions` | `model.service.ts:3350-3359` | No (always-on) | same, all owners in page | always-on baseline cost |
| associated models | `model.controller.ts:1637-1647` | Yes | same, all associated owners | lower volume |
| single `getModel` | `model.controller.ts:307-324` | Yes | 1 owner `findUnique`, short-circuited | small — leave as-is |
| Path | file:line | Gated by flag? | Per-request fan-out | Cost |
| ------------------------------------------- | ------------------------------- | -------------- | ------------------------------------------------------------------------------------ | -------------------------------------------- |
| Browse feed `model.getAll` → `getModelsRaw` | `model.service.ts:1428-1447` | **Yes** | 1 DB query + deserialize of full `settings` for **all N feed owners** (N up to ~100) | **Dominant** — highest volume on api-primary |
| v1 models list `getModelsWithVersions` | `model.service.ts:3350-3359` | No (always-on) | same, all owners in page | always-on baseline cost |
| associated models | `model.controller.ts:1637-1647` | Yes | same, all associated owners | lower volume |
| single `getModel` | `model.controller.ts:307-324` | Yes | 1 owner `findUnique`, short-circuited | small — leave as-is |
The synchronous longtask = JSON deserialization of N large `settings` blobs per request,
attributed to the feed because it is the hottest gated caller.
## Fix design (minimal, deterministic, byte-identical privacy)
Add a tiny read-through per-user cache of the **three derived hide-default booleans only**,
mirroring the proven `getValidCreatorMembershipMap` pattern in the same dependency-light module:
@@ -100,6 +108,7 @@ existing `userSettingsCache` and membership cache, and it can only make the cach
OWN setting change; it can never expose another creator's metric that membership would hide.
## Verification plan
1. **Compile:** `NODE_OPTIONS=--max_old_space_size=8192 npx tsc --noEmit` (default heap OOMs).
2. **Unit tests:** cache hit/miss/batch-backfill/fail-open/bust + byte-identical-vs-raw-settings
(a stored 3-bool object resolves the same hidden metrics as reading full settings), and the
@@ -109,11 +118,12 @@ OWN setting change; it can never expose another creator's metric that membership
baseline. Metrics: per-request CPU (Pyroscope / cpuprofile) and the event-loop **longtask**
metric (the +71% signal). Success = the ON/OFF gap collapses (longtask ON ≈ OFF).
4. **Cache-hit confirmation:** watch the new key populate (`packed:caches:user-metric-privacy-
defaults:*`) and DB `user` findMany volume from these paths drop after warmup; confirm no
defaults:*`) and DB `user` findMany volume from these paths drop after warmup; confirm no
change in emitted hidden-metric values on a hide-owner model (spot-check a known CP member's
card + version stats look identical pre/post).
## Residual uncertainty (honest)
Static analysis proves the feed unconditionally does an uncached full-`settings` fetch per
request that #3322 left in place and that the common hot path never calls membership — so this
IS a real, dominant, always-run synchronous cost the A/B flag toggles. What static reading
claudedocs/red-test-triage-2026-08-10.md
+35 -33
View File
@@ -1,5 +1,7 @@
# Triage: the "16 pre-existing test failures on main"
**Status (added 2026-08-21):** Historical analysis. Verified at baseline `a43e49a4ba` and `2a2fe66428`. The 16 failures were environment-caused, not code-caused. The worktree/submodule/.envrc traps documented here are now in CLAUDE.md (PR #3567).
**Measured 2026-08-10.** Baseline commit `a43e49a4ba` (`origin/main`, "5.0.2262"), plus a
second baseline at `2a2fe66428` — the tip of `main` on 2026-08-08, the day the ticket was
filed. Every run below was made in a clean throwaway worktree off the remote tip, with a
@@ -14,7 +16,7 @@ versions, and so are the `packages` and `apps` projects. All five suites the tic
by name pass, each collecting a non-zero number of tests.
The failures were real observations of a real problem — but the problem was in the
*environment the suite was run in*, not in the tests. A fresh `git worktree` does not check
_environment the suite was run in_, not in the tests. A fresh `git worktree` does not check
out submodules and does not carry the repo's gitignored `.envrc`, and each of those omissions
produces a distinct block of false reds. That trap has been documented in prose in `CLAUDE.md`
since #3567 (2026-08-04) and was walked into anyway, four days later, by three agents in a row.
@@ -23,15 +25,15 @@ since #3567 (2026-08-04) and was walked into anyway, four days later, by three a
Three numbers, not one, because the failure mode at issue moves between them.
| Run | Commit | Node | Failed FILES | Failed TESTS | Passed TESTS | Skipped | exit |
|---|---|---|---|---|---|---|---|
| unit, submodule **absent** | `a43e49a4ba` | 22.22.2 (flake) | **73** / 891 | **0** | 12,782 | 4 | 1 |
| unit, submodule present | `a43e49a4ba` | 22.22.2 (flake) | 0 / 891 | 0 | 13,753 | 1 | 0 |
| unit, submodule present | `a43e49a4ba` | 24.18.1 (`.nvmrc`/CI) | 0 / 891 | 0 | 13,753 | 1 | 1 † |
| unit, submodule present | `a43e49a4ba` | 26.5.0 (ambient) | 1 / 891 | **7** | 13,746 | 1 | 1 |
| unit, submodule present | `2a2fe66428` (2026-08-08) | 22.22.2 (flake) | 0 / 889 | 0 | 13,713 | 1 | 0 |
| `packages` | `a43e49a4ba` | 22.22.2 | 0 / 73 (2 skipped) | 0 | 1,002 | 4 | 0 |
| `apps` | `a43e49a4ba` | 22.22.2 | 0 / 53 | 0 | 507 | 0 | 0 |
| Run | Commit | Node | Failed FILES | Failed TESTS | Passed TESTS | Skipped | exit |
| -------------------------- | ------------------------- | --------------------- | ------------------ | ------------ | ------------ | ------- | ---- |
| unit, submodule **absent** | `a43e49a4ba` | 22.22.2 (flake) | **73** / 891 | **0** | 12,782 | 4 | 1 |
| unit, submodule present | `a43e49a4ba` | 22.22.2 (flake) | 0 / 891 | 0 | 13,753 | 1 | 0 |
| unit, submodule present | `a43e49a4ba` | 24.18.1 (`.nvmrc`/CI) | 0 / 891 | 0 | 13,753 | 1 | 1 † |
| unit, submodule present | `a43e49a4ba` | 26.5.0 (ambient) | 1 / 891 | **7** | 13,746 | 1 | 1 |
| unit, submodule present | `2a2fe66428` (2026-08-08) | 22.22.2 (flake) | 0 / 889 | 0 | 13,713 | 1 | 0 |
| `packages` | `a43e49a4ba` | 22.22.2 | 0 / 73 (2 skipped) | 0 | 1,002 | 4 | 0 |
| `apps` | `a43e49a4ba` | 22.22.2 | 0 / 53 | 0 | 507 | 0 | 0 |
† 0 failed tests but a non-zero exit — see "Finding 3".
@@ -45,7 +47,7 @@ Of the 73 failing files in the submodule-absent run, **72 collected ZERO tests**
(`prisma-inconsistent-orphan-relations.test.ts`) collected 6 and skipped 3, and failed via an
unhandled rejection rather than an assertion.
That is the whole shape of the trap: 72 files that *fail* without *failing a test*. The run
That is the whole shape of the trap: 72 files that _fail_ without _failing a test_. The run
reports **0 failed tests** and 12,782 passed. The green run reports 13,753 passed. So the
broken run silently removed **971 tests** while its failed-test count read zero. A reader
comparing failed-test counts sees "0 either way"; a reader comparing passed-test totals to a
@@ -61,23 +63,23 @@ Every suite the ticket names is **green and non-vacuous**. Each was mutation-tes
it guards was broken on purpose, and the guard was watched to go red **with its own specific
error message**, on an input no earlier check short-circuits.
| Suite | Kind | State on `main` | Verdict |
|---|---|---|---|
| `src/server/services/__tests__/no-wholesale-module-mock.test.ts` | quality guard (ESLint RuleTester, 97 tests) | green | **Keep. Not vacuous — proven.** |
| `src/server/middleware/__tests__/block-scope.normalize-endpoint.test.ts` | route-drift guard (27 tests) | green | **Keep. Not vacuous — proven.** |
| `src/server/services/blocks/__tests__/app-spend-tier-privilege.test.ts` | privilege-surface drift guard (9 tests) | green | **Keep. Not vacuous — proven.** |
| `src/server/services/comics/__tests__/orchestrator-chat.wait-unit.test.ts` | ledger drift guard (12 tests) | green | **Keep. Not vacuous — proven.** |
| `scripts/__tests__/typecheck.test.ts` | wrapper-behaviour guard (5 tests) | green | **Keep. Not vacuous — proven.** |
| Suite | Kind | State on `main` | Verdict |
| -------------------------------------------------------------------------- | ------------------------------------------- | --------------- | ------------------------------- |
| `src/server/services/__tests__/no-wholesale-module-mock.test.ts` | quality guard (ESLint RuleTester, 97 tests) | green | **Keep. Not vacuous — proven.** |
| `src/server/middleware/__tests__/block-scope.normalize-endpoint.test.ts` | route-drift guard (27 tests) | green | **Keep. Not vacuous — proven.** |
| `src/server/services/blocks/__tests__/app-spend-tier-privilege.test.ts` | privilege-surface drift guard (9 tests) | green | **Keep. Not vacuous — proven.** |
| `src/server/services/comics/__tests__/orchestrator-chat.wait-unit.test.ts` | ledger drift guard (12 tests) | green | **Keep. Not vacuous — proven.** |
| `scripts/__tests__/typecheck.test.ts` | wrapper-behaviour guard (5 tests) | green | **Keep. Not vacuous — proven.** |
### Mutation evidence
| Guard | Mutation applied | Result |
|---|---|---|
| Guard | Mutation applied | Result |
| -------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `no-wholesale-module-mock` | Reintroduced the historical **textual** check in `eslint-local-rules.js` — treat any factory whose source text contains `importOriginal` as safe | 97 → **29 failed / 68 passed**, `AssertionError: Should have 1 error but had 0: []`. The killed cases are exactly the laundering cases (unused `importOriginal` param, a comment mentioning it, the bare string `'importOriginal'` as a value). |
| route drift | Added `src/pages/api/v1/blocks/mutation-probe.ts` exporting `withBlockScope(...)` | 27 → **2 failed**: *"the allowlist is EXACTLY the static segments of the wrapped routes"* and *"pins the current set, so adding a route is a deliberate act"*, both diffing on the literal `"mutation-probe"`. |
| spend-tier privilege | Added `const __probe = { spendTier: 'premium' }` to `src/pages/api/v1/developer/block-manifests.ts` (a publisher-reachable module) | 9 → **3 failed**, incl. `AssertionError: src/pages/api/v1/developer/block-manifests.ts must not reference spendTier in code`. |
| wait-unit ledger | Added `src/server/services/comics/mutation-probe.ts` with `export const probeQuery = { wait: 60000 }` | 12 → **2 failed**: *"no statically-resolvable wait exceeds the seconds envelope"* and *"matches the known ledger of numeric wait sites"*, naming `server/services/comics/mutation-probe.ts:60000`. |
| typecheck wrapper | Moved the one-line crash verdict from stdout to stderr in `scripts/typecheck.mjs` | 5 → **3 failed**, `AssertionError: expected '' to contain 'TYPECHECK CRASHED'`. |
| route drift | Added `src/pages/api/v1/blocks/mutation-probe.ts` exporting `withBlockScope(...)` | 27 → **2 failed**: _"the allowlist is EXACTLY the static segments of the wrapped routes"_ and _"pins the current set, so adding a route is a deliberate act"_, both diffing on the literal `"mutation-probe"`. |
| spend-tier privilege | Added `const __probe = { spendTier: 'premium' }` to `src/pages/api/v1/developer/block-manifests.ts` (a publisher-reachable module) | 9 → **3 failed**, incl. `AssertionError: src/pages/api/v1/developer/block-manifests.ts must not reference spendTier in code`. |
| wait-unit ledger | Added `src/server/services/comics/mutation-probe.ts` with `export const probeQuery = { wait: 60000 }` | 12 → **2 failed**: _"no statically-resolvable wait exceeds the seconds envelope"_ and _"matches the known ledger of numeric wait sites"_, naming `server/services/comics/mutation-probe.ts:60000`. |
| typecheck wrapper | Moved the one-line crash verdict from stdout to stderr in `scripts/typecheck.mjs` | 5 → **3 failed**, `AssertionError: expected '' to contain 'TYPECHECK CRASHED'`. |
Every mutation was reverted and the five suites re-run together afterwards: **150 tests, all
passing**, tracked tree byte-clean.
@@ -89,14 +91,14 @@ had an opinion about" the new `/api/testing/eventloop-stall` route added in #375
It would not have, and this is worth knowing rather than assuming. That guard's walk matches
only files whose default export is wrapped in `withBlockScope(...)`. `eventloop-stall.ts` is
not block-scoped, so the guard is correctly silent about it. It *is* live for the routes it
not block-scoped, so the guard is correctly silent about it. It _is_ live for the routes it
covers — the mutation above proves that — but it is a **block-scope allowlist** drift guard,
not a general new-route detector. No such general guard exists. If one is wanted, that is a
new piece of work, not a repair.
## Phase 3 — what actually produced the 16
Not reproduced exactly, and I will not claim otherwise. What *is* measured:
Not reproduced exactly, and I will not claim otherwise. What _is_ measured:
**Finding 1 — the submodule (this is the big one).** Detailed above: 73 files fail, 72 collect
zero tests, failed-test count reads 0, 971 tests vanish. An independent agent measuring a
@@ -112,9 +114,9 @@ undefined (reading 'clear')` on `window.localStorage` under happy-dom. Under the
engine env: I measured **6** `PrismaClientInitializationError: could not locate the Query Engine
for runtime "linux-nixos"` unhandled rejections.
`CLAUDE.md` already records this exact pair from a previous encounter: *"system Node 26.5.0
`CLAUDE.md` already records this exact pair from a previous encounter: _"system Node 26.5.0
against the flake's 22.22.2 produced 7 spurious `window.localStorage is undefined` failures
under happy-dom plus 8 Prisma `linux-nixos` engine errors — every one a false red."* **7 + 8 =
under happy-dom plus 8 Prisma `linux-nixos` engine errors — every one a false red."_ **7 + 8 =
15**, against a reported 16. That is the closest match to the ticket's number that any
mechanism here produces, it is the mechanism the repo had already written down, and the two
halves are independently reproducible.
@@ -131,7 +133,7 @@ Flagged, not fixed — the fix is environment-shaped and overlaps PR #3779.
clean tree". `git stash` is repo-**global**: `refs/stash` lives in the common git dir and is
shared by every one of this clone's worktrees. Three agents stashing concurrently across a
shared clone are not each producing a clean tree; they are pushing and popping one shared
stack. That is consistent with three agents reporting an *identical* set from three different
stack. That is consistent with three agents reporting an _identical_ set from three different
branches, and it is a hazard worth retiring independently of this ticket. Stated as mechanism,
not as measurement — I did not reproduce it.
@@ -141,7 +143,7 @@ One file added: **`src/__tests__/submodules-checked-out.test.ts`**.
It converts Finding 1 from a silent subtraction into one legible red line. It reads the
submodule paths out of `.gitmodules` rather than hardcoding them, so a submodule added later is
covered without anyone remembering, and it asserts three specific entry points so a *partial*
covered without anyone remembering, and it asserts three specific entry points so a _partial_
checkout — which produces the identical vanish-without-failing symptom — is caught too. It
imports nothing from the submodule, because a guard that fails to load for the very reason it
exists to report is not a guard.
@@ -150,10 +152,10 @@ Controls run on it, both directions:
- **Green state:** 5 tests collected, 5 passed.
- **Negative control, empty gitlink directory:** 4 failed / 1 passed, `AssertionError:
submodule "event-engine-common" is present but EMPTY — an uninitialised gitlink. Fix: git
submodule update --init --recursive`.
submodule "event-engine-common" is present but EMPTY — an uninitialised gitlink. Fix: git
submodule update --init --recursive`.
- **Negative control, directory absent entirely:** 4 failed / 1 passed, `AssertionError:
submodule "event-engine-common" is not checked out. …`.
submodule "event-engine-common" is not checked out. …`.
- **Reachability:** in both broken states the file still **collected 5 tests** while 72 other
files were collecting zero. It is visible precisely when everything else has gone quiet.
- **Vacuity control:** the `it.each` populations are derived, so an unparseable `.gitmodules`
claudedocs/runner-scouting-2026-08-15.md
+24 -22
View File
@@ -1,43 +1,45 @@
# Is vitest the right runner? Scouting Bun and node:test
**Status (added 2026-08-21):** Recommendation unchanged — stay on vitest. The measurement was single-shot and not re-run; the ~20x ratio on the like-for-like file is a point estimate.
Recorded 2026-08-15. Measurement and a recommendation; no migration was attempted, and no box time
was used — every probe is a single-process module import, not a suite run.
> **Two corrections were made to this document after first publication, and both are stated in place
> below rather than silently edited out.** (1) A per-module ratio derived from `inventory.json`'s
> static counts is retracted; that artifact over-counted by up to 75x, selectively. (2) The headline
> comparison was not like-for-like — it set vitest's `collect` for a *test file* against a probe
> importing only the *source module* beneath it, which is a smaller graph. The gap was overstated as
> comparison was not like-for-like — it set vitest's `collect` for a _test file_ against a probe
> importing only the _source module_ beneath it, which is a smaller graph. The gap was overstated as
> ~1600x; it is ~20x. **The recommendation did not change under either correction.**
**Recommendation: stay on vitest — but the reason overturns the cost model we had been optimising
against.** The per-module constant is *not* inherent to our module graph. On the one file all three
against.** The per-module constant is _not_ inherent to our module graph. On the one file all three
runtimes can load, bun loads the same 82-module closure ~15-20x faster than vitest's `collect`.
## The measurement
**The like-for-like row is the test file, not the source module.** vitest's `collect` loads the
whole test-file closure — the file, its imports, and `vitest` itself. An early version of this doc
compared that against a probe importing only the *source* module underneath it, which is a different
compared that against a probe importing only the _source_ module underneath it, which is a different
and much smaller graph. Corrected:
| target | real closure | vitest `collect` | bun | node + tsx |
|---|---|---|---|---|
| `model-substitution.test.ts` | 82 modules | **5298 ms** | **259 ms** (median of 5: 250–262) | ✗ cannot import `vitest` under CJS |
| target | real closure | vitest `collect` | bun | node + tsx |
| ---------------------------- | ------------ | ---------------- | --------------------------------- | ---------------------------------- |
| `model-substitution.test.ts` | 82 modules | **5298 ms** | **259 ms** (median of 5: 250–262) | ✗ cannot import `vitest` under CJS |
So **~20x on the one file all three could be asked to load**, not the three orders of magnitude an
earlier draft claimed.
Source-module-only probes, which are *not* comparable to `collect` and are kept only to show the
Source-module-only probes, which are _not_ comparable to `collect` and are kept only to show the
runtimes' floor:
| source module | bun | node + tsx |
|---|---|---|
| `model-substitution.ts` | 3.3 ms | 8.5 ms |
| `challenge.constants.ts` | 4.0 ms | 10.4 ms |
| `placement.ts` | 3.7 ms | 122.7 ms |
| `image.schema.ts` | 68.6 ms | 514 ms |
| `utils/metadata/audit.ts` | 162.6 ms | 1755 ms |
| source module | bun | node + tsx |
| ------------------------- | -------- | ---------- |
| `model-substitution.ts` | 3.3 ms | 8.5 ms |
| `challenge.constants.ts` | 4.0 ms | 10.4 ms |
| `placement.ts` | 3.7 ms | 122.7 ms |
| `image.schema.ts` | 68.6 ms | 514 ms |
| `utils/metadata/audit.ts` | 162.6 ms | 1755 ms |
🔴 **A per-module ratio published here earlier is RETRACTED.** It divided `collect` by
`inventory.json`'s static module counts, and that artifact was later found wrong by up to 75x and
@@ -65,11 +67,11 @@ has — and everything downstream that divides by a module count rests on it.
**What both of this document's errors had in common**, since the pattern generalises past this
question: each was a denominator error, and each produced a number that was right about the thing it
measured and wrong about what that thing was. The second was catchable alone by asking what `collect`
actually includes before dividing by anything. It is easy to check that two *runtimes* are comparable
and forget to check that the two *quantities* are.
actually includes before dividing by anything. It is easy to check that two _runtimes_ are comparable
and forget to check that the two _quantities_ are.
**So the cost is the module runner, not the modules.** That is consistent with the tracer result from
this morning — 569 module *bodies* executing in ~0.4 s against a 25.4 s import phase — and it locates
this morning — 569 module _bodies_ executing in ~0.4 s against a 25.4 s import phase — and it locates
the missing time in vite-node's per-module fetch/instantiate, not in compile-and-evaluate as we had
been assuming. The two hypotheses make the same prediction for pool choice, which is why swapping
pools did not distinguish them; a different runtime does.
@@ -77,7 +79,7 @@ pools did not distinguish them; a different runtime does.
⚠️ **Read the limits before quoting any of it.** These probes import one graph in one process, with no
test framework, no mock interception, and no isolation between files. vitest's `collect` includes the
setup file, the mock machinery, and a fresh registry per test file. The gap is real but it is not
apples to apples, and nothing here shows bun *running our tests* faster — only loading our modules
apples to apples, and nothing here shows bun _running our tests_ faster — only loading our modules
faster. `node + tsx` could not even import a test file: `Vitest cannot be imported in a CommonJS
module using require()`.
@@ -97,7 +99,7 @@ flattering-slice trap: the runner whose win is startup was measured only where s
load these files" is a hard failure observed directly and does not rest on a count.
**Module-scope env aborts the import under every runtime.** `~/env/server` validates at module scope,
so a bare import throws `Invalid environment variables` under bun *and* node. Satisfying it with a
so a bare import throws `Invalid environment variables` under bun _and_ node. Satisfying it with a
synthesised env got past the gate; `cache-helpers` then **hung past 300 s** under bun, which looks
like a module-scope client construction that never settles. Not diagnosed further.
@@ -121,7 +123,7 @@ vitest APIs or its reporter contract. None of that is wall-clock, and all of it
**One thing a switch would NOT fix.** `IS_BUILD` / `IS_DATAPACKET` are read at module scope and set
both ways by different tests. Any runner that shares a process between files has that limit, so the
two-project split's *existence* is runner-independent. A cheaper runner could change the size of the
two-project split's _existence_ is runner-independent. A cheaper runner could change the size of the
residue, not remove it.
## Recommendation
@@ -134,7 +136,7 @@ mock sites plus six pieces of first-party tooling.
than the graph, which says:
- shrinking the graph (fewer modules per worker) attacks a term whose real work is a fraction of a
millisecond per module — the leverage is in how many times vite-node *instantiates* a module, not in
millisecond per module — the leverage is in how many times vite-node _instantiates_ a module, not in
how many modules exist;
- `isolate: false` is the only lever that removes instantiations rather than reducing their count,
which is consistent with it being the largest measured effect all day (collect 4565 s → 280 s);
claudedocs/services-a-m-handover.md
+2
View File
@@ -1,5 +1,7 @@
# Handover — `src/server/services/__tests__` a–m shared-mock migration
**Status (added 2026-08-21):** Historical handover log. 127 of 129 files migrated at time of handover; 2 hold-outs remained. The branch `perf/test-mock-migration-services-a-m` is the reference.
josh, 2026-08-15, continued and closed out by liz through batches 6–9 (her sections are marked).
Kept as a LOG rather than rewritten to current state: which claims were superseded, and by what, is
the thing a successor most needs — josh's _"the three easiest remaining files"_ with the ✅ beneath
claudedocs/services-a-m-parameterised-client-analysis.md
+2
View File
@@ -1,5 +1,7 @@
# The `dbRead`/`dbWrite` alias split where the service picks its client at runtime
**Status (added 2026-08-21):** Historical analysis. Bucket classifications were verified at branch base `17f994221e`; the 6 files and their routing defaults are documented per-case.
Written by josh, 2026-08-15, for the `src/server/services/__tests__` a–m slice of the shared-mock
migration (branch `perf/test-mock-migration-services-a-m`). These are the buckets in that slice
where the routing decision is **not** readable off the production source in the usual way, so it is
claudedocs/test-perf-measurement-envelope-2026-08-15.md
+7 -5
View File
@@ -1,5 +1,7 @@
# The measuring box moves further than most of the effects we measured
**Status (added 2026-08-21):** Historical measurement. The ~20% drift envelope is a point estimate from one contaminated pair; a clean pair has not been taken. The methodological conclusions (pair controls, cost-comparable control groups) remain valid.
Recorded 2026-08-15 from the unit-suite performance work. This is the number that decides which of
the day's other numbers are readable, so it belongs beside them rather than inside any one PR.
@@ -14,7 +16,7 @@ drift-3 wall 253.8s collect 5845s +20.5% collect, +17.2% wall
```
⚠️ **That pair was contaminated** — another agent's suite started during the second run; five vitest
workers were live on the box immediately afterwards. So `+20.5%` is not a drift *figure*. It is a
workers were live on the box immediately afterwards. So `+20.5%` is not a drift _figure_. It is a
demonstration that this box can move that far while nothing under measurement changes, which is the
part that matters: **any comparison assembled from two different windows sits inside that envelope.**
@@ -25,7 +27,7 @@ take one; it is the denominator for everything else.
**1. Quote in-pair controls, not cross-window deltas.** A wall-clock difference between a run taken
now and a run taken an hour ago is unreadable. The readable form is a single pair with a control
group inside it — files the change cannot have affected — reported *first*, before the headline.
group inside it — files the change cannot have affected — reported _first_, before the headline.
`scripts/test-perf/compare-runs.mjs` does this and prints the control group with the header "if this
moved, the headline below is drift, not the change."
@@ -42,7 +44,7 @@ Two seconds of absolute noise rendered as a large percentage. A good control was
**3. Dose-response is suggestive, not conclusive, when the axis is confounded.** Pre-bundling five
external packages showed a clean-looking gradient — files reaching none of them moved −6.9%, files
reaching one or two moved −31%/−30%. That argues the effect is real rather than ambient, *except*
reaching one or two moved −31%/−30%. That argues the effect is real rather than ambient, _except_
that mean per-file `collect` rises monotonically with exposure (0.32s → 10.43s), so "sorts by
exposure" and "sorts by file weight" are not separated by that data. The contaminated drift pair
sorted by exposure too, inversely. **Load can produce a gradient on any axis that correlates with
@@ -62,7 +64,7 @@ instant.** Three failures in one day, three different shapes:
their MCP servers plus the dev-server daemon are ~30 node processes at rest;
- a genuine zero sample that was simply the gap between two of someone's short runs.
The check that works is *for the workload, not the runtime*:
The check that works is _for the workload, not the runtime_:
```powershell
Get-CimInstance Win32_Process -Filter "Name='node.exe'" |
@@ -78,6 +80,6 @@ window as a commitment from whoever holds the box, not as an inference from a pr
Every one of these is the same mistake wearing different clothes: **a measurement passing because the
thing being measured never happened.** A run that stopped early. A config that emitted no chunk. A
module that never loaded. A gate that could not fire. Verifying that the change was *engaged* — the
module that never loaded. A gate that could not fire. Verifying that the change was _engaged_ — the
chunk on disk, the module body executing, the worker actually spawned — costs seconds and is the only
thing that separates a result from a coincidence.
claudedocs/turbopack-chunk-hash-collision-2026-08-18.md
+1 -1
View File
@@ -1,6 +1,6 @@
# Turbopack "assets emitted to the same output path" — root cause and options
**Date:** 2026-08-18 · **Status:** diagnosed, not fixed · **Next:** 16.3.0 (`^16.3.0`)
**Date:** 2026-08-18 · **Status (corrected 2026-08-21):** diagnosed, not fixed. Repo is on `^16.3.1`; the doc's "Next: 16.3.0" was a forward-looking expectation, not the current version. 16.3.1 re-rolls the collision but does not fix the mechanism (see §Next 16.3.1 is a re-roll, not a fix).
## Symptom
claudedocs/typecheck-tests-gap-2026-08-12.md
+127 -120
View File
@@ -1,7 +1,13 @@
# `pnpm typecheck` is structurally blind to every file under `src/**/__tests__/`
**Status: proposal. Nothing here is a mass fix.** Measured 2026-08-12 against `main` at
`f19574a9cb`, on Node 22.22.2 / TypeScript 5.9.2.
**Status (corrected 2026-08-21): merged, but NOT WIRED — the gate runs nowhere.** PR #3868
merged 2026-08-13 (its title reads `PROPOSAL (do not merge)`, which is misleading); PR #4189
extended it to `scripts/` 2026-08-20. `tsconfig.tests.json`, `scripts/ci/typecheck-tests-gate.mjs`
and its baseline are all on `main`, and the gate has its own unit tests — but **nothing invokes
it**: no `package.json` script (`typecheck` runs the base config, which still excludes
`src/**/__tests__/**`), and no workflow reference. Verified at `312a91ad7c`. So the ratchet that
was meant to stop the inflow currently fires on nothing, and reads as done.
Original measurement: 2026-08-12 against `main` at `f19574a9cb`, Node 22.22.2 / TypeScript 5.9.2.
---
@@ -13,7 +19,7 @@
"src/**/__tests__/**"
```
`include` still lists `src`, so the *app* tree is checked normally — but no file under a
`include` still lists `src`, so the _app_ tree is checked normally — but no file under a
`__tests__/` directory ever enters the program. `pnpm typecheck` therefore does not report
type errors in those files leniently; it cannot see them at all. A deliberate type error
planted in such a file is reported as **0 errors, exit 0** (demonstrated in §6).
@@ -22,12 +28,12 @@ This is not a hypothetical. It has produced two distinct failures on one pull re
nineteen real type errors accumulated unnoticed in a single new test file, and a first
attempt to measure the problem produced a false all-clear.
Two things it does *not* cover, worth stating so the scope is honest:
Two things it does _not_ cover, worth stating so the scope is honest:
- **`tests/` and `test/` (top-level) are already checked.** They are in `include` and are not
excluded. This is only about `src/**/__tests__/`.
- **242 test files under `src/` are already checked**, because they are `*.test.ts` files that
do not live in a `__tests__/` directory. The exclusion is keyed on the *directory*, not on
do not live in a `__tests__/` directory. The exclusion is keyed on the _directory_, not on
the filename.
- **`packages/*/src/**/__tests__/` is already checked too** — 68 such files are in the program
today, because the exclude pattern is anchored at the repo-root `src/`, not applied
@@ -41,7 +47,7 @@ on which directory you happened to put it in.
## 2. Why the exclusion exists
It is not a performance decision. What it *is* has been corrected once already — see the
It is not a performance decision. What it _is_ has been corrected once already — see the
correction note below, which changes the reading materially.
The `exclude` entry has been touched exactly **once** in the repository's history:
@@ -55,6 +61,7 @@ That commit's diff on `tsconfig.json` is the single added line. The same commit
file, in a commit whose subject is about neither.
> ### 🔴 Correction (2026-08-13): this file previously claimed that `exif-parser.test.ts` was
>
> "the first file the repository ever put in a `src/**/__tests__/` directory", and that
> "nobody decided that test files should not be typechecked". **Both are false.** Verified:
>
@@ -73,9 +80,9 @@ file, in a commit whose subject is about neither.
>
> **Nine such files already existed** when the exclude line was added, the earliest on
> **2026-01-23** — over a month earlier, across six separate commits by different authors.
> The convention was established and in use; the exclusion came *afterwards*.
> The convention was established and in use; the exclusion came _afterwards_.
>
> An earlier revision of this document said *eight*, and quoted the grep
> An earlier revision of this document said _eight_, and quoted the grep
> `^src/.*/__tests__/`. That pattern requires at least one path segment between `src/` and
> `__tests__/`, so it cannot match `src/__tests__/setup.ts` — a file added by the very same
> commit as two of the others. The corrected pattern is `^src/(.*/)?__tests__/`, which is
@@ -108,11 +115,11 @@ Measured on this machine, `pnpm typecheck` (which sets `--max-old-space-size=819
`scripts/typecheck.mjs`; a bare `tsc --noEmit` at the default heap aborts and prints **zero**
diagnostics, which reads as clean):
| program | cold (no `.tsbuildinfo`) | warm |
|---|---|---|
| `tsconfig.json` (today) | 112 s | 34 s |
| `tsconfig.tests.json` (tests included) | 176 s | 39 s |
| **cost of including tests** | **+64 s (+57%)** | **+5 s (+15%)** |
| program | cold (no `.tsbuildinfo`) | warm |
| -------------------------------------- | ------------------------ | --------------- |
| `tsconfig.json` (today) | 112 s | 34 s |
| `tsconfig.tests.json` (tests included) | 176 s | 39 s |
| **cost of including tests** | **+64 s (+57%)** | **+5 s (+15%)** |
**Read these as an order of magnitude, not a constant.** All four were run back-to-back on one
developer machine (8 cores) that had other work on it; an earlier, differently-scheduled pair
@@ -159,21 +166,21 @@ tool whose failure mode is "the file you care about reports clean" is worse than
### By error code
| code | count | what it is |
|---|---|---|
| TS2345 | 147 | argument not assignable — mostly hand-built fixtures/mocks narrower than the real parameter |
| TS2493 | 126 | tuple `[]` has no element at index 0 — reading `mock.calls[0]` off a mock typed with no args |
| TS2322 | 126 | type not assignable — literal fixtures missing fields of the type they are cast to |
| TS2352 | 102 | unsafe `as` conversion, usually from `undefined` |
| TS2339 | 58 | property does not exist — e.g. `.mock` on a union of `Mock \| (() => void)` |
| TS2556 | 50 | spread argument is not a tuple |
| TS2737 | 41 | **BigInt literals require target ≥ ES2020** — see the caveat below |
| TS18048 | 34 | possibly `undefined` |
| others | 117 | 20 further codes, none above 27 |
| code | count | what it is |
| ------- | ----- | -------------------------------------------------------------------------------------------- |
| TS2345 | 147 | argument not assignable — mostly hand-built fixtures/mocks narrower than the real parameter |
| TS2493 | 126 | tuple `[]` has no element at index 0 — reading `mock.calls[0]` off a mock typed with no args |
| TS2322 | 126 | type not assignable — literal fixtures missing fields of the type they are cast to |
| TS2352 | 102 | unsafe `as` conversion, usually from `undefined` |
| TS2339 | 58 | property does not exist — e.g. `.mock` on a union of `Mock \| (() => void)` |
| TS2556 | 50 | spread argument is not a tuple |
| TS2737 | 41 | **BigInt literals require target ≥ ES2020** — see the caveat below |
| TS18048 | 34 | possibly `undefined` |
| others | 117 | 20 further codes, none above 27 |
**Caveat on TS2737 (41 errors, 5.1%, across 15 files).** These are an artifact of the app
config's `"target": "ES2018"`, inherited by the measurement config. The test files run under
Vitest on Node 22, where `1n` is fine. They are *real* errors under the target the repository
Vitest on Node 22, where `1n` is fine. They are _real_ errors under the target the repository
declares, and the fix is a one-character change (`BigInt(1)`), but they are not test-logic
defects. Overriding `target` in `tsconfig.tests.json` would remove them — and would also
destroy the one-line-delta property that makes the measurement trustworthy, since changing
@@ -190,31 +197,31 @@ src/server/db/__tests__/kysely-prisma-parity.test.ts(5,29):
`kysely` is not a dependency of this repository (only `prisma-kysely` is) and is not installed.
That import cannot resolve at runtime either, so this suite contributes zero tests — the exact
failure class already documented in the repo's own notes about suites that fail to *collect*
failure class already documented in the repo's own notes about suites that fail to _collect_
and therefore report "no tests" rather than a failure. The typecheck would have named it on the
day it landed.
### By area
| area | errors | dirty files |
|---|---|---|
| `src/server/services/` | 329 | 66 |
| `src/server/routers/` | 231 | 19 |
| `src/server/__tests__/` | 56 | 8 |
| `src/env/__tests__/` | 39 | 1 |
| `src/server/jobs/` | 30 | 7 |
| `src/server/rewards/` | 20 | 2 |
| remainder (10 areas) | 96 | 38 |
| area | errors | dirty files |
| ----------------------- | ------ | ----------- |
| `src/server/services/` | 329 | 66 |
| `src/server/routers/` | 231 | 19 |
| `src/server/__tests__/` | 56 | 8 |
| `src/env/__tests__/` | 39 | 1 |
| `src/server/jobs/` | 30 | 7 |
| `src/server/rewards/` | 20 | 2 |
| remainder (10 areas) | 96 | 38 |
### Concentration — this is what decides whether a ratchet is viable
| | errors | share of 801 |
|---|---|---|
| top 1 file | 77 | 9.6% |
| top 5 files | 266 | 33.2% |
| top 10 files | 388 | 48.4% |
| top 20 files | 493 | 61.5% |
| top 50 files | 658 | 82.1% |
| | errors | share of 801 |
| ------------ | ------ | ------------ |
| top 1 file | 77 | 9.6% |
| top 5 files | 266 | 33.2% |
| top 10 files | 388 | 48.4% |
| top 20 files | 493 | 61.5% |
| top 50 files | 658 | 82.1% |
Heavily concentrated. The long tail is thin: 52 files have exactly one error, 89 have three or
fewer. **Fifty files carry four fifths of the debt**, and they are identifiable by name.
@@ -223,14 +230,14 @@ fewer. **Fifty files carry four fifths of the debt**, and they are identifiable
Grouping the 141 dirty files by the date they were **first added**:
| month first added | dirty files | errors they contain |
|---|---|---|
| 2026-02 | 1 | 2 |
| 2026-03 | 1 | 1 |
| 2026-05 | 1 | 4 |
| **2026-06** | **39** | **316** |
| **2026-07** | **59** | **248** |
| **2026-08** (12 days) | **40** | **230** |
| month first added | dirty files | errors they contain |
| --------------------- | ----------- | ------------------- |
| 2026-02 | 1 | 2 |
| 2026-03 | 1 | 1 |
| 2026-05 | 1 | 4 |
| **2026-06** | **39** | **316** |
| **2026-07** | **59** | **248** |
| **2026-08** (12 days) | **40** | **230** |
**138 of 141 dirty files, and 794 of 801 errors, are less than three months old.** For context,
the whole convention is new — 857 of the ~905 test files ever added under `src/**/__tests__/`
@@ -250,46 +257,46 @@ the "legacy" it would grandfather is three months old and 82%-concentrated in fi
### A. A separate, non-blocking `pnpm typecheck:tests`
*What it is:* add the script, tell people to run it, report it in CI as a warning.
_What it is:_ add the script, tell people to run it, report it in CI as a warning.
*Against:* this is, functionally, the status quo plus a script. The 801 errors accrued under a
_Against:_ this is, functionally, the status quo plus a script. The 801 errors accrued under a
regime where anyone could have run `tsc -p` with a modified exclude at any time. A report that
nobody must act on does not change an inflow of 19 errors/day. It costs the same compute as a
blocking gate and buys none of the enforcement.
*Verdict:* worth having as a local-development affordance, but it is not a fix. This proposal
_Verdict:_ worth having as a local-development affordance, but it is not a fix. This proposal
deliberately does **not** add it: `pnpm typecheck -p tsconfig.tests.json` already works today
(the wrapper forwards its arguments to `tsc`), so the convenience alias can be one line of
`package.json` whenever someone wants it, and this pull request stays purely additive.
### B. Delete the exclusion and require green
*Against:* permanently red for as long as 801 errors take to fix. **A permanently-red gate is
_Against:_ permanently red for as long as 801 errors take to fix. **A permanently-red gate is
worse than no gate: it trains everyone to click through, and it takes the credibility of the
other checks with it.**
*Verdict:* no. Not as a first move.
_Verdict:_ no. Not as a first move.
### C. Gate only the test files changed in the pull request
*Against:* two problems. First, it saves nothing: `tsc` is a whole-program checker, so the
entire program is built regardless and restricting the *report* to changed files does not
reduce the cost. Second, it is blind in the direction that matters most — a change to *app*
code that breaks an *unchanged* test file passes, which is precisely the coupling a typecheck
_Against:_ two problems. First, it saves nothing: `tsc` is a whole-program checker, so the
entire program is built regardless and restricting the _report_ to changed files does not
reduce the cost. Second, it is blind in the direction that matters most — a change to _app_
code that breaks an _unchanged_ test file passes, which is precisely the coupling a typecheck
exists to catch.
*Verdict:* no.
_Verdict:_ no.
### D. Fix the debt outright
*For:* 141 files, and fifty of them carry 82%. This is a real option, not a fantasy — it is
_For:_ 141 files, and fifty of them carry 82%. This is a real option, not a fantasy — it is
days of work, not months, and the concentration means it can be attacked in ranked order with
visible progress.
*Against:* on its own it does not hold. The measured inflow (~230 errors in the last twelve
_Against:_ on its own it does not hold. The measured inflow (~230 errors in the last twelve
days) would restore the debt in roughly five weeks. Fixing without gating buys five weeks.
*Verdict:* do it — **second**, behind a gate, and use §3's ranked file list to sequence it.
_Verdict:_ do it — **second**, behind a gate, and use §3's ranked file list to sequence it.
### E. ✅ Recommended: a per-file ratchet, then burn the top 50 down
@@ -309,7 +316,7 @@ This is the shape this repository already runs for schema drift
detector reports what exists, the gate answers the narrower question of whether this change
made it worse. Reusing that shape means no new concept for reviewers to learn.
**Why a *per-file* count rather than a single total.** A repo-wide total lets a pull request
**Why a _per-file_ count rather than a single total.** A repo-wide total lets a pull request
that fixes ten errors in an old file introduce ten new ones in a new file and pass. Per-file
counts make the two independent, which is the whole point given §3's finding that the inflow —
not the stock — is the problem.
@@ -385,13 +392,13 @@ because the class is more instructive than the fix: **every one of them treated
diagnostics" as evidence of cleanliness.** All four were demonstrated live against this
repository, not against a stub:
| condition | old gate | new gate |
|---|---|---|
| `TYPECHECK_HEAP_MB=abc` (wrapper exits 2, prints no crash marker) | `PASS`, exit 0 | exit 3, names the exit status |
| wrapper exits 127 (binary missing) | `PASS`, exit 0 | exit 3 |
| wrapper exits 134 (V8 heap abort) | `PASS`, exit 0 | exit 3 |
| tsc `pretty` output — 801 real diagnostics | `0 error(s) across 0 file(s) … PASS`, exit 0 | `801 error(s) across 141 file(s)` |
| `--write-baseline` while the check cannot run | writes `0 error(s) across 0 file(s)`, exit 0 | exit 3, file untouched |
| condition | old gate | new gate |
| ----------------------------------------------------------------- | -------------------------------------------- | --------------------------------- |
| `TYPECHECK_HEAP_MB=abc` (wrapper exits 2, prints no crash marker) | `PASS`, exit 0 | exit 3, names the exit status |
| wrapper exits 127 (binary missing) | `PASS`, exit 0 | exit 3 |
| wrapper exits 134 (V8 heap abort) | `PASS`, exit 0 | exit 3 |
| tsc `pretty` output — 801 real diagnostics | `0 error(s) across 0 file(s) … PASS`, exit 0 | `801 error(s) across 141 file(s)` |
| `--write-baseline` while the check cannot run | writes `0 error(s) across 0 file(s)`, exit 0 | exit 3, file untouched |
The old "CRASH IS NOT CLEAN" control was **spelled, not structural**: it matched the literal
string `TYPECHECK CRASHED`, which `scripts/typecheck.mjs` emits from exactly one of its four
@@ -401,24 +408,24 @@ and a spawn error — print no such marker and sailed straight through.
Five controls now stand between a run and a verdict, and none depends on a downstream tool
remembering to print a particular string:
1. **Exit-status truth table.** The outcome is decided by *(exit status, signal, parsed
diagnostic count)*. A non-zero exit with zero parsed diagnostics is a **failure to run**,
1. **Exit-status truth table.** The outcome is decided by _(exit status, signal, parsed
diagnostic count)_. A non-zero exit with zero parsed diagnostics is a **failure to run**,
never a clean tree — one rule covering exits 2, 127, 134 and any future sibling.
2. **Parse control.** `pretty` is a legal `compilerOptions` key, inherited through `extends`,
and tsc also enables it under a TTY. It reshapes every diagnostic from
`path(l,c): error TS…` to `path:l:c - error TS…`. It is now forced off on the command line
(`--pretty false` beats the config), **both** formats are parsed anyway, ANSI is stripped
first, and the parse must **account for every line** carrying the `error TS` marker — not
merely parse *something*. That last clause is the fix for a cliff at exactly zero: the
merely parse _something_. That last clause is the fix for a cliff at exactly zero: the
control originally fired only when the parser understood **nothing**, so a run that
understood 5 of 801 marker lines was accepted and then reported "136 file(s) now clean".
Measured on this repository, `unparsed === 0` across 801 diagnostics and 650 continuation
lines; the divergent shapes that do exist in tsc are the *fileless* diagnostics (TS18003
lines; the divergent shapes that do exist in tsc are the _fileless_ diagnostics (TS18003
"No inputs were found in config file", TS6053 "File not found"), which are precisely the
outputs meaning the program was not built as intended.
3. **Plausibility.** A parsed total of `0` against a baseline of `N > 0` is refused, and so is a
**collapse** — a total under 25% of the baseline. The zero was only the extreme of that
spectrum, and the ratchet scores the whole spectrum as *progress*: every file the instrument
spectrum, and the ratchet scores the whole spectrum as _progress_: every file the instrument
fails to see is a file that looks `fixed`. "Everything got fixed" and "the instrument is
broken" produce the same number, and only one of them is common. On a verdict run a collapse
**shouts but does not block** (it cannot hide a regression, so blocking would obstruct a
@@ -432,7 +439,7 @@ remembering to print a particular string:
`max(fixed 400, 90% of the count stored in the baseline)` — 938 → 844 here. Both halves of
that `max` are load-bearing, and each fixes a different defect: a bare 400 let 57% of the
test tree vanish with the control still green (vanished files score as `fixed`, i.e. PASS),
while a bare *derived* floor was only ever as good as a number in a JSON file —
while a bare _derived_ floor was only ever as good as a number in a JSON file —
`testFilesInProgram: 1` produced a floor of **zero**, a control no program can fail, while
still logging "derived from the baseline". A corrupt, negative, non-integer or absurd
recorded count is **refused**, not silently downgraded to the fallback: an absent value and
@@ -459,7 +466,7 @@ Three properties now hold, and the first is the one that matters:
- **Scope.** It is honoured on `--write-baseline` **only**, so it cannot make a verdict run
green. This does not weaken its legitimate use: on the day the test tree is genuinely clean
the required action *is* to regenerate the baseline, after which the baseline total is 0 and
the required action _is_ to regenerate the baseline, after which the baseline total is 0 and
the control never fires again. A verdict run that trips the control is correctly telling you
to do that regeneration.
- **It is refused, not ignored, on a verdict run.** Silently ignoring a control-disabling
@@ -467,7 +474,7 @@ Three properties now hold, and the first is the one that matters:
believes it is on, and the log says nothing.
- **It requires a written reason**, not a truthy token. `=1` is refused; the value must be a
human-readable justification, and it is echoed into the output, so the log of a disarmed run
carries *why* it was disarmed. A control that can be switched off by typing `1` is one nobody
carries _why_ it was disarmed. A control that can be switched off by typing `1` is one nobody
has to justify switching off.
A disarmed regeneration prints a banner, and its success line says
@@ -495,7 +502,7 @@ environment could not run the check.
### Known behaviour: a pure `git mv` blocks
A renamed test file is a new path with no baseline entry (→ **BLOCK**) plus an old path that
disappeared (→ scored `fixed`). The gate cannot see file *content*, so it will not
disappeared (→ scored `fixed`). The gate cannot see file _content_, so it will not
auto-forgive "this looks like a rename" — that is a hole a genuine regression fits through.
What it does instead is **name the probable former path** in the block output, so the author is
told the remedy (`--write-baseline` in the same PR) rather than hunting a regression that is not
@@ -625,7 +632,7 @@ diagnostic formats; and every cell of the exit-status truth table.
function, and a call site in the gate that decides whether to act on it — and until the final
hardening round only the first was covered. Mutating `if (!drift.ok)` to `if (false)` in the
gate left the suite **78/78 green**: `diffExcludes` was thoroughly tested and the control was
genuinely live in production, but nothing asserted the gate *read* it. That is the same class
genuinely live in production, but nothing asserted the gate _read_ it. That is the same class
this document exists to close, found inside this document's own suite. The gate's call sites are
now driven end-to-end through a `TYPECHECK_TESTS_CONFIG_DIR` seam, and each case asserts the
**specific message** of the control under test rather than the exit code — several of these
@@ -640,55 +647,55 @@ summarised after it.
Each mutation was applied to the real source and confirmed to turn the suite **red for that
guard's own cases**, with the tree checksum-verified back to pristine afterwards:
| mutant | guard broken | cases failed |
|---|---|---|
| M1 | non-zero exit + 0 diagnostics must not read as clean | 9 |
| M2 | a measured zero against a non-empty baseline is refused | 3 |
| M3 | pretty-format diagnostics are still counted | 3 |
| M4 | the file-count floor tracks the baseline, not a magic 400 | 4 |
| M5 | `packages/*/src/**/__tests__` is not this gate's business | 4 |
| M6 | a rename is reported as a rename | 2 |
| M7 | the two tsconfig exclude lists differ by exactly one entry | 3 |
| M8 | a baseline measured against another config is rejected | 2 |
| M9 | a run killed by a signal is refused | 1 |
| M10 | a coloured pretty run is still counted | 1 |
| M11 | `--write-baseline` refuses an implausible measurement | 1 |
| M12 | the gate acts on the run classification | 4 |
| M13 | `--write-baseline` borrows the floor from the baseline it replaces | 1 |
| mutant | guard broken | cases failed |
| ------ | ------------------------------------------------------------------ | ------------ |
| M1 | non-zero exit + 0 diagnostics must not read as clean | 9 |
| M2 | a measured zero against a non-empty baseline is refused | 3 |
| M3 | pretty-format diagnostics are still counted | 3 |
| M4 | the file-count floor tracks the baseline, not a magic 400 | 4 |
| M5 | `packages/*/src/**/__tests__` is not this gate's business | 4 |
| M6 | a rename is reported as a rename | 2 |
| M7 | the two tsconfig exclude lists differ by exactly one entry | 3 |
| M8 | a baseline measured against another config is rejected | 2 |
| M9 | a run killed by a signal is refused | 1 |
| M10 | a coloured pretty run is still counted | 1 |
| M11 | `--write-baseline` refuses an implausible measurement | 1 |
| M12 | the gate acts on the run classification | 4 |
| M13 | `--write-baseline` borrows the floor from the baseline it replaces | 1 |
A guard that no mutation can turn red is not a guard, and a green suite says nothing on its own
about which of its assertions are load-bearing.
**The second battery (19 mutants, after the hardening round).** It covers the guards added in
that round *and* the gate's call sites, and it carries its own controls:
that round _and_ the gate's call sites, and it carries its own controls:
| mutant | guard broken | cases failed |
|---|---|---|
| `F1-unparsed-guard` | a partial parse is refused, not only a total one | 6 |
| `F1-collapse-guard` | a collapse against the baseline is refused | 5 |
| `F2-math-max` | the derived floor may raise the fixed one, never lower it | 4 |
| `F2-derived-strict` | "derived" is only claimed when the baseline actually raised the floor | 1 |
| `F2-noninteger-refusal` | a non-integer recorded count is refused | 4 |
| `F2-nonpositive-refusal` | a zero/negative recorded count is refused | 2 |
| `F2-absurd-ceiling` | an absurd count is refused rather than made permanently red | 2 |
| `F3-verdict-scope` | the escape hatch is honoured on `--write-baseline` only | 3 |
| `F3-bare-token` | the hatch requires a reason, not `=1` | 10 |
| `BL-total-crosscheck` | `totalErrors` must agree with the sum of the entries | 1 |
| `F5-drift-callsite` | **the gate acts on the drift control** | 2 |
| `F5-listed-callsite` | the gate acts on the positive control arm's own verdict | 1 |
| `F5-floorresult-callsite` | the gate acts on an unusable floor | 4 |
| `F5-allowance-callsite` | the gate acts on a refused escape hatch | 1 |
| `F5-provenance-echo` | provenance is echoed | 1 |
| `F5-collapse-shout` | a collapse is shouted on the verdict path | 1 |
| `CONTROL-breaking-compare` (must die) | the ratchet blocks at all | 8 |
| `CONTROL-equivalent-rename` (must **survive**) | — | 0 ✅ |
| `CONTROL-equivalent-comment` (must **survive**) | — | 0 ✅ |
| mutant | guard broken | cases failed |
| ----------------------------------------------- | --------------------------------------------------------------------- | ------------ |
| `F1-unparsed-guard` | a partial parse is refused, not only a total one | 6 |
| `F1-collapse-guard` | a collapse against the baseline is refused | 5 |
| `F2-math-max` | the derived floor may raise the fixed one, never lower it | 4 |
| `F2-derived-strict` | "derived" is only claimed when the baseline actually raised the floor | 1 |
| `F2-noninteger-refusal` | a non-integer recorded count is refused | 4 |
| `F2-nonpositive-refusal` | a zero/negative recorded count is refused | 2 |
| `F2-absurd-ceiling` | an absurd count is refused rather than made permanently red | 2 |
| `F3-verdict-scope` | the escape hatch is honoured on `--write-baseline` only | 3 |
| `F3-bare-token` | the hatch requires a reason, not `=1` | 10 |
| `BL-total-crosscheck` | `totalErrors` must agree with the sum of the entries | 1 |
| `F5-drift-callsite` | **the gate acts on the drift control** | 2 |
| `F5-listed-callsite` | the gate acts on the positive control arm's own verdict | 1 |
| `F5-floorresult-callsite` | the gate acts on an unusable floor | 4 |
| `F5-allowance-callsite` | the gate acts on a refused escape hatch | 1 |
| `F5-provenance-echo` | provenance is echoed | 1 |
| `F5-collapse-shout` | a collapse is shouted on the verdict path | 1 |
| `CONTROL-breaking-compare` (must die) | the ratchet blocks at all | 8 |
| `CONTROL-equivalent-rename` (must **survive**) | — | 0 ✅ |
| `CONTROL-equivalent-comment` (must **survive**) | — | 0 ✅ |
Two findings came out of running it, and both are worth recording because each is an instance of
something this document argues elsewhere:
- **`F5-floorresult-callsite` initially SURVIVED.** The case meant to cover it used
`testFilesInProgram: 1`, which after the `Math.max` fix is a *valid* input (floor 400) — so it
`testFilesInProgram: 1`, which after the `Math.max` fix is a _valid_ input (floor 400) — so it
exercised the floor comparison, not the "is this floor usable at all" refusal. With that call
site neutered, `floor` is `null`, `testFilesInProgram < null` is false, and the gate sails past
**both** checks into a verdict. A dedicated case with a genuinely corrupt count now kills it.
@@ -696,7 +703,7 @@ something this document argues elsewhere:
spot looks like from the inside.
- **`CONTROL-equivalent-rename` was initially reported KILLED**, which by the rule stated for
this kind of battery means the harness is broken. It was not: the "equivalent" mutation renamed
only the *declaration* of a local, leaving the interpolation `${pct}` dangling — a
only the _declaration_ of a local, leaving the interpolation `${pct}` dangling — a
`ReferenceError`, i.e. a genuinely breaking mutation mislabelled as equivalent. The harness was
right and the control was wrong. It is recorded here rather than quietly corrected, because
"the equivalent control died" is exactly the signal one is tempted to explain away.