You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
coverage-crap: replace staleness detectors with a coverage run stamp #3468
The blocking touched-function check (#3426, PRs #3460 → #3454) uses four problem kinds to tell when coverage/ wasn't written by this checkout's own run: map_partial, unscored_touched, unmeasured_touched and enumeration_missing.
lcov and the json map come from the same Vitest coverage map, so in a fresh run they can't disagree. The real question is just "is this coverage/ stale?" Four detectors is more machinery than that needs, and they still miss two cases: new files, and estimates below the threshold.
Proposal
Have test:coverage (or a small Vitest reporter) stamp HEAD and a tree hash into coverage/.
Have scripts/coverage-crap/model.ts refuse with "no trustworthy verdict" when the stamp doesn't match the checkout.
Replace the four staleness kinds with that one check.
Keep corrupt_touched, plus a declared "uninstrumented" set read from coverage.exclude.
Completion
A stale coverage/ always fails PR mode, including when the diff adds new files.
The four staleness kinds and their tests are deleted.
CONTRIBUTING.md and docs/agents/testing.md describe the single rule.
Suggested by the maintainer's review of #3460. Part of #3414's follow-ups. Blocked by #3460 and #3454.
Scope additions from a read-only review of the merged #3460 and #3454. Neither can happen in CI, where coverage runs fresh on the same checkout with src/** included, but both are reachable locally. The stamp proposed above fixes both:
Freshness against source. Matching file lists and function locations can produce a clean verdict from old counters if a function body is replaced in place. Record source hashes (or the HEAD + tree-hash stamp) with the coverage, and reject a mismatch before scoring.
Eligibility from config, not absence. A file that is missing from both reports is treated as outside coverage scope. For example, a src/new.ts added after the coverage run is reported as non-blocking at an estimated CRAP 42, and reports from another checkout pass the same way. Derive eligibility from the resolved Vitest coverage config, refuse eligible files that are missing, and reject paths outside the analyzed checkout.
Use fallow's own map check. The health JSON's summary.istanbul_files_matched / istanbul_files_total read 1724/1724 on a fresh map and 1722/1724 on a stale one. Refusing when matched < total catches a map from the wrong root, where every finding falls back to an estimate and --all would print "no function(s)". It also catches files deleted since the coverage run. It could replace the lcov cross-check and the parseLcov import.
Known defects to clear while replacing the detectors:
A sanitizer-dropped file stays in mapPaths (scripts/coverage-crap/model.ts:~318). Its complex functions then come back as unmeasured_touched, with the wrong fix advice. The "named once" test passes no estimates, so it doesn't catch this.
A function inside c8 ignore (for example conformance-harness.ts) would block forever with "run again".
In CI, coverage/ is wiped and both reports come from one process, so the staleness kinds can only fire locally. The stamp should be designed for the local case.
Activity
thymikee commentedon Oct 12, 2026
Scope additions from a read-only review of the merged #3460 and #3454. Neither can happen in CI, where coverage runs fresh on the same checkout with
src/**included, but both are reachable locally. The stamp proposed above fixes both:src/new.tsadded after the coverage run is reported as non-blocking at an estimated CRAP 42, and reports from another checkout pass the same way. Derive eligibility from the resolved Vitest coverage config, refuse eligible files that are missing, and reject paths outside the analyzed checkout.thymikee commentedon Oct 12, 2026
Two more inputs from an audit of the merged gate:
summary.istanbul_files_matched/istanbul_files_totalread 1724/1724 on a fresh map and 1722/1724 on a stale one. Refusing when matched < total catches a map from the wrong root, where every finding falls back to an estimate and--allwould print "no function(s)". It also catches files deleted since the coverage run. It could replace the lcov cross-check and theparseLcovimport.mapPaths(scripts/coverage-crap/model.ts:~318). Its complex functions then come back asunmeasured_touched, with the wrong fix advice. The "named once" test passes no estimates, so it doesn't catch this.c8 ignore(for exampleconformance-harness.ts) would block forever with "run again".coverage/is wiped and both reports come from one process, so the staleness kinds can only fire locally. The stamp should be designed for the local case.refactor(coverage): verdict from the stamp, and four detectors out
refactor(coverage): verdict from the stamp, and four detectors out
refactor(coverage): four staleness detectors out, the stamp verdict in
refactor(coverage): four staleness detectors out, the stamp verdict in
refactor(coverage): four staleness kinds out, the stamp verdict in
refactor(coverage): four staleness kinds out, the stamp verdict in
refactor(coverage): four staleness kinds out, the stamp verdict in
refactor(coverage): four staleness kinds out, the stamp verdict in
refactor(coverage): four staleness kinds out, the stamp verdict in (#…