Repository navigation
ci(codecov): expand the coverage globs the uploader never expanded - #414
Conversation
The `files:` line on every coverage upload read as configuration and did nothing. `codecov/codecov-action` does not expand `**` itself, and nothing routes the value through a shell, so the pattern arrived at the CLI literally, matched no file, and the CLI fell back to its own tree-wide search. That fallback is what made the defect survive review: it produced a plausible answer rather than an empty one. On master's full build it found every jacoco report in the tree, so the `unit` and `integration` flags each received BOTH halves - the tell being that master's two flags reported an identical figure, which can only happen if they hold the same data. On a pull request each suite job has only produced its own half, so the same fallback found only that half. The per-flag gates were therefore comparing unlike sets, and `codecov/project/unit` and `codecov/project/integration` failed on branches containing no Java at all. This is the same defect this repository already fixed once, for the test-results upload in the same file, and the fix here mirrors that one: a preceding step expands the pattern with `find`, writes the real comma-joined list to `$GITHUB_OUTPUT` using the heredoc delimiter form rather than `echo` (a filename may legally contain a newline, which in the plain form injects an extra step-output line), and the upload sets `disable_search: true` so the fallback cannot silently re-widen the set. An upload whose collector found nothing is skipped rather than handed an empty `files:`, because empty is exactly what re-opens the fallback. Not every site wanted identical treatment, and the difference is the point: - The PR `test` matrix keeps BOTH jacoco patterns under `flags: matrix.suite`. A suite job produces only what it produces, and narrowing which half reaches which flag is the separate open question in docs/inflight/ci-codecov-flags-not-like-for-like.md. Expanding a pattern that matched nothing neither answers that nor depends on it. - The disabled `ak-experimental` job gets the same treatment as the matrix. It is `if: false` today, which is precisely why it is fixed now - re-enabling it for the Kafka 4.x work would otherwise reintroduce the defect under a flag nobody is watching. - The master `build` job collects the two halves into two SEPARATE step outputs and uploads one per flag. One half per flag is the split the per-flag gates compare, so the separation has to be real there rather than declared. Both halves come from one collector step because it is one job and one tree walk. Also corrected, one character, in the neighbouring test-results collector: `printf '%s'` piped into `wc -l` counts newlines, so with no trailing newline the log undercounted the collected reports by one every time. Behaviour is unaffected; only the log line was wrong. Verified locally: the workflow parses as YAML, no step id collides within a job, no `**` remains in any `files:` value, all five `codecov-action` calls now carry `disable_search`, every collector block is shellcheck-clean, and the collector shell was executed against a simulated multi-module tree - with both halves present, with one half only, and with none - producing the right lists and the right empty-value skip. `bin/check-all.sh` passes, and `bin/check-copyright-headers.sh` was re-run with `COPYRIGHT_CHECK_REQUIRE_FORK_POINT=1` so a shallow-clone false green could not hide behind it. THE FIX ITSELF IS NOT PROVEN, AND COULD NOT BE PROVEN BEFORE MERGING. The comparison exists only on Codecov's side, and only once both the base and the head have re-uploaded under real file lists. The proof is deferred to CI: the first per-flag comparison after master has run the `build` job with these uploads - master's two flags no longer reporting an identical figure, and a branch that changes no Java no longer showing a delta. Inside that window a pull request is still compared against a base assembled the old way, so a red per-flag gate there is the old defect being measured, not a regression. Defect-class sweep, since a fix that removes today's instance invites tomorrow's: the class is a glob handed to a consumer that does not expand it. Every other `**` under `.github/workflows/` goes to something that DOES expand it - `hashFiles()`, the `path:` input of actions/upload-artifact, actions/cache and actions/download-artifact, and a git pathspec in pr-checklist.yml - and codecov-action was the only non-expanding consumer in the tree. All of its call sites are fixed here. No other instance found. docs/ci.md's Codecov section and the inflight note are updated in the same pass: the diagnosis moves to past tense, and the note now tracks the outstanding proof and the condition for closing it rather than the defect. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UH2q829FDKQcqQseE8Cp7q
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
✅ Duplicate Code ReportTwo engines run in parallel for cross-validation. Each has its own thresholds tuned to its baseline - the real safety net is the per-engine "max increase vs base" check. ✅ PMD CPD
No new clones introduced by this PR. ✅ jscpd (language-agnostic)
No new clones introduced by this PR. Powered by astubbs/duplicate-code-cross-check |
[superseded - a quarantined test changed outcome] 🧪🔒 Quarantine Lane Report
🔴 expected while the owner PR is open · 🟡🎲 flapper, pass proves nothing · 🚨 a deterministic quarantined test passing means its fix landed: delete its Updated for Superseded by a newer quarantine lane report. |
|
🟢 Throughput — OKThis branch measured about 1% faster than master, on the one test this measures. That is INSIDE this test's own run-to-run spread of about 17%, so read it as a reading and not as a result - re-running the same commit moves it by about as much.
Allowable range 🟢 ≥ 0.70 · 🟡 0.50–0.70 (about a 30% loss) · 🔴 < 0.50 (about a 50% loss) What the numbers mean, and what they cannot tell youThe one that gets misread. Why a shape and not a rate. A rate depends on which runner you drew. A shape does not: every test here processes a fixed number of records, so a runner twice as slow doubles the subject and the controls together and leaves their ratio alone. That is the whole trick, and it is why the reported rate is shown last and labelled as this machine only. Reading the comparison. By conservation, not by correction. Every test in this lane processes a fixed number of records, so within one run the ratio of one test's time to another's is invariant under machine speed — a runner twice as slow doubles both terms and leaves the ratio alone. There is no machine-index correction to be wrong, because nothing needed correcting. Per-method times, not class times. A class time is Reference is the median of 7 recent What this still cannot do. It removes machine-to-machine variance. It does not remove this test's own run-to-run variance, measured at about 30% on a single unchanged commit while its controls stayed within 5%. That is a property of the test, not of the comparison, and no arithmetic here can touch it — which is why the reference is a median and the bounds are deliberately coarse. 🟡 means look at this; only 🔴 is outside the measured spread. Runs used: b917ae6, 54e8870, 3ae5dc3, beb01e1, 276fdff, 8f51d42, a694102 Since the previous push: ratio 1.104 -> 1.01, share 1.435 -> 1.594, rate 104587 -> 69988 (-33.1%). One push of difference sits inside this test's measured spread - read it as movement, not as a result. Updated for |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #414 +/- ##
============================================
+ Coverage 79.07% 81.10% +2.03%
- Complexity 1187 1366 +179
============================================
Files 79 90 +11
Lines 4267 4833 +566
Branches 451 464 +13
============================================
+ Hits 3374 3920 +546
- Misses 699 728 +29
+ Partials 194 185 -9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…one file Two documentation-accuracy corrections from the local ce-code-review pass on this branch; no behaviour changes, and the collector shell is byte-identical. - The `build` job's collector comment said the two halves are collected in one step "because this is one job and one tree walk". The step runs `find` twice, once per half, so the second half of that claim was false. The comment now gives the reason that is true - it is one job - and keeps the part that matters: the outputs stay separate to preserve the per-flag split. - The lane table in docs/ci.md still described master's uploads as "one file per flag" and the PR matrix's as "both files". With the globs expanded, the `unit` flag receives every module's jacoco/jacoco.xml and `integration` every module's jacoco-it/jacoco.xml - one HALF per flag, several files each. The table now says so, and the matrix row says "both halves" to match. Review outcome recorded for the record: correctness, project-standards and an independent cross-model adversarial pass (Codex) found no defect in the mechanism; the only actionable items were these two wording mismatches. The peer's one advisory - a report path containing a comma would split the comma-joined `files:` list - is a limitation of the action's own input contract shared by the pre-existing test-results collector, and no module path in this repository contains a comma; it is noted, not fixed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UH2q829FDKQcqQseE8Cp7q
…R's own CI run The Chaos Pain Suite on this branch killed ChaosChurnStormIT at the fleet-level NO_PROGRESS bound - the same arm, the same test, the same at-the-bound shape as every row already in test-no-progress-window-may-not-transfer-to-w1.md, so the sighting goes there, with its seed. Why it is worth a row rather than a shrug: the branch changes workflow YAML only, so no product code differed from master when it fired - which is the branch- independence control the note has wanted. It also reproduces the autopsy reporting gap on a fresh run: the autopsy printed violations (0) while the fleet-level VIOLATION line sat a hundred lines above it. Recorded before merge because the run log expires and the seed goes with it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UH2q829FDKQcqQseE8Cp7q
[superseded - a quarantined test changed outcome] 🧪🔒 Quarantine Lane Report
🔴 expected while the owner PR is open · 🟡🎲 flapper, pass proves nothing · 🚨 a deterministic quarantined test passing means its fix landed: delete its Since the previous push: Updated for Superseded by a newer quarantine lane report. |
…merge The row names this PR by number and its head sha - both durable facts, so bin/check-branch-self-reference.sh only wanted the attestation marker. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UH2q829FDKQcqQseE8Cp7q
[superseded - a quarantined test changed outcome] 🧪🔒 Quarantine Lane Report
🔴 expected while the owner PR is open · 🟡🎲 flapper, pass proves nothing · 🚨 a deterministic quarantined test passing means its fix landed: delete its Since the previous push: Updated for Superseded by a newer quarantine lane report. |
🧪🔒 Quarantine Lane Report
🔴 expected while the owner PR is open · 🟡🎲 flapper, pass proves nothing · 🚨 a deterministic quarantined test passing means its fix landed: delete its No quarantined test changed outcome since the previous push. Updated for |
…nd got the cause wrong `node bin/inflight.mjs prior-art codecov` returns four codecov notes and marks exactly one as NOT ON origin/master: this one. The other three arrived with #409 in the master merge this branch already carries, hours before the note was written. ci-codecov-flags-not-like-for-like.md owns the flag-set mismatch and its fix in #400. ci-the-coverage-uploads-still-use-the-inert-glob.md owns what is actually wrong with the per-flag gates - the uploader does not expand `**`, so the pattern matches nothing and the CLI falls back to a tree-wide search; master's full build therefore hands the WHOLE tree to both the unit and the integration flag, while a PR's suite job has only its own half. That is why the two flags report an identical number on the default branch, which both that note and this one used as evidence for different conclusions. Its collision check also lists #414, open since before the diagnosis started here, making exactly that fix with `disable_search: true` and expanded globs - and proving it from the upload logs rather than by inference. So the note was a third statement of a fact this repo already had twice, and its own mechanism - "the base report is partial" - was wrong. A partial base is real and separate (a CANCELLED master run uploads nothing, which is why the base totals here read 83 files against a head of 93), but it is not what makes the per-flag gates red. That observation is offered on #414 rather than kept here, because that PR owns the file it belongs in. The rule this broke is in AGENTS.md and I quoted it in a merge commit two hours before ignoring it: grep and find read the working tree, prior-art reads every ref. Both offending notes were IN the working tree by then, so even the weaker check would have worked - what failed was searching at all before writing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01228AtRiHyTzeH7SjcXzgNj
|
One observation from the other side of this, offered rather than pushed — it belongs in your note's file, which this PR owns. A cancelled master run is a second, independent way the same gates go wrong, and it is not the glob. Master's CI at Measured on #207 while that was the base: the diff block read base 83 files / 4448 lines against head 93 / 4894, on a PR that adds 3 files. The two are separable, and the files count tells them apart in one line:
The second matched what your note already predicts for the post-merge window — "a red or no-data Also worth recording somewhere: Take or leave any of it; no change requested here. |
|
@claude review this |
|
Claude finished @astubbs's task in 5m 4s —— View job Review
SummaryThis is a careful, well-diagnosed fix. I compared master's
I did not find any bug in the shell logic itself — the Minor finding (docs only, not the codecov fix)
Note on astubbs's commentThe observation about a cancelled master run being a second, independent way the per-flag gates go wrong (distinguishable from this PR's glob defect by whether Verification limitsPer this session's tool grant (from master's copy of the Claude workflow, not this PR's), I could read files and run isolated single git/show commands but not arbitrary |
…unting seeds Two pieces of review feedback on this PR, both documentation. The cancelled-master-run observation is folded into the inflight note this PR owns, as offered: a master run cancelled before it uploaded leaves Codecov with no report for that base commit, so every PR comparing against it falls back to older master data and the per-flag gates go red for a reason that is not the glob. The two causes are separable by the files count in the diff block - equal on both sides is the glob, base short by more than the PR adds is the missing upload - and that tell is recorded in the note with its measurement, and in one paragraph of docs/ci.md, which owns the Codecov diagnosis and outlives the note. The automated review found the NO_PROGRESS note still saying three seeds were available after this PR's own row made it four. Fixed by removing the count rather than bumping it: docs/inflight/AGENTS.md forbids written counts for exactly this reason, and the sentence now points at the table, which is where the set lives. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UH2q829FDKQcqQseE8Cp7q
…PR's Unit Tests lane ReactorBatchTest.simpleBatchTest, KEY ordering, failed this PR's Unit Tests lane with the ledgered signature - Expected size: 3 but was: 4 on the 30s alias. The branch is workflow YAML and docs only, so it is master state, and the sighting goes in test-untracked-ci-flakes.md before this PR merges because the log that carries the payload expires. The payload is the point: keys 49, 74, 74, 74, 59 by offset, a three-way collision on one key, arriving as the two singletons paired and the three colliding records one to a batch. That is the same shape as the 2026-08-25 and 2026-09-01 sightings, with a different collided key each time, and it is what the expectation-versus-input reading predicts. The diagnosis is unchanged and the pin-the-keys experiment is still unrun; what has changed is that the flake has now failed a PR's required lane two days running, on branches that could not have caused it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UH2q829FDKQcqQseE8Cp7q
|
Both pieces of feedback on this PR are handled in 6675aaf.
Folded into
Fixed differently: the count is removed rather than bumped. Also in this push, 540fa1d: this PR's |
One conflict, in docs/inflight/test-untracked-ci-flakes.md: master rewrote the ProducerManagerTest row (#265 fixed it, #262 lifted the quarantine) on the line above the simpleBatchTest row this branch rewrote for its fifth sighting. Both edits kept; the stray blank line master removed stays removed. Resolved by editing the markers, and the removals in the file were read against both parents - each is the other side's own change. master's edit to .github/workflows/maven.yml (a comment block on the Lincheck lane) auto-merged in a region this branch does not touch. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UH2q829FDKQcqQseE8Cp7q
Four commits, no Java on master's side, so nothing here reaches this branch's code. The one that matters to this PR is #414: the codecov glob fix has landed, so the two per-flag gates this branch has been carrying as red are now fixed at their source rather than merely attributed. #414 ALSO CARRIES THE OBSERVATION THIS BRANCH OFFERED IT Its note now holds "A second, independent way the same gates go red - and the one-line tell that separates them": master's run at beb01e1 was cancelled, so Codecov holds no report for that commit and every PR comparing against it falls back to older master data. That was offered as a comment rather than written into a third note here, and it landed in the file that owns the subject - which is the outcome the never-state-a-fact-twice rule is for. THE CONFLICT test-untracked-ci-flakes.md again, and trivially: master raised simpleBatchTest from four sightings to five. Master's row is the superset and is taken whole; this branch's four rows that master has never seen are kept beside it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01228AtRiHyTzeH7SjcXzgNj
…control that narrows its hypothesis
The core unit suite failed once on this branch immediately after merging master,
at `anEmptyCallerContextIsHandledAndNothingLeaks` with `expected: null but was:
{}` - the same line and the same message master's note already records from
#262 the day before.
WHY THE SECOND SIGHTING IS WORTH MORE THAN A TALLY
The two branches have no code in common: #262 is transactional, this one
is offset decoding and documentation, and neither touches anything MDC-related.
The note previously had to argue master-state from a neighbouring run of the same
tree; two branches failing the same way is the branch-independence that argument
wanted.
AND A CONTROL THAT BEARS ON THE HYPOTHESIS RATHER THAN REPEATING IT
The class alone passed 5 of 5, on the same tree, minutes after the suite lost.
The note's hypothesis is that a sibling in this class does `MDC.put` and arms the
precondition depending on JUnit's method order - but if that were so, running the
class by itself should still lose whenever a putting method was ordered first. It
did not. One green isolated run cannot prove within-class ordering never arms it,
so the note says that; what it does is point the search at a different class
sharing the fork's JUnit thread, which is cheaper to falsify than the original
wording implies.
The note's "deliberately not quarantined" section now reads two sightings rather
than one. The disposition is unchanged and deliberately so: rule 1 wants a
diagnosis or a ledger, and a second sighting with an untested hypothesis is still
not a diagnosis.
Also attests the #207 reference #414's note picked up from this
branch's comment. The gate's PR-number arm reads whole files by design - a PR
number can only be about its own PR - so master's sentence about this one needs
this branch's attestation, which is exactly the branch that supplied it. The
sentence was already past tense; only the marker was missing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01228AtRiHyTzeH7SjcXzgNj
…s fix Master's own `full build (master)` job at ebd8bcb failed: 581 tests, 1 failure, `MdcContextPropagationTest.anEmptyCallerContextIsHandledAndNothingLeaks`, the same assertion the two branch sightings above carry. That ends the master-state argument outright. The note had been making it from branch evidence - #262 first, this branch second, neither touching anything MDC-related - and a failure on master needs no attribution at all. THE PART THAT IS NOT ABOUT THIS TEST The `build` job is the only one uploading coverage on master. It fails here, so nothing uploads. #414 merged the corrected coverage globs and named its own post-merge proof - master's `unit` and `integration` flags no longer reporting one identical figure - and that proof HAS NOT HAPPENED: `node bin/inflight.mjs codecov` still shows both at 79.12%, because the job that would produce the corrected upload has not completed since the fix landed. Three of the four master runs after it were cancelled by the next push and the fourth is this failure. So every open PR keeps comparing against a base assembled the old way and keeps seeing red per-flag gates that read as its own fault. The cost of this test is no longer one confusing red in a suite: it is master red, no coverage upload, a merged fix that cannot take effect, and a misleading gate on every PR. Recorded rather than acted on. Quarantining is still the owner's call and the note's own argument against guessing stands - what changed is the price of leaving it, which the next person deciding should be able to read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01228AtRiHyTzeH7SjcXzgNj
Four commits, no Java touching this branch's paths. Three conflicts, all the same shape as before - two writers appending sightings to the same ledgers - and this time both writers were recording the SAME two flakes, so neither side's count was right and taking either whole would have lost real observations. MdcContextPropagationTest: master had three sightings (one local on #262, two on #416's CI across consecutive heads); this branch had a fourth, its own local suite, and a fifth that matters more than the rest - MASTER'S OWN `full build` at ebd8bcb. Unioned to five, with the master one called out, because it is the one that stops being a test problem: `build` is master's only coverage uploader, so its failure is why #414's merged glob fix has not taken effect and why every open PR still sees red per-flag gates. committedOffsetRemoved[1] latest: master had #407, #409 and #416; this branch had #407, #207 and #409. Union is four, and the row now says which of them captured what - #407 the surviving record, #409 a rate, the other two neither. Master also re-added a processInKeyOrder row at two sightings where this branch already carries the six-sighting version from an earlier merge; its copy is dropped rather than landing a duplicate row under one test name. bin/check-all.sh 15/15, core compiles. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01228AtRiHyTzeH7SjcXzgNj
1 participant
Description
The two
codecov/project/*gates go red on every PR that changes no Java, and the cause is aninert glob.
codecov-actiondoes not expand**, and nothing routesfiles:through a shell,so every coverage upload's pattern arrived literally, matched nothing, and the CLI fell back to a
tree-wide search. On master's full build that fallback finds every jacoco report, so the
unitand
integrationflags each received the whole tree and reported an identical number. On a PR eachsuite job has only its own half. The per-flag comparison was between unlike sets.
Proven from the upload logs, not inferred: master's
unitupload declared one glob and loggedSome files were not foundfollowed by a find-count far larger than one glob could match.The fix mirrors what #409 did for the test-results upload in the same
file: a
findstep expands the patterns into real paths, passed via$GITHUB_OUTPUTwith aheredoc delimiter (a filename may contain a newline), and every upload sets
disable_search: truesothe fallback cannot re-widen the set. Uploads are skipped when nothing is found rather than handed an
empty
files:.The four sites did not want identical treatment: master's
buildgets one collector, twooutputs, one half per flag, because one-half-per-flag is what the gates compare and the separation
has to be real rather than declared. The PR matrix keeps both patterns - a suite job produces only
what it produces.
ak-experimentalisif: falsetoday and was fixed precisely so re-enabling itdoes not reintroduce the defect under a flag nobody watches.
What this PR proved on its own head, and what only the merge can. The unflagged project total on
the fixed head landed within rounding of the pre-fix head's, so the explicit lists are complete:
nothing the tree-wide search used to pick up has been lost, only the tagging changed. The per-flag
comparison itself exists only on Codecov after both sides re-upload, and until master does, every PR
is still compared against a base assembled the old way, so a red per-flag gate in that window is the
old defect being measured, this PR's own included. The proof after merge is master's two flags no
longer reporting the same figure, and the next no-Java branch showing a zero delta.
Defect-class sweep: every other
**under.github/workflows/reaches a consumer that expands it(
hashFiles(), upload/download-artifact, a git pathspec).codecov-actionwas the onlynon-expanding consumer; all five of its call sites are now fixed.
Checklist
docs/ci.mdCodecov section moved to past tense; the inflight note now tracks the outstanding proofdocs/features/- N/A - CI configuration, no user-facing featuredocs/inflight/working note (pr-/branch-) started at the PR's first commit - the existingci-the-coverage-uploads-still-use-the-inert-glob.mdis this PR's note and is updated in itce-simplifyandce-code-reviewlocally - both ran after the PR opened (ce-simplify-code,ce-code-reviewwith a Codex adversarial pass); one doc-accuracy commit came out of it, zero defects🤖 Generated with Claude Code
https://claude.ai/code/session_01UH2q829FDKQcqQseE8Cp7q