Run the test suite against TypeScript sources behind an uncheatable coverage gate - #44
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. Node has stripped types without a flag since 22.18.0, the floor this package already declares in engines, so this needs no new tooling. 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, because Node omits files that never load from its coverage report entirely instead of reporting them at zero - a threshold alone is therefore satisfiable by narrowing what the suite imports. The walk is dynamic, so a module added later is required automatically. It also pins TZ=UTC so the measurement is machine-independent, chains build:test so the test tree and the gate script stay typechecked now that release:check no longer runs npm test, deletes any previous lcov report so a run producing none cannot pass on stale data, and rejects an `ignore` entry whose compiled output contains runtime code. Thresholds are pinned at the measured baseline (63 line, 76 branch, 77 function) so the number can only ratchet up. CI runs the gate on Node 22 and 26. Tracked in pm-graph-5cu7.
|
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}
WalkthroughTests now execute TypeScript sources directly through a new coverage gate. The gate validates required modules, enforces thresholds, and integrates with release checks and CI across Node.js 22 and 26. ChangesTypeScript coverage enforcement
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CI
participant CoverageGate
participant NodeTest
participant Lcov
CI->>CoverageGate: Run npm run coverage
CoverageGate->>CoverageGate: Enumerate required TypeScript sources
CoverageGate->>NodeTest: Run tests with thresholds and TZ=UTC
NodeTest->>Lcov: Write coverage/lcov.info
CoverageGate->>Lcov: Parse reported source files
CoverageGate-->>CI: Pass or fail the coverage gate
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 GuideIntroduces a TypeScript-native coverage gate that runs tests directly against the TS sources, enforces ratcheting coverage thresholds configured in package.json, validates ignore entries against compiled output, and wires this gate into both local scripts and CI (including a Node 22/26 matrix), while updating tests to import from src instead of dist and pinning Node TS import options. Sequence diagram for running tests through the TypeScript coverage gatesequenceDiagram
actor Developer
participant Npm as NpmScripts
participant Gate as coverage_gate_ts
participant Node as node_test_runner
participant Lcov as lcov_report
Developer->>Npm: npm run coverage
Npm->>Gate: node scripts/coverage-gate.ts
Gate->>Gate: readFileSync(package.json)
Gate->>Gate: collectSources()
Gate->>Gate: readFileSync(tsconfig.json)
Gate->>Gate: validate ignore entries
Gate->>Node: spawnSync(process.execPath, ["--test", "--experimental-test-coverage", ...])
Node-->>Lcov: write lcov.info
Node-->>Gate: exit status
Gate->>Gate: readFileSync(lcovPath)
Gate->>Gate: parse reported source files
Gate->>Gate: compare reported vs required
alt missing required sources
Gate-->>Developer: exit 1 (list missing files)
else thresholds met and all files reported
Gate-->>Developer: exit 0 (coverage-gate: N source file(s) reported)
end
Flow diagram for coverage-gate.ts enforcing uncheatable coverageflowchart LR
A[Start coverage-gate.ts] --> B["readFileSync(package.json) and load coverageGate"]
B --> C["collectSources() from coverageGate.sources"]
C --> D["readFileSync(tsconfig.json) to get outDir and rootDir"]
D --> E[Validate coverageGate.ignore entries against compiled JS]
E --> F{Validation ok?}
F -->|No| X[Exit 1 with error]
F -->|Yes| G["Prepare coverage/lcov.info (mkdirSync, rmSync)"]
G --> H["spawnSync(process.execPath, --test ... thresholds ... --test-coverage-include=files)"]
H --> I{Test runner status == 0?}
I -->|No| X
I -->|Yes| J["readFileSync(lcovPath) and parse SF: lines"]
J --> K[Build reported set of source files]
K --> L{All required files reported?}
L -->|No| M[Print list of missing files and guidance]
M --> X
L -->|Yes| N[Log thresholds met message]
N --> O[Exit 0]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Greptile SummaryAdds an uncheatable TypeScript coverage gate and runs tests against
Confidence Score: 5/5This PR appears safe to merge; no blocking failures remain from prior Greptile findings or eligible follow-up issues. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| scripts/coverage-gate.ts | New gate script runs Node coverage on TS sources, enforces thresholds, and fails if any required source never loads. |
| package.json | Adds coverage script and coverageGate thresholds; release:check and test now exercise TS sources. |
| .github/workflows/ci.yml | CI test job matrices Node 22/26 and runs npm run coverage instead of npm test. |
| tsconfig.json | Enables allowImportingTsExtensions and rewriteRelativeImportExtensions for direct .ts test imports. |
| tsconfig.test.json | Typechecks test and scripts trees with noEmit instead of emitting dist-test. |
| test/smoke.test.ts | Switches imports from dist to src TypeScript entrypoint (same pattern across the test suite). |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[npm run coverage] --> B[build + build:test]
B --> C[scripts/coverage-gate.ts]
C --> D[Walk coverageGate.sources]
C --> E[Validate ignore via tsc --showConfig emit]
C --> F[rm stale coverage/lcov.info]
C --> G["node --test --experimental-test-coverage TZ=UTC"]
G --> H{runner status}
H -->|non-zero| I[exit with runner status]
H -->|0| J[Parse lcov SF paths]
J --> K{all required files reported?}
K -->|no| L[fail: name missing sources]
K -->|yes| M[pass: thresholds already enforced by node]
Reviews (5): 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: 4
🤖 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 `@package.json`:
- Line 17: Update the release:check script to remove the redundant npm run build
step, allowing the coverage command to own the project build while preserving
the remaining validation commands and their order.
- Around line 19-23: Retain source coverage while adding release-only validation
of the compiled or packed artifact. In package.json, update the release workflow
to build and execute an artifact smoke or contract suite. Apply equivalent
built-artifact validation in test/analytics.test.ts,
test/diagram-commands.test.ts, test/explain-command.test.ts,
test/export-and-contract.test.ts, and test/impact-command.test.ts, including
extension activation/entry-point checks and package contract checks as
applicable.
- Around line 106-109: Update the coverage thresholds in the thresholds
configuration to the stated decimal values: 63.21 for lines, 76.46 for branches,
and 77.40 for functions, preserving the existing threshold keys and structure.
In `@scripts/coverage-gate.ts`:
- Around line 228-230: Preserve decimal coverage thresholds through the
coverageGate configuration used by the flags in scripts/coverage-gate.ts: keep
lines, branches, and functions at the measured 63.21, 76.46, and 77.40 values
rather than truncating them to integers in package.json. Update the
corresponding baseline note in .agents/pm/features/pm-graph-5cu7.toon to record
the same decimal thresholds.
🪄 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: 13d20787-5966-49aa-a2f4-c77bced345f6
⛔ Files ignored due to path filters (2)
dist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (16)
.agents/pm/features/pm-graph-5cu7.toon.agents/pm/history/pm-graph-5cu7.jsonl.github/workflows/ci.yml.gitignoreCHANGELOG.mdpackage.jsonscripts/coverage-gate.tstest/analytics.test.tstest/diagram-commands.test.tstest/explain-command.test.tstest/export-and-contract.test.tstest/id-resolution.test.tstest/impact-command.test.tstest/smoke.test.tstsconfig.jsontsconfig.test.json
… 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 was referenced Aug 3, 2026
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 obtainable numbers pointed at generated JavaScript offsets rather than lines an author can act on, and no threshold was wired into any gate.What changed
Tests run against the TypeScript source. Node has stripped types without a flag since 22.18.0, the floor this package already declares in
engines.scripts/coverage-gate.tsis the gate. It enforces per-dimension thresholds and reconciles the reported file list against a walk of the source tree:The gate carries four further hardenings, each from a review finding on the sibling PRs in this rollout: the source list is walked dynamically rather than frozen (so a module added later is required automatically);
TZ=UTCis pinned when spawning the runner (coverage was otherwise a property of the host's timezone); the previous lcov report is deleted first (a run producing no report was otherwise reconciled against the last one's file); andcoverageGate.ignoreentries are validated against their compiled output, since a type-only module emitsexport {};and nothing else — that exemption was previously the one supported way to remove executable code from both the measured and the required set.The test tree stays typechecked.
coveragechainsbuild:test, sincerelease:checkno longer runsnpm test.Thresholds ratchet. Pinned in
package.jsonundercoverageGate. CI runs the gate on a Node 22 + 26 matrix.Measured baseline
63.21% line / 76.46% branch / 77.40% function over the single source module.
Line and function coverage are the weakest in this package and are the target of the follow-up work toward the mandated 100.
Verification
npm run release:check— passes end to end (typecheck, coverage gate, prod audit, pack dry-run, changelog check)src/makes the gate exit 1 naming it; exit 0 again once removedCI legs are unobserved locally and are confirmed by this PR.
pm items
Summary by Sourcery
Run tests directly against TypeScript sources under a strict, configuration-driven coverage gate and wire it into local scripts and CI.
New Features:
Enhancements:
Build:
CI:
Summary by cubic
Run tests against TypeScript sources and add a coverage gate that enforces thresholds and fails if any
srcfile isn’t loaded. CI runs the gate on Node 22 and 26. Meetspm-graph-5cu7acceptance criteria.New Features
src/*.tsdirectly; Node 22+ runs.tsnatively.scripts/coverage-gate.tsenforces thresholds and requires allsrcfiles via a dynamic walk.TZ=UTC, removes stalecoverage/lcov.info, exits with runner status first, strips block comments for type‑only checks, and resolves effectivetsconfigviatsc --showConfig(fails closed if it cannot be resolved).npm run coverage; wired intorelease:checkand CI; thresholds pinned inpackage.json(63 lines / 76 branches / 77 functions).Migration
npm run coveragelocally;npm teststill runs.tstests directly.coverageGate.ignore.Written for commit 31a620e. Summary will update on new commits.