mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
Zachary Lowden
69b5ce5497
* fix(typecheck): close the scripts/ blind spot — 31 files now checked, 5 quarantined behind a ratchet The root tsconfig named `scripts/local-dev/*.ts` in BOTH `include` and `exclude`. Exclude wins, so nothing under `scripts/` was typechecked at all — not even the one directory the include list appeared to name. Measured on23cecb57c0: `tsc -p tsconfig.json --listFilesOnly` contained 0 files under `scripts/`, and a deliberate `const x: number = 'nope'` planted in `scripts/oneoffs/parse_header.ts` produced `OK — 0 type errors`, exit 0. Both CI tiers run that config, so the gap could not close on its own. Adding `scripts/**/*.ts(x)` puts 36 files in the program and surfaces 198 pre-existing errors, all in 5 files. The other 31 are clean and are checked from now on by the real `pnpm typecheck` in both tiers, as is every file added under `scripts/` later. The 5 are excluded BY NAME — enumerable, and shrinkable in a reviewable diff — and are NOT left unchecked: `tsconfig.scripts.json` is the root config without those entries, and `scripts/ci/typecheck-scripts-gate.mjs` ratchets their per-file counts. Excluding them and walking away would recreate the very defect being fixed. 164 of the 198 are one file with one cause: it imports an untyped `.mjs` with no JSDoc, so under `allowJs` tsc infers `request({ worktree, args = [] } = {})` as `{ args?: never[] }` (40x TS2353) and the run object's `null` initializers as type `null` (123x TS18047/TS2531). Not 164 defects; typing that module is the fix and is left to a follow-up. The gate mirrors `typecheck-tests-gate.mjs` and IMPORTS its five controls rather than copying them (`classifyEmptyAllowance` gains an `envName`/ `gateScript` parameter, defaulted, so both gates report their own). What is genuinely different lives in `typecheck-scripts-compare.mjs`: the positive control anchors at the repo root instead of matching the substring `/scripts/`, because this repo pulls `.claude/skills/**/scripts/*.mjs` into the program transitively and every vendored package has a `scripts/` directory — a substring control could stay green with the whole measured tree gone. `tsconfig.tests.json` is updated in step: the sibling gate's config-drift control caught the base-exclude change immediately and refused to run, which is the control working. Verification - positive control: planted error in scripts/oneoffs/ -> 1 error, exit 2 under the new config; the SAME error under main's config -> "OK — 0 type errors", exit 0. - ratchet proven both ways against the real tsc: worsening gen_seed 2->3 BLOCKS (exit 1, names the delta); clearing it PASSES (exit 0, "1 file(s) now clean"). - 42 new tests; 10 mutants (one per control) each killed by their specifically named test, with an unmutated green control on both ends of the battery. - root `pnpm typecheck`: 0 errors. Both gate suites: 189 passed. Known-unrelated: the sibling tests gate is already BLOCKED on main (869/160 vs its 784/140 baseline) — verified identical at the base ref, not caused here, and it is not wired into CI. * fix(typecheck): exclude scripts/__tests__ from the root program; gate becomes a report, not a CI block #4181 landed scripts/__tests__/dev-server-daemon-port.test.ts and turned both the root typecheck and this PR's gate red. Reproduced locally on the merged head: 4 errors, all in that one new file, all TS2345 "not assignable to parameter of type 'ProcessEnv'". The gate was working as designed. The design was wrong. Root cause, read at the source rather than inferred: scripts/__tests__ exists to test the untyped `.mjs` modules under .claude/skills and .claude/hooks by importing them. None carries JSDoc, so under `allowJs` tsc infers their signatures from the implementation and every caller inherits the result. `daemonPortsGuarded(env = process.env)` infers as requiring a full ProcessEnv, so `{ DEV_DAEMON_PORT: '9555' }` is an error. It is the same mechanism as the 164 errors already recorded for dev-server-test-queue.test.ts. That makes the failure systemic, not a backlog: 5 of the 15 files there carry this class, and ALL 15 were added within one month. Quarantining files one-by-one would have fired several times a month, always for a defect in a dependency the test author did not write, and the fastest remedy would have been to add another exclude line — training the reflex the quarantine existed to prevent. So scripts/__tests__/** is now excluded as a DIRECTORY, exactly as src/**/__tests__/** already is, and the gate is no longer wired into CI. Its CI job is removed; .github/workflows/lint.yml is byte-identical to main again. What this costs, stated plainly: scripts/__tests__/** is UNCHECKED. That is the pre-existing state, not a regression, and it is measured rather than hidden — tsconfig.scripts.json re-includes it and the gate reports per-file counts on demand (202 across 6 files). What it still buys, measured by --listFilesOnly rather than asserted: 15 non-test files under scripts/ (oneoffs 6, local-dev 3, test-perf 2, root 2, metric-migration 2) go from unchecked to checked by the real `pnpm typecheck` in BOTH tiers, along with any file added to those directories later. The earlier claim of "31" was wrong — it counted scripts/__tests__ files and transitively-imported .mjs. gen_seed.ts stays excluded by name: its 2 errors are a genuinely broken import (`~/server/db/notifDb`, deleted in927e31bfee). notifDbWrite now lives in apps/notifications behind a different API, so restoring it is a cross-workspace migration, not a one-line fix. Verification - root typecheck on the merged head: 0 errors. - positive control: planted error in scripts/oneoffs/ -> caught (1 error, exit 2), so the remaining coverage is real and not a vacuous glob. - negative control: a NEW deliberately-broken test file dropped into scripts/__tests__/ -> 0 root errors. The tripwire is gone. The gate still reports that same file, so it is excluded, not invisible. - ratchet both ways against real tsc: gen_seed 2->3 BLOCKS (exit 1, names the delta); untouched tree PASSES (exit 0). - new invariant test, watched fail: a baseline entry for a root-COVERED file is rejected by name, so this baseline cannot be used to park a failure that `pnpm typecheck` is actually red on. Plus a negative control proving its predicate can reject. - drift-control mutant re-run against the new glob-shaped quarantine: killed by 3 tests including the end-to-end one. - 190 tests pass across both gate suites; prettier and eslint clean.