Skip to content

Commit

Permalink
Fix issue #276: Allow the openhands-resolver to also read full commen…
Browse files Browse the repository at this point in the history
…ts from PR (#277)

* Fix issue #276: Allow the openhands-resolver to also read full comments from PR

* Fix pr #277: Fix issue #276: Allow the openhands-resolver to also read full comments from PR
  • Loading branch information
openhands-agent authored Nov 8, 2024
1 parent 9485ec8 commit d801f94
Show file tree
Hide file tree
Showing 3 changed files with 140 additions and 11 deletions.
46 changes: 43 additions & 3 deletions openhands_resolver/issue_definitions.py
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,29 @@ def __download_pr_metadata(self, pull_number: int) -> tuple[list[str], list[str]


# Override processing of downloaded issues
def _get_pr_comments(self, pr_number: int) -> list[str] | None:
"""Download comments for a specific pull request from Github."""
url = f"https://api.github.com/repos/{self.owner}/{self.repo}/issues/{pr_number}/comments"
headers = {
"Authorization": f"token {self.token}",
"Accept": "application/vnd.github.v3+json",
}
params = {"per_page": 100, "page": 1}
all_comments = []

while True:
response = requests.get(url, headers=headers, params=params)
response.raise_for_status()
comments = response.json()

if not comments:
break

all_comments.extend([comment["body"] for comment in comments])
params["page"] += 1

return all_comments if all_comments else None

def get_converted_issues(self) -> list[GithubIssue]:
all_issues = self._download_issues_from_github()
converted_issues = []
Expand All @@ -339,6 +362,10 @@ def get_converted_issues(self) -> list[GithubIssue]:
body = issue.get("body") if issue.get("body") is not None else ""
closing_issues, review_comments, review_threads, thread_ids = self.__download_pr_metadata(issue["number"])
head_branch = issue["head"]["ref"]

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

issue_details = GithubIssue(
owner=self.owner,
repo=self.repo,
Expand All @@ -349,7 +376,8 @@ def get_converted_issues(self) -> list[GithubIssue]:
review_comments=review_comments,
review_threads=review_threads,
thread_ids=thread_ids,
head_branch=head_branch
head_branch=head_branch,
thread_comments=thread_comments
)

converted_issues.append(issue_details)
Expand Down Expand Up @@ -385,8 +413,20 @@ def get_instruction(self, issue: GithubIssue, prompt_template: str, repo_instruc
review_thread_file_str = json.dumps(review_thread_files, indent=4)
images.extend(self._extract_image_urls(review_thread_str))


instruction = template.render(issues=issues_str, review_comments=review_comments_str, review_threads=review_thread_str, files=review_thread_file_str, repo_instruction=repo_instruction)
# Format thread comments if they exist
thread_context = ""
if issue.thread_comments:
thread_context = "\n\nPR Thread Comments:\n" + "\n---\n".join(issue.thread_comments)
images.extend(self._extract_image_urls(thread_context))

instruction = template.render(
issues=issues_str,
review_comments=review_comments_str,
review_threads=review_thread_str,
files=review_thread_file_str,
thread_context=thread_context,
repo_instruction=repo_instruction
)
return instruction, images


Expand Down
69 changes: 69 additions & 0 deletions tests/test_issue_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,75 @@ def test_pr_handler_guess_success_with_thread_comments():
assert success_list == [True]
assert "successfully address" in explanation

def test_pr_handler_get_converted_issues_with_comments():
# Mock the necessary dependencies
with patch('requests.get') as mock_get:
# Mock the response for PRs
mock_prs_response = MagicMock()
mock_prs_response.json.return_value = [{
'number': 1,
'title': 'Test PR',
'body': 'Test Body',
'head': {'ref': 'test-branch'}
}]

# Mock the response for PR comments
mock_comments_response = MagicMock()
mock_comments_response.json.return_value = [
{'body': 'First comment'},
{'body': 'Second comment'}
]

# Mock the response for PR metadata (GraphQL)
mock_graphql_response = MagicMock()
mock_graphql_response.json.return_value = {
'data': {
'repository': {
'pullRequest': {
'closingIssuesReferences': {'edges': []},
'reviews': {'nodes': []},
'reviewThreads': {'edges': []}
}
}
}
}

# Set up the mock to return different responses
# We need to return empty responses for subsequent pages
mock_empty_response = MagicMock()
mock_empty_response.json.return_value = []

mock_get.side_effect = [
mock_prs_response, # First call for PRs
mock_empty_response, # Second call for PRs (empty page)
mock_comments_response, # Third call for PR comments
mock_empty_response, # Fourth call for PR comments (empty page)
]

# Mock the post request for GraphQL
with patch('requests.post') as mock_post:
mock_post.return_value = mock_graphql_response

# Create an instance of PRHandler
handler = PRHandler('test-owner', 'test-repo', 'test-token')

# Get converted issues
prs = handler.get_converted_issues()

# Verify that we got exactly one PR
assert len(prs) == 1

# Verify that thread_comments are set correctly
assert prs[0].thread_comments == ['First comment', 'Second comment']

# Verify other fields are set correctly
assert prs[0].number == 1
assert prs[0].title == 'Test PR'
assert prs[0].body == 'Test Body'
assert prs[0].owner == 'test-owner'
assert prs[0].repo == 'test-repo'
assert prs[0].head_branch == 'test-branch'

def test_pr_handler_guess_success_no_comments():
# Create a PR handler instance
handler = PRHandler('test-owner', 'test-repo', 'test-token')
Expand Down
36 changes: 28 additions & 8 deletions tests/test_resolve_issues.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,16 +125,21 @@ def get_mock_response(url, *args, **kwargs):

def test_download_pr_from_github():
handler = PRHandler("owner", "repo", "token")
mock_response = MagicMock()
mock_response.json.side_effect = [
mock_pr_response = MagicMock()
mock_pr_response.json.side_effect = [
[
{"number": 1, "title": "PR 1", "body": "This is a pull request", "head": {"ref": "b1"}},
{"number": 2, "title": "My PR", "body": "This is another pull request", "head": {"ref": "b2"}},
{"number": 3, "title": "PR 3", "body": "Final PR", "head": {"ref": "b3"}},
],
None,
]
mock_response.raise_for_status = MagicMock()
mock_pr_response.raise_for_status = MagicMock()

# Mock for PR comments response
mock_comments_response = MagicMock()
mock_comments_response.json.return_value = [] # No PR comments
mock_comments_response.raise_for_status = MagicMock()

# Mock for GraphQL request (for download_pr_metadata)
mock_graphql_response = MagicMock()
Expand Down Expand Up @@ -193,7 +198,12 @@ def test_download_pr_from_github():

mock_graphql_response.raise_for_status = MagicMock()

with patch('requests.get', return_value=mock_response):
def get_mock_response(url, *args, **kwargs):
if "/comments" in url:
return mock_comments_response
return mock_pr_response

with patch('requests.get', side_effect=get_mock_response):
with patch('requests.post', return_value=mock_graphql_response):
issues = handler.get_converted_issues()

Expand Down Expand Up @@ -666,14 +676,19 @@ def test_guess_success_invalid_output():

def test_download_pr_with_review_comments():
handler = PRHandler("owner", "repo", "token")
mock_response = MagicMock()
mock_response.json.side_effect = [
mock_pr_response = MagicMock()
mock_pr_response.json.side_effect = [
[
{"number": 1, "title": "PR 1", "body": "This is a pull request", "head": {"ref": "b1"}},
],
None,
]
mock_response.raise_for_status = MagicMock()
mock_pr_response.raise_for_status = MagicMock()

# Mock for PR comments response
mock_comments_response = MagicMock()
mock_comments_response.json.return_value = [] # No PR comments
mock_comments_response.raise_for_status = MagicMock()

# Mock for GraphQL request with review comments but no threads
mock_graphql_response = MagicMock()
Expand All @@ -697,7 +712,12 @@ def test_download_pr_with_review_comments():

mock_graphql_response.raise_for_status = MagicMock()

with patch('requests.get', return_value=mock_response):
def get_mock_response(url, *args, **kwargs):
if "/comments" in url:
return mock_comments_response
return mock_pr_response

with patch('requests.get', side_effect=get_mock_response):
with patch('requests.post', return_value=mock_graphql_response):
issues = handler.get_converted_issues()

Expand Down

0 comments on commit d801f94

Please sign in to comment.