Skip to content

ci: shard the coverage lane 4 ways and make a skipped Codecov upload loud - #5423

Merged
os-support-ai merged 1 commit into
mainfrom
claude/issue-5403-shard-coverage-job
Aug 20, 2026
Merged

ci: shard the coverage lane 4 ways and make a skipped Codecov upload loud#5423
os-support-ai merged 1 commit into
mainfrom
claude/issue-5403-shard-coverage-job

Conversation

@os-support-ai

@os-support-aios-support-ai commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Fixes#5403

Both halves live in the same test-coverage job, so this is one PR: shard the coverage lane 4 ways with a blob-report merge, and stop the Codecov upload from disappearing in silence.

Read this first — the card's numbers were stale, and the re-measurement moved the conclusion in the other direction

#5403 was written while the coverage job was failing 100% of the time, and its unblock comment predicted that #5402 would have made the unsharded arm faster than the 39 min 05 s measured before it. Re-measured on aa949ba2 (current main, the first commit carrying #5402's fix), both arms on the same commit and the same runner class, elapsed from each job's own started_at/completed_at:

armround 1 (run 32387182880)round 2 (run 32387778786)
slowest shard job11 min 25 s10 min 52 s
the merge job~1 min 15 s (13 s of it the merge itself)
sharded lane, wall clock~12 min 40 s~12 min 07 s
unsharded, one runner39 min 51 s tests / 40 min 19 s job39 min 29 s tests / 40 min 00 s job *

* the second unsharded sample is from run 32388915404, not from round 2 — round 2's own unsharded leg was still running when this was written, and rounds 1 and 4 are two independent samples of the same arm on the same tree.

The gap did not narrow. 39 min 51 s and 39 min 29 s are the old 39 min 05 s back within run-to-run noise — #5402 removed ~40 s from a ~2,400 s run, which is what its own numbers said it would. And measured at the level the cap acts on, the job took 40 min 19 s, so timeout-minutes: 40 would have killed it roughly twenty seconds before the suite finished — and the second sample landed on 40 min 00 s exactly: the fifth firing of a timeout that has already fired four times, and a job killed at 40 minutes uploads nothing. Unsharded, this lane no longer reliably reaches its own upload step. Sharded it is a third of that, with 28 minutes of headroom under the same cap.

⚠️ The same run also demonstrates the thing that makes the number hard to see: its coverage artifact upload failed with if-no-files-found: error, because the suite is red on this commit and vitest had already deleted coverage/. See the findings at the bottom.

Two more measurements point the same way, and neither is in the card:

  • The push lane's own coverage job on this very commit was cancelled at 22 min 20 s, by the next merge (cancel-in-progress keyed on github.ref) — run 32385114700, job Test (coverage), Upload coverage to Codecov conclusion skipped. A third silent path to "Codecov got nothing".
  • Over the last 30 consecutive push/main intervals (2026-08-19T15:31Z .. 2026-08-20T15:38Z) the median gap between merges is 25.2 minutes. A ~13-minute lane survives to completion in 20 of 30 of those windows; a ~39-minute one in 12 of 30.

Half 1 — four shards, and exactly one place a report is produced

test-coverage becomes a 4-way matrix that writes .vitest-reports/blob-N-4.json (the shard index is in the file name, so four artifacts merge into one directory without colliding — measured, not assumed). coverage-report downloads all four, refuses to continue unless all four arrived, merges them with --merge-reports, and uploads that one complete report.

The shard legs neutralise coverage.thresholds; the merge does not. Without that, sharding would evaluate whole-suite thresholds five times per push — once per shard over a quarter of the suite, plus once on the merged report. Those four extra evaluations are assertions this lane has never made and can only produce a false red. With it, the threshold gate runs exactly once, over the complete report, as it did unsharded.

⚠️ That override is not here because shards were seen to fail thresholds. That was the prediction, and CI refuted it: a shard leg run with the thresholds left alone passes (PROTO shard 1/4 WITHOUT the threshold override, conclusion=success, in both rounds). The v8 provider reports only files a run actually loaded, so one shard's percentage lands near the whole suite's. The override stays for the reason above, restated honestly, rather than the one that motivated writing it.

Equivalence — measured, and it holds inside the noise floor

Both arms on the same commit, compared file-by-file (coverage-final.json vs coverage-final.json), plus a second sharded round so the delta can be read against run-to-run noise rather than against zero:

filesonlyUnshardedonlyMergedstatementsdeltafiles with fewer covered statements
merged vs unsharded (same commit)1288 / 12880055516 vs 55521 of 84749+5 statements, +0.0059 pp1
merged vs merged (two identical sharded rounds)1288 / 12880055521 vs 55518 of 84749-3 statements, -0.0035 pp2

The acceptance criteria #5403 names are met: same file count both ways, onlyUnsharded=0, onlyMerged=0, and a delta of the same order as the noise floor — the card measured 3 statements between two identical sharded runs, and this measurement reproduces exactly 3. The merged report is not identical to the unsharded one; it is equivalent to within the variation two identical sharded runs already show, which is the strongest statement this comparison can support. Note the sign: the merged report has more covered statements than the unsharded one, not fewer.

The card's own figures were 1287 files and -0.0437 pp on an earlier commit; the file count moved by one because the tree moved (this is aa949ba2, not the commit #5395 measured).

⚠️ Both arms here are red runs — the suite fails on this commit (#5421) — so the instrument's unsharded leg needed --coverage.reportOnFailure=true to keep its report from being deleted before it could be captured. Both arms therefore describe the same suite in the same state, which is what makes them comparable; it is not the shipped recipe, and the shipped recipe is unchanged.

Evidence: runs 32388915404 (both arms) and 32391890554 (second sharded round); the comparison is run 32393342485, whose two artifact names carry the numbers above — the Actions log CDN, artifact bodies and a workflow-token branch push are all unreachable from the measuring session, and artifact metadata is not.

Half 2 — a skipped upload is now loud, ablated in both directions on real CI

The upload runs only when all four shards are green and all four blobs arrived. A final step then runs on every path and states the outcome — ::error annotation plus a job-summary block — and fails the job when Codecov did not receive a report.

That last part is the load-bearing one, and it was ablated rather than argued. Two jobs, identical except that one has the verdict step deleted, both driven by a genuinely failed needs:

jobdownload / merge / uploadverdict stepjob conclusion
with the verdict stepall skippedfailurefailure — says Codecov got nothing
verdict step deletedall skippedsuccess

The second row is the defect in one line: a job whose every step was skipped is a green check sitting next to a Codecov that received nothing. Reproduced in runs 32387182880, 32387778786 and 32388816770.

fail_ci_if_error flips false to true for the same reason: with false, an upload that errors reports success, and the verdict step would then truthfully report a lie. The cost is that a Codecov outage paints this lane red — nothing blocks on it, so the cost is attention, which is the point.

The reportOnFailure decision routed here by #5402: A, leave the default falsevitest.config.mts is untouched

Sharding resolves this half on its own, which the card suspected and #5402's comment explained. The artefact that survives a red run is now the blob, not coverage/: each shard uploads .vitest-reports/blob-N-4.json with if: always(), and a blob carries both the failing results and that runner's raw coverage. So a red coverage run is diagnosable from artifacts today — strictly more so than reportOnFailure: true would make it, since a blob can be merged locally into the full report.

Setting true would add a partial coverage/ directory on a red runner that nothing collects, and one more thing that could be uploaded by mistake. The two are one decision, as the card says: with the upload gated on all-four-green, reportOnFailure: true would upload nothing on red anyway.

⚠️ One honest cost: the throwaway measuring instrument did need --coverage.reportOnFailure=true on its unsharded leg, because on a red suite the unsharded arm deletes coverage/ before it can be captured. That is a measurement need, met with a CLI flag, not a reason to change the committed default.

Reverse-verification: predicted, then observed

legpredictedobserved
shard leg without the threshold overridered on thresholdsgreen — refuted; rationale rewritten to match
red-arm merge with the verdict stepred, and says whyred, verdict step failure
red-arm merge with the verdict step deletedgreen (the bug)green, every step skipped
local gates after the ci.yml editdependabot-merge-gate red on 4 unclassified namesred exactly there, plus two more the prediction missed (see below)
merged report still judged by thresholdsred under --coverage.thresholds.statements=99red — but not conclusive: the merged blobs carry a failing shard, so both legs go red for that reason. Claim now rests on the code path, and says so.
merged vs unsharded coverageequal file sets, delta inside the noise floorconfirmed: 1288/1288, 0/0, +0.0059 pp against a -0.0035 pp noise floor

Files outside the card's declared surface — both gate-forced, declared on the issue before this PR

The card scoped this to ci.yml (plus vitest.config.mts if needed). Renaming and adding a job in ci.yml turns three repo gates red on its own:

  • scripts/dependabot-merge-gate.mjsthese checks run on every pull request but the gate does not classify them: Test (coverage shard 1/4) ... 4/4. The four new names are classified in NOT_A_GATE (push-only, skipped on a PR by design). REQUIRED_CONTEXTS is untouched — nothing is added to or removed from the gating set.
  • content/docs/guide/ci-cd-pipeline.md — its ci.yml job table is pinned to ci.yml's jobs, in both directions, and its "Appears as" column to each job's name:. Updated, together with the two other places on that page that name this job.

A fourth failure came from the same run and is fixed inside ci.yml: the first draft counted blobs with a stderr-suppressing ls, which merge-queue-reporting.test.ts rejects repo-wide as the fail-CLOSED spelling #3723 removed.

Local gates after the amendment: scripts/ — 58 files, 1553 tests, all passing; check:control-bytes OK.

Two findings this run turned up, filed separately, not fixed here

Deliberately unchanged

The measuring instrument lives on claude/issue-5403-proto and is not part of this PR.


Generated by Claude Code

…loud (#5403)
`ci.yml`'s `test-coverage` job ran the whole suite unsharded on every push to
`main`, and its `Upload coverage to Codecov` step carried no `if:` — so it
inherited the implicit `success()` and was skipped whenever the suite failed:
311 of 373 coverage jobs, 120 of the 121 most recent. From outside, "Codecov
received nothing" and "Codecov received an unchanged report" are the same
picture, which is how four dark days passed unnoticed.
Half 1 — the lane is now four shards plus a merge. Measured on aa949ba with
both arms on the same commit and the same runner class (instrument on branch
`claude/issue-5403-proto`, runs 32387182880 / 32387778786, elapsed from each
job's own timestamps):
4 shards, slowest shard job 11 min 25 s / 10 min 52 s
+ the merge job ~1 min 15 s (13 s of it the merge itself)
the unsharded run 39 min 51 s of tests, 40 min 19 s of job
The card predicted #5402 would have made the unsharded arm faster than the
39 min 05 s measured before it. It has not — that is the old figure back
within noise. Measured at the level the cap acts on, the job took 40 min 19 s,
so `timeout-minutes: 40` would have killed it about twenty seconds before the
suite finished: the fifth occurrence of a timeout that has already fired four
times, and a job killed at 40 minutes uploads nothing.
Half 2 — the upload only runs when all four shards are green and all four blob
reports arrived, so a partial report can never be published as if complete; and
a final step runs on every path to state whether Codecov received the commit,
failing the job when it did not. Ablated both ways on real CI: with the verdict
step the job is red and says why; with that one step deleted and nothing else
changed, the identical job is GREEN with every step skipped — the silence this
card exists to remove. `fail_ci_if_error` flips to `true` for the same reason:
with `false` an upload that errors reports success.
The shard legs neutralise `coverage.thresholds` and the merge does not, so the
threshold gate is still evaluated exactly once per push over the complete
report, as it was when the job was unsharded.
Two files outside the card's declared surface, both gate-forced and declared on
the issue before the PR: `scripts/dependabot-merge-gate.mjs` classifies the four
new shard check names (its own test fails otherwise, by design), and
`content/docs/guide/ci-cd-pipeline.md`'s job table is pinned to `ci.yml`'s jobs
by `ci-cd-pipeline-doc.test.ts`. Nothing is added to or removed from the gating
set.
No test is deleted, skipped, quarantined or given a raised timeout, and
`timeout-minutes: 40` is unchanged.
Part of #5403
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Shard Test (coverage) 4 ways and stop the Codecov upload being skipped by its implicit success()

2 participants

@os-support-ai@claude