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'}]))