Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions gittensor/classes.py
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ class MinerEvaluation:
open_pull_requests: List[PullRequest] = field(default_factory=list)
closed_pull_requests: List[PullRequest] = field(default_factory=list)
stale_closed_pull_requests: List[PullRequest] = field(default_factory=list)
stale_merged_pull_requests: List[PullRequest] = field(default_factory=list)

# Populated by gittensor.validator.oss_contributions.mirror.combine.combine
# when the mirror scoring path runs. Empty for legacy-only evaluations.
Expand Down Expand Up @@ -510,6 +511,13 @@ def add_stale_closed_pull_request(self, raw_pr: Dict):
PullRequest.from_graphql_response(raw_pr, self.uid, self.hotkey, self.github_id)
)

def add_stale_merged_pull_request(self, raw_pr: Dict):
"""Track a stale MERGED PR so storage can refresh its pull_requests row."""
bt.logging.info(f'Stale MERGED PR #{raw_pr["number"]} in {parse_repo_name(raw_pr["repository"])}')
self.stale_merged_pull_requests.append(
PullRequest.from_graphql_response(raw_pr, self.uid, self.hotkey, self.github_id)
)


@dataclass
class ScoreBreakdown:
Expand Down
16 changes: 16 additions & 0 deletions gittensor/utils/github_api_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -827,6 +827,21 @@ def try_add_open_or_closed_pr(
miner_eval.add_closed_pull_request(pr_raw)


def _maybe_add_stale_merged_pr(
miner_eval: MinerEvaluation,
pr_raw: Dict,
lookback_date_filter: datetime,
) -> None:
"""Capture merged PRs whose mergedAt predates the lookback window so storage
can sync the previously-stored OPEN row (parallel to #769's stale-CLOSED path).
"""
merged_at = pr_raw.get('mergedAt')
if not merged_at:
return
if parse_github_iso_to_utc(merged_at) < lookback_date_filter:
miner_eval.add_stale_merged_pull_request(pr_raw)


def should_skip_merged_pr(
pr_raw: Dict,
repository_full_name: str,
Expand Down Expand Up @@ -1014,6 +1029,7 @@ def load_miners_prs(
)

if should_skip:
_maybe_add_stale_merged_pr(miner_eval, pr_raw, lookback_date_filter)
bt.logging.debug(skip_reason or '')
continue

Expand Down
16 changes: 16 additions & 0 deletions gittensor/validator/storage/queries.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,22 @@
updated_at = NOW()
"""

# Targeted UPDATE that syncs pr_state to MERGED on existing rows whose PR
# merged outside the lookback window. The pr_state guard keeps it idempotent
# and preserves historical scoring fields on rows already captured as MERGED.
REFRESH_STALE_MERGED_PR_STATE = """
UPDATE pull_requests
SET
pr_state = 'MERGED',
merged_at = %s,
collateral_score = 0.0,
updated_at = NOW()
WHERE
number = %s
AND repository_full_name = %s
AND pr_state != 'MERGED'
"""

# Targeted state-only refresh for stale-closed PRs (avoids overwriting scored columns)
REFRESH_STALE_PR_STATES = """
UPDATE pull_requests
Expand Down
25 changes: 25 additions & 0 deletions gittensor/validator/storage/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
CLEANUP_STALE_MINER_EVALUATIONS_BY_HOTKEY,
CLEANUP_STALE_MINERS,
CLEANUP_STALE_MINERS_BY_HOTKEY,
REFRESH_STALE_MERGED_PR_STATE,
REFRESH_STALE_PR_STATES,
SET_MINER,
)
Expand Down Expand Up @@ -218,6 +219,30 @@ def store_pull_requests_bulk(self, pull_requests: List[PullRequest], commit: boo
self.logger.error(f'Error in bulk pull request storage: {e}')
return 0

def refresh_stale_merged_pr_states(self, pull_requests: List[PullRequest], commit: bool = True) -> int:
"""Targeted UPDATE-only refresh of pr_state/merged_at on existing rows.

Skips rows already at pr_state='MERGED' so historical scoring fields are
preserved; rows that don't exist are silent no-ops (never inserts).
"""
eligible = [pr for pr in pull_requests if pr.merged_at is not None]
if not eligible:
return 0

values = [(pr.merged_at, pr.number, pr.repository_full_name) for pr in eligible]

try:
with self.get_cursor() as cursor:
cursor.executemany(REFRESH_STALE_MERGED_PR_STATE, values)
if commit:
self.db.commit()
return len(values)
except Exception as e:
if commit:
self.db.rollback()
self.logger.error(f'Error refreshing stale merged PR states: {e}')
return 0

def refresh_stale_pr_states(self, pull_requests: List[PullRequest], commit: bool = True) -> int:
"""Update pr_state to CLOSED for stale PRs without touching scoring columns.

Expand Down
3 changes: 3 additions & 0 deletions gittensor/validator/utils/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ def _adapt_mirror(scored_list):
result.stored_counts['stale_closed_pull_requests'] = self.repo.refresh_stale_pr_states(
miner_eval.stale_closed_pull_requests, commit=False
)
result.stored_counts['stale_merged_pull_requests'] = self.repo.refresh_stale_merged_pr_states(
miner_eval.stale_merged_pull_requests, commit=False
)
result.stored_counts['issues'] = self.repo.store_issues_bulk(miner_eval.get_all_issues(), commit=False)
result.stored_counts['file_changes'] = self.repo.store_file_changes_bulk(
miner_eval.get_all_file_changes(), commit=False
Expand Down
138 changes: 133 additions & 5 deletions tests/validator/test_unscored_stale_prs.py
Original file line number Diff line number Diff line change
@@ -1,19 +1,27 @@
# The MIT License (MIT)
# Copyright © 2025 Entrius

"""Regression tests for stale-closed PR capture in a storage-only bucket.
"""Regression tests for stale-closed and stale-merged PR capture in storage-only buckets.

Covers the invariant that stale-CLOSED PRs (created before the lookback window):
- do not enter `closed_pull_requests` (preserves the #406 drop-from-scoring)
- do enter `stale_closed_pull_requests` so storage can refresh pr_state
Covers two invariants:

stale-CLOSED PRs (created before the lookback window, #769):
- do not enter ``closed_pull_requests`` (preserves the #406 drop-from-scoring)
- do enter ``stale_closed_pull_requests`` so storage can refresh ``pr_state``
- are not reflected in total counts or scoring buckets

stale-MERGED PRs (merged before the lookback window):
- do not enter ``merged_pull_requests`` (preserves the lookback exclusion)
- do enter ``stale_merged_pull_requests`` so storage can sync a previously-
stored OPEN row to MERGED without re-entering scoring
- are not reflected in total counts or scoring buckets
"""

from datetime import datetime, timedelta, timezone
from unittest.mock import patch

from gittensor.classes import MinerEvaluation, PRState
from gittensor.utils.github_api_tools import try_add_open_or_closed_pr
from gittensor.utils.github_api_tools import _maybe_add_stale_merged_pr, try_add_open_or_closed_pr


def _pr_node(number: int, created_at: str, closed_at: str, state: str = 'CLOSED') -> dict:
Expand Down Expand Up @@ -130,3 +138,123 @@ def test_stale_closed_storage_bucket_does_not_inflate_any_totals(_):
assert miner_eval.total_closed_prs == 0
assert miner_eval.total_prs == 0
assert len(miner_eval.stale_closed_pull_requests) == 3


# ---------------------------------------------------------------------------
# Stale-MERGED PR capture (storage-only refresh path)
# ---------------------------------------------------------------------------


def _merged_pr_node(number: int, created_at: str, merged_at: str) -> dict:
return {
'number': number,
'title': f'merged PR {number}',
'state': 'MERGED',
'repository': {
'name': 'jan',
'owner': {'login': 'janhq'},
'defaultBranchRef': {'name': 'main'},
},
'headRepository': {'name': 'jan', 'owner': {'login': 'contributor'}},
'author': {'login': 'contributor'},
'authorAssociation': 'CONTRIBUTOR',
'mergedBy': {'login': 'maintainer'},
'mergedAt': merged_at,
'createdAt': created_at,
'closedAt': merged_at,
'lastEditedAt': None,
'additions': 10,
'deletions': 5,
'commits': {'totalCount': 1},
'baseRefName': 'main',
'baseRefOid': 'abc',
'headRefName': 'feature',
'headRefOid': 'def',
'bodyText': '',
'closingIssuesReferences': {'nodes': []},
'changesRequestedReviews': {'nodes': []},
'labels': {'nodes': []},
'timelineItems': {'nodes': []},
}


@patch('gittensor.utils.github_api_tools.bt.logging')
def test_stale_merged_pr_goes_to_storage_only_bucket(_):
miner_eval = MinerEvaluation(uid=74, hotkey='hk', github_id='1', github_pat='fake')
now = datetime.now(timezone.utc)
lookback = now - timedelta(days=35)
stale_merge = (lookback - timedelta(days=20)).strftime('%Y-%m-%dT%H:%M:%SZ')
created = (lookback - timedelta(days=40)).strftime('%Y-%m-%dT%H:%M:%SZ')

_maybe_add_stale_merged_pr(miner_eval, _merged_pr_node(7629, created, stale_merge), lookback)

assert len(miner_eval.merged_pull_requests) == 0, 'stale merged PR must not enter scoring bucket'
assert len(miner_eval.stale_merged_pull_requests) == 1, 'stale merged PR must enter storage-only bucket'
assert miner_eval.stale_merged_pull_requests[0].number == 7629
assert miner_eval.stale_merged_pull_requests[0].pr_state == PRState.MERGED
assert miner_eval.stale_merged_pull_requests[0].merged_at is not None


@patch('gittensor.utils.github_api_tools.bt.logging')
def test_stale_merged_pr_not_counted_in_totals(_):
miner_eval = MinerEvaluation(uid=74, hotkey='hk', github_id='1', github_pat='fake')
now = datetime.now(timezone.utc)
lookback = now - timedelta(days=35)
stale_merge = (lookback - timedelta(days=20)).strftime('%Y-%m-%dT%H:%M:%SZ')
created = (lookback - timedelta(days=40)).strftime('%Y-%m-%dT%H:%M:%SZ')

_maybe_add_stale_merged_pr(miner_eval, _merged_pr_node(7629, created, stale_merge), lookback)

assert miner_eval.total_merged_prs == 0
assert miner_eval.total_open_prs == 0
assert miner_eval.total_closed_prs == 0
assert miner_eval.total_prs == 0


@patch('gittensor.utils.github_api_tools.bt.logging')
def test_fresh_merged_pr_is_not_captured_as_stale(_):
"""Regression: PRs merged within the lookback window must NOT enter the
stale-merged bucket — they belong on the normal scoring path.
"""
miner_eval = MinerEvaluation(uid=74, hotkey='hk', github_id='1', github_pat='fake')
now = datetime.now(timezone.utc)
lookback = now - timedelta(days=35)
fresh_merge = (now - timedelta(days=5)).strftime('%Y-%m-%dT%H:%M:%SZ')
created = (now - timedelta(days=10)).strftime('%Y-%m-%dT%H:%M:%SZ')

_maybe_add_stale_merged_pr(miner_eval, _merged_pr_node(1, created, fresh_merge), lookback)

assert len(miner_eval.stale_merged_pull_requests) == 0


@patch('gittensor.utils.github_api_tools.bt.logging')
def test_merged_pr_without_merged_at_is_skipped(_):
"""Defensive: malformed MERGED PR with no mergedAt must not crash and must
not be captured (we cannot prove it's stale)."""
miner_eval = MinerEvaluation(uid=74, hotkey='hk', github_id='1', github_pat='fake')
now = datetime.now(timezone.utc)
lookback = now - timedelta(days=35)
created = (lookback - timedelta(days=40)).strftime('%Y-%m-%dT%H:%M:%SZ')

pr = _merged_pr_node(2, created, merged_at='')
pr['mergedAt'] = None

_maybe_add_stale_merged_pr(miner_eval, pr, lookback)

assert len(miner_eval.stale_merged_pull_requests) == 0


@patch('gittensor.utils.github_api_tools.bt.logging')
def test_multiple_stale_merged_prs_accumulate(_):
miner_eval = MinerEvaluation(uid=74, hotkey='hk', github_id='1', github_pat='fake')
now = datetime.now(timezone.utc)
lookback = now - timedelta(days=35)
stale_merge = (lookback - timedelta(days=20)).strftime('%Y-%m-%dT%H:%M:%SZ')
created = (lookback - timedelta(days=40)).strftime('%Y-%m-%dT%H:%M:%SZ')

for i in range(3):
_maybe_add_stale_merged_pr(miner_eval, _merged_pr_node(200 + i, created, stale_merge), lookback)

assert len(miner_eval.stale_merged_pull_requests) == 3
assert miner_eval.total_merged_prs == 0
assert miner_eval.total_prs == 0
47 changes: 47 additions & 0 deletions tests/validator/utils/test_storage_mirror.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ def _make_storage_with_mock_repo():
mock_repo = MagicMock()
mock_repo.set_miner.return_value = 1
mock_repo.store_pull_requests_bulk.return_value = 0 # actual count irrelevant
mock_repo.refresh_stale_merged_pr_states.return_value = 0
mock_repo.refresh_stale_pr_states.return_value = 0
mock_repo.store_issues_bulk.return_value = 0
mock_repo.store_file_changes_bulk.return_value = 0
Expand Down Expand Up @@ -186,6 +187,52 @@ def test_stale_closed_prs_are_stored_separately(self):
for call in mock_repo.store_pull_requests_bulk.call_args_list:
assert not any(pr.number == 7 for pr in call.args[0])

def test_stale_merged_prs_use_targeted_refresh_path(self):
"""Stale-MERGED PRs route to ``refresh_stale_merged_pr_states`` (a
targeted UPDATE) rather than the full ``store_pull_requests_bulk``
upsert, so historical scoring fields on already-MERGED rows are not
clobbered with dataclass defaults.
"""
storage, mock_repo = _make_storage_with_mock_repo()

eval_ = MinerEvaluation(uid=1, hotkey='hk', github_id='123')
eval_.stale_merged_pull_requests = [_legacy_pr(7629, state=PRState.MERGED)]

storage.store_evaluation(eval_)

# Targeted refresh, not a bulk upsert.
mock_repo.refresh_stale_merged_pr_states.assert_called_once()
refresh_arg = mock_repo.refresh_stale_merged_pr_states.call_args.args[0]
assert len(refresh_arg) == 1
assert refresh_arg[0].number == 7629
assert refresh_arg[0].pr_state == PRState.MERGED
assert refresh_arg[0].merged_at is not None

# Stale-merged storage must not inflate scoring totals.
assert eval_.total_merged_prs == 0
assert eval_.total_prs == 0

# And it must not be passed to the bulk upsert path
# (3 calls: merged, open, closed). Stale-closed PRs go through
# refresh_stale_pr_states, not the bulk upsert.
assert mock_repo.store_pull_requests_bulk.call_count == 3
for call in mock_repo.store_pull_requests_bulk.call_args_list:
assert call.args[0] == [] or call.args[0][0].number != 7629

def test_empty_stale_merged_list_still_invokes_refresh(self):
"""Even with no stale-merged PRs, the refresh call happens (returns 0)
so the storage transaction shape stays predictable across rounds.
"""
storage, mock_repo = _make_storage_with_mock_repo()

eval_ = MinerEvaluation(uid=1, hotkey='hk', github_id='123')
# stale_merged_pull_requests left empty by default

storage.store_evaluation(eval_)

mock_repo.refresh_stale_merged_pr_states.assert_called_once()
assert mock_repo.refresh_stale_merged_pr_states.call_args.args[0] == []


def test_cleanup_stale_called_with_commit_false():
"""cleanup_stale_miner_data must be called with commit=False inside the transaction.
Expand Down
Loading