Skip to content

Commit b236d01

Browse files
minion1227claude
andauthored
fix(cli): distinguish GitHub lookup failure from empty submissions (#1492) (#1554)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 05ce577 commit b236d01

5 files changed

Lines changed: 97 additions & 11 deletions

File tree

gittensor/cli/issue_commands/helpers.py

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
from contextlib import nullcontext
1414
from decimal import Decimal, InvalidOperation
1515
from pathlib import Path
16-
from typing import Any, Callable, ContextManager, Dict, List, Optional, Tuple, TypeVar
16+
from typing import Any, Callable, ContextManager, Dict, List, NoReturn, Optional, Tuple, TypeVar
1717

1818
import click
1919
import requests
@@ -197,7 +197,7 @@ def print_warning(message: str) -> None:
197197
err_console.print(f'\n[yellow]{message}[/yellow]\n', highlight=True)
198198

199199

200-
def handle_exception(as_json: bool, message: str, error_type: str = 'cli_error') -> None:
200+
def handle_exception(as_json: bool, message: str, error_type: str = 'cli_error') -> NoReturn:
201201
"""Emit a CLI error in JSON or human format and exit non-zero."""
202202
if as_json:
203203
emit_error_json(message, error_type=error_type)
@@ -254,8 +254,13 @@ def fetch_open_issue_pull_requests(
254254
repository_full_name: str,
255255
issue_number: int,
256256
as_json: bool,
257-
) -> list:
258-
"""Fetch open PR submissions for a GitHub issue."""
257+
) -> Optional[list]:
258+
"""Fetch open PR submissions for a GitHub issue.
259+
260+
Returns a (possibly empty) list of PRs, or ``None`` when the GitHub lookup
261+
fails. Callers must treat ``None`` as a failure (not "no submissions"); see
262+
``find_prs_for_issue``.
263+
"""
259264
token = os.environ.get('GITTENSOR_MINER_PAT') or ''
260265
if not token and not as_json:
261266
print_warning('No GitHub token found; set GITTENSOR_MINER_PAT to fetch GitHub issue submissions')
@@ -270,7 +275,8 @@ def fetch_open_issue_pull_requests(
270275
token=token or None,
271276
open_only=True,
272277
)
273-
# Intentionally return GitHub tool output as-is (no CLI schema mapping yet).
278+
# Intentionally return GitHub tool output as-is (no CLI schema mapping yet);
279+
# this includes the None failure sentinel, which the caller must handle.
274280
return prs
275281
except Exception as e:
276282
raise click.ClickException(f'Failed to fetch PR submissions from GitHub: {e}')

gittensor/cli/issue_commands/submissions.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,17 @@ def issues_submissions(
8181
except click.ClickException as e:
8282
handle_exception(as_json, str(e), click_error_type(e))
8383

84+
# None is the GitHub lookup-failure sentinel (rate limit, network/GraphQL
85+
# error). Surface it as an explicit error instead of reporting an empty
86+
# submission list, which would be a false negative for monitoring/automation.
87+
if pull_requests is None:
88+
handle_exception(
89+
as_json,
90+
f'GitHub lookup failed for {repo_name}#{issue_number}; '
91+
'submissions could not be determined. Please retry.',
92+
'github_lookup_failed',
93+
)
94+
8495
if as_json:
8596
submissions = [
8697
{

gittensor/utils/github_api_tools.py

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -329,14 +329,26 @@ def find_prs_for_issue(
329329
issue_number: int,
330330
open_only: bool = True,
331331
token: Optional[str] = None,
332-
) -> List[PRInfo]:
333-
"""Find PRs that reference an issue via GraphQL cross-reference data."""
332+
) -> Optional[List[PRInfo]]:
333+
"""Find PRs that reference an issue via GraphQL cross-reference data.
334+
335+
Returns a (possibly empty) list on success, or ``None`` when the GraphQL
336+
lookup fails (rate limit, network error, GraphQL errors, missing issue
337+
payload, or an exception). ``None`` is a failure sentinel that callers must
338+
distinguish from ``[]`` ("no referencing PRs exist") — mirroring the
339+
``solver_lookup_failed`` signaling in ``find_solver_from_closure_event`` /
340+
``check_github_issue_closed``. The empty-list return for a falsy token is a
341+
precondition-not-met case, not a transient failure.
342+
"""
334343
if token:
335344
try:
336-
prs = _search_issue_referencing_prs_graphql(repo, issue_number, token, open_only=open_only)
337-
return prs or []
345+
# Propagate the None failure sentinel from the GraphQL helper as-is;
346+
# collapsing it to [] would make a lookup failure indistinguishable
347+
# from a genuinely empty submission list.
348+
return _search_issue_referencing_prs_graphql(repo, issue_number, token, open_only=open_only)
338349
except Exception as exc:
339350
bt.logging.debug(f'GraphQL PR fetch failed for {repo}#{issue_number}: {exc}')
351+
return None
340352

341353
return []
342354

tests/cli/test_issue_submission.py

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,49 @@ def test_submissions_human_no_open_prs_message(cli_root, runner, sample_issue):
112112
assert 'No open submissions available' in result.output
113113

114114

115+
def test_submissions_json_lookup_failure_returns_structured_error(cli_root, runner, sample_issue):
116+
"""A GitHub lookup failure (find_prs_for_issue -> None, propagated as None by
117+
fetch_open_issue_pull_requests) must surface as `success: false` with a
118+
`github_lookup_failed` type — not a false-negative `submission_count: 0`."""
119+
with (
120+
patch('gittensor.cli.issue_commands.submissions.get_contract_address', return_value='0xabc'),
121+
patch('gittensor.cli.issue_commands.submissions.resolve_network', return_value=('ws://x', 'test')),
122+
patch('gittensor.cli.issue_commands.submissions.fetch_issue_from_contract', return_value=sample_issue),
123+
patch('gittensor.cli.issue_commands.submissions.fetch_open_issue_pull_requests', return_value=None),
124+
):
125+
result = runner.invoke(
126+
cli_root,
127+
['issues', 'submissions', '--id', '42', '--json'],
128+
catch_exceptions=False,
129+
)
130+
131+
assert result.exit_code != 0
132+
payload = json.loads(result.stdout)
133+
assert payload['success'] is False
134+
assert payload['error']['type'] == 'github_lookup_failed'
135+
assert 'submission_count' not in payload
136+
137+
138+
def test_submissions_human_lookup_failure_errors_instead_of_no_submissions(cli_root, runner, sample_issue):
139+
"""In human mode a lookup failure must error out, not print the misleading
140+
'No open submissions available' message used for a genuinely empty list."""
141+
with (
142+
patch('gittensor.cli.issue_commands.submissions.get_contract_address', return_value='0xabc'),
143+
patch('gittensor.cli.issue_commands.submissions.resolve_network', return_value=('ws://x', 'test')),
144+
patch('gittensor.cli.issue_commands.submissions.fetch_issue_from_contract', return_value=sample_issue),
145+
patch('gittensor.cli.issue_commands.submissions.fetch_open_issue_pull_requests', return_value=None),
146+
):
147+
result = runner.invoke(
148+
cli_root,
149+
['issues', 'submissions', '--id', '42'],
150+
catch_exceptions=False,
151+
)
152+
153+
assert result.exit_code != 0
154+
assert 'No open submissions available' not in result.output
155+
assert 'GitHub lookup failed' in result.output
156+
157+
115158
def test_submissions_json_contract_read_failure_returns_structured_error(cli_root, runner):
116159
"""`fetch_issue_from_contract` now converts contract-read failures to a
117160
`ClickException`, which `submissions` routes through `handle_exception` —

tests/utils/test_github_api_tools.py

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,12 +172,26 @@ def test_find_prs_returns_empty_when_graphql_empty(mock_graphql):
172172

173173

174174
@patch('gittensor.utils.github_api_tools._search_issue_referencing_prs_graphql')
175-
def test_find_prs_returns_empty_when_graphql_errors(mock_graphql):
175+
def test_find_prs_returns_none_when_graphql_errors(mock_graphql):
176+
# An exception during the GraphQL lookup is a failure, not "no PRs" — it must
177+
# surface as the None sentinel so callers can distinguish it from [].
176178
mock_graphql.side_effect = RuntimeError('boom')
177179

178180
result = find_prs_for_issue('owner/repo', 12, open_only=True, token='fake_token')
179181

180-
assert result == []
182+
assert result is None
183+
mock_graphql.assert_called_once_with('owner/repo', 12, 'fake_token', open_only=True)
184+
185+
186+
@patch('gittensor.utils.github_api_tools._search_issue_referencing_prs_graphql')
187+
def test_find_prs_returns_none_when_graphql_lookup_fails(mock_graphql):
188+
# The GraphQL helper returns None on rate limit / network / GraphQL errors;
189+
# find_prs_for_issue must propagate it rather than collapsing to [].
190+
mock_graphql.return_value = None
191+
192+
result = find_prs_for_issue('owner/repo', 12, open_only=True, token='fake_token')
193+
194+
assert result is None
181195
mock_graphql.assert_called_once_with('owner/repo', 12, 'fake_token', open_only=True)
182196

183197

0 commit comments

Comments
 (0)