fix(frontend): make the typecheck and coverage gates actually validate - #576
Open
umzcio wants to merge 3 commits into
Open
fix(frontend): make the typecheck and coverage gates actually validate#576umzcio wants to merge 3 commits into
umzcio wants to merge 3 commits into
Conversation
`tsc --noEmit` is a no-op on this solution-style tsconfig (`files: []` + project references) — `npx tsc --noEmit --listFiles` emitted 0 files, so the typecheck gate in `frontend-ci` was vacuous. Switch to `tsc -b`, which builds both referenced configs (`tsconfig.app.json` and `tsconfig.node.json`, both already `noEmit: true`) and now checks 1440 files. Verified: introduced a deliberate `string` to `number` assignment in main.tsx and confirmed `npm run typecheck` failed with `TS2322: Type 'string' is not assignable to type 'number'.`, then reverted and confirmed it passes again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
v8 only instrumented files a test happened to import (no coverage.include),
so the 40% gate measured an import-driven denominator (45.92% lines,
1404/3057) rather than the whole src/ tree (7.87%, 1404/17839), and was
non-monotonic: importing a large untested file into its first test could
lower the reported number below the gate. Set coverage.include to
'src/**/*.{ts,tsx}' with excludes for test files, main.tsx, and .d.ts files,
so the denominator is fixed and adding a test can only raise the number.
Measured with the plan 009 test tranche in place: statements 8.1%
(1673/20643), branches 6.97% (1347/19312), functions 6.37% (401/6289),
lines 8.37% (1493/17837). Thresholds set just below each, rounded down:
statements 8, branches 6, functions 6, lines 8 — single-sourced in
vitest.config.ts.
Remove the duplicated --coverage.thresholds.lines=40 flag from Makefile's
frontend-test target and frontend/package.json's ci script, and fix the
stale coverage figures in CLAUDE.md and the Makefile comment.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Jul 25, 2026
…eline The thresholds set alongside the whole-src-tree coverage denominator (statements/lines 8%, branches/functions 6%) were measured wrong — actual whole-src coverage is ~6.4% lines / 6.1% statements / 5.1% functions / 5.2% branches, so CI failed on every push regardless of changes. Lower the thresholds to sit just below the real numbers (matching the stated intent: 'set just below the honest whole-src measurement'), and correct CLAUDE.md's stale ~8% note to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two CI gates in
frontend/currently pass without validating anything. This makesboth real.
npm run typecheckchecks zero files. It is defined astsc --noEmit, butfrontend/tsconfig.jsonis solution-style ("files": []plusreferences), andtsc --noEmitwithout-bdoes nothing on such a config. Verified withnpx tsc --noEmit --listFiles, which emits 0 files. Somake frontend-typecheckand the typecheck step of
frontend-cihave been vacuous. Type errors were only evercaught incidentally by
npm run build, which does runtsc -b. Changing the scriptto
tsc -bmakes it check 1,444 files.The coverage gate measures about a fifth of the codebase. v8 only instruments
files that a test imports, and
vitest.config.tssets nocoverage.include, so thedenominator is "whatever the tests happened to pull in". Measured before this change:
45.9% lines (1,404/3,057) against a real tree of 17,839 lines — an honest 7.9%.
Worse, the gate is non-monotonic: adding the first test to a large untested file
lowers the reported percentage by widening the denominator. Adding one test that
imports
ConfigTab(~2,400 lines) would have dropped coverage to roughly 31% andturned CI red. The gate actively penalised writing the first test for any big module.
Setting a fixed whole-
srcdenominator makes coverage monotonic — adding a test cannow only ever raise it — and comparable over time. The headline number drops to
single digits because that is the truth about a ~95k-line frontend.
Changes
frontend/package.json:typecheckscripttsc --noEmit→tsc -b.frontend/vitest.config.ts: addcoverage.include: ['src/**/*.{ts,tsx}'], anexcludelist (test files,src/main.tsx,*.d.ts), andcoverage.thresholdsre-baselined just under the honest measurement.
vitest.config.ts. Removed the duplicated--coverage.thresholds.lines=40flag from thefrontend-testMakefile target andfrom
package.json'sciscript.Makefile: replace the stale "set just below current measured (~44%)" comment withone explaining the whole-
srcdenominator and the monotonicity property.CLAUDE.md: the documented figure said 35%, the Makefile enforced 40%, and neitherdescribed what was being measured. Corrected.
Test Plan
make ci)make release-check) — n/a, no packaging changeCHANGELOG.mdfor user-facing or operator-facing changes — not done; this is operator-facing (CI behaviour), please advise on preferred wordingCLAUDE.mdcorrected; no install/release path changeVerification performed:
npx tsc --noEmit --listFiles | wc -l→ 0 before,npx tsc -b --listFiles | wc -l→ 1444 after.const x: number = 'not a number'into asrcfile and confirmed
npm run typecheckfails withTS2322: Type 'string' is not assignable to type 'number'; reverted and confirmedit passes again. Exit 0 alone was not accepted as evidence, since that is precisely
the signal that hid the problem.
make frontend-ciexits 0.grep -rn "thresholds.lines" Makefile frontend/package.jsonreturns nothing.Note for reviewers:
tsc -bsurfaced no pre-existing type-error backlog, becausenpm run buildhad been running it all along. The broken script was buying falseconfidence, not hiding real damage.
Related Issues
Closes #