fix: merge coverage reports by file path in aggregation#78
Conversation
When multiple reports cover the same file, the previous aggregation concatenated file entries and summed statements, counting each line in the denominator once per report while hits only came from whichever report covered it. This deflated the overall rate roughly to a weighted average of the per-report rates rather than the union. Aggregation now groups FileCoverage entries by path and merges their lines via max(hits) before summing metrics. Files with unique paths are unchanged. Branch/method counts use max() across reports as a best-effort union, since LineCoverage doesn't carry per-branch hit state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Parsers differ on how statements map to lines: cobertura/lcov/ jacoco/istanbul emit one statement per line entry, but Go counts semantic blocks where one block can span many lines. The initial merge collapsed statements to mergedLines.length, corrupting Go's per-file statement count. Use max() across reports for statements (same file ⇒ same count), and derive coveredStatements from the merged line union only when statements align 1:1 with lines; otherwise fall back to max().
Some parsers emit multiple FileCoverage entries per source file (e.g., cobertura produces one <class> per type, and a single file can contain several types or inner classes). Those are disjoint parts and must be summed. The previous code treated them as cross-report duplicates and unioned them, undercounting the denominator and allowing coveredStatements to exceed statements. Aggregation now merges in two phases: intra-report entries with the same path are summed, then cross-report entries are unioned via max() on per-line hits.
sumFileGroup and mergeFileGroup shared ~40 lines of derivation logic (sort, missing/partial lines, rate calculation, return shape). Pull that into finalizeMergedFile, leaving each function focused on its aggregation strategy. Also collapse sumFileGroup's six reduce passes into one loop and factor the rate formula into calculateRate.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 647252a. Configure here.
max() across reports understates the union when different reports exercise different blocks (e.g., report A covers block 1, report B covers block 2 — true union is both, but max(covered_A, covered_B) still only credits one). Use min(statements, sum) instead: strictly ≥ max() for same-file reports, and tracks the common real-world pattern of test suites covering different paths. Overstates only when reports cover identical blocks, which is the less common case. Exact reconstruction would require preserving block structure through FileCoverage; that's a bigger refactor.
|
Apologies for the churn. This started out as a pretty straightforward fix (merge coverage reports by path). It grew a bit under the AI hardening process... 😅 For reference, I've validated the fixes here: |
MathurAditya724
left a comment
MathurAditya724
left a comment
There was a problem hiding this comment.
Went through the aggregation logic end to end and this is a genuine fix — the old aggregateResults concatenated file entries and summed metrics.statements, so a file covered by two reports counted its lines in the denominator twice while hits came from only one report. Your 3/4 → 3/8 example is exactly what happens.
The two-phase merge (sum disjoint same-path entries within a report, then union across reports) is the right structure, and I verified the parts I was most worried about:
- Go block counting: the
lineAlignedcheck (f.statements === f.lines.length) correctly distinguishes line-based parsers (cobertura/lcov/jacoco/istanbul, exact union from the line map) from Go, wherestatementsis semantic blocks and one block spans many lines. Confirmed againstgo-parser.ts— it sumsblock.numStmtsfor statements but emits one line entry per physical line, so they're intentionally not 1:1. - Intra- vs cross-report: summing disjoint cobertura
<class>entries within a report, then unioning across reports, avoids both undercounting the denominator and letting covered exceed total. The disjoint-class test nails this.
One caveat on the Go min(statements, sum) clamp, which you already flag in the comment — I reproduced it: two reports that each cover only block 1 of a two-block file report 100% when the true union is 50%, because the clamp can't tell "same block covered twice" from "two different blocks covered." It overstates only when reports cover overlapping blocks; for the common case (suites exercising different paths) it's accurate, and it's strictly better than the old double-counted denominator either way. Fine to ship with the comment as-is, but might be worth a one-line note in the PR body so reviewers/users know merged Go rates can round up slightly when suites overlap.
All 197 tests pass, dist is rebuilt, and the added code is lint-clean. The pre-existing tsc any/threshold-type errors are on main, not from this change. Solid work.
2 participants
Problem
When multiple reports cover the same file,
aggregateResultsconcatenatesFileCoverageentries and sums statements. Each line in a shared file counts in the denominator once per report, while hits come only from whichever report covered it.Example — a 4-line file, report A hits lines 1+2, report B hits lines 2+3:
Observed on a real project with two suites sharing coverage of one module: 86.59% → 58.24% purely from double-counting.
Solution
Group
FileCoverageentries by path and merge their lines viamax(hits)before summing:max()across reports as a best-effort (same file ⇒ same branch count;LineCoveragedoesn't carry per-branch hit state).One new test covers the union semantics. All existing tests pass.