fix(mirror): coerce null author_login so ghost-authored merged PRs aren't dropped#1632
Open
ebios-star wants to merge 1 commit into
Open
fix(mirror): coerce null author_login so ghost-authored merged PRs aren't dropped#1632ebios-star wants to merge 1 commit into
ebios-star wants to merge 1 commit into
Conversation
…en't dropped
MirrorPullRequest.author_login is typed `str`, but from_dict parsed it with
`data.get('author_login', '')`. That default only applies when the key is
absent — an explicit JSON `null` (which the mirror serializes for a
ghost/deleted GitHub author) passes straight through as `None`.
A `None` author_login then reaches the merged-PR eligibility gate, where
`_should_skip_merged_mirror_pr` does
`pr.merged_by_login.lower() == pr.author_login.lower()`. `None.lower()`
raises AttributeError, which `_maybe_add_pr`'s load-time `except Exception`
swallows — so the PR is silently dropped from `merged_prs` and the miner
loses credit for a legitimately merged contribution.
Coerce the null to '' the same way the sibling `review_summary or {}` /
`labels or []` fields in the same constructor already do, honouring the
declared `str` type. The other login-based model (MirrorIssue) is typed
`Optional[str]` and is intentionally left untouched.
Adds regression tests at both the parse boundary (null/absent -> '') and the
crash site (the self-merge gate no longer raises on a null author).
ebios-star
force-pushed
the
fix/mirror-null-author-login
branch
from
7b154a0 to
7743b16
Compare
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
1 participant
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.
Bug
MirrorPullRequest.author_loginis typedstr, butfrom_dictparses it withdata.get('author_login', ''). That default is only used when the key is absent — an explicit JSONnullreturnsNone, not''. The mirror serializesauthor_login: nullfor a ghost/deleted GitHub author, soNoneslips into a field the rest of the code treats as astr.That
Nonethen reaches the merged-PR eligibility gate in_should_skip_merged_mirror_pr:None.lower()raisesAttributeError. The crash is swallowed by the load-timeexcept Exceptionin_maybe_add_pr, so the PR is never appended tomerged_prs— a legitimately merged contribution is silently dropped, lowering that miner'smerged_count/ credibility.Fix
Coerce the null to
''exactly the way the siblingreview_summary or {}andlabels or []fields in the same constructor already handle an explicit null, honouring the declaredstrtype. A null author can't be a self-merge, so''correctly fails the self-merge comparison instead of crashing.The other login-bearing model,
MirrorIssue, is intentionally typedOptional[str]and is left untouched.Tests
test_mirror_models.py:author_loginnull / absent both parse to''.test_scoring.py: the self-merge gate no longer raises on a null-author merged PR (reproduces the swallowedAttributeErrorbefore the fix).Full suite green (967 passed);
ruff check,ruff format --check, andpyrightclean.