From 1f18065daa32af9f6696f18770f0ecea83923a5e Mon Sep 17 00:00:00 2001 From: leonace924 Date: Thu, 12 Feb 2026 17:25:16 -0500 Subject: [PATCH 1/4] fix(graphql): reduce page size on 5xx errors to recover from expensive queries GitHub returns 502 when a GraphQL query is too expensive to process. Halve the page limit on 502/503/504 responses (floor of 10) before retrying, so the retry loop can self-heal instead of failing 8 times with the same oversized request. --- gittensor/utils/github_api_tools.py | 26 +++++--- tests/utils/test_github_api_tools.py | 88 ++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 8 deletions(-) diff --git a/gittensor/utils/github_api_tools.py b/gittensor/utils/github_api_tools.py index cd5c0e4..c35bed4 100644 --- a/gittensor/utils/github_api_tools.py +++ b/gittensor/utils/github_api_tools.py @@ -354,13 +354,14 @@ def get_github_graphql_query( max_attempts = 8 headers = {'Authorization': f'Bearer {token}', 'Content-Type': 'application/json'} - variables = { - 'userId': global_user_id, - 'limit': min(100, max_prs - merged_pr_count), - 'cursor': cursor, - } + limit = min(100, max_prs - merged_pr_count) for attempt in range(max_attempts): + variables = { + 'userId': global_user_id, + 'limit': limit, + 'cursor': cursor, + } try: response = requests.post( f'{BASE_GITHUB_API_URL}/graphql', @@ -375,9 +376,18 @@ def get_github_graphql_query( # error - log and retry if attempt < (max_attempts - 1): backoff_delay = min(5 * (2**attempt), 30) # cap at 30s - bt.logging.warning( - f'GraphQL request for PRs failed with status {response.status_code} (attempt {attempt + 1}/{max_attempts}), retrying in {backoff_delay}s...' - ) + # Reduce page size on server-side errors (query may be too expensive) + if response.status_code in (502, 503, 504) and limit > 10: + limit = max(limit // 2, 10) + bt.logging.warning( + f'GraphQL request for PRs failed with status {response.status_code} ' + f'(attempt {attempt + 1}/{max_attempts}), reducing page size to {limit} and retrying in {backoff_delay}s...' + ) + else: + bt.logging.warning( + f'GraphQL request for PRs failed with status {response.status_code} ' + f'(attempt {attempt + 1}/{max_attempts}), retrying in {backoff_delay}s...' + ) time.sleep(backoff_delay) else: bt.logging.error( diff --git a/tests/utils/test_github_api_tools.py b/tests/utils/test_github_api_tools.py index beac169..a02c71a 100644 --- a/tests/utils/test_github_api_tools.py +++ b/tests/utils/test_github_api_tools.py @@ -101,6 +101,94 @@ def test_retry_on_502_then_success(self, mock_logging, mock_sleep, mock_post, gr # Verify exponential backoff: 5s, 10s mock_sleep.assert_has_calls([call(5), call(10)]) + @patch('gittensor.utils.github_api_tools.requests.post') + @patch('gittensor.utils.github_api_tools.time.sleep') + @patch('gittensor.utils.github_api_tools.bt.logging') + def test_502_halves_page_size_on_retry(self, mock_logging, mock_sleep, mock_post, graphql_params): + """Test that 502 errors cause the page size (limit) to be halved on each retry.""" + mock_response_502 = Mock(status_code=502, text='502 Bad Gateway') + mock_response_200 = Mock(status_code=200) + + mock_post.side_effect = [mock_response_502, mock_response_502, mock_response_200] + + result = get_github_graphql_query(**graphql_params) + + assert result.status_code == 200 + # Initial limit=100, halved to 50 after first 502, halved to 25 after second 502 + limits = [c.kwargs['json']['variables']['limit'] for c in mock_post.call_args_list] + assert limits == [100, 50, 25] + + @patch('gittensor.utils.github_api_tools.requests.post') + @patch('gittensor.utils.github_api_tools.time.sleep') + @patch('gittensor.utils.github_api_tools.bt.logging') + def test_page_size_floors_at_10(self, mock_logging, mock_sleep, mock_post, graphql_params): + """Test that page size never drops below 10 even after many 502s.""" + mock_response_502 = Mock(status_code=502, text='502 Bad Gateway') + mock_post.return_value = mock_response_502 + + get_github_graphql_query(**graphql_params) + + # 100 -> 50 -> 25 -> 12 -> 10 -> 10 -> 10 -> 10 + limits = [c.kwargs['json']['variables']['limit'] for c in mock_post.call_args_list] + assert limits == [100, 50, 25, 12, 10, 10, 10, 10] + + @patch('gittensor.utils.github_api_tools.requests.post') + @patch('gittensor.utils.github_api_tools.time.sleep') + @patch('gittensor.utils.github_api_tools.bt.logging') + def test_non_5xx_does_not_reduce_page_size(self, mock_logging, mock_sleep, mock_post, graphql_params): + """Test that non-5xx errors (e.g. 401) do not reduce page size.""" + mock_response_401 = Mock(status_code=401, text='Unauthorized') + mock_response_200 = Mock(status_code=200) + + mock_post.side_effect = [mock_response_401, mock_response_401, mock_response_200] + + result = get_github_graphql_query(**graphql_params) + + assert result.status_code == 200 + limits = [c.kwargs['json']['variables']['limit'] for c in mock_post.call_args_list] + assert limits == [100, 100, 100], 'Page size should stay at 100 for non-5xx errors' + + @patch('gittensor.utils.github_api_tools.requests.post') + @patch('gittensor.utils.github_api_tools.time.sleep') + @patch('gittensor.utils.github_api_tools.bt.logging') + def test_503_and_504_also_reduce_page_size(self, mock_logging, mock_sleep, mock_post, graphql_params): + """Test that 503 and 504 errors also trigger page size reduction.""" + mock_response_503 = Mock(status_code=503, text='Service Unavailable') + mock_response_504 = Mock(status_code=504, text='Gateway Timeout') + mock_response_200 = Mock(status_code=200) + + mock_post.side_effect = [mock_response_503, mock_response_504, mock_response_200] + + result = get_github_graphql_query(**graphql_params) + + assert result.status_code == 200 + limits = [c.kwargs['json']['variables']['limit'] for c in mock_post.call_args_list] + assert limits == [100, 50, 25] + + @patch('gittensor.utils.github_api_tools.requests.post') + @patch('gittensor.utils.github_api_tools.time.sleep') + @patch('gittensor.utils.github_api_tools.bt.logging') + def test_small_initial_limit_not_reduced_below_10(self, mock_logging, mock_sleep, mock_post): + """Test that a small initial limit (e.g. 15) floors correctly at 10.""" + mock_response_502 = Mock(status_code=502, text='502 Bad Gateway') + mock_response_200 = Mock(status_code=200) + + mock_post.side_effect = [mock_response_502, mock_response_502, mock_response_200] + + params = { + 'token': 'fake_github_token', + 'global_user_id': 'MDQ6VXNlcjEyMzQ1', + 'merged_pr_count': 85, + 'max_prs': 100, + 'cursor': None, + } + result = get_github_graphql_query(**params) + + assert result.status_code == 200 + # Initial limit = min(100, 100-85) = 15, halved to 10, stays at 10 + limits = [c.kwargs['json']['variables']['limit'] for c in mock_post.call_args_list] + assert limits == [15, 10, 10] + @patch('gittensor.utils.github_api_tools.requests.post') @patch('gittensor.utils.github_api_tools.time.sleep') @patch('gittensor.utils.github_api_tools.bt.logging') From 464a50f92eca82a1bdb4a8c01c4ec22a481e4baa Mon Sep 17 00:00:00 2001 From: leonace924 Date: Thu, 12 Feb 2026 18:07:35 -0500 Subject: [PATCH 2/4] fix(api): add retry logic to get_pull_request_file_changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This REST endpoint had no retry logic — a single transient failure would silently return an empty list, causing the PR to score 0. Add 3 attempts with exponential backoff (matching the pattern used by all other API functions in the module). --- gittensor/utils/github_api_tools.py | 51 +++++++++--- tests/utils/test_github_api_tools.py | 112 +++++++++++++++++++++++++++ 2 files changed, 151 insertions(+), 12 deletions(-) diff --git a/gittensor/utils/github_api_tools.py b/gittensor/utils/github_api_tools.py index c35bed4..ca66c69 100644 --- a/gittensor/utils/github_api_tools.py +++ b/gittensor/utils/github_api_tools.py @@ -248,7 +248,10 @@ def get_github_account_age_days(token: str) -> Optional[int]: def get_pull_request_file_changes(repository: str, pr_number: int, token: str) -> Optional[List[FileChange]]: """ - Get the diff for a specific PR by repository name and PR number + Get the diff for a specific PR by repository name and PR number. + + Uses retry logic with exponential backoff for transient failures. + Args: repository (str): Repository in format 'owner/repo' pr_number (int): PR number @@ -256,21 +259,45 @@ def get_pull_request_file_changes(repository: str, pr_number: int, token: str) - Returns: List[FileChanges]: List object with file changes or None if error """ + max_attempts = 3 headers = make_headers(token) - try: - response = requests.get( - f'{BASE_GITHUB_API_URL}/repos/{repository}/pulls/{pr_number}/files', headers=headers, timeout=15 - ) - if response.status_code == 200: - file_diffs = response.json() - return [FileChange.from_github_response(pr_number, repository, file_diff) for file_diff in file_diffs] + for attempt in range(max_attempts): + try: + response = requests.get( + f'{BASE_GITHUB_API_URL}/repos/{repository}/pulls/{pr_number}/files', headers=headers, timeout=15 + ) + if response.status_code == 200: + file_diffs = response.json() + return [FileChange.from_github_response(pr_number, repository, file_diff) for file_diff in file_diffs] + + if attempt < (max_attempts - 1): + backoff_delay = min(5 * (2**attempt), 30) + bt.logging.warning( + f'File changes request for PR #{pr_number} in {repository} failed with status {response.status_code} ' + f'(attempt {attempt + 1}/{max_attempts}), retrying in {backoff_delay}s...' + ) + time.sleep(backoff_delay) + else: + bt.logging.error( + f'File changes request for PR #{pr_number} in {repository} failed with status {response.status_code} ' + f'after {max_attempts} attempts' + ) - return [] + except requests.exceptions.RequestException as e: + if attempt < (max_attempts - 1): + backoff_delay = min(5 * (2**attempt), 30) + bt.logging.warning( + f'File changes request error for PR #{pr_number} in {repository} ' + f'(attempt {attempt + 1}/{max_attempts}): {e}, retrying in {backoff_delay}s...' + ) + time.sleep(backoff_delay) + else: + bt.logging.error( + f'File changes request for PR #{pr_number} in {repository} failed after {max_attempts} attempts: {e}' + ) - except Exception as e: - bt.logging.error(f'Error getting file changes for PR #{pr_number} in {repository}: {e}') - return [] + return [] def execute_graphql_query( diff --git a/tests/utils/test_github_api_tools.py b/tests/utils/test_github_api_tools.py index a02c71a..2268cbd 100644 --- a/tests/utils/test_github_api_tools.py +++ b/tests/utils/test_github_api_tools.py @@ -27,6 +27,7 @@ get_github_graphql_query = github_api_tools.get_github_graphql_query get_github_id = github_api_tools.get_github_id get_github_account_age_days = github_api_tools.get_github_account_age_days +get_pull_request_file_changes = github_api_tools.get_pull_request_file_changes # ============================================================================ @@ -380,5 +381,116 @@ def test_get_github_account_age_retry_logic(self, mock_sleep, mock_get, clear_gi assert mock_get.call_count == 2 +# ============================================================================ +# File Changes Retry Logic Tests +# ============================================================================ + + +class TestFileChangesRetryLogic: + """Test suite for retry logic in get_pull_request_file_changes.""" + + @patch('gittensor.utils.github_api_tools.requests.get') + def test_successful_request_no_retry(self, mock_get): + """Test that a successful request returns file changes without retrying.""" + mock_response = Mock(status_code=200) + mock_response.json.return_value = [ + { + 'filename': 'test.py', + 'status': 'modified', + 'changes': 2, + 'additions': 1, + 'deletions': 1, + 'patch': '@@ -1 +1 @@\n-old\n+new', + }, + ] + mock_get.return_value = mock_response + + result = get_pull_request_file_changes('owner/repo', 1, 'fake_token') + + assert mock_get.call_count == 1 + assert result is not None + assert len(result) == 1 + + @patch('gittensor.utils.github_api_tools.requests.get') + @patch('gittensor.utils.github_api_tools.time.sleep') + @patch('gittensor.utils.github_api_tools.bt.logging') + def test_retry_on_502_then_success(self, mock_logging, mock_sleep, mock_get): + """Test that 502 triggers retry and succeeds on second attempt.""" + mock_502 = Mock(status_code=502, text='Bad Gateway') + mock_200 = Mock(status_code=200) + mock_200.json.return_value = [ + {'filename': 'test.py', 'status': 'modified', 'changes': 1, 'additions': 1, 'deletions': 0, 'patch': ''}, + ] + + mock_get.side_effect = [mock_502, mock_200] + + result = get_pull_request_file_changes('owner/repo', 1, 'fake_token') + + assert mock_get.call_count == 2 + assert mock_sleep.call_count == 1 + assert len(result) == 1 + + @patch('gittensor.utils.github_api_tools.requests.get') + @patch('gittensor.utils.github_api_tools.time.sleep') + @patch('gittensor.utils.github_api_tools.bt.logging') + def test_gives_up_after_three_attempts(self, mock_logging, mock_sleep, mock_get): + """Test that function gives up after 3 failed attempts and returns empty list.""" + mock_500 = Mock(status_code=500, text='Internal Server Error') + mock_get.return_value = mock_500 + + result = get_pull_request_file_changes('owner/repo', 1, 'fake_token') + + assert mock_get.call_count == 3 + assert mock_sleep.call_count == 2 + assert result == [] + mock_logging.error.assert_called() + + @patch('gittensor.utils.github_api_tools.requests.get') + @patch('gittensor.utils.github_api_tools.time.sleep') + @patch('gittensor.utils.github_api_tools.bt.logging') + def test_retry_on_connection_error_then_success(self, mock_logging, mock_sleep, mock_get): + """Test that connection errors trigger retry.""" + import requests + + mock_200 = Mock(status_code=200) + mock_200.json.return_value = [] + + mock_get.side_effect = [requests.exceptions.ConnectionError('refused'), mock_200] + + result = get_pull_request_file_changes('owner/repo', 1, 'fake_token') + + assert mock_get.call_count == 2 + assert mock_sleep.call_count == 1 + assert result == [] + + @patch('gittensor.utils.github_api_tools.requests.get') + @patch('gittensor.utils.github_api_tools.time.sleep') + @patch('gittensor.utils.github_api_tools.bt.logging') + def test_gives_up_after_three_connection_errors(self, mock_logging, mock_sleep, mock_get): + """Test that function gives up after 3 connection errors.""" + import requests + + mock_get.side_effect = requests.exceptions.ConnectionError('refused') + + result = get_pull_request_file_changes('owner/repo', 1, 'fake_token') + + assert mock_get.call_count == 3 + assert result == [] + mock_logging.error.assert_called() + + @patch('gittensor.utils.github_api_tools.requests.get') + @patch('gittensor.utils.github_api_tools.time.sleep') + @patch('gittensor.utils.github_api_tools.bt.logging') + def test_exponential_backoff_timing(self, mock_logging, mock_sleep, mock_get): + """Test that backoff delays are 5s, 10s for 3 attempts.""" + mock_500 = Mock(status_code=500, text='Internal Server Error') + mock_get.return_value = mock_500 + + get_pull_request_file_changes('owner/repo', 1, 'fake_token') + + mock_sleep.assert_has_calls([call(5), call(10)]) + assert mock_sleep.call_count == 2 + + if __name__ == '__main__': pytest.main([__file__, '-v']) From 9ad68798beb8efbe4f44f8a9453eff321c61a14a Mon Sep 17 00:00:00 2001 From: eureka928 Date: Fri, 13 Feb 2026 18:49:27 +0100 Subject: [PATCH 3/4] fix(graphql): remove redundant limit guard and clarify log message Address PR #185 review feedback: remove redundant `and limit > 10` guard since `max(limit // 2, 10)` already floors at 10, and simplify the warning log from "reducing page size to" to "page size set to". --- gittensor/utils/github_api_tools.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gittensor/utils/github_api_tools.py b/gittensor/utils/github_api_tools.py index ca66c69..6e2fe36 100644 --- a/gittensor/utils/github_api_tools.py +++ b/gittensor/utils/github_api_tools.py @@ -404,11 +404,11 @@ def get_github_graphql_query( if attempt < (max_attempts - 1): backoff_delay = min(5 * (2**attempt), 30) # cap at 30s # Reduce page size on server-side errors (query may be too expensive) - if response.status_code in (502, 503, 504) and limit > 10: + if response.status_code in (502, 503, 504): limit = max(limit // 2, 10) bt.logging.warning( f'GraphQL request for PRs failed with status {response.status_code} ' - f'(attempt {attempt + 1}/{max_attempts}), reducing page size to {limit} and retrying in {backoff_delay}s...' + f'(attempt {attempt + 1}/{max_attempts}), page size set to {limit}, retrying in {backoff_delay}s...' ) else: bt.logging.warning( From e0ce9ce166cf74b09a58856d69cd81937d61f424 Mon Sep 17 00:00:00 2001 From: eureka928 Date: Fri, 13 Feb 2026 18:55:17 +0100 Subject: [PATCH 4/4] fix(api): use max_attempts instead of max_attempts - 1 in file changes retry Address PR review feedback: change retry guard from `attempt < (max_attempts - 1)` to `attempt < max_attempts` and move error logging after the loop for clarity. --- gittensor/utils/github_api_tools.py | 19 ++++++++----------- tests/utils/test_github_api_tools.py | 6 +++--- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/gittensor/utils/github_api_tools.py b/gittensor/utils/github_api_tools.py index 6e2fe36..41a8eec 100644 --- a/gittensor/utils/github_api_tools.py +++ b/gittensor/utils/github_api_tools.py @@ -262,6 +262,7 @@ def get_pull_request_file_changes(repository: str, pr_number: int, token: str) - max_attempts = 3 headers = make_headers(token) + last_error = None for attempt in range(max_attempts): try: response = requests.get( @@ -271,32 +272,28 @@ def get_pull_request_file_changes(repository: str, pr_number: int, token: str) - file_diffs = response.json() return [FileChange.from_github_response(pr_number, repository, file_diff) for file_diff in file_diffs] - if attempt < (max_attempts - 1): + last_error = f'status {response.status_code}' + if attempt < max_attempts: backoff_delay = min(5 * (2**attempt), 30) bt.logging.warning( f'File changes request for PR #{pr_number} in {repository} failed with status {response.status_code} ' f'(attempt {attempt + 1}/{max_attempts}), retrying in {backoff_delay}s...' ) time.sleep(backoff_delay) - else: - bt.logging.error( - f'File changes request for PR #{pr_number} in {repository} failed with status {response.status_code} ' - f'after {max_attempts} attempts' - ) except requests.exceptions.RequestException as e: - if attempt < (max_attempts - 1): + last_error = str(e) + if attempt < max_attempts: backoff_delay = min(5 * (2**attempt), 30) bt.logging.warning( f'File changes request error for PR #{pr_number} in {repository} ' f'(attempt {attempt + 1}/{max_attempts}): {e}, retrying in {backoff_delay}s...' ) time.sleep(backoff_delay) - else: - bt.logging.error( - f'File changes request for PR #{pr_number} in {repository} failed after {max_attempts} attempts: {e}' - ) + bt.logging.error( + f'File changes request for PR #{pr_number} in {repository} failed after {max_attempts} attempts: {last_error}' + ) return [] diff --git a/tests/utils/test_github_api_tools.py b/tests/utils/test_github_api_tools.py index 2268cbd..9f7bf01 100644 --- a/tests/utils/test_github_api_tools.py +++ b/tests/utils/test_github_api_tools.py @@ -441,7 +441,7 @@ def test_gives_up_after_three_attempts(self, mock_logging, mock_sleep, mock_get) result = get_pull_request_file_changes('owner/repo', 1, 'fake_token') assert mock_get.call_count == 3 - assert mock_sleep.call_count == 2 + assert mock_sleep.call_count == 3 assert result == [] mock_logging.error.assert_called() @@ -488,8 +488,8 @@ def test_exponential_backoff_timing(self, mock_logging, mock_sleep, mock_get): get_pull_request_file_changes('owner/repo', 1, 'fake_token') - mock_sleep.assert_has_calls([call(5), call(10)]) - assert mock_sleep.call_count == 2 + mock_sleep.assert_has_calls([call(5), call(10), call(20)]) + assert mock_sleep.call_count == 3 if __name__ == '__main__':