fix: Revert "ci: discover staged Codecov flags instead of a hardcoded matrix"#6837
Conversation
Automated Reviewer SuggestionsBased on the
|
There was a problem hiding this comment.
Pull request overview
This PR reverts the prior change that dynamically discovered staged codecov-* artifacts for the Codecov deferred upload workflow, returning to a hardcoded upload matrix because Codecov uploads stopped working after the previous change.
Changes:
- Replaced dynamic artifact discovery with a fixed
matrix.includelist of Codecov flags incodecov-upload.yml. - Updated
build.ymlcomments to reflect that new platform services require a correspondingcodecov-upload.ymlmatrix row to avoid dropping coverage.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| .github/workflows/codecov-upload.yml | Reverts to a hardcoded flag matrix and adjusts download/upload guards accordingly. |
| .github/workflows/build.yml | Updates guidance comment to keep Codecov upload flag list in sync with staged artifacts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 381 | 0.233 | 25,142/32,693/32,693 us | 🔴 +10.2% / 🔴 +104.2% |
| 🔴 | bs=100 sw=10 sl=64 | 782 | 0.477 | 125,458/157,780/157,780 us | 🔴 +9.6% / 🔴 +48.3% |
| 🟢 | bs=1000 sw=10 sl=64 | 894 | 0.546 | 1,113,447/1,230,591/1,230,591 us | 🟢 -9.2% / 🔴 +20.7% |
Baseline details
Latest main dda9e83 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 381 tuples/sec | 423 tuples/sec | 767.66 tuples/sec | -9.9% | -50.4% |
| bs=10 sw=10 sl=64 | MB/s | 0.233 MB/s | 0.258 MB/s | 0.469 MB/s | -9.7% | -50.3% |
| bs=10 sw=10 sl=64 | p50 | 25,142 us | 23,137 us | 12,623 us | +8.7% | +99.2% |
| bs=10 sw=10 sl=64 | p95 | 32,693 us | 29,672 us | 16,011 us | +10.2% | +104.2% |
| bs=10 sw=10 sl=64 | p99 | 32,693 us | 29,672 us | 18,856 us | +10.2% | +73.4% |
| bs=100 sw=10 sl=64 | throughput | 782 tuples/sec | 814 tuples/sec | 998.44 tuples/sec | -3.9% | -21.7% |
| bs=100 sw=10 sl=64 | MB/s | 0.477 MB/s | 0.497 MB/s | 0.609 MB/s | -4.0% | -21.7% |
| bs=100 sw=10 sl=64 | p50 | 125,458 us | 123,853 us | 100,463 us | +1.3% | +24.9% |
| bs=100 sw=10 sl=64 | p95 | 157,780 us | 143,989 us | 106,400 us | +9.6% | +48.3% |
| bs=100 sw=10 sl=64 | p99 | 157,780 us | 143,989 us | 117,712 us | +9.6% | +34.0% |
| bs=1000 sw=10 sl=64 | throughput | 894 tuples/sec | 876 tuples/sec | 1,034 tuples/sec | +2.1% | -13.6% |
| bs=1000 sw=10 sl=64 | MB/s | 0.546 MB/s | 0.535 MB/s | 0.631 MB/s | +2.1% | -13.5% |
| bs=1000 sw=10 sl=64 | p50 | 1,113,447 us | 1,126,331 us | 973,294 us | -1.1% | +14.4% |
| bs=1000 sw=10 sl=64 | p95 | 1,230,591 us | 1,355,445 us | 1,019,213 us | -9.2% | +20.7% |
| bs=1000 sw=10 sl=64 | p99 | 1,230,591 us | 1,355,445 us | 1,049,896 us | -9.2% | +17.2% |
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,524.34,200,128000,381,0.233,25141.73,32693.08,32693.08
1,100,10,64,20,2558.99,2000,1280000,782,0.477,125457.79,157780.41,157780.41
2,1000,10,64,20,22372.44,20000,12800000,894,0.546,1113446.81,1230590.91,1230590.91
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6837 +/- ##
=========================================
Coverage 76.28% 76.28%
Complexity 3452 3452
=========================================
Files 1161 1161
Lines 45916 45916
Branches 5099 5099
=========================================
Hits 35025 35025
Misses 9327 9327
Partials 1564 1564
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…oad (apache#6840) ### What changes were proposed in this PR? Reverts apache#6730 ("ci: route all Codecov flags through the deferred workflow_run upload"). Together with the already-merged apache#6837 (revert of apache#6824), this fully backs out the deferred `workflow_run` Codecov upload: the inline `codecov/codecov-action` steps are restored in `build.yml` for every flag, and both `codecov-upload.yml` and `.github/scripts/stage-codecov.sh` are removed. The deferred approach regressed coverage — because the upload moved out of the Required Checks run, Codecov built each PR's comment from carryforward before the deferred upload landed, so every PR showed its own flag as `Carriedforward` instead of a fresh delta (full analysis in apache#6839). Restoring inline uploads brings back fresh per-PR coverage. ### Any related issues, documentation, discussions? Closes apache#6839 ### How was this PR tested? Revert of CI-only workflow changes; no product code is affected. Verified on the branch that `build.yml` restores the inline `codecov-action` upload steps for all flags (frontend, amber, amber-integration, the six platform services, pyamber, agent-service) and that `codecov-upload.yml` and `stage-codecov.sh` are gone. Fresh (non-carryforward) coverage is confirmed once this merges and the next PR's Codecov comment again shows its own flag with a delta. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8 [1M context])
apache#6852) ### What changes were proposed in this PR? Step 1 of a Codecov carryforward reset (see apache#6850). The deferred `workflow_run` upload experiment (apache#6730, apache#6824) — since reverted (apache#6840, apache#6837) — left Codecov's carryforward chain stuck: every PR renders its **own** flag as `Carriedforward` instead of a fresh delta, so a PR's real coverage change never surfaces (full diagnosis in apache#6839). Reverting the upload path alone does not un-stick the chain; the poisoned base report on `main` has to be flushed. This PR removes `flag_management.default_rules.carryforward` so the next full `main` build rebaselines Codecov against the current flag set, dropping the stale carried sessions. **This is intentionally temporary.** A follow-up PR restores `carryforward: true` once `main` has a clean full-build baseline. During the window, single-stack PRs may show non-run flags as `?` — expected, and the reason carryforward exists. Ordered operational steps (per Codecov's [reset guidance](https://community.codecov.com/t/remove-old-carryforward-flag/3428)): 1. Merge this PR. 2. Let a full build run on `main` (all 11 flags upload fresh; nothing carried). 3. Merge the follow-up PR re-enabling carryforward. 4. Rebase open PRs onto the new `main`. ### Any related issues, documentation, discussions? Closes apache#6850 ### How was this PR tested? Config-only change. Validated `codecov.yml` parses as valid YAML (`yaml.safe_load`) and that the top-level keys are now `ignore`, `coverage`, `comment` — the `flag_management` block is removed as intended. No behavior to unit-test; the effect is verifiable on `main` post-merge by confirming PR comments show fresh flag deltas again. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (claude-opus-4-8)
What changes were proposed in this PR?
Reverts #6824 ("ci: discover staged Codecov flags instead of a hardcoded matrix").
That change was part of the deferred
workflow_runCodecov upload, which regressed coverage reporting: every PR began showing its own flag asCarriedforwardinstead of a fresh delta, because moving the upload out of the Required Checks run lets Codecov build the PR comment from carryforward before the deferred upload lands (full analysis in #6839). This revert is the first step of backing the deferred approach out.Any related issues, documentation, discussions?
Related to #6839. This reverts #6824 only; #6730 — which introduced the deferred upload and removed the inline
codecov-actionsteps — also needs reverting to fully restore fresh coverage and resolve #6839.How was this PR tested?
Revert of a CI-only workflow change; no product code is affected, and
workflow_runworkflows do not run on a PR in any case. The workflow YAML is covered by the existing CI lint. End-to-end recovery is verified once the inline uploads are restored (via the #6730 revert) and a PR again shows a fresh, non-carryforward flag.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8 [1M context])