From db1009b859a0c343ac8fee9d2f8bf5852bce0e6e Mon Sep 17 00:00:00 2001 From: John Baillie Date: Fri, 22 May 2026 10:28:16 -0400 Subject: [PATCH 1/2] fix(validator): count only canonical valid solved issues (#1269) Move acc.valid_solved += 1 past every credibility-only gate (same-account, one-issue-per-PR canonical, token threshold). Previously the increment fired on token-score alone, before the canonical check, so seven sibling issues closed by one qualifying PR could satisfy the per-repo MIN_VALID_SOLVED_ISSUES eligibility gate even though only one was scoreable. Adds regression coverage for both single-repo and per-repo eligibility, plus a credibility-only assertion for the same-account path, and updates two existing tests whose assertions encoded the buggy 7-counts-as-7 behavior. Fixes #1269. Co-Authored-By: Claude Opus 4.7 (1M context) --- gittensor/validator/issue_discovery/scan.py | 10 +- tests/validator/issue_discovery/test_scan.py | 124 +++++++++++++++++-- 2 files changed, 121 insertions(+), 13 deletions(-) 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..c8a187d6 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 @@ -1203,13 +1205,17 @@ 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 satisfy the eligibility threshold — preventing one qualifying PR + # from unlocking MIN_VALID_SOLVED_ISSUES through duplicate issue rows. 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 assert e_a.is_issue_eligible - assert e_b.is_issue_eligible + assert not e_b.is_issue_eligible # ``issue_token_score`` accumulates only over SCORED PRs (default # ``_scored_mirror_pr`` token_score is 100.0), so this is the @@ -1217,10 +1223,107 @@ def _per_miner(github_id, since_by_repo=None): # 6 (shared PR 100 is canonical for A only and credibility-only for B). assert e_a.issue_token_score == 700.0 assert e_b.issue_token_score == 600.0 - # All solving PRs share identical scoring inputs at this issue mix, so - # the discovery_score ratio collapses to 7:6. - 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) + # B's ineligibility zeroes its issue_discovery_score regardless of the + # six canonical issues that would otherwise have scored. + assert e_a.issue_discovery_score > 0 + assert e_b.issue_discovery_score == 0 + + 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 @@ -1269,9 +1372,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 From 09e37381166388ce57942f31e1233c44763e7613 Mon Sep 17 00:00:00 2001 From: John Baillie Date: Fri, 22 May 2026 11:06:06 -0400 Subject: [PATCH 2/2] fix(test): adjust assertions for current MIN_VALID_SOLVED_ISSUES default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two test fixes uncovered by actually running pytest locally: 1. test_two_miners_shared_pr_only_earliest_scores — MIN_VALID_SOLVED_ISSUES dropped from 7 to 3 since #1269 was filed, so B (valid_solved == 6 after the canonical fix) remains eligible. Restore the 7:6 score-ratio assertion; the dedicated eligibility-gate regression lives in test_non_canonical_siblings_do_not_satisfy_valid_solved_gate where valid_solved == 1 < 3. 2. test_all_mirror_miner_below_threshold_passes_spam — this test relied on eight sibling issues sharing one solving PR all counting as valid_solved (the #1269 bug). Give each issue its own solving PR and seed all eight so the spam-mult assertion still has an eligible miner to assert against post-fix. All 56 tests in tests/validator/issue_discovery/ now pass locally. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/validator/issue_discovery/test_scan.py | 30 ++++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/tests/validator/issue_discovery/test_scan.py b/tests/validator/issue_discovery/test_scan.py index c8a187d6..5296ca05 100644 --- a/tests/validator/issue_discovery/test_scan.py +++ b/tests/validator/issue_discovery/test_scan.py @@ -1022,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( @@ -1208,14 +1217,17 @@ def _per_miner(github_id, since_by_repo=None): # 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 satisfy the eligibility threshold — preventing one qualifying PR - # from unlocking MIN_VALID_SOLVED_ISSUES through duplicate issue rows. + # 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 == 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 not e_b.is_issue_eligible + assert e_b.is_issue_eligible # ``issue_token_score`` accumulates only over SCORED PRs (default # ``_scored_mirror_pr`` token_score is 100.0), so this is the @@ -1223,10 +1235,10 @@ def _per_miner(github_id, since_by_repo=None): # 6 (shared PR 100 is canonical for A only and credibility-only for B). assert e_a.issue_token_score == 700.0 assert e_b.issue_token_score == 600.0 - # B's ineligibility zeroes its issue_discovery_score regardless of the - # six canonical issues that would otherwise have scored. - assert e_a.issue_discovery_score > 0 - assert e_b.issue_discovery_score == 0 + # All solving PRs share identical scoring inputs at this issue mix, so + # the discovery_score ratio collapses to 7:6. + 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