-
Notifications
You must be signed in to change notification settings - Fork 24
fix: move unique_repos tracking before file changes check to prevent KeyError #71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: test
Are you sure you want to change the base?
Conversation
|
@LandynDev , @SmartDever02 Can you plz check this PR? |
|
your PRs are not pointed to the |
LandynDev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You raise a good point here. I'd actually prefer a fix more directly in the miner_eval.add_pull_request helper function. Can we move it there so that when a PR is added via this helper function, it just automatically appends to our unique_repos_contributed_to: Set[str]?
Move repository tracking from scoring.py to MinerEvaluation.add_pull_request() so that unique_repos_contributed_to is automatically populated when PRs are added via the helper function. This prevents KeyError in apply_cross_miner_multipliers_and_finalize when calculating uniqueness multipliers, as repos are now tracked immediately upon PR addition rather than during scoring (which may skip PRs with no file changes).
aee7a58 to
4ef6654
Compare
Fixed as what you want, Please check again. Thanks. |
LandynDev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove some of these comments? We don't need to be maxing the diff out, like this is super verbose and unnecessary for a simple helper function:
Automatically tracks the repository in unique_repos_contributed_to
to prevent KeyError in apply_cross_miner_multipliers_and_finalize
when calculating uniqueness multipliers.
So can we restore the doc string of the add_pull_request function?
Then im ready to GTM 👍 |
Problem
Fixes #66
The apply_cross_miner_multipliers_and_finalize function crashes with KeyError when a PR has no file changes.
Root Cause
In score_pull_requests, the line:
miner_eval.unique_repos_contributed_to.add(pr.repository_full_name)
was placed AFTER the continue statement that skips PRs with no file changes. This means PRs without file changes never get their repository added to unique_repos_contributed_to.
Later, apply_cross_miner_multipliers_and_finalize tries to access repo_counts[pr.repository_full_name] which raises KeyError because the repo was never counted.
Fix
Move unique_repos_contributed_to.add() to the beginning of the loop, before the file changes check. This ensures ALL valid PRs contribute to repository uniqueness regardless of whether they have file changes.
Impact
Contribution by Gittensor, learn more at https://gittensor.io/