Repository navigation
Run the test suite against TypeScript sources behind an uncheatable coverage gate - #22
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 sources directly. 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 file import its siblings as ./x.ts while tsc still emits ./x.js into dist, leaving the published artifact's module graph unchanged. The gate is scripts/coverage-gate.ts. Beyond enforcing per-dimension thresholds it reconciles the reported file list against a walk of the source tree, 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 source file makes the gate exit 1 and name it. Thresholds are pinned at the measured baseline (76 line, 78 branch, 75 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 two source files: 76.14 line, 78.93 branch, 75.71 function. The weakest is index.ts at 73.08/78.30/73.03; projects.ts is at 97.00/81.41/100.00. Both are the targets of the follow-up work to reach the mandated 100. Tracked in pm-github-uv1a.
|
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}
WalkthroughChangesThe project now runs tests against TypeScript sources, enforces configured line, branch, and function coverage thresholds, verifies required sources appear in LCOV output, and executes the coverage gate during release checks and CI on Node 22 and 26. Coverage gate
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CI
participant CoverageGate
participant NodeTestRunner
participant LCOV
CI->>CoverageGate: Run npm run coverage
CoverageGate->>NodeTestRunner: Execute TypeScript tests with coverage thresholds
NodeTestRunner->>LCOV: Write coverage/lcov.info
CoverageGate->>LCOV: Compare reported files with required sources
CoverageGate-->>CI: Return success or failure
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 GuideRuns the Node test suite directly against TypeScript sources and introduces a scripted coverage gate that enforces ratcheting coverage thresholds and verifies that all configured source files appear in the coverage report, wiring it into local scripts and CI on a Node 22/26 matrix without changing the published JS artifact graph. Flow diagram for the coverage-gate TypeScript scriptflowchart TD
A[Start coverage-gate.ts] --> B[Read package.json]
B --> C{coverageGate block present?}
C -- No --> Z1[Print error and exit 1]
C -- Yes --> D[Collect source files from coverageGate.sources]
D --> E{Any source files found?}
E -- No --> Z2[Print error and exit 1]
E -- Yes --> F[Build required list excluding coverageGate.ignore]
F --> G[Ensure coverage directory and lcov.info path]
G --> H[spawnSync node --test with coverage flags and config.tests]
H --> I{spawnSync error?}
I -- Yes --> Z3[Print error and exit 1]
I -- No --> J[Read lcov.info and collect reported files]
J --> K{lcov.info readable?}
K -- No --> Z4[Print error and exit 1]
K -- Yes --> L[Compute missing = required - reported]
L --> M{missing files?}
M -- Yes --> Z5[Print missing files and exit 1]
M -- No --> N{Test exit status === 0?}
N -- No --> Z6[Exit with test status]
N -- Yes --> O[Print success message and exit 0]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Greptile SummaryRuns the package test suite against TypeScript sources and enforces coverage via a custom gate in CI and release checks.
Confidence Score: 5/5The PR appears safe to merge; no blocking failures remain from this follow-up pass. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| scripts/coverage-gate.ts | Coverage gate runs Node test coverage, enforces thresholds, reconciles loaded sources, and verifies ignore entries are type-only. |
| package.json | Adds coverage script and coverageGate config; release:check and test scripts point at TS tests and the gate. |
| .github/workflows/ci.yml | CI test job matrices Node 22/26 and runs the coverage gate. |
| tsconfig.json | Enables allowImportingTsExtensions and rewriteRelativeImportExtensions for .ts imports with unchanged emit. |
| tsconfig.test.json | Switches test project to noEmit and includes test/scripts sources for typecheck without dist-test emit. |
Reviews (9): Last reviewed commit: "Fail closed when the effective tsconfig ..." | Re-trigger Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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-github-uv1a.toon:
- Line 20: Correct the baseline measurement date from July 29, 2026 to July 28,
2026 in the feature entry at .agents/pm/features/pm-github-uv1a.toon:20-20 and
the matching body payload at .agents/pm/history/pm-github-uv1a.jsonl:2-2; keep
all coverage metrics and other event data unchanged.
🪄 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: c0068dcd-1d62-4353-a489-3732536fd943
📒 Files selected for processing (17)
.agents/pm/features/pm-github-uv1a.toon.agents/pm/history/pm-github-uv1a.jsonl.github/workflows/ci.yml.gitignoreindex.tspackage.jsonscripts/coverage-gate.tstest/atomic.test.tstest/comments-sync.test.tstest/dryrun-preview.test.tstest/helpers/comment-sync-child.tstest/import-lock.test.tstest/link-deps.test.tstest/projects.test.tstest/smoke.test.tstsconfig.jsontsconfig.test.json
…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.
CI measured 78.93% branch where this host measured marginally higher, so the threshold rejected its own baseline. CI is the authoritative gate, so the ratchet is set from its floor.
…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.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@scripts/coverage-gate.ts`:
- Around line 98-100: Update the configuration loading used by the coverage
mapping logic around tsConfig, tsOutDir, and tsRootDir to resolve the effective
TypeScript compiler options, including the tsconfig.test.json extends chain,
rather than parsing tsconfig.json directly with JSON.parse. Use TypeScript’s
config parser or an equivalent resolution mechanism so emitted-file paths,
especially scripts/**/*.ts, are mapped against the inherited outDir and rootDir.
🪄 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: 52b75309-3e80-4cb0-b53a-59e1192e57c7
📒 Files selected for processing (1)
scripts/coverage-gate.ts
… 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
1 participant
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 in this package was never measured. Tests imported the compiled output from
dist/, so the only numbers obtainable pointed at generated JavaScript offsets rather than lines an author can act on, and no threshold was wired into any gate — coverage could regress silently on every merge.What changed
Tests run against the TypeScript sources. 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 and no build step for the test run. Tests import source modules directly (../index.ts,../projects.ts), which puts the coverage report on real source lines.rewriteRelativeImportExtensionskeeps the published artifact unchanged. Source files now import siblings as./x.ts;tscstill emits./x.jsintodist/, so the shipped module graph is identical.scripts/coverage-gate.tsis the gate. It enforces per-dimension thresholds and reconciles the reported file list against a walk of the source tree. That second check is why this is a script rather than a bare--test-coverage-linesflag:Negative control, run before opening this PR: adding an untested file and listing it in
coverageGate.sourcesmakes the gate exit 1 and name the file. The check is not vacuous.Thresholds ratchet. Pinned in
package.jsonundercoverageGateat the measured baseline so the number can only move up, and visible in review as a deliberate diff whenever it changes.release:checkruns the gate in place of a bare test run, and CI runs it on a Node 22 + 26 matrix.Measured baseline
Both source files, reported against TypeScript lines:
index.tsis the target of the follow-up work to reach the mandated 100.Verification
npm run check— passesnpm run coverage— 172 tests, 0 failures, both source files present in the reportnpm run release:check— passes end to end (typecheck, build, coverage gate, prod audit, pack dry-run, changelog check)The Node 22 matrix leg is unobserved locally (this host runs 26.5.0) and is confirmed by this PR's CI.
pm items
Summary by Sourcery
Enforce a source-level coverage gate by running the Node test suite directly against TypeScript entrypoints and wiring coverage thresholds into local and CI workflows.
New Features:
Enhancements:
Build:
CI:
Tests:
Chores:
Summary by cubic
Run tests directly against TypeScript sources behind a strict coverage gate that enforces thresholds and checks file presence. CI runs the gate on Node 22 and 26; the published JS output is unchanged.
--test-coverage-include; rejects missing, non‑TS, or.d.tsentries; normalizes lcovSF:paths to repo‑relative POSIX.coverage/lcov.infobefore each run; exits with runner status before reading the report; pinsTZ=UTCfor stable coverage.coverageGate.ignoreby verifying emitted JS is type‑only (strips block and line comments); resolves effective tsconfig viatsc --showConfigand fails closed if it cannot be resolved.build:testintonpm run coverage;tsconfig.test.jsonset to no‑emit with reporootDir; thresholds inpackage.jsonset to lines 76, branches 78, functions 75;npm testrunsnode --testontest/*.test.ts.Written for commit 293c9e7. Summary will update on new commits.