Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve GitHub resolver robustness and efficiency #5462

Closed
wants to merge 10 commits into from
111 changes: 59 additions & 52 deletions openhands/resolver/issue_definitions.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,17 +150,12 @@ def get_converted_issues(
Returns:
List of Github issues.
"""

if not issue_numbers:
raise ValueError('Unspecified issue number')

all_issues = self._download_issues_from_github()
logger.info(f'Limiting resolving to issues {issue_numbers}.')
all_issues = [
issue
for issue in all_issues
if issue['number'] in issue_numbers and 'pull_request' not in issue
]
all_issues = [issue for issue in all_issues if issue['number'] in issue_numbers]

if len(issue_numbers) == 1 and not all_issues:
raise ValueError(f'Issue {issue_numbers[0]} not found')
Expand Down Expand Up @@ -509,57 +504,69 @@ def get_converted_issues(
if not issue_numbers:
raise ValueError('Unspecified issue numbers')

all_issues = self._download_issues_from_github()
enyst marked this conversation as resolved.
Show resolved Hide resolved
logger.info(f'Limiting resolving to issues {issue_numbers}.')
all_issues = [issue for issue in all_issues if issue['number'] in issue_numbers]

logger.info(f'Fetching issues {issue_numbers}.')
converted_issues = []
for issue in all_issues:
# For PRs, body can be None
if any([issue.get(key) is None for key in ['number', 'title']]):
logger.warning(f'Skipping #{issue} as it is missing number or title.')
continue
headers = {
'Authorization': f'token {self.token}',
'Accept': 'application/vnd.github.v3+json',
}

# Handle None body for PRs
body = issue.get('body') if issue.get('body') is not None else ''
(
closing_issues,
closing_issues_numbers,
review_comments,
review_threads,
thread_ids,
) = self.__download_pr_metadata(issue['number'], comment_id=comment_id)
head_branch = issue['head']['ref']

# Get PR thread comments
thread_comments = self._get_pr_comments(
issue['number'], comment_id=comment_id
)
for issue_number in issue_numbers:
try:
url = f'https://api.github.com/repos/{self.owner}/{self.repo}/issues/{issue_number}'
response = requests.get(url, headers=headers)
response.raise_for_status()
issue = response.json()

closing_issues = self.__get_context_from_external_issues_references(
closing_issues,
closing_issues_numbers,
body,
review_comments,
review_threads,
thread_comments,
)
# For PRs, body can be None
if any([issue.get(key) is None for key in ['number', 'title']]):
logger.warning(
f'Skipping #{issue} as it is missing number or title.'
)
continue

# Handle None body for PRs
body = issue.get('body') if issue.get('body') is not None else ''
(
closing_issues,
closing_issues_numbers,
review_comments,
review_threads,
thread_ids,
) = self.__download_pr_metadata(issue['number'], comment_id=comment_id)
head_branch = issue['head']['ref']

# Get PR thread comments
thread_comments = self._get_pr_comments(
issue['number'], comment_id=comment_id
)

issue_details = GithubIssue(
owner=self.owner,
repo=self.repo,
number=issue['number'],
title=issue['title'],
body=body,
closing_issues=closing_issues,
review_comments=review_comments,
review_threads=review_threads,
thread_ids=thread_ids,
head_branch=head_branch,
thread_comments=thread_comments,
)
closing_issues = self.__get_context_from_external_issues_references(
closing_issues,
closing_issues_numbers,
body,
review_comments,
review_threads,
thread_comments,
)

converted_issues.append(issue_details)
issue_details = GithubIssue(
owner=self.owner,
repo=self.repo,
number=issue['number'],
title=issue['title'],
body=body,
closing_issues=closing_issues,
review_comments=review_comments,
review_threads=review_threads,
thread_ids=thread_ids,
head_branch=head_branch,
thread_comments=thread_comments,
)

converted_issues.append(issue_details)
except requests.exceptions.RequestException as e:
logger.warning(f'Failed to fetch issue {issue_number}: {str(e)}')

return converted_issues

Expand Down
77 changes: 77 additions & 0 deletions tests/unit/test_issue_definitions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import pytest
import responses

from openhands.core.config import LLMConfig
from openhands.resolver.issue_definitions import PRHandler


@pytest.fixture
def pr_handler():
return PRHandler(
owner='test-owner',
repo='test-repo',
token='test-token',
llm_config=LLMConfig(model='test-model'),
)


@responses.activate
def test_get_converted_issues_fetches_specific_issues(pr_handler):
# Mock the specific issue endpoint
responses.add(
responses.GET,
'https://api.github.com/repos/test-owner/test-repo/issues/123',
json={
'number': 123,
'title': 'Test Issue',
'body': 'Test body',
'state': 'open',
'head': {'ref': 'test-branch'},
'pull_request': {
'url': 'https://github.com/test-owner/test-repo/pull/123'
}, # This makes it a PR
},
status=200,
)

# Mock the PR metadata endpoint
responses.add(
responses.POST,
'https://api.github.com/graphql',
json={
'data': {
'repository': {
'pullRequest': {
'closingIssuesReferences': {'edges': []},
'url': 'https://github.com/test-owner/test-repo/pull/123',
'reviews': {'nodes': []},
'reviewThreads': {'edges': []},
}
}
}
},
status=200,
)

# Mock the PR comments endpoint
responses.add(
responses.GET,
'https://api.github.com/repos/test-owner/test-repo/issues/123/comments',
json=[],
status=200,
)

# Test fetching a specific issue
issues = pr_handler.get_converted_issues(issue_numbers=[123])

assert len(issues) == 1
assert issues[0].number == 123
assert issues[0].title == 'Test Issue'

# Print out all the requests that were made
for i, call in enumerate(responses.calls):
print(f'Request {i+1}: {call.request.method} {call.request.url}')

# Verify that only the necessary requests were made
assert len(responses.calls) == 3 # Issue, PR metadata, and PR comments
assert '/issues/123' in responses.calls[0].request.url
Loading