Add per-area coverage table and top uncovered files to coverage summary - #8427
Open
Amaury Chamayou (achamayou) wants to merge 1 commit into
Open
Amaury Chamayou (achamayou) wants to merge 1 commit into
Amaury Chamayou (achamayou) wants to merge 1 commit into
Conversation
The coverage job summary only showed overall line/branch totals, making it impossible to see which areas of the code base moved between runs or where the most uncovered lines are concentrated. This adds: - extract_file_coverage: parses per-file rows of the llvm-cov report by shape, tolerating timestamps, ANSI codes, and reports without branch columns. - area_of / aggregate_by_area: groups files by the first 3 directory components (e.g. src/node/rpc, include/ccf/ds). - render_areas: a Markdown table sorted by missed lines, with line and branch coverage deltas in percentage points against the most recent previous run. - render_top_files: the top 15 files by missed lines. Existing trend chart behaviour and output are unchanged when the report has no per-file rows. Unit tests are added in scripts/tests/coverage_summary_test.py and wired into ci-checks.sh via a new coverage-summary-tests.sh check, documented in the formatting-and-linting skill's check inventory table. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot started reviewing on behalf of
Amaury Chamayou (achamayou)
September 22, 2026 22:42
View session
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Two moderate issues in scripts/coverage_summary.py remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Adds per-area coverage deltas and a ranked list of uncovered files to the coverage summary, with tests and CI integration.
Changes:
- Parses and aggregates per-file coverage data.
- Renders area and top-file tables.
- Adds unit tests and registers the CI check.
- Documents the new check.
Review findings:
- Moderate (3 votes): History selection should fall back when the newest log lacks usable per-file data.
- Moderate (1 vote): File-row validation must reject non-report log lines with numeric tokens.
| File | Description |
|---|---|
scripts/tests/coverage_summary_test.py |
Tests parsing, aggregation, rendering, history, and main behavior. |
scripts/coverage-summary-tests.sh |
Runs coverage summary unit tests. |
scripts/coverage_summary.py |
Implements parsing, aggregation, historical deltas, and summary tables. |
scripts/ci-checks.sh |
Registers the new CI check. |
.github/skills/formatting-and-linting/SKILL.md |
Documents the check inventory entry. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+458
to
+462
| previous_path: Optional[str] = latest_history_path(args.history) | ||
| if previous_path is not None: | ||
| previous_text: Optional[str] = _read_text(previous_path) | ||
| if previous_text is not None: | ||
| previous_files: List[FileCoverage] = extract_file_coverage(previous_text) |
This branch has not been deployed
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.
Motivation
The Coverage workflow's job summary only showed overall line/branch coverage totals, making it impossible to see which areas of the code base moved between runs or where the most uncovered lines are concentrated.
Implementation summary
scripts/coverage_summary.pynow also renders, after the existing trend charts:src/node/rpc,include/ccf/ds), sorted by missed lines, with line/branch coverage deltas in percentage points against the most recent previous run (extract_file_coverage,area_of,aggregate_by_area,render_areas).render_top_files).Per-file rows are parsed by row shape (tolerating timestamps, ANSI codes,
-percentages and reports without branch columns), so this works whether the "report" is the raw llvm-cov output or a full job log. Existing trend chart behaviour is unchanged, and when the report has no per-file rows the output is identical to before.Added
scripts/tests/coverage_summary_test.py(unit tests for parsing, aggregation, rendering, history selection andmain) and wired it intoscripts/ci-checks.shvia a newscripts/coverage-summary-tests.shcheck, documented in the formatting-and-linting skill's check inventory table.Validated end-to-end against real coverage run data (run 35761555333 and previous run 35759554753): output contains all four sections, with
src/nodefirst in the area table andsrc/node/node_state.hfirst in the top-files table, matching expected line/missed counts.Safety and compatibility
No runtime impact: this only changes the CI coverage job summary script and adds a CI check; it does not touch any product code, API, data format, consensus/KV behaviour, or mixed-version/recovery paths.