fix(validator): don't reject merged PRs with a missing base_ref#1619
Open
jaytbarimbao-collab wants to merge 1 commit into
Open
fix(validator): don't reject merged PRs with a missing base_ref#1619jaytbarimbao-collab wants to merge 1 commit into
jaytbarimbao-collab wants to merge 1 commit into
Conversation
check_merged_branch_eligibility gated the base_ref check with
`not branch_matches_pattern(base_ref or '', acceptable)`. When the mirror
omits base_ref on pre-backfill rows (base_ref is Optional[str] -> None),
`base_ref or ''` becomes '', which never matches the acceptable branch
set, so a valid MERGED PR is dropped at load time and never enters
merged_prs — earning no OSS contribution score despite counting toward
eligibility.
Gate only when base_ref is present, so a missing base_ref falls through
rather than false-positive-blocking. This matches:
- the head_ref check in the same function (`if acceptable and head_ref
and ...`), which already skips on a falsy head_ref;
- the function's own docstring ("missing head_ref/head_repo_full_name
skips the head_ref check rather than false-positive-blocking on older
data");
- issue discovery, which calls this same shared gate only when
`base_ref is not None` (issue_discovery/scan.py), covered by
test_missing_base_ref_falls_through_to_solved.
Add a regression test asserting a merged PR with base_ref=None falls
through the eligibility gate. A real non-matching base_ref is still
blocked (existing tests unchanged).
Fixes entrius#1599
Author
|
@anderdc @LandynDev fixes #1599 (bug, filed by @luciferlive112116). The shared |
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.
Summary
check_merged_branch_eligibilitygated the base_ref check withnot branch_matches_pattern(base_ref or '', acceptable). When the mirror omitsbase_refon pre-backfill rows (base_refisOptional[str]→None),base_ref or ''becomes'', which never matches the acceptable branch set — so a valid MERGED PR is dropped at load time and never entersmerged_prs, earning no OSS contribution score even though it still counts toward eligibility.The fix gates only when
base_refis present, so a missingbase_reffalls through rather than false-positive-blocking. This matches three existing precedents:if acceptable and head_ref and ...), which already skips on a falsyhead_ref;base_ref is not None(issue_discovery/scan.py), covered bytest_missing_base_ref_falls_through_to_solved.Related Issues
Fixes #1599
Type of Change
Testing
TestEligibilityGate::test_missing_base_ref_falls_through_not_blockedasserts a merged PR withbase_ref=Nonefalls through the gate. Fails ontestbefore the fix (blocked), passes after. A real non-matchingbase_refis still blocked (existing eligibility tests unchanged, all 21 green).ruff+pyrightclean.Checklist