diff --git a/gittensor/validator/issue_discovery/scan.py b/gittensor/validator/issue_discovery/scan.py index c30a12e3..729307b8 100644 --- a/gittensor/validator/issue_discovery/scan.py +++ b/gittensor/validator/issue_discovery/scan.py @@ -463,10 +463,6 @@ async def _score_miner_issues( ) continue - # Valid-solved gate: solving PR must meet the repo's token threshold. - if cached.token_score >= cfg.min_token_score_for_valid_issue: - acc.valid_solved += 1 - # Same-account: discoverer == solver gets credibility only, no score if issue.author_github_id == solving_pr.author_github_id: bt.logging.debug( @@ -494,6 +490,12 @@ async def _score_miner_issues( ) continue + # Past every credibility-only gate (same-account, canonical one-issue- + # per-PR, token threshold). Non-canonical siblings would have continued + # above; counting them here would let one qualifying PR satisfy the + # valid-solved eligibility gate through duplicate issue rows. + acc.valid_solved += 1 + adapted = _mirror_issue_for_scoring(issue, solving_pr, repo_config, base_score=cached.base_score) if adapted is None: continue diff --git a/tests/validator/issue_discovery/test_scan.py b/tests/validator/issue_discovery/test_scan.py index bc994c67..5296ca05 100644 --- a/tests/validator/issue_discovery/test_scan.py +++ b/tests/validator/issue_discovery/test_scan.py @@ -307,6 +307,8 @@ def test_self_issue_counts_credibility_but_no_score(self): ) ) assert eval_.total_solved_issues == 1 # credibility counts + # Same-account is credibility-only — must not satisfy the valid-solved gate. + assert eval_.total_valid_solved_issues == 0 # But no discovery_earned_score because self-solve assert eval_.issue_discovery_score == 0 @@ -1020,12 +1022,21 @@ def test_all_mirror_miner_below_threshold_passes_spam(self): ) for i in range(2) ] - solved_issues = [_issue_dict(issue_number=300 + i, author_github_id=f'discoverer{i}') for i in range(8)] + # Give each solved issue its own solving PR so each is canonical and + # contributes to valid_solved. Sharing one PR across all eight would + # leave seven non-canonical siblings credibility-only post-#1269 and + # the miner would fall below MIN_VALID_SOLVED_ISSUES, defeating the + # spam-mult assertion this test exists to make. + solved_issues = [ + _issue_dict(issue_number=300 + i, author_github_id=f'discoverer{i}', solved_by_pr=300 + i) for i in range(8) + ] client = Mock() client.get_miner_issues.return_value = _response(open_issues + solved_issues) eval_ = _eval() - eval_.merged_prs = [_scored_mirror_pr('entrius/gittensor-ui', 100, token_score=100.0)] + eval_.merged_prs = [ + _scored_mirror_pr('entrius/gittensor-ui', pr_number, token_score=100.0) for pr_number in range(300, 308) + ] _run( run_issue_discovery( @@ -1203,11 +1214,18 @@ def _per_miner(github_id, since_by_repo=None): ) ) - # Both miners count the shared-PR issue toward credibility. + # Both miners count the shared-PR issue toward credibility, but only + # the canonical owner (A) counts it toward the valid-solved gate. + # B's shared-PR issue is one-issue-per-PR credibility-only, so it must + # not increment valid_solved — preventing one qualifying PR from + # padding the gate via duplicate issue rows (#1269). assert e_a.total_solved_issues == 7 assert e_b.total_solved_issues == 7 assert e_a.total_valid_solved_issues == 7 - assert e_b.total_valid_solved_issues == 7 + assert e_b.total_valid_solved_issues == 6 + # Both stay above MIN_VALID_SOLVED_ISSUES (currently 3) and remain + # eligible. The orthogonal eligibility-gate assertion lives in + # ``test_non_canonical_siblings_do_not_satisfy_valid_solved_gate``. assert e_a.is_issue_eligible assert e_b.is_issue_eligible @@ -1222,6 +1240,103 @@ def _per_miner(github_id, since_by_repo=None): assert e_a.issue_discovery_score > e_b.issue_discovery_score > 0 assert e_a.issue_discovery_score / e_b.issue_discovery_score == pytest.approx(7 / 6, rel=1e-2) + def test_non_canonical_siblings_do_not_satisfy_valid_solved_gate(self): + """Regression for #1269: seven sibling issues all closed by one + qualifying PR must count as one canonical valid solved issue, not + seven. Otherwise a single solving PR could satisfy the per-repo + MIN_VALID_SOLVED_ISSUES eligibility gate through duplicate rows.""" + client = Mock() + issues = [ + _issue_dict( + issue_number=50 + i, + author_github_id='A', + solved_by_pr=100, + solving_pr_author='SOLVER', + created_at=f'2026-04-{i + 1:02d}T00:00:00Z', + ) + for i in range(7) + ] + client.get_miner_issues.return_value = _response(issues) + + eval_ = _eval(uid=1, github_id='A') + seed = MinerEvaluation(uid=99, hotkey='hkS', github_id='SEED') + seed.merged_prs = [_scored_mirror_pr('entrius/gittensor-ui', 100)] + + _run( + run_issue_discovery( + {1: eval_, 99: seed}, + _mirror_repos('entrius/gittensor-ui'), + _EMPTY_LANGS, + _EMPTY_TOKEN_CONFIG, + client=client, + ) + ) + + # All seven count for credibility (solved/closed) but only the canonical + # owner increments valid_solved — so the miner remains ineligible. + assert eval_.total_solved_issues == 7 + assert eval_.total_valid_solved_issues == 1 + assert eval_.issue_discovery_issues == [] + assert not eval_.is_issue_eligible + assert eval_.issue_discovery_score == 0 + + def test_non_canonical_siblings_per_repo_eligibility(self): + """Multi-repo variant of the #1269 regression. Per-repo eligibility + means each repo's valid_solved is independent; a single solving PR + in one repo must not pad another repo's count, and a shared PR + within a repo must not pad that repo's count via duplicate rows.""" + client = Mock() + + # Repo A: 7 unique-PR canonical issues — repo-A eligible. + repo_a_issues = [ + _issue_dict( + issue_number=10 + i, + author_github_id='A', + solved_by_pr=200 + i, + solving_pr_author='SOLVER', + repo='entrius/gittensor-ui', + ) + for i in range(7) + ] + # Repo B: 7 siblings on a single shared PR — only 1 canonical, repo-B + # must remain ineligible despite seven credibility hits. + repo_b_issues = [ + _issue_dict( + issue_number=50 + i, + author_github_id='A', + solved_by_pr=100, + solving_pr_author='SOLVER', + repo='entrius/other-repo', + created_at=f'2026-04-{i + 1:02d}T00:00:00Z', + ) + for i in range(7) + ] + client.get_miner_issues.return_value = _response(repo_a_issues + repo_b_issues) + + eval_ = _eval(uid=1, github_id='A') + seed = MinerEvaluation(uid=99, hotkey='hkS', github_id='SEED') + seed.merged_prs = [_scored_mirror_pr('entrius/gittensor-ui', pr_number) for pr_number in range(200, 207)] + [ + _scored_mirror_pr('entrius/other-repo', 100) + ] + + _run( + run_issue_discovery( + {1: eval_, 99: seed}, + _mirror_repos('entrius/gittensor-ui', 'entrius/other-repo'), + _EMPTY_LANGS, + _EMPTY_TOKEN_CONFIG, + client=client, + ) + ) + + # Repo A's 7 canonical issues drive the evaluation-wide totals; + # repo B contributes 7 solved + 1 valid_solved (canonical only). + assert eval_.total_solved_issues == 14 + assert eval_.total_valid_solved_issues == 8 + # Only the 7 repo-A canonical issues land in the scored set. + assert len(eval_.issue_discovery_issues) == 7 + assert {issue.number for issue in eval_.issue_discovery_issues} == set(range(10, 17)) + def test_within_miner_one_issue_per_pr_still_holds(self): """One miner authoring two issues both closed by the same PR — the earlier-created issue scores; the later one is credibility-only. @@ -1269,9 +1384,12 @@ def test_within_miner_one_issue_per_pr_still_holds(self): ) ) - # 8 solved (both shared-PR issues counted for credibility), eligible. + # 8 solved (both shared-PR issues counted for credibility), eligible + # on the strength of 7 canonical valid solved (6 unique PRs + 1 + # canonical issue on the shared PR — the later sibling is one-issue- + # per-PR credibility-only and does not increment valid_solved). assert eval_.total_solved_issues == 8 - assert eval_.total_valid_solved_issues == 8 + assert eval_.total_valid_solved_issues == 7 assert eval_.is_issue_eligible # ``issue_token_score`` only accumulates over SCORED PRs (default # ``_scored_mirror_pr`` token_score is 100.0). 7 distinct scoring PRs