Skip to content

fix(devx): type-check-coverage sees include-shaped hidden test layers (#7353) - #7921

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-7353-type-check-coverage-include-shape
Aug 12, 2026
Merged

fix(devx): type-check-coverage sees include-shaped hidden test layers (#7353)#7921
os-zhuang merged 1 commit into
mainfrom
claude/issue-7353-type-check-coverage-include-shape

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#7353

scripts/check-type-check-coverage.mjs only. No package's tsconfig*.json is touched — this changes what the gate can see, not what the repo declares.

The defect

TESTS_COVERED asked one per-config question — does some exclude name the tests:

excludesTests: (parsed.exclude??[]).some((pattern)=>TEST_GLOB.test(pattern)),

So it detected a config that steered tsc away from the test layer and never a config that had simply never steered tsc toward it. roots was computed from include a few lines above and fed configCovers for the PINS_CHECKED walk, but never fed this.

The dispatch's second reading is confirmed: the package-level aggregate anyExcludesTests && testsInvoked.length === 0 makes the hole strictly bigger. A package with no exclude at all has anyExcludesTests === false, so it is judged covered at the aggregate level too, regardless of include. The include-shaped package was invisible at both levels.

Also confirmed: the countRoots comment hands the sibling-test/ case to PINS_CHECKED, and PINS_CHECKED only collects files carrying @ts-expect-error. All 65 newly-visible files carry zero pin directives, so the hand-off caught none of them — reported by neither half, exactly as the card says.

Census — all 78 workspace packages (root included), at b9f930b

The census the card asks for, and the reason the ratchet move is small. The key question is not "does this package have a typecheck script" but which tsc program accounts for its test files:

bucketpackagesfiles
test layer already counted (exclude-shaped)19719
newly visible (include-shaped)365
tests read by an accounted program56

My first census pass over-counted this at 16 packages / 422 files by treating "no typecheck script" as "nothing reads the tests". That was wrong: for those packages measureDebt runs tsc -p tsconfig.json, and every one of them already has its tests under that config's include (e.g. metadata-protocol's 72 test files live in src/, which its include: ["src/**/*"] reaches). Their tests are counted in DEBT, not hidden. Corrected count: 3 packages, 65 files, 198 raw errors. No staging is needed and there is no separate follow-up card to file.

Which packages flip status — the full list

packagefiles hiddenraw errorsshape
@objectstack/cli56188include: ["src"], no exclude at all, 56 tests in a sibling test/ tree
@objectstack/metadata-fs66include: ["src/**/*"], no exclude naming tests, all 6 tests in test/
@objectstack/example-showcase34include names src/test, the e2e/ Playwright tree beside it is unread

Nothing else changes status. All 19 existing TEST_DEBT packages keep the same verdict and the same file count — for each of them the old include-roots count and the new unread-file count were already identical (719 both ways), so their numbers move by zero.

packages/cli is the headline: 188 raw tsc errors in a test layer that no gate, no ledger and no CI job had ever read. 159 of the 188 are TS2835 plus the TS7006 cascade it causes — the NodeNext pair from the top-of-ledger note, one repair rather than 159.

The fix

Decide it per file, against the programs that account for the package:

constaccounted=invoked.length>0 ? invoked : (primary ? [primary] : []);consthiddenTests=unreadTests(testRels,accounted);

The second clause is load-bearing. A package with no typecheck script has no invoked program, but it is not unmeasured — measureDebt runs tsc -p tsconfig.json, so a test file that config reads is counted in DEBT rather than hidden. Without it, 353 files would move into the ledger that means "src checks, tests are hidden" purely because DEBT already owns them.

Per file also makes partial coverage sayable, which the per-config form could not express: cli keeps 54 of its 110 test files under include and 56 outside it, and the honest number is 56 — not 0, and not 110.

measureTestDebt learns the same lesson: dropping exclusions is only half of un-hiding, because only half of hiding is an exclusion. It now adds the unread files to includeone at a timee2e/**/* would have pulled e2e/global-setup.ts into app-showcase's number and billed the test layer 6 errors from a file that is not a test — and neutralises rootDir, which is src in most of these packages: widening past it makes tsc answer with one TS6059 per added file and nothing else. Measured both ways on cli: 56 TS6059 with rootDir kept, 188 real errors with it neutralised. Under --noEmit there is no output layout for rootDir to protect, so a TS6059 there measures the tape measure. For the 19 existing entries the generated config is byte-identical to before.

Before / after gate output

before: test layer: 19 package(s) still exclude their own tests
(719 files hidden as counted by this run, 1607 frozen raw errors in TEST_DEBT).
after: test layer: 22 package(s) still hide their own tests from tsc
(784 files hidden as counted by this run, 1805 frozen raw errors in TEST_DEBT).

Intermediate state, before the three ledger entries were added — the gate correctly refusing the newly-seen packages:

check-type-check-coverage: 3 problem(s)
• @objectstack/example-showcase (examples/app-showcase): 3 of its test file(s) sit outside
every tsc program that accounts for this package ...
• @objectstack/cli (packages/cli): 56 of its test file(s) ...
• @objectstack/metadata-fs (packages/metadata-fs): 6 of its test file(s) ...

Reverse verification

A green run proves little here, so the load-bearing evidence is a planted regression. Planted a TS2322 in packages/metadata-fs/test/contract.test.ts — a package #7312 never touched — and ran both detectors against the same tree:

stepresult
pnpm --filter @objectstack/metadata-fs exec tsc --noEmitGREEN, exit 0 — the package's own advertised typecheck never reads the file
old detector (origin/main, run in-tree)OK19 package(s) ... 719 files, metadata-fs not mentioned
new detector, structural passOK — 22 package(s) ... 784 files (structural pass runs no compiler, as designed)
new detector, --re-measureRED, exit 1@objectstack/metadata-fs: TEST_DEBT records 6 raw tsc error(s), tsc --noEmit now reports 7 (+1)

That is the card's mechanism reproduced on a fresh package and then closed: a type error that used to leave bothtsc --noEmit and this gate green now has exactly one gate that goes red on it.

The three new entries are recorded exactly, with no bootstrap margin, so the first new error in any of these layers goes red immediately rather than being absorbed.

Relationship to #7888 (same file)

Read before writing, per the dispatch. My --re-measure at b9f930bconfirms#7888's reading: 9 entries, 273 raw errors of surplus — identical total, identical entry set. My change moves none of them; the three new entries carry zero surplus, so the figure is 273 before and after.

One correction to #7888's table: @objectstack/plugin-auth measures 107, not 106 (131 → 107 is the −24 its own surplus column and its 273 total already assume — the measured cell is the typo, the arithmetic is right). Nothing here fixes#7888; --lower remains its own deliberate PR.

Gates run locally

gateresult
node scripts/check-type-check-coverage.mjs --self-test✓ 23 semantic + 24 observation (was 16) + 15 re-measure + 12 built-closure + 9 auto-lowering
pnpm check:type-check-coverage✓ OK
pnpm check:type-check-debt (full --re-measure, 36 entries, 255s)✓ OK — 1987 raw errors, none above its recorded number
node scripts/check-nul-bytes.mjs✓ OK (7237 files)
pnpm exec eslint scripts/check-type-check-coverage.mjs --no-inline-config✓ clean
build closureturbo run build --filter='./packages/*' --filter='./packages/*/*' — 70/70

Self-test grew by 8 observation cases: 5 pinning unreadTests (the include-shaped form with no exclude anywhere; partial coverage; the #5286 sibling-config repair; the empty-program case) and 3 pinning accountedPrograms (including the DEBT-territory clause, so the 353-file over-reach cannot be reintroduced silently).

Changeset

None — scripts/-only gate change, no published package, nothing reader-visible ships. Route is the skip-changeset label at review. Deliberately not adding an empty-frontmatter changeset (#5471 / #4898).


Generated by Claude Code

…#7353)
TESTS_COVERED asked one per-config question -- does some `exclude` name the
tests -- so it detected a config that steered tsc AWAY from the test layer and
never a config that had simply never steered tsc TOWARD it. A package with no
`exclude` at all and `include: ["src"]` beside a sibling `test/` tree reported
as fully test-covered while nothing compiled a line of it. #7312 moved two
example apps from hiding their tests to compiling them and this gate's headline
did not move by one file, because neither app had ever counted toward it.
Decide it per FILE instead, against the tsc programs that ACCOUNT for the
package: the configs the `typecheck` script invokes, or -- where there is no
such script -- `tsconfig.json`, which is what `measureDebt` runs, so a test file
that config reads is counted in DEBT rather than hidden. Without that second
clause every DEBT package's whole test tree would land in TEST_DEBT for the sole
reason that DEBT already owns it.
Per file also makes partial coverage sayable: `packages/cli` keeps 54 of its 110
test files under `include` and 56 outside it, and 56 is the honest number.
`measureTestDebt` learns the same lesson -- dropping exclusions is only half of
un-hiding, because only half of hiding is an exclusion. It now adds the unread
files to `include` one at a time (a directory glob would have pulled
`e2e/global-setup.ts` into app-showcase's number) and neutralises `rootDir`,
which is `src` in most of these packages and otherwise answers with one TS6059
per added file: cli measured 56 TS6059 that way and 188 real errors without it.
Census over all 78 workspace packages: 3 flip, 65 files, 198 raw errors.
@objectstack/cli 56 files, 188 errors (include-shaped)
@objectstack/metadata-fs 6 files, 6 errors (include-shaped)
@objectstack/example-showcase 3 files, 4 errors (e2e/ beside include)
test layer: 19 -> 22 packages, 719 -> 784 files, 1607 -> 1805 frozen errors
Not new debt: all three were in this state before the ledger existed, and only
the question changed. The 9 pre-existing ledger surpluses (#7888) are untouched.
Fixes#7353
@vercel

vercelBot commented Aug 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 12, 2026 4:29am

Request Review

@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

Check Changeset is red and I am deliberately not fixing it in the diff — it needs the skip-changeset label, which is the PM's to apply.

The gate's own message names the resolution:

This PR adds no changeset. If it releases nothing (including any skills/** change — see #5947), apply the skip-changeset label; otherwise run pnpm changeset and name the packages. An empty-frontmatter changeset is NOT a third option any more: the step below rejects newly added ones (#5471) […] an all-empty set stalls the release silently and greenly (#4898).

This PR releases nothing. The entire diff is scripts/check-type-check-coverage.mjs — a CI gate's detector, its inline --self-test cases, and the TEST_DEBT ledger constants. It is not a workspace member, no package's files field includes it, and nothing reader-visible ships. So route 1 (pnpm changeset) has no package to name, and route 3 (empty frontmatter) is closed and was never worth taking. The label is the only correct answer.

Both other options were considered and rejected on the record in the PR body's Changeset section before this run went red; my dispatch also names the label as the route explicitly.

Rest of CI at 872c8ed: everything else green or skipped, with ESLint and TypeScript Type Check still running — the latter is the one that matters here, since it runs pnpm check:type-check-coverage and pnpm check:type-check-debt. Both passed locally against a fully built closure (--re-measure: 36 entries, 255s, none above its recorded number), so I expect them green; if either comes back red I will drive it.


Generated by Claude Code

@os-zhuangos-zhuang added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 12, 2026 — with Claude
@os-zhuang
os-zhuang marked this pull request as ready for review August 12, 2026 04:50
@os-zhuang
os-zhuang added this pull request to the merge queueAug 12, 2026
Merged via the queue into main with commit 59ac0c6Aug 12, 2026
23 of 24 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-7353-type-check-coverage-include-shape branch August 12, 2026 05:00
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/mskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants

@os-zhuang@claude