Skip to content

Conversation

@0xsatoshi99
Copy link
Contributor

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

  • Prevents validator crashes when PRs have no file changes
  • Ensures correct uniqueness calculation for all valid PRs
  • Minimal change: 2 lines moved, 2 lines of comments added
  • Zero breaking changes

Contribution by Gittensor, learn more at https://gittensor.io/

@0xsatoshi99
Copy link
Contributor Author

@LandynDev , @SmartDever02 Can you plz check this PR?

@anderdc
Copy link
Collaborator

anderdc commented Dec 5, 2025

your PRs are not pointed to the test branch. These will not get merged unless that is fixed

@0xsatoshi99 0xsatoshi99 changed the base branch from main to test December 5, 2025 23:42
Copy link
Collaborator

@LandynDev LandynDev left a 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).
@0xsatoshi99 0xsatoshi99 force-pushed the fix/unique-repos-keyerror branch from aee7a58 to 4ef6654 Compare December 16, 2025 00:20
@0xsatoshi99
Copy link
Contributor Author

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]?

Fixed as what you want, Please check again. Thanks.

Copy link
Collaborator

@LandynDev LandynDev left a 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?

@LandynDev
Copy link
Collaborator

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 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants