From c76b4456b6e35381c33606183a94c00a1a15824b Mon Sep 17 00:00:00 2001 From: YB0y Date: Thu, 30 Apr 2026 18:30:26 +0200 Subject: [PATCH] fix(validator): handle unavailable mirror scoring data Treat scoring_data_stored=False as an availability signal instead of a valid zero-score result in both mirror scoring paths. For issue discovery, solving PR file data that is unavailable now returns None and stays out of the per-cycle cache so later lookups can retry instead of hitting a cached zero. For OSS contributions, merged mirror PRs that cannot be scored are removed before finalization so they cannot inflate credibility or rescue reward eligibility. Closes #836 Closes #904 --- .../validator/issue_discovery/mirror_scan.py | 8 + .../oss_contributions/mirror/scoring.py | 22 +- .../issue_discovery/test_mirror_scan.py | 57 +++++- .../test_scoring_data_stored_eligibility.py | 190 ++++++++++++++++++ 4 files changed, 268 insertions(+), 9 deletions(-) create mode 100644 tests/validator/oss_contributions/mirror/test_scoring_data_stored_eligibility.py diff --git a/gittensor/validator/issue_discovery/mirror_scan.py b/gittensor/validator/issue_discovery/mirror_scan.py index 8fdb7eb80..bf4e89e2f 100644 --- a/gittensor/validator/issue_discovery/mirror_scan.py +++ b/gittensor/validator/issue_discovery/mirror_scan.py @@ -408,6 +408,14 @@ def _resolve_solving_pr_score( ) return None + if not files_response.scoring_data_stored: + cache_stats.fetch_failures += 1 + bt.logging.warning( + f'Mirror scoring data unavailable for solving PR #{solving_pr.pr_number} ' + f'({issue.repo_full_name}) — issue #{issue.issue_number} not scored' + ) + return None + file_changes, file_contents = mirror_files_to_legacy( issue.repo_full_name, solving_pr.pr_number, files_response.files ) diff --git a/gittensor/validator/oss_contributions/mirror/scoring.py b/gittensor/validator/oss_contributions/mirror/scoring.py index 487a2383b..dc73af3cc 100644 --- a/gittensor/validator/oss_contributions/mirror/scoring.py +++ b/gittensor/validator/oss_contributions/mirror/scoring.py @@ -93,11 +93,17 @@ def score_mirror_miner_prs( ] for label, scored_prs in pr_groups: + scored_merged_prs: List[ScoredMirrorPR] = [] for i, scored in enumerate(scored_prs, start=1): bt.logging.info( f'\n[{i}/{len(scored_prs)}] {label} PR #{scored.pr.pr_number} in {scored.pr.repo_full_name}' ) - score_mirror_pr(scored, mirror_eval, mirror_repos, programming_languages, token_config, client) + was_scored = score_mirror_pr(scored, mirror_eval, mirror_repos, programming_languages, token_config, client) + if label == 'MERGED' and was_scored: + scored_merged_prs.append(scored) + + if label == 'MERGED': + scored_prs[:] = scored_merged_prs # ============================================================================ @@ -112,13 +118,13 @@ def score_mirror_pr( programming_languages: Dict[str, LanguageConfig], token_config: TokenConfig, client: MirrorClient, -) -> None: - """Score a single mirror PR. Populates ScoredMirrorPR scoring fields in place.""" +) -> bool: + """Score a single mirror PR. Returns True when scoring fields were populated.""" pr = scored.pr repo_config = mirror_repos.get(pr.repo_full_name) if not repo_config: bt.logging.warning(f'{pr.repo_full_name} not in mirror_repos. Skipping...') - return + return False # Eligibility gate for MERGED PRs already ran at LOAD time (see # load_mirror_miner_prs → _should_skip_merged_mirror_pr). By this point @@ -128,19 +134,19 @@ def score_mirror_pr( # Mirror signals it has no stored files for this PR (pending backfill, in-flight # file job, etc.) — skip the round trip. if not pr.scoring_data_stored: - return + return False # Fetch file contents via the mirror's lazy /pulls/.../files endpoint. try: files = client.get_pr_files(pr.repo_full_name, pr.pr_number).files except MirrorRequestError as e: bt.logging.warning(f'Mirror file fetch failed for PR #{pr.pr_number}: {e}') - return + return False scored.files = files if not files: bt.logging.warning(f'No files returned for PR #{pr.pr_number}') - return + return False file_changes, file_contents = mirror_files_to_legacy(pr.repo_full_name, pr.pr_number, files) @@ -154,6 +160,8 @@ def score_mirror_pr( # — score sets per-PR state, finalize rolls up eval-level totals across # both paths). + return True + # ============================================================================ # Eligibility gate (MERGED PRs) diff --git a/tests/validator/issue_discovery/test_mirror_scan.py b/tests/validator/issue_discovery/test_mirror_scan.py index feba79d94..6a5dd2abe 100644 --- a/tests/validator/issue_discovery/test_mirror_scan.py +++ b/tests/validator/issue_discovery/test_mirror_scan.py @@ -85,7 +85,9 @@ def _scored_mirror_pr( return scored -def _empty_files_response(repo: str, pr_number: int) -> MirrorPullRequestFilesResponse: +def _empty_files_response( + repo: str, pr_number: int, scoring_data_stored: bool = True +) -> MirrorPullRequestFilesResponse: return MirrorPullRequestFilesResponse.from_dict( { 'repo_full_name': repo, @@ -93,7 +95,7 @@ def _empty_files_response(repo: str, pr_number: int) -> MirrorPullRequestFilesRe 'head_sha': 'h', 'base_sha': 'b', 'merge_base_sha': 'mb', - 'scoring_data_stored': True, + 'scoring_data_stored': scoring_data_stored, 'files': [], } ) @@ -609,6 +611,57 @@ def test_resolve_increments_fetch_failures_on_request_error(self): # Failed lookups are NOT cached (so a retry is possible) assert cache == {} + def test_resolve_skips_caching_when_scoring_data_unavailable(self): + from gittensor.validator.issue_discovery.mirror_scan import ( + _CacheStats, + _resolve_solving_pr_score, + ) + + client = Mock() + client.get_pr_files.return_value = _empty_files_response('entrius/gittensor-ui', 100, scoring_data_stored=False) + cache = {} + stats = _CacheStats() + + issue = MirrorIssue.from_dict(_issue_dict()) + assert issue.solving_pr is not None + result = _resolve_solving_pr_score( + issue, issue.solving_pr, cache, stats, client, _EMPTY_LANGS, _EMPTY_TOKEN_CONFIG + ) + + assert result is None + assert cache == {} + assert stats.hits == 0 + assert stats.misses == 1 + assert stats.fetch_failures == 1 + + def test_unavailable_scoring_data_does_not_poison_cross_miner_cache(self): + from gittensor.validator.issue_discovery.mirror_scan import ( + _CacheStats, + _resolve_solving_pr_score, + ) + + client = Mock() + client.get_pr_files.return_value = _empty_files_response('entrius/gittensor-ui', 100, scoring_data_stored=False) + cache = {} + stats = _CacheStats() + + first_issue = MirrorIssue.from_dict(_issue_dict(issue_number=51)) + second_issue = MirrorIssue.from_dict(_issue_dict(issue_number=52)) + assert first_issue.solving_pr is not None and second_issue.solving_pr is not None + + first = _resolve_solving_pr_score( + first_issue, first_issue.solving_pr, cache, stats, client, _EMPTY_LANGS, _EMPTY_TOKEN_CONFIG + ) + second = _resolve_solving_pr_score( + second_issue, second_issue.solving_pr, cache, stats, client, _EMPTY_LANGS, _EMPTY_TOKEN_CONFIG + ) + + assert first is None and second is None + assert cache == {} + assert stats.misses == 2 + assert stats.fetch_failures == 2 + assert client.get_pr_files.call_count == 2 + class TestOpenIssueSpamSourceIsMirror: """The open-issue spam multiplier sources its count from mirror's response, diff --git a/tests/validator/oss_contributions/mirror/test_scoring_data_stored_eligibility.py b/tests/validator/oss_contributions/mirror/test_scoring_data_stored_eligibility.py new file mode 100644 index 000000000..e1d856bab --- /dev/null +++ b/tests/validator/oss_contributions/mirror/test_scoring_data_stored_eligibility.py @@ -0,0 +1,190 @@ +from unittest.mock import Mock + +import pytest + +classes = pytest.importorskip('gittensor.classes') +mirror_combine = pytest.importorskip('gittensor.validator.oss_contributions.mirror.combine') +mirror_client_module = pytest.importorskip('gittensor.utils.mirror.client') +mirror_eval_module = pytest.importorskip('gittensor.validator.oss_contributions.mirror.evaluation') +mirror_models = pytest.importorskip('gittensor.utils.mirror.models') +mirror_scoring = pytest.importorskip('gittensor.validator.oss_contributions.mirror.scoring') +reward_module = pytest.importorskip('gittensor.validator.oss_contributions.normalize') +scoring_module = pytest.importorskip('gittensor.validator.oss_contributions.scoring') +scored_pr_module = pytest.importorskip('gittensor.validator.oss_contributions.mirror.scored_pr') +load_weights = pytest.importorskip('gittensor.validator.utils.load_weights') + +MinerEvaluation = classes.MinerEvaluation +MirrorFile = mirror_models.MirrorFile +MirrorMinerEvaluation = mirror_eval_module.MirrorMinerEvaluation +MirrorPullRequest = mirror_models.MirrorPullRequest +MirrorRequestError = mirror_client_module.MirrorRequestError +RepositoryConfig = load_weights.RepositoryConfig +ScoredMirrorPR = scored_pr_module.ScoredMirrorPR +combine = mirror_combine.combine +finalize_miner_scores = scoring_module.finalize_miner_scores +normalize_rewards_linear = reward_module.normalize_rewards_linear +score_mirror_miner_prs = mirror_scoring.score_mirror_miner_prs + + +def _pr( + pr_number: int, + github_id: str, + repo: str, + state: str = 'MERGED', + scoring_data_stored: bool = True, +) -> MirrorPullRequest: + return MirrorPullRequest.from_dict( + { + 'repo_full_name': repo, + 'pr_number': pr_number, + 'title': f'PR {pr_number}', + 'body': 'b', + 'state': state, + 'author_github_id': github_id, + 'author_login': f'user-{github_id}', + 'author_association': 'CONTRIBUTOR', + 'created_at': '2026-04-15T00:00:00Z', + 'closed_at': '2026-04-18T10:00:00Z' if state in ('CLOSED', 'MERGED') else None, + 'merged_at': '2026-04-18T10:00:00Z' if state == 'MERGED' else None, + 'last_edited_at': None, + 'edited_after_merge': False, + 'hours_since_merge': 1.0 if state == 'MERGED' else None, + 'merged_by_login': 'maintainer' if state == 'MERGED' else None, + 'base_ref': 'main', + 'head_ref': f'feature/{pr_number}', + 'head_repo_full_name': repo, + 'default_branch': 'main', + 'head_sha': 'h', + 'base_sha': 'b', + 'merge_base_sha': 'mb', + 'additions': 1, + 'deletions': 0, + 'commits_count': 1, + 'scoring_data_stored': scoring_data_stored, + 'review_summary': { + 'maintainer_changes_requested_count': 0, + 'changes_requested_count': 0, + 'approved_count': 1, + 'commented_count': 0, + }, + 'labels': [], + 'linked_issues': [], + } + ) + + +def _file() -> MirrorFile: + return MirrorFile.from_dict( + { + 'filename': 'src/code.py', + 'previous_filename': None, + 'status': 'modified', + 'additions': 1, + 'deletions': 0, + 'changes': 1, + 'is_binary': False, + 'head_content': 'x = 2\n', + 'base_content': 'x = 1\n', + } + ) + + +def _mirror_eval( + uid: int, + github_id: str, + repo: str, + scored_merged: int, + unavailable_merged: int = 0, + closed: int = 0, +) -> MirrorMinerEvaluation: + eval_ = MirrorMinerEvaluation(uid=uid, hotkey=f'hk{uid}', github_id=github_id) + eval_.merged_prs = [ + ScoredMirrorPR(pr=_pr(i, github_id, repo, scoring_data_stored=True)) for i in range(1, scored_merged + 1) + ] + eval_.merged_prs.extend( + ScoredMirrorPR(pr=_pr(i, github_id, repo, scoring_data_stored=False)) + for i in range(scored_merged + 1, scored_merged + unavailable_merged + 1) + ) + eval_.closed_prs = [ + ScoredMirrorPR(pr=_pr(100 + i, github_id, repo, state='CLOSED', scoring_data_stored=False)) + for i in range(closed) + ] + return eval_ + + +def test_unscored_mirror_merged_prs_do_not_rescue_credibility_gate(monkeypatch): + def fake_base_score(scored, _file_changes, _file_contents, _programming_languages, _token_config): + scored.token_score = 10.0 + return 100.0 + + monkeypatch.setattr(mirror_scoring, '_calculate_base_score', fake_base_score) + + client = Mock() + client.get_pr_files.return_value = Mock(files=[_file()]) + + mirror_repos = { + 'entrius/gittensor-ui': RepositoryConfig(weight=0.5, mirror_enabled=True), + 'entrius/gittensor': RepositoryConfig(weight=1.0, mirror_enabled=True), + } + + affected_mirror = _mirror_eval( + uid=7, + github_id='700', + repo='entrius/gittensor-ui', + scored_merged=5, + unavailable_merged=3, + closed=3, + ) + control_mirror = _mirror_eval( + uid=8, + github_id='800', + repo='entrius/gittensor', + scored_merged=5, + ) + + score_mirror_miner_prs(affected_mirror, mirror_repos, {}, Mock(), client=client) + score_mirror_miner_prs(control_mirror, mirror_repos, {}, Mock(), client=client) + + affected = MinerEvaluation(uid=7, hotkey='hk7', github_id='700') + control = MinerEvaluation(uid=8, hotkey='hk8', github_id='800') + combine(affected, affected_mirror) + combine(control, control_mirror) + + miner_evals = {7: affected, 8: control} + finalize_miner_scores(miner_evals) + rewards = normalize_rewards_linear(miner_evals) + + assert affected.is_eligible is False + assert affected.credibility == pytest.approx(5 / 7) + assert affected.total_score == 0.0 + assert control.is_eligible is True + assert rewards[7] == 0.0 + assert rewards[8] == 1.0 + + +@pytest.mark.parametrize('mode', ['scoring_data_unavailable', 'fetch_error', 'empty_files']) +def test_unscored_merged_prs_are_removed_before_finalization(mode): + mirror_eval = _mirror_eval( + uid=7, + github_id='700', + repo='entrius/gittensor-ui', + scored_merged=1, + ) + client = Mock() + + if mode == 'scoring_data_unavailable': + mirror_eval.merged_prs[0].pr.scoring_data_stored = False + elif mode == 'fetch_error': + client.get_pr_files.side_effect = MirrorRequestError('boom') + else: + client.get_pr_files.return_value = Mock(files=[]) + + score_mirror_miner_prs( + mirror_eval, + {'entrius/gittensor-ui': RepositoryConfig(weight=0.5, mirror_enabled=True)}, + {}, + Mock(), + client=client, + ) + + assert mirror_eval.merged_prs == []