fix(issue-discovery): clear stale per-repo issue fields on the scoring path#1611
Open
kevin8600 wants to merge 1 commit into
Open
fix(issue-discovery): clear stale per-repo issue fields on the scoring path#1611kevin8600 wants to merge 1 commit into
kevin8600 wants to merge 1 commit into
Conversation
…g path _finalize_repo_issue_scores rewrote only the repos seen this round while _roll_up_issue_totals sums over every repo_evaluation, so a repo scored in a prior round but absent this round (e.g. on an OSS cache-restored evaluation) kept stale issue values that leaked into the round-level totals. The no-issues path already clears every repo via _clear_issue_discovery_fields; extract that per-repo reset into a shared helper and apply it on the scoring path too.
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
Issue discovery has two per-round reset paths that must clear a miner's per-repo issue fields identically, but they drifted:
_clear_issue_discovery_fields(no-issues path) zeroes the issue fields on everyrepo_evaluation._finalize_repo_issue_scores(scoring path) only rewrites the repos seen this round (sorted(set(repo_acc) | set(open_counts))), while_roll_up_issue_totalssums over allrepo_evaluations.So a repo scored in a prior round but absent this round keeps stale per-repo values that leak into the round-level totals.
Root cause. The only way a
repo_evaluationcarries prior-round issue data into a new round is the OSS cache-restore path: ongithub_pr_fetch_failed,store_or_use_cached_evaluationreplaces the eval with the cached one (which carriesrepo_evaluations). Issue discovery is expected to recompute fresh fields on top (see theforward.pycomment: "cached UIDs now have fresh issue-discovery fields"), but the scoring path never clears the repos it doesn't revisit, so_roll_up_issue_totalssums stale + fresh.Impact. Inflated per-repo issue rows (persisted via
bulk_store_evaluation) and round-leveltotal_solved_issues/issue_discovery_score/is_issue_eligible, which also get re-written to the evaluation cache (update_issue_discovery) and compound across rounds. Consumed by CLIminer scoreand analytics. Current-round emission allocation reads the freshly-rebuiltissue_discovery_issueslist, so this is a stored-data/reporting correctness bug, not an emissions change.Fix. Extract the per-repo reset into a shared
_reset_repo_issue_fieldshelper (removing the duplication that let the two paths drift) and call it at the top of_finalize_repo_issue_scores, mirroring_clear_issue_discovery_fields.Related Issues
Fixes #1610
Type of Change
Testing
Added
test_successful_scoring_clears_stale_issue_fields_for_repos_absent_this_round: pre-populates arepo_evaluationwith prior-round issue data (simulating the OSS cache restore), returns mirror issues only for a different enabled repo, and asserts the stale repo and the round-level roll-up are both zeroed. The test fails onmain(assert 7 == 0) and passes with the fix. Full issue-discovery suite green (73 passed);ruff,ruff format, andvultureclean.Checklist