fix: prefer maintainer-authored issue in calculate_issue_multiplier#673
Merged
anderdc merged 4 commits intoentrius:testfrom Apr 22, 2026
Merged
Conversation
Picking valid_issues[0] made the multiplier depend on GraphQL closingIssuesReferences response ordering, which isn't documented as stable. A PR closing both a maintainer and a regular-author issue could flip between 1.66x and 1.33x run-to-run, and closing more valid issues could lower the multiplier. Now the maintainer-authored valid issue wins deterministically when one exists. Same-class determinism fix as entrius#633 / PR entrius#636 (label ordering).
Contributor
Author
|
Open to suggestions and feedback from @e35ventura and @anderdc. |
…us#680 Tests set closed_at before merged_at, which entrius#680 now treats as invalid. Flipping the hour offset matches GitHub's real behavior (closing-keyword PRs close linked issues at or after merge time).
anderdc
approved these changes
Apr 22, 2026
anderdc
added a commit
that referenced
this pull request
Apr 24, 2026
Audit found five real behavioral differences beyond the edited_after_merge gate scope. Each fix and why it matters: 1. calculate_open_pr_collateral_score crashes on ScoredMirrorPR (runtime bug) Legacy reads pr.number; ScoredMirrorPR exposes it only at pr.pr_number. Every miner with an OPEN mirror PR would crash at finalize time. Add .number and .repository_full_name property proxies to ScoredMirrorPR so the function accepts either type via duck-typing. 2. Eligibility gate runs at SCORE time in mirror, LOAD time in legacy Rejected merged PRs (self-merge w/o approval, base_ref mismatch, etc.) stayed in mirror_merged_prs with token_score=0, inflating len(merged_prs) inside check_eligibility and boosting credibility artificially. Moved _should_skip_merged_mirror_pr from score_mirror_pr to load's _maybe_add_pr, matching legacy's should_skip_merged_pr flow. Rejected PRs never enter the list now. 3. _is_valid_linked_issue used abs() for days_diff Legacy: `if days_diff > MAX or days_diff < 0: return False` — rejects issues closed BEFORE the PR merged (impossible for the PR to be the solver). Mirror's abs() made a -1 day diff look like +1 day and pass the MAX check. Replaced with signed days_diff and the < 0 check. 4. state_reason check was scoped to MERGED PRs only in mirror Legacy's _is_completed_when_closed applies regardless of PR state — a CLOSED issue with state_reason=NOT_PLANNED should never grant the issue multiplier, even for OPEN PR collateral. Pulled the state_reason gate out of the is_merged block. 5. _calculate_issue_multiplier took valid[0] without maintainer preference Legacy PR #673 (bc04ad9) updated calculate_issue_multiplier to prefer a maintainer-authored valid issue over response ordering. Mirror lagged. Applied the same next((m for ...), valid[0]) pattern. Also found gap #6 (not fixed in this commit, mirror-side change needed): The mirror API filters by p.created_at >= since, whereas legacy effectively uses merged_at >= lookback for MERGED PRs. A PR created >35d ago but merged within the lookback window is missed by mirror. Needs mirror SQL to `p.created_at >= since OR p.merged_at >= since`. Will draft as a separate plan for the mirror repo. Tests (+11 new): - TestEligibilityGateAtLoadTime: 3 tests (self-merge, base_ref mismatch, missing merged_at) all verifying rejected PRs never enter merged_prs - TestLinkedIssueValidity.test_issue_closed_before_pr_merged_blocks (abs fix) - TestLinkedIssueValidity.test_closed_issue_not_planned_blocks_open_pr_too (state_reason scope fix) - TestLinkedIssueValidity.test_open_issue_on_open_pr_still_valid (regression guard — OPEN issues on OPEN PRs should still pass) - TestIssueMultiplierPreference: 2 tests (prefer-maintainer + fallback) - TestCollateralScoreAcceptsScoredMirrorPR: 3 tests (no-crash, .number property, .repository_full_name property) Full suite: 632 passed.
8 tasks
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.
Picking valid_issues[0] made the multiplier depend on GraphQL closingIssuesReferences response ordering, which isn't documented as stable. A PR closing both a maintainer and a regular-author issue could flip between 1.66x and 1.33x run-to-run, and closing more valid issues could lower the multiplier.
Fixes #671