Repository navigation
Run the test suite against TypeScript sources behind an uncheatable coverage gate - #52
Merged
Merged
Conversation
…overage gate Coverage was unmeasured here. The suite imported the compiled output from dist/, so the only numbers obtainable pointed at generated JavaScript offsets rather than lines an author edits, and no threshold was wired into any gate. Tests now import the TypeScript source directly (the package is a single index.ts at the repo root, so tests import ../index.ts). Node has stripped types without a flag since 22.18.0, which is the floor this package already declares in engines, so this needs no new tooling. Enabling rewriteRelativeImportExtensions lets a source import its siblings as .ts while tsc still emits .js into dist; this package has no relative source imports today, so the published artifact is byte-identical, but the flag is harmless to set and keeps the door open for future multi-file sources. The gate is scripts/coverage-gate.ts. Beyond enforcing per-dimension thresholds it reconciles the reported file list against the configured source list, and that second check is the reason it is a script rather than a bare flag: Node omits files that never load from its coverage report entirely instead of reporting them at zero, so a threshold alone is satisfiable by narrowing what the suite imports. A negative control confirms the check is not vacuous - adding an untested __probe.ts source makes the gate exit 1 and name it, and removing it returns exit 0. Thresholds are pinned at the measured baseline (70 line, 80 branch, 80 function) so the number can only ratchet up. release:check now runs the gate in place of a bare test run, and CI runs it on both Node 22 and 26. Baseline across the single source file index.ts: 70.53 line, 80.42 branch, 80.43 function. Tracked in pm-linear-v618.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
WalkthroughThe PR changes tests to import TypeScript sources directly, adds a configurable coverage gate with threshold and loaded-source checks, updates release and CI commands, and runs the test matrix on Node.js 22 and 26. ChangesTypeScript coverage enforcement
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reviewer's GuideAdds a TypeScript-aware coverage gate that runs Node’s native test coverage against source Flow diagram for the new TypeScript coverage gate executionflowchart LR
A[npm run coverage] --> B[coverage-gate.ts]
B --> C[Read coverageGate from package.json]
C --> D[Collect required .ts sources]
D --> E[Run node --test with --experimental-test-coverage<br>and thresholds for lines/branches/functions]
E --> F[Write coverage/lcov.info]
F --> G[Parse lcov and build reported file set]
G --> H{All required sources reported?}
H -- No --> I[Exit 1<br>and list missing files]
H -- Yes --> J[Exit with test status<br>and print success message]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.agents/pm/features/pm-linear-v618.toon:
- Line 23: Correct the duplicated baseline measurement date from July 29, 2026
to July 28, 2026 in .agents/pm/features/pm-linear-v618.toon:23-23 and keep the
corresponding historical patch value consistent in
.agents/pm/history/pm-linear-v618.jsonl:1-1.
In `@package.json`:
- Around line 35-40: Update the coverage script in package.json (lines 35-40) to
run npm run build:test before scripts/coverage-gate.ts, allowing release:check
and CI to exercise the test build through coverage. The referenced
tsconfig.test.json (lines 7-10) and .github/workflows/ci.yml (lines 43-44)
require no direct changes because they are corrected through the package.json
coverage flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c59969fd-9a94-4657-b901-fe04596eba21
📒 Files selected for processing (10)
.agents/pm/features/pm-linear-v618.toon.agents/pm/history/pm-linear-v618.jsonl.github/workflows/ci.yml.gitignorepackage.jsonscripts/coverage-gate.tstest/atomic.test.tstest/smoke.test.tstsconfig.jsontsconfig.test.json
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Greptile SummaryWires a real coverage gate over TypeScript sources and CI.
Confidence Score: 5/5The PR appears safe to merge; the prior typecheck gap is addressed and no blocking failure remains. coverage now runs build:test, tsconfig.test.json typechecks tests and scripts with a valid rootDir, and no remaining gate or typecheck hole is evident on the changed path.
|
| Filename | Overview |
|---|---|
| package.json | coverage script chains build, build:test, and the gate; release:check uses coverage instead of bare test. |
| tsconfig.test.json | noEmit typecheck for tests and scripts with rootDir at repo root so tsc accepts the tree. |
| scripts/coverage-gate.ts | Runs node --test with coverage, enforces thresholds, and fails on never-loaded sources. |
| .github/workflows/ci.yml | Matrix Node 22/26 and runs coverage instead of separate build/test steps. |
Reviews (8): Last reviewed commit: "Fail closed when the effective tsconfig ..." | Re-trigger Greptile
…urce walk Brings this package onto the final form of the gate after three defects were found and fixed on the reference implementation while this rollout was in flight. Pin TZ=UTC when spawning the runner. Code that branches on a timestamp's UTC offset takes different paths under a local offset than under UTC, so the reported percentage moved between a contributor's machine and CI and a threshold pinned to one failed on the other. Chain build:test into the coverage script. release:check no longer runs `npm test`, so tsconfig.test.json - the only config that typechecks the test tree and the gate script itself - had dropped out of every gate, and Node strips types without checking them. Adding it exposed that the config also inherited rootDir from the base config, which tsc rejects once the test build stops emitting; rootDir is now overridden to the repository root. Enumerate sources by walking a directory instead of listing files. A file list freezes the required set, so a module added later was never required and an untested new file slipped the gate entirely - the exact blind spot the presence check exists to close. The walk prunes non-source directories, and the coverage include flags are derived from the enumerated list so the measured set and the required set are the same set by construction. Thresholds re-measured against the corrected gate.
…e edges Three review findings raised on this rollout, all valid. Report the runner's exit status before the never-loaded diagnostic. A failing suite or an unmet threshold can legitimately leave sources out of the lcov output, and the gate then blamed a missing import for what was actually a test failure the author needed to see first. Normalise lcov `SF:` paths to repository-relative POSIX form. The reporter emits working-directory-relative paths on Linux, but that is not contractual and absolute paths have been seen on Windows runners; unnormalised, the presence check inverts into a permanently red build accusing every source file of never loading. Diagnose a non-existent `coverageGate.sources` entry instead of throwing a raw ENOENT stack out of statSync, which was the one failure mode in this script that did not produce a `coverage-gate:` message.
…s, ignore abuse
Delete the previous lcov report before the run. mkdirSync left an existing
coverage/lcov.info in place, so a run that produced no report at all was
checked against the previous run's file - the gate could pass by reading
history instead of by measuring anything.
Validate `coverageGate.ignore`. The exemption exists for type-only modules,
which erase to nothing and can never appear in a report, but it was trusted
blindly - making it the one supported way to remove an executable module from
both the measured set and the required set, the exact escape this gate exists
to prevent. Entries are now checked against the compiled output: TypeScript
emits `export {};` and nothing else for a module that erases completely.
Reject a non-TypeScript or declaration-file entry in `coverageGate.sources`
rather than requiring a file that can never be reported.
… at all The status check sat between the report parse and the presence check, so it covered the "sources missing from the report" diagnostic but not the "no report was written" one directly above it. A runner that exits non-zero without producing lcov - a test file that fails to load, an aborted run - still hit the missing-report branch first and exited with that message, so CI showed a coverage-report failure where the author needed to see a test failure. The check now runs immediately after the spawn, before the report is touched.
…only The type-only check normalised away line comments but not block comments, so a documented type-only module whose JSDoc survived emit would have been read as runtime code and rejected - a false failure on the exemption path. Applied as hardening rather than as a reproduced bug: with tsc's default emit settings the leading JSDoc on a fully-erased module is not carried into the output. Comment-preserving configurations exist, the normalisation is a single expression, and a false rejection on this path is confusing enough to pre-empt.
… as JSON The ignored-module check located a file's emitted output by reading outDir and rootDir straight out of tsconfig.json with JSON.parse. That is wrong twice over: tsconfig may be JSONC, where JSON.parse throws on a perfectly valid config, and either option may be inherited through an `extends` chain, where reading direct fields silently yields the wrong paths and the check then looks for compiled output that was never going to be there. It now asks `tsc --showConfig`, which returns the flattened configuration the compiler actually used, and warns and falls back to the conventional defaults if the compiler cannot be reached. The resolution runs only when `coverageGate.ignore` is non-empty, so no package pays for it unless it uses the exemption.
The fallback warned and assumed outDir dist / rootDir "." when `tsc --showConfig` could not be reached. That was the wrong default for this consumer: the resolved paths feed the check deciding whether an exempted module is genuinely type-only, so guessing the emit layout can clear an executable module by inspecting the wrong file - the one outcome this gate exists to make impossible. Leniency here is indistinguishable from a bypass. It now exits 1 and prints the compiler's stderr.
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.
Why
Coverage was unmeasured here. The suite imported the compiled output from
dist/, so the only obtainable numbers pointed at generated JavaScript offsets an author cannot act on, and no threshold was wired into any gate.What changed
../index.ts) instead of../dist/index.js, so the coverage report lands on editable source lines. Node strips types natively from 22.18.0, the floor this package already declares inengines— no new tooling.tsconfig.jsonenablesrewriteRelativeImportExtensions+allowImportingTsExtensions. This package has no relative source imports today, so the publisheddist/artifact is byte-identical; the flag keeps the door open for future multi-file sources.scripts/coverage-gate.ts(copied verbatim frompm-changelog) enforces per-dimension thresholds and reconciles the reported file list against the configured source list. That second check is why it is a script, not a bare flag: Node omits files that never load from its coverage report entirely rather than reporting them at zero, so a threshold alone is satisfiable by narrowing what the suite imports.release:checkruns the gate in place of a bare test run; CI runs it on a Node 22 + 26 matrix.Measured baseline (single source file
index.ts)Thresholds are pinned at the measured floor so the number can only ratchet up.
Negative control
A throwaway
__probe.tssource was added tocoverageGate.sources. No test imported it, so the gate exited 1 and named__probe.tsas never loaded. After removing the probe the gate exited 0 again. The presence check is not vacuous.pm item
Resolves pm-linear-v618 after merge.
Summary by Sourcery
Enforce a coverage gate over the TypeScript sources by running Node’s built-in test coverage against .ts test files and wiring the gate into local release checks and CI.
New Features:
Enhancements:
Build:
CI:
Chores:
Summary by cubic
Run tests against the TypeScript source and enforce a coverage gate that also fails when untested files don’t appear in the report. Uses a dynamic source walk, pins TZ=UTC, and runs in CI on Node 22 and 26; satisfies Linear pm-linear-v618.
New Features
../index.tsso coverage maps to authored lines; Node 22+ executes.ts.coverageGate.sources; only required files are included.npm run coveragebuilds, typechecks tests and the gate, then runs the gate; CI runs this job;coverage/is gitignored;tsconfigenablesrewriteRelativeImportExtensionsandallowImportingTsExtensions; tests arenoEmitwith repo-rootrootDir.Bug Fixes
lcovSF:paths to repo-relative POSIX to avoid false “never loaded” on Windows.coverageGate.sourcesentry does not exist.coverage/lcov.infobefore the run to prevent passing on stale data.coverageGate.ignoreentries are type-only against emitted JS (stripping block comments); resolveoutDir/rootDirviatsc --showConfigand fail closed if it cannot be resolved; reject non-TS or.d.tsentries incoverageGate.sources.Written for commit 7c3ba6c. Summary will update on new commits.