Resolve patch coverage from Codecov first and name the source of every verdict - #4397
Merged
Merged
Conversation
DonOmalVindula
added
Type/Improvement
trigger-pr-builder
Contributor
|
Warning Review limit reached
Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe patch coverage gate now reads thresholds and ignore paths from ChangesCodecov patch coverage gate
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow
participant CodecovTarget
participant GitHubAPI
participant GateAction
participant ConfigReader
participant CoverableCheck
participant CodecovCheck
participant LocalCoverage
Workflow->>CodecovTarget: Resolve applicable head SHA
CodecovTarget->>GitHubAPI: Fetch merge-group pull request SHA
GitHubAPI-->>CodecovTarget: Return head SHA
CodecovTarget->>GateAction: Pass SHA and token
GateAction->>ConfigReader: Read Codecov configuration
ConfigReader-->>GateAction: Export FAIL_UNDER and ignore globs
GateAction->>CoverableCheck: List coverable changes
CoverableCheck-->>GateAction: Return coverable status
GateAction->>CodecovCheck: Poll codecov/patch check
CodecovCheck-->>GateAction: Return verdict or unresolved state
GateAction->>LocalCoverage: Compute local coverage if unresolved
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/actions/patch-coverage-gate/compute-local-patch-coverage.sh:
- Around line 60-68: Update the no-artifact fallback that determines COVERABLE
so it loads and applies the patterns from CODECOV_IGNORE_FILE before classifying
changed files, rather than relying only on hard-coded exclusions. Reuse the
existing EXCLUDES population and matching logic, ensuring ignored source-only
changes are treated as non-coverable when coverage artifacts are unavailable.
In @.github/actions/patch-coverage-gate/read-codecov-config.sh:
- Around line 38-41: Update the PyYAML fallback output in the `patch`
configuration parsing so `target` is replaced with an empty string only when it
is `None`, preserving numeric zero consistently with the `yq` path and
validator.
- Around line 35-46: Replace the eval-based configuration loading around the
Python heredoc with data-only parsing of its two output lines, so target and
threshold values from codecov.yml are assigned without shell re-evaluation.
Preserve the existing default handling for missing target and threshold, and
continue writing ignore globs through the Python block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 111b909c-c808-4cd0-98bf-d620183e1a26
📒 Files selected for processing (5)
.github/actions/patch-coverage-gate/action.yml.github/actions/patch-coverage-gate/check-codecov-api.sh.github/actions/patch-coverage-gate/compute-local-patch-coverage.sh.github/actions/patch-coverage-gate/read-codecov-config.sh.github/workflows/pr-builder.yml
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
DonOmalVindula
force-pushed
the
ci/gate-parity
branch
2 times, most recently
from
July 28, 2026 09:18
c05fbbf to
2233fa6
Compare
DonOmalVindula
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/actions/patch-coverage-gate/check-codecov-api.sh:
- Around line 61-70: Update the Codecov comparison validation near CODECOV_BASE
and MERGE_BASE to also extract and validate the response’s .head_commit against
HEAD_SHA. If the head commit is missing or differs, emit the existing fallback
warning and exit successfully so local patch coverage is used; retain the
current base and merge-base checks for valid matching comparisons.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: cbdf633b-f957-47ea-85c1-77c9289c7fc6
📒 Files selected for processing (6)
.github/actions/patch-coverage-gate/action.yml.github/actions/patch-coverage-gate/check-codecov-api.sh.github/actions/patch-coverage-gate/compute-local-patch-coverage.sh.github/actions/patch-coverage-gate/list-coverable-changes.sh.github/actions/patch-coverage-gate/read-codecov-config.sh.github/workflows/pr-builder.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/actions/patch-coverage-gate/action.yml
- .github/workflows/pr-builder.yml
12 tasks
DonOmalVindula
force-pushed
the
ci/gate-parity
branch
from
July 30, 2026 02:51
2233fa6 to
ef980e6
Compare
DonOmalVindula
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/actions/patch-coverage-gate/action.yml:
- Around line 60-67: Pass the gate’s expected base reference into the mirror
helper from the codecov-api step in
.github/actions/patch-coverage-gate/action.yml (lines 60-67), then update
mirror-codecov-check.sh (lines 32-53) to validate the Codecov comparison’s base
or base_commit against BASE_REF before mirroring; skip the verdict when the
bases do not match.
In @.github/actions/patch-coverage-gate/read-codecov-config.sh:
- Around line 30-46: Normalize Codecov ignore rules before local filtering:
update read-codecov-config.sh to distinguish directory-prefix, glob/fnmatch,
regex, and unsupported entries; update list-coverable-changes.sh to apply the
normalized compatible exclusions when selecting files; and update
compute-local-patch-coverage.sh to pass only compatible fnmatch/glob exclusions
to diff-cover, guarding or excluding unsupported regex syntax. Apply these
changes at .github/actions/patch-coverage-gate/read-codecov-config.sh:30-46,
.github/actions/patch-coverage-gate/list-coverable-changes.sh:18-20, and
.github/actions/patch-coverage-gate/compute-local-patch-coverage.sh:63-78.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 44eec152-530e-4521-99c8-18e7e8ca400e
📒 Files selected for processing (7)
.github/actions/patch-coverage-gate/action.yml.github/actions/patch-coverage-gate/check-codecov-api.sh.github/actions/patch-coverage-gate/compute-local-patch-coverage.sh.github/actions/patch-coverage-gate/list-coverable-changes.sh.github/actions/patch-coverage-gate/mirror-codecov-check.sh.github/actions/patch-coverage-gate/read-codecov-config.sh.github/workflows/pr-builder.yml
💤 Files with no reviewable changes (1)
- .github/actions/patch-coverage-gate/check-codecov-api.sh
DonOmalVindula
force-pushed
the
ci/gate-parity
branch
from
July 30, 2026 03:08
ef980e6 to
20fda22
Compare
DonOmalVindula
changed the title
DonOmalVindula
force-pushed
the
ci/gate-parity
branch
from
July 30, 2026 03:13
20fda22 to
3bcd20b
Compare
DonOmalVindula
added
trigger-pr-builder
DonOmalVindula
force-pushed
the
ci/gate-parity
branch
2 times, most recently
from
July 30, 2026 04:53
eae3ada to
2bf7565
Compare
DonOmalVindula
force-pushed
the
ci/gate-parity
branch
from
July 30, 2026 05:00
2bf7565 to
a581bfc
Compare
DonOmalVindula
enabled auto-merge
July 30, 2026 05:14
brionmario
approved these changes
Jul 30, 2026
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
2 participants
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.
Purpose
On #4404 the gate failed at 76.74% while
codecov/patchpassed with "Coverage not affected". Two verdicts for one diff is not acceptable, and the cause was that the gate read Codecov's compare API whilecodecov/patchreports from a check run. Those are different measurements:Refs #4268.
Approach
Patch coverage is now resolved from the most authoritative source available, in order, and every message names the source it came from.
Source 1: the
codecov/patchcheck run.mirror-codecov-check.shpolls the check run for the commit and returns Codecov's own conclusion, echoing its summary line ("91.70% of diff hit (target 80.00%)"). When Codecov reports, the gate is identical to Codecov by construction, because it is Codecov's verdict.A verdict is final. A failure here fails the gate rather than continuing to a more lenient source, otherwise a pull request Codecov rejected could pass on a fallback. Only the absence of a verdict after ten 30-second polls, or a cancelled or stale check, continues to source 2.
Source 2: the Codecov compare API.
check-codecov-api.shwaits for a complete report (six sessions, matchingnotify.after_n_builds) and enforces its patch total. It is still Codecov's data, so it is preferred over computing locally, but it is a different measurement from the check run, so it is only consulted when there is no verdict to mirror. When its comparison base is not this branch's merge base, the range difference is reported in the summary rather than hidden.Source 3: local computation.
compute-local-patch-coverage.shruns diff-cover over the archived coverage artifacts, pinned to this diff's base. Deterministic and always correctly scoped, but not identical to Codecov, so it is the last resort. It exists because Codecov does not always report at all: fork pull requests, merge queue commits and processing stalls leave the check pending indefinitely, which is why this gate was written.Before any source runs,
list-coverable-changes.shchecks whether the diff can affect coverage at all. If no file that reports coverage changed, the gate passes immediately without polling Codecov, and says so.Rules come from codecov.yml. The threshold was hardcoded as
fail-under: "79"in the workflow and the ignored paths were hardcoded again in the gate script.read-codecov-config.shnow derivesFAIL_UNDERfromcoverage.status.patch.default.targetminus.threshold, which is when Codecov passes a patch status, and feedsignoretodiff-cover --exclude. Globs travel via a file so they are never word split or filename expanded. It prefersyqand falls back to PyYAML, mirroring howbuild.shreads YAML, and fails loudly on a non-numeric target such asauto.Merge queue runs read the verdict from the head of the pull request the queue entry came from, taken out of the queue ref, because the queue commit is not what Codecov reported on. That is the premise
codecov-merge-queue-statusalready relies on.How #4404 behaves now
Its
codecov/patchcheck run issuccesswith "Coverage not affected when comparing 54a4882...045dbe1". Source 1 mirrors that and the gate passes, matching Codecov instead of contradicting it.Worth being explicit about the consequence: parity means inheriting Codecov's leniency. #4404 adds 578 lines and Codecov considers coverage unaffected, so the gate now passes it where the old compare-API number would have failed it. If untested new code should be blocked, that is a stricter policy than parity with
codecov/patchand needs a separate decision.Verification
The source chain was simulated against the composite's real
ifexpressions: no coverable files runs nothing; a source 1 verdict runs source 1 only; a silent source 1 continues to source 2; a silent source 2 downloads artifacts and computes locally. A failing verdict from source 1 or 2 exits non-zero and stops the composite.The query source 1 issues was run against #4404's head commit and returns
successwith Codecov's title, so the gate would pass that pull request. Against the realcodecov.ymlthe reader derivesFAIL_UNDER=79(80 − 1) and the same six globs that were hardcoded, so the fallback's rules are unchanged;target: autoand a missing file exit 1 with a clear message, and a missingthresholddefaults to 0. The chain was exercised againstdiff-cover==10.4.0in a scratch repository: the globs arrive as one--excludeflag with the patterns intact, the derived threshold is enforced, and a change underdocs/is ignored.On the accuracy of source 3: replaying PR 4371's six real coverage artifacts through the gate's own conversion and diff-cover invocation, against that pull request's actual merge base, gives 93.08% (242 of 260 lines) where
codecov/patchreports 91.70%. The uncovered count matches Codecov exactly (35 lines = 25 misses + 10 partials) when scoped to Codecov's own base, so the coverage math agrees; the residual gap is in which lines reach the denominator, most likely partial-line handling. That gap is the reason source 3 is last, and the reason the summary always names the source.Related Issues
Checklist
breaking changelabel added.Security checks