From c3604007e5b40143ab966e5332d8351459b850e3 Mon Sep 17 00:00:00 2001 From: jaytbarimbao-collab <300663773+jaytbarimbao-collab@users.noreply.github.com> Date: Fri, 10 Jul 2026 15:15:37 -0400 Subject: [PATCH] fix(scoring): count merged PR repo attribution before mirror file-fetch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit score_pr() only added a MERGED PR's repo to eval_.unique_repos_contributed_to at the end of the function, after three early-return paths that fire whenever the mirror's file-diff data isn't available yet (pending backfill, a transient MirrorRequestError, or an empty files result) — all explicitly normal transient states per the function's own comments. Since a PR is already counted toward eligibility/total_merged_prs independent of file-fetch status, this let unique_repos_contributed_to (persisted as unique_repos_count, surfaced via `gitt miner score`) silently undercount based on mirror-fetch timing rather than any property of the PR. Move the MERGED repo-attribution above the early-return branches so it no longer depends on this round's file-fetch outcome. Same bug class previously fixed in the pre-mirror scoring path (#65/#71), reintroduced when scoring moved to the mirror-based flow. Closes #1616 --- .../oss_contributions/mirror/scoring.py | 13 ++- .../oss_contributions/mirror/test_scoring.py | 89 +++++++++++++++++++ 2 files changed, 98 insertions(+), 4 deletions(-) diff --git a/gittensor/validator/oss_contributions/mirror/scoring.py b/gittensor/validator/oss_contributions/mirror/scoring.py index f0256434..ba16ac35 100644 --- a/gittensor/validator/oss_contributions/mirror/scoring.py +++ b/gittensor/validator/oss_contributions/mirror/scoring.py @@ -133,6 +133,13 @@ async def score_pr( has_fixed_base = repo_config.fixed_base_score is not None scoring_cfg = resolve_scoring(repo_config.scoring) + # Repo attribution for a merged PR must not depend on this round's mirror + # file-fetch outcome (pending backfill, transient fetch error, empty diff) — + # those are normal transient states, not a property of the PR itself. Set + # this before any early return below. See #1616. + if pr.state == 'MERGED': + eval_.unique_repos_contributed_to.add(pr.repo_full_name) + # Mirror signals it has no stored files for this PR (pending backfill, in-flight # file job, etc.) — skip the round trip unless the repo explicitly supplies # a fixed base score. @@ -182,10 +189,8 @@ async def score_pr( _calculate_pr_multipliers(scored, repo_config, scoring_cfg) - if pr.state == 'MERGED': - eval_.unique_repos_contributed_to.add(pr.repo_full_name) - # Token totals are aggregated later in finalize_miner_scores; this - # function only sets per-PR state. + # Token totals are aggregated later in finalize_miner_scores; this + # function only sets per-PR state. # ============================================================================ diff --git a/tests/validator/oss_contributions/mirror/test_scoring.py b/tests/validator/oss_contributions/mirror/test_scoring.py index 10160023..697c6212 100644 --- a/tests/validator/oss_contributions/mirror/test_scoring.py +++ b/tests/validator/oss_contributions/mirror/test_scoring.py @@ -21,6 +21,8 @@ import pytest +classes_module = pytest.importorskip('gittensor.classes') +client_module = pytest.importorskip('gittensor.utils.mirror.client') scoring_module = pytest.importorskip( 'gittensor.validator.oss_contributions.mirror.scoring', reason='Requires gittensor mirror subpackage', @@ -46,6 +48,8 @@ MirrorLinkedIssue = mirror_models.MirrorLinkedIssue MirrorFile = mirror_models.MirrorFile RepositoryConfig = load_weights.RepositoryConfig +MinerEvaluation = classes_module.MinerEvaluation +MirrorRequestError = client_module.MirrorRequestError def _pr( @@ -383,6 +387,91 @@ def test_fixed_base_score_scores_without_stored_files(self): assert scored.base_score == pytest.approx(7.5) +class TestUniqueReposContributedTo: + """A MERGED PR must count toward eval_.unique_repos_contributed_to regardless of + this round's mirror file-fetch outcome — see #1616.""" + + def _eval(self) -> MinerEvaluation: + return MinerEvaluation(uid=1, hotkey='5F' + 'a' * 46) + + def test_counts_when_scoring_data_not_stored(self): + scored = ScoredPR(pr=_pr(state='MERGED')) + scored.pr.scoring_data_stored = False + eval_ = self._eval() + client = Mock() + + asyncio.run( + score_pr( + scored, + eval_=eval_, + master_repositories={scored.pr.repo_full_name: _config()}, + programming_languages={}, + token_config=Mock(), + client=client, + ) + ) + + client.get_pr_files.assert_not_called() + assert scored.pr.repo_full_name in eval_.unique_repos_contributed_to + + def test_counts_when_mirror_fetch_fails(self): + scored = ScoredPR(pr=_pr(state='MERGED')) + eval_ = self._eval() + client = Mock() + client.get_pr_files.side_effect = MirrorRequestError('mirror unavailable') + + asyncio.run( + score_pr( + scored, + eval_=eval_, + master_repositories={scored.pr.repo_full_name: _config()}, + programming_languages={}, + token_config=Mock(), + client=client, + ) + ) + + assert scored.pr.repo_full_name in eval_.unique_repos_contributed_to + + def test_counts_when_no_files_returned(self): + scored = ScoredPR(pr=_pr(state='MERGED')) + eval_ = self._eval() + client = Mock() + client.get_pr_files.return_value.files = [] + + asyncio.run( + score_pr( + scored, + eval_=eval_, + master_repositories={scored.pr.repo_full_name: _config()}, + programming_languages={}, + token_config=Mock(), + client=client, + ) + ) + + assert scored.pr.repo_full_name in eval_.unique_repos_contributed_to + + def test_not_counted_for_closed_pr(self): + scored = ScoredPR(pr=_pr(state='CLOSED')) + scored.pr.scoring_data_stored = False + eval_ = self._eval() + client = Mock() + + asyncio.run( + score_pr( + scored, + eval_=eval_, + master_repositories={scored.pr.repo_full_name: _config()}, + programming_languages={}, + token_config=Mock(), + client=client, + ) + ) + + assert scored.pr.repo_full_name not in eval_.unique_repos_contributed_to + + class TestFixedBaseScore: def test_fixed_base_replaces_token_base_but_keeps_token_breakdown_and_multipliers(self, monkeypatch): scored = ScoredPR(pr=_pr(labels=[{'name': 'feature', 'actor_github_id': '1', 'actor_association': 'OWNER'}]))