Skip to content

Commit 9df5104

Browse files
committed
fix: prevent scoring data loss and fix fragile test in stale PR refresh (#769 follow-up)
1 parent 316f815 commit 9df5104

4 files changed

Lines changed: 45 additions & 3 deletions

File tree

gittensor/validator/storage/queries.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,17 @@
9090
updated_at = NOW()
9191
"""
9292

93+
# Targeted state-only refresh for stale-closed PRs (avoids overwriting scored columns)
94+
REFRESH_STALE_PR_STATES = """
95+
UPDATE pull_requests
96+
SET pr_state = 'CLOSED',
97+
updated_at = NOW()
98+
FROM (VALUES %s) AS stale(pr_number, repo)
99+
WHERE pull_requests.number = stale.pr_number
100+
AND pull_requests.repository_full_name = stale.repo
101+
AND pull_requests.pr_state != 'CLOSED'
102+
"""
103+
93104
# Issue Queries
94105
BULK_UPSERT_ISSUES = """
95106
INSERT INTO issues (

gittensor/validator/storage/repository.py

Lines changed: 25 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

@@ -225,6 +226,30 @@ def store_pull_requests_bulk(self, pull_requests: List[PullRequest], commit: boo
225226
self.logger.error(f'Error in bulk pull request storage: {e}')
226227
return 0
227228

229+
def refresh_stale_pr_states(self, pull_requests: List[PullRequest], commit: bool = True) -> int:
230+
"""Update pr_state to CLOSED for stale PRs without touching scoring columns.
231+
232+
Uses a targeted UPDATE so previously-computed scores (earned_score, base_score,
233+
credibility_multiplier, etc.) are preserved on rows that were already scored.
234+
Only rows currently stored as non-CLOSED are affected.
235+
"""
236+
if not pull_requests:
237+
return 0
238+
values = [(pr.number, pr.repository_full_name) for pr in pull_requests]
239+
try:
240+
with self.get_cursor() as cursor:
241+
from psycopg2.extras import execute_values
242+
243+
execute_values(cursor, REFRESH_STALE_PR_STATES, values)
244+
if commit:
245+
self.db.commit()
246+
return len(values)
247+
except Exception as e:
248+
if commit:
249+
self.db.rollback()
250+
self.logger.error(f'Error refreshing stale PR states: {e}')
251+
return 0
252+
228253
def store_issues_bulk(self, issues: List[Issue], commit: bool = True) -> int:
229254
"""
230255
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)