Skip to content

Commit b5e2d1f

Browse files
committed
fix: prevent scoring data loss and fix fragile test in stale PR refresh (entrius#769 follow-up)
1 parent 49615fc commit b5e2d1f

4 files changed

Lines changed: 42 additions & 3 deletions

File tree

gittensor/validator/storage/queries.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,16 @@
100100
updated_at = NOW()
101101
"""
102102

103+
# Targeted state-only refresh for stale-closed PRs (avoids overwriting scored columns)
104+
REFRESH_STALE_PR_STATES = """
105+
UPDATE pull_requests
106+
SET pr_state = 'CLOSED',
107+
updated_at = NOW()
108+
WHERE number = %s
109+
AND repository_full_name = %s
110+
AND pr_state != 'CLOSED'
111+
"""
112+
103113
# Issue Queries
104114
BULK_UPSERT_ISSUES = """
105115
INSERT INTO issues (

gittensor/validator/storage/repository.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
CLEANUP_STALE_MINER_EVALUATIONS_BY_HOTKEY,
2424
CLEANUP_STALE_MINERS,
2525
CLEANUP_STALE_MINERS_BY_HOTKEY,
26+
REFRESH_STALE_PR_STATES,
2627
SET_MINER,
2728
)
2829

@@ -217,6 +218,28 @@ def store_pull_requests_bulk(self, pull_requests: List[PullRequest], commit: boo
217218
self.logger.error(f'Error in bulk pull request storage: {e}')
218219
return 0
219220

221+
def refresh_stale_pr_states(self, pull_requests: List[PullRequest], commit: bool = True) -> int:
222+
"""Update pr_state to CLOSED for stale PRs without touching scoring columns.
223+
224+
Uses a targeted UPDATE so previously-computed scores (earned_score, base_score,
225+
credibility_multiplier, etc.) are preserved on rows that were already scored.
226+
Only rows currently stored as non-CLOSED are affected.
227+
"""
228+
if not pull_requests:
229+
return 0
230+
values = [(pr.number, pr.repository_full_name) for pr in pull_requests]
231+
try:
232+
with self.get_cursor() as cursor:
233+
cursor.executemany(REFRESH_STALE_PR_STATES, values)
234+
if commit:
235+
self.db.commit()
236+
return len(values)
237+
except Exception as e:
238+
if commit:
239+
self.db.rollback()
240+
self.logger.error(f'Error refreshing stale PR states: {e}')
241+
return 0
242+
220243
def store_issues_bulk(self, issues: List[Issue], commit: bool = True) -> int:
221244
"""
222245
Bulk insert/update issues with efficient SQL conflict resolution

gittensor/validator/utils/storage.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ def _adapt_mirror(scored_list):
7171
result.stored_counts['closed_pull_requests'] = self.repo.store_pull_requests_bulk(
7272
miner_eval.closed_pull_requests + _adapt_mirror(miner_eval.mirror_closed_prs), commit=False
7373
)
74-
result.stored_counts['stale_closed_pull_requests'] = self.repo.store_pull_requests_bulk(
74+
result.stored_counts['stale_closed_pull_requests'] = self.repo.refresh_stale_pr_states(
7575
miner_eval.stale_closed_pull_requests, commit=False
7676
)
7777
result.stored_counts['issues'] = self.repo.store_issues_bulk(miner_eval.get_all_issues(), commit=False)

tests/validator/utils/test_storage_mirror.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ def _make_storage_with_mock_repo():
8686
mock_repo = MagicMock()
8787
mock_repo.set_miner.return_value = 1
8888
mock_repo.store_pull_requests_bulk.return_value = 0 # actual count irrelevant
89+
mock_repo.refresh_stale_pr_states.return_value = 0
8990
mock_repo.store_issues_bulk.return_value = 0
9091
mock_repo.store_file_changes_bulk.return_value = 0
9192
mock_repo.set_miner_evaluation.return_value = True
@@ -173,12 +174,17 @@ def test_stale_closed_prs_are_stored_separately(self):
173174

174175
storage.store_evaluation(eval_)
175176

176-
stale_call = mock_repo.store_pull_requests_bulk.call_args_list[3]
177-
stale_arg = stale_call.args[0]
177+
# Stale PRs must go through refresh_stale_pr_states (targeted UPDATE), not the
178+
# full-column UPSERT, so previously-scored rows are not overwritten with defaults.
179+
mock_repo.refresh_stale_pr_states.assert_called_once()
180+
stale_arg = mock_repo.refresh_stale_pr_states.call_args.args[0]
178181
assert len(stale_arg) == 1
179182
assert stale_arg[0].number == 7
180183
assert stale_arg[0].pr_state == PRState.CLOSED
181184
assert eval_.total_closed_prs == 0
185+
# Verify the stale PR did not leak into store_pull_requests_bulk
186+
for call in mock_repo.store_pull_requests_bulk.call_args_list:
187+
assert not any(pr.number == 7 for pr in call.args[0])
182188

183189

184190
def test_cleanup_stale_called_with_commit_false():

0 commit comments

Comments
 (0)