ci: discover staged Codecov flags instead of a hardcoded matrix#6824
Merged
aglinxinyuan merged 3 commits intoJul 23, 2026
Merged
Conversation
Follow-up to apache#6730. The deferred Codecov Upload matrix hardcoded all 11 flags and tried to download each from the source run. On a label-gated PR (e.g. a frontend-only PR that only stages codecov-frontend) the other 10 legs hit "Artifact not found" — noise that looks like a name mismatch — and because the download used continue-on-error, a genuinely failed download was swallowed, the leg went green, and notify-failure never fired. Add a discover job that lists the source run's actual codecov-* artifacts and drives the upload matrix via fromJSON, and drop continue-on-error on the download. Now only the flags that were actually built are processed (no spurious errors), and a listed artifact that fails to download/upload fails the leg so notify-failure fires. Also removes the hardcoded flag list, so adding a platform service no longer needs a matching row here (build.yml comment updated). Harden the discover github-script to read the source run id from env rather than interpolating a workflow_dispatch-supplied run_id into the script body.
Contributor
Automated Reviewer SuggestionsBased on the
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the deferred Codecov Upload GitHub Actions workflow by dynamically discovering which codecov-* artifacts were actually produced by the upstream “Required Checks” run, then uploading only those flags—eliminating spurious “artifact not found” errors and ensuring real download/upload failures surface correctly.
Changes:
- Add a
discoverjob to listcodecov-*artifacts from the source run and drive the upload matrix viafromJSON. - Remove
continue-on-errorfrom artifact download so missing/failed downloads correctly fail the corresponding matrix leg and trigger failure notification. - Update
build.ymlcomments to reflect that Codecov flags are now discovered dynamically (no hardcoded matrix sync).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
.github/workflows/codecov-upload.yml |
Introduces discover job and switches upload matrix to dynamic flags; makes download failures fail loudly and adjusts failure notification dependencies. |
.github/workflows/build.yml |
Updates staging/upload comment to reflect dynamic Codecov artifact discovery (no manual flag matrix maintenance). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Per Copilot review on apache#6824: - Validate the source run id in the discover script and fail with a clear message instead of letting a non-numeric workflow_dispatch run_id become NaN and produce a confusing artifacts-API error. - Default the upload matrix to `[]` (fromJSON(flags || '[]')) so an empty-string output can never throw during matrix evaluation; the job `if` still skips the empty case.
Contributor
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🟢 | bs=10 sw=10 sl=64 | 481 | 0.293 | 20,605/27,906/27,906 us | 🟢 -7.4% / 🔴 +73.1% |
| 🔴 | bs=100 sw=10 sl=64 | 913 | 0.558 | 105,402/145,488/145,488 us | 🔴 +13.2% / 🔴 +35.5% |
| ⚪ | bs=1000 sw=10 sl=64 | 1,125 | 0.687 | 887,709/907,468/907,468 us | ⚪ within ±5% / 🟢 -14.1% |
Baseline details
Latest main 0ba5d1a from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 481 tuples/sec | 472 tuples/sec | 760.2 tuples/sec | +1.9% | -36.7% |
| bs=10 sw=10 sl=64 | MB/s | 0.293 MB/s | 0.288 MB/s | 0.464 MB/s | +1.7% | -36.9% |
| bs=10 sw=10 sl=64 | p50 | 20,605 us | 22,240 us | 12,781 us | -7.4% | +61.2% |
| bs=10 sw=10 sl=64 | p95 | 27,906 us | 30,107 us | 16,118 us | -7.3% | +73.1% |
| bs=10 sw=10 sl=64 | p99 | 27,906 us | 30,107 us | 19,074 us | -7.3% | +46.3% |
| bs=100 sw=10 sl=64 | throughput | 913 tuples/sec | 978 tuples/sec | 988.68 tuples/sec | -6.6% | -7.7% |
| bs=100 sw=10 sl=64 | MB/s | 0.558 MB/s | 0.597 MB/s | 0.603 MB/s | -6.5% | -7.5% |
| bs=100 sw=10 sl=64 | p50 | 105,402 us | 99,562 us | 101,448 us | +5.9% | +3.9% |
| bs=100 sw=10 sl=64 | p95 | 145,488 us | 128,498 us | 107,410 us | +13.2% | +35.5% |
| bs=100 sw=10 sl=64 | p99 | 145,488 us | 128,498 us | 120,258 us | +13.2% | +21.0% |
| bs=1000 sw=10 sl=64 | throughput | 1,125 tuples/sec | 1,131 tuples/sec | 1,023 tuples/sec | -0.5% | +10.0% |
| bs=1000 sw=10 sl=64 | MB/s | 0.687 MB/s | 0.69 MB/s | 0.624 MB/s | -0.4% | +10.0% |
| bs=1000 sw=10 sl=64 | p50 | 887,709 us | 883,915 us | 983,888 us | +0.4% | -9.8% |
| bs=1000 sw=10 sl=64 | p95 | 907,468 us | 923,691 us | 1,030,100 us | -1.8% | -11.9% |
| bs=1000 sw=10 sl=64 | p99 | 907,468 us | 923,691 us | 1,056,513 us | -1.8% | -14.1% |
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,416.05,200,128000,481,0.293,20604.62,27906.22,27906.22
1,100,10,64,20,2189.46,2000,1280000,913,0.558,105402.02,145487.57,145487.57
2,1000,10,64,20,17770.87,20000,12800000,1125,0.687,887709.45,907468.36,907468.36
Yicong-Huang
approved these changes
Jul 23, 2026
Address review (apache#6824): unlike the upload job's download (which fails hard so a real failure surfaces), notify-failure's download is only fetching a codecov-* artifact to read the PR number for the comment. Keep continue-on-error so a missing artifact degrades to "no comment" via the `if: pr != ''` guard instead of hard-failing notify-failure — which isn't a required check, so failing it would be invisible noise. Comment-only change.
aglinxinyuan
enabled auto-merge
July 23, 2026 09:02
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6824 +/- ##
=========================================
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:
|
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Jul 23, 2026
This was referenced Jul 23, 2026
Open
Mrudhulraj
pushed a commit
to Mrudhulraj/texera
that referenced
this pull request
Jul 23, 2026
… matrix" (apache#6837) ### What changes were proposed in this PR? Reverts apache#6824 ("ci: discover staged Codecov flags instead of a hardcoded matrix"). That change was part of the deferred `workflow_run` Codecov upload, which regressed coverage reporting: every PR began showing its **own** flag as `Carriedforward` instead 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 apache#6839). This revert is the first step of backing the deferred approach out. ### Any related issues, documentation, discussions? Related to apache#6839. This reverts apache#6824 only; apache#6730 — which introduced the deferred upload and removed the inline `codecov-action` steps — also needs reverting to fully restore fresh coverage and resolve apache#6839. ### How was this PR tested? Revert of a CI-only workflow change; no product code is affected, and `workflow_run` workflows 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 apache#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])
mengw15
pushed a commit
to mengw15/texeraFork
that referenced
this pull request
Jul 23, 2026
…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])
This was referenced Jul 24, 2026
Yicong-Huang
pushed a commit
to Yicong-Huang/texera
that referenced
this pull request
Jul 24, 2026
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)
renovate-bot
pushed a commit
to renovate-bot/apache-_-texera
that referenced
this pull request
Jul 24, 2026
…#6856) ### What changes were proposed in this PR? Step 2 (final) of the Codecov carryforward reset started in apache#6852. apache#6852 removed `flag_management.default_rules.carryforward` and let `main` rebuild once under the current flag set, flushing the stuck carryforward sessions left by the reverted deferred-upload experiment (apache#6730/apache#6824). This PR re-enables carryforward so single-stack PRs keep backfilling non-run flags from the base report. Net effect versus pre-apache#6852: **identical functional config** — the whole exercise leaves no lasting config change; it only rebaselines Codecov's carryforward data on `main`. **⚠️ Verification gate.** This PR's own Codecov comment is the A/B test for the reset: - apache#6852 (carryforward **off**) showed non-run flags as `?`. - This PR (carryforward **on**) must show each PR's own flag as a **fresh** delta again, and must NOT resurrect any stale flag as a `0.00%` carried-forward row. Do not merge until the Codecov comment here is confirmed clean. ### Any related issues, documentation, discussions? Follows apache#6852. Closes apache#6851. ### How was this PR tested? Config-only change. Validated `codecov.yml` parses as valid YAML and that the functional blocks (`flag_management`, `coverage`, `comment`) match the pre-apache#6852 configuration, restoring `carryforward: true`. Behavioral verification is the Codecov comment on this PR (see the verification gate above). ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (claude-opus-4-8)
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 changes were proposed in this PR?
Follow-up to #6730. The deferred
Codecov Uploadworkflow hardcoded all 11 flags in its matrix and tried to download each from the source run. On a label-gated PR — e.g. a frontend-only PR that only stagescodecov-frontend— the other 10 legs hit##[error] Artifact not found for name: codecov-<flag>, which looks like a name mismatch; and because the download usedcontinue-on-error: true, a genuinely failed download was swallowed, the leg went green, andnotify-failurenever fired.Fix: add a
discoverjob that lists the source run's actualcodecov-*artifacts and drives the upload matrix from that (fromJSON), and dropcontinue-on-erroron the download.notify-failurefires (previously it went silently green).platformservice no longer needs a matching matrix row here.Observed on the first live run after #6730 merged (run 29984668372, triggered by a frontend-only PR's checks):
frontenduploaded fine, but 10 legs loggedArtifact not foundfor the label-gated flags and the run still went green.Any related issues, documentation, discussions?
Follow-up to #6730 (reported on #6685).
How was this PR tested?
workflow_runonly activates from the copy of the file on the default branch, so this can't run on its own PR. Validated by YAML lint and an adversarial review of the discover → upload → notify flow: empty-flags cleanly skips (no failure, no notify), the dynamicfromJSONmatrix handles one or many flags, a download/upload failure now fails the leg and firesnotify-failure,amber-integration(results-only) skips the coverage step viahashFiles, and theworkflow_dispatchpath resolves the run id.workflow_dispatch(run_id) entry remains for manual post-merge validation against a finished Required Checks run.Was this PR authored or co-authored using generative AI tooling?
Yes.
Generated-by: Claude Code (Opus 4.8 [1M context])