COMP: Fix the Codecov wiring that was dropping three modules from the percentage - #629
Merged
Conversation
The reported whole-tree percentage was measured over a subset of the extension. Three faults, all silent -- no warning, no failed job: `flags.<flag>.paths` in .codecov.yml removes files from the report rather than merely un-flagging them, and the list had fallen behind the module layout: it named LiverMarkups/ (module deleted) and Modeling/ (never existed), placed LiverResectionsLib/ one level too high, pointed the py flag at **/Testing/Python/ (which the ignore list discards anyway), and omitted Liver/, LiverSegmentation/ and SlicerLiverInteractionLib/. Those three modules were therefore absent from the percentage even though the coverage job measured them. Flag membership now comes from the upload alone. A single codecov-action step with `files: a,b` + `flags: cxx,py` is one session carrying both flags, so neither flag isolated its language -- the tell was `sessions: 1` on a repo with two flags. Upload once per flag instead. SlicerLiverInteractionLib was missing from the pytest --cov targets, so the base four modules share was never measured at all, and the three newer staged Lib packages had no .coveragerc [paths] alias, leaving their launched-leg records on build-tree paths instead of merging onto the source files. Because the first two faults change the denominator, they also made the percentage non-comparable across commits: the 56.29 -> 51.61 step in August falls on an ADR-plus-workflow commit that adds no source. Coverage stays informational per ADR-0021; this fixes what the number is computed over, not whether it gates.
Both faults fixed in the preceding commit corrupt the reported percentage without surfacing an error, and both are easy to reintroduce: a flag path list that silently drops files, and a single upload step that collapses two flags into one session. Also note the standing obligation: a new Python sub-package staged into qt-scripted-modules/ needs a .coveragerc [paths] alias, or its launched-leg records never merge onto the source file. Adds the reading rule that would have caught this sooner -- when a coverage swing has no plausible cause in the diff, check the file and line totals before treating it as a test-quality regression.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## preview #629 +/- ##
===========================================
+ Coverage 52.00% 54.75% +2.75%
===========================================
Files 122 139 +17
Lines 14863 17449 +2586
Branches 2504 2519 +15
===========================================
+ Hits 7729 9555 +1826
- Misses 6746 7503 +757
- Partials 388 391 +3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
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.
What this fixes
The Codecov percentage on
previewwas being computed over a subset of the extension, and had become non-comparable across commits. Three independent faults, all silent — no warning, no failed job, just a wrong number.1. Flag
pathswere dropping three whole modulesflags.<flag>.pathsin.codecov.ymlremoves files from the report, it does not merely un-flag them. The list had fallen behind the module layout:LiverMarkups/Modeling/LiverResectionsLib/LiverResections/LiverResectionsLib/**/Testing/Python/ignorelist below itLiver/,LiverSegmentation/,SlicerLiverInteractionLib/So the shell (11
.py), Stage-2 segmentation (29.py) and the shared interaction lib (13.py) contributed zero to the reported figure, even though the coverage job was measuring the first two. Confirmed against the live report: onlyLiverResections,LiverVolumetryandVascularTerritoriesappear.Flag membership now comes from the upload alone, which is the thing that already knows the answer.
2. Two flags, one session
A single
codecov-actionstep withfiles: a,b+flags: cxx,pyis one upload carrying both flags, so neither flag isolates its language. The tell issessions: 1on a repo with two flags — visible on every commit in the API. Now one upload step per flag.3. Unmeasured and unmerged code
SlicerLiverInteractionLibwas missing from the--covtargets, so the base four modules share was never measured at all.LiverVolumetryLib,VascularTerritoriesLibandSlicerLiverInteractionLibare staged intoqt-scripted-modules/likeLiverResectionsLib, but had no.coveragerc[paths]alias — their launched-leg records stayed on build-tree paths instead of merging onto the source files.Why the trend line was lying
Faults 1 and 2 change the denominator, so the percentage moves on its own. The 56.29% -> 51.61% step on 2026-08-20 lands on an ADR-plus-workflow commit that adds no source, while the measured base jumped from 118 files / 12,478 lines to 122 / 14,524. Read as a test-quality regression, that would have been wrong.
For the record, the current 52.0% is still soft — it will move once the three missing modules enter the denominator, probably downward, and that will be the first honest number rather than a regression.
Scope
Configuration and documentation only. No test, source or build-logic changes. Coverage remains informational per ADR-0021 — this fixes what the number is computed over, not whether it gates.
ADR-0021 gains a "Configuration traps" section recording both faults, the staged-package
[paths]obligation, and the reading rule that would have caught this sooner: when a coverage swing has no plausible cause in the diff, check the file and line totals before believing it.Verification
.codecov.yml,.coveragercandci.ymlparse (yaml.safe_load,configparser).coveragejob now carries exactly twocodecov-actionsteps,coverage-cxx.xml/cxxandcoverage-py.xml/py.qt-scripted-modules/Lib package now has a[paths]alias (checked against theCMakeLists.txtDESTINATION_DIRentries).Validate GitHub Workflows.The real proof is the next post-merge run: expect
sessions: 2and the three modules present in the file list.Reviewer's map
.codecov.yml— load-bearing: theflagsblock. The comment explains whypathsmust not come back..github/workflows/ci.yml— load-bearing: the upload step split. Mechanical: one added--covline..coveragerc— mechanical: three aliases matching the existing pattern.Docs/adr/0021-coverage-measurement.md— documentation of the above.Authored with assistance from Claude (Anthropic Claude Opus 5).