Compare commits

...

18 Commits

Author SHA1 Message Date
openhands
327cc09f31 Fix pr #5449: Fix issue #5448: When using the GitHub resolver on a PR, automatically pipe in failure info 2025-01-07 02:31:18 +00:00
Graham Neubig
5dd5ba66df Merge branch 'main' into openhands-fix-issue-5448 2024-12-28 20:32:30 -05:00
openhands
ff8b615151 Use relative path in test_pr_status_in_prompt.py 2024-12-29 01:25:43 +00:00
openhands
c6f0afdc8d Fix duplicate linting information in PR status
1. Add check to avoid duplicate CI check entries
2. Remove separate linting section since it's redundant with CI check information
3. Update tests to match new behavior
2024-12-29 00:59:30 +00:00
openhands
1209022393 Fix failing tests in test_pr_status.py
Update test assertions to match new behavior where 'This PR has no merge conflicts' is only included when there's already some status information.
2024-12-29 00:54:01 +00:00
openhands
53ec2bcdb1 Fix PR status not showing in prompt
1. Add pr_status section to basic-followup.jinja template
2. Add unit tests to verify PR status in prompts
2024-12-29 00:47:13 +00:00
openhands
347e35a1f2 Fix linting issues and update PR status handling 2024-12-28 20:53:41 +00:00
openhands
3822033672 Rename test case for failed lint check 2024-12-28 20:27:50 +00:00
openhands
bfc3b1b082 Fix handling of unknown or pending PR statuses 2024-12-28 20:20:08 +00:00
openhands
0c0faae376 Add blank line between import statements 2024-12-28 20:03:09 +00:00
openhands
739dbdc91f Format pyproject.toml 2024-12-28 19:57:22 +00:00
openhands
4680c54137 Fix PR status message and update tests 2024-12-28 19:57:14 +00:00
Graham Neubig
9b5b14fd89 Merge branch 'main' into openhands-fix-issue-5448 2024-12-28 14:30:16 -05:00
Graham Neubig
b100cc872d Merge branch 'main' into openhands-fix-issue-5448 2024-12-25 17:52:03 -05:00
Graham Neubig
07027f3adc Merge branch 'main' into openhands-fix-issue-5448 2024-12-25 17:06:45 -05:00
Graham Neubig
66aa3395ee Merge branch 'main' into openhands-fix-issue-5448 2024-12-22 13:05:31 -05:00
openhands
07cef7cc95 style: Format code with ruff and fix linting issues 2024-12-07 19:58:42 +00:00
openhands
9df2dae82c Fix issue #5448: When using the GitHub resolver on a PR, automatically pipe in failure info 2024-12-07 19:27:45 +00:00
8 changed files with 769 additions and 0 deletions

149
investigate_pr_14.py Normal file
View File

@@ -0,0 +1,149 @@
import os
import logging
import traceback
from openhands.resolver.issue_definitions import PRHandler
from openhands.core.config import LLMConfig
# Set up logging
logging.basicConfig(level=logging.DEBUG, format='%(asctime)s - %(name)s - %(levelname)s - %(message)s')
logger = logging.getLogger(__name__)
# Modify the PRHandler class to add logging
class DebugPRHandler(PRHandler):
def __get_pr_status(self, pull_number: int):
logger.debug(f"Fetching PR status for PR #{pull_number}")
try:
result = super().__get_pr_status(pull_number)
logger.debug(f"PR status result: {result}")
# Print detailed information about the API response
print(f"Detailed PR status for PR #{pull_number}:")
print(f"Mergeable status: {result[0]}")
print("Failed checks:")
for check in result[1]:
print(f" Name: {check['name']}")
print(f" Description: {check['description']}")
print(" ---")
return result
except Exception as e:
logger.error(f"Error in __get_pr_status: {str(e)}")
logger.debug(traceback.format_exc())
raise
def get_instruction(self, issue, prompt_template, repo_instruction=None):
logger.debug(f"Generating instruction for PR #{issue.number}")
logger.debug(f"Prompt template: {prompt_template}")
logger.debug(f"Issue data: {issue}")
try:
result = super().get_instruction(issue, prompt_template, repo_instruction)
logger.debug(f"Generated instruction: {result[0]}")
return result
except Exception as e:
logger.error(f"Error in get_instruction: {str(e)}")
logger.debug(traceback.format_exc())
raise
def get_converted_issues(self, issue_numbers=None, comment_id=None):
logger.debug(f"Getting converted issues: {issue_numbers}")
try:
result = super().get_converted_issues(issue_numbers, comment_id)
logger.debug(f"Converted issues: {result}")
# Print detailed information about each converted issue
for issue in result:
print(f"\nDetailed information for PR #{issue.number}:")
print(f"Title: {issue.title}")
print(f"Merge conflicts: {issue.has_merge_conflicts}")
print("Failed checks:")
if issue.failed_checks:
for check in issue.failed_checks:
print(f" Name: {check['name']}")
print(f" Description: {check['description']}")
print(" ---")
else:
print(" No failed checks")
return result
except Exception as e:
logger.error(f"Error in get_converted_issues: {str(e)}")
logger.debug(traceback.format_exc())
raise
def main():
try:
print("Starting main function")
# GitHub token
github_token = os.environ.get('GITHUB_TOKEN')
if not github_token:
raise ValueError("GITHUB_TOKEN environment variable is not set")
print("GitHub token retrieved")
# Create PRHandler instance
llm_config = LLMConfig(model='gpt-3.5-turbo') # Adjust as needed
pr_handler = DebugPRHandler('neubig', 'pr-viewer', github_token, llm_config)
print("PRHandler instance created")
# Call __get_pr_status directly
print("Calling __get_pr_status for PR #14")
has_conflicts, failed_checks = pr_handler._PRHandler__get_pr_status(14)
print(f"PR #14 status: has_conflicts={has_conflicts}, failed_checks={failed_checks}")
# Fetch PR #14
print("Fetching PR #14")
issues = pr_handler.get_converted_issues([14])
if not issues:
logger.error("Failed to fetch PR #14")
return
print(f"Fetched {len(issues)} issues")
pr = issues[0]
print(f"PR data: {pr}")
# Generate instruction
prompt_template = """
{{ pr_status }}
{{ body }}
{% if review_comments %}
Review Comments:
{{ review_comments }}
{% endif %}
{% if review_threads %}
Review Threads:
{{ review_threads }}
{% endif %}
{% if files %}
Files:
{{ files }}
{% endif %}
{% if thread_context %}
Thread Context:
{{ thread_context }}
{% endif %}
{% if repo_instruction %}
Repository Instruction:
{{ repo_instruction }}
{% endif %}
"""
print("Generating instruction")
instruction, _ = pr_handler.get_instruction(pr, prompt_template)
print("Instruction generated")
print(f"Final instruction:\n{instruction}")
except Exception as e:
print(f"An error occurred in main: {str(e)}")
print(traceback.format_exc())
if __name__ == "__main__":
try:
main()
except Exception as e:
print(f"An error occurred in main: {str(e)}")
print(traceback.format_exc())

View File

@@ -18,3 +18,5 @@ class GithubIssue(BaseModel):
review_threads: list[ReviewThread] | None = None
thread_ids: list[str] | None = None
head_branch: str | None = None
has_merge_conflicts: bool | None = None
failed_checks: list[dict[str, str | None]] | None = None

View File

@@ -296,6 +296,129 @@ class PRHandler(IssueHandler):
super().__init__(owner, repo, token, llm_config)
self.download_url = 'https://api.github.com/repos/{}/{}/pulls'
def __get_pr_status(
self, pull_number: int
) -> tuple[bool | None, list[dict[str, str | None]]]:
"""Get the PR's merge conflict status and CI check status.
Args:
pull_number: The number of the pull request to query.
Returns:
A tuple containing:
- bool | None: Whether the PR has merge conflicts (None if unknown)
- list[dict[str, str | None]]: List of failed CI checks with their details (empty list if no failures)
"""
query = """
query($owner: String!, $repo: String!, $pr: Int!) {
repository(owner: $owner, name: $repo) {
pullRequest(number: $pr) {
mergeable
commits(last: 1) {
nodes {
commit {
statusCheckRollup {
contexts(first: 100) {
nodes {
... on StatusContext {
context
state
description
}
... on CheckRun {
name
conclusion
text
summary
title
}
}
}
}
}
}
}
}
}
}
"""
variables = {'owner': self.owner, 'repo': self.repo, 'pr': pull_number}
url = 'https://api.github.com/graphql'
headers = {
'Authorization': f'Bearer {self.token}',
'Content-Type': 'application/json',
}
response = requests.post(
url, json={'query': query, 'variables': variables}, headers=headers
)
response.raise_for_status()
response_json = response.json()
# Print raw API response for debugging
print(f'Raw API response for PR #{pull_number}:')
print(json.dumps(response_json, indent=2))
pr_data = (
response_json.get('data', {}).get('repository', {}).get('pullRequest', {})
)
# Check mergeable status
mergeable = pr_data.get('mergeable')
has_conflicts = None if mergeable == 'UNKNOWN' else (mergeable == 'CONFLICTING')
# Check CI status
failed_checks = []
seen_checks = set() # Track seen check names to avoid duplicates
commits = pr_data.get('commits', {}).get('nodes', [])
if commits:
status_rollup = commits[0].get('commit', {}).get('statusCheckRollup', {})
contexts = status_rollup.get('contexts', {}).get('nodes', [])
for context in contexts:
# Handle both StatusContext and CheckRun types
if 'state' in context: # StatusContext
if context['state'] in ['FAILURE', 'ERROR']:
name = context['context']
if name not in seen_checks:
seen_checks.add(name)
failed_checks.append(
{
'name': name,
'description': context.get('description')
or 'No description provided',
}
)
elif 'conclusion' in context: # CheckRun
if context['conclusion'] in [
'FAILURE',
'TIMED_OUT',
'CANCELLED',
'ACTION_REQUIRED',
]:
name = context['name']
if name not in seen_checks:
seen_checks.add(name)
description = (
context.get('text')
or context.get('summary')
or context.get('title')
or 'No description provided'
)
failed_checks.append(
{
'name': name,
'description': description,
}
)
print(f'Processed PR status for PR #{pull_number}:')
print(f'has_conflicts: {has_conflicts}')
print(f'failed_checks: {failed_checks}')
return has_conflicts, failed_checks
def __download_pr_metadata(
self, pull_number: int, comment_id: int | None = None
) -> tuple[list[str], list[int], list[str], list[ReviewThread], list[str]]:
@@ -571,6 +694,9 @@ class PRHandler(IssueHandler):
issue['number'], comment_id=comment_id
)
# Get PR status (merge conflicts and CI checks)
has_conflicts, failed_checks = self.__get_pr_status(issue['number'])
closing_issues = self.__get_context_from_external_issues_references(
closing_issues,
closing_issues_numbers,
@@ -592,6 +718,8 @@ class PRHandler(IssueHandler):
thread_ids=thread_ids,
head_branch=head_branch,
thread_comments=thread_comments,
has_merge_conflicts=has_conflicts,
failed_checks=failed_checks,
)
converted_issues.append(issue_details)
@@ -639,6 +767,43 @@ class PRHandler(IssueHandler):
thread_context = '\n---\n'.join(issue.thread_comments)
images.extend(self._extract_image_urls(thread_context))
# Add PR status information
pr_status = ''
if issue.has_merge_conflicts is None:
pr_status += 'The merge status of this PR is currently unknown or pending.'
elif issue.has_merge_conflicts:
pr_status += 'This PR has merge conflicts that need to be resolved.'
elif not issue.has_merge_conflicts:
pr_status += 'This PR has no merge conflicts'
if issue.failed_checks is None:
pr_status += '\nThe CI check status is currently unknown or pending.'
elif issue.failed_checks:
pr_status += '\nThe following CI checks have failed:\n'
for check in issue.failed_checks:
pr_status += f"- {check['name']}: {check['description']}\n"
pr_status += 'Please examine the GitHub workflow files, reproduce the problem locally, and fix and test it locally.'
# Add note about running tests locally
pr_status += '\nPlease run the failing checks locally to fix the issues.'
elif not issue.failed_checks:
if pr_status:
pr_status += ' and all CI checks have passed.'
else:
pr_status += (
'This PR has no merge conflicts and all CI checks have passed.'
)
# Add a note about the lack of detailed information
if issue.failed_checks and all(
check['description'] == 'No description provided'
for check in issue.failed_checks
):
pr_status += '\n\nNote: Detailed information about the failed checks is not available. Please check the GitHub Actions tab for more information.'
# Remove leading newline if present
pr_status = pr_status.lstrip()
instruction = template.render(
issues=issues_str,
review_comments=review_comments_str,
@@ -646,6 +811,7 @@ class PRHandler(IssueHandler):
files=review_thread_file_str,
thread_context=thread_context,
repo_instruction=repo_instruction,
pr_status=pr_status,
)
return instruction, images

View File

@@ -6,6 +6,11 @@ An environment has been set up for you to start working. You may assume all nece
# Issues addressed
{{ issues }}
{% if pr_status %}
# PR Status
{{ pr_status }}
{% endif %}
# Review comments
{{ review_comments }}

View File

@@ -100,6 +100,7 @@ reportlab = "*"
[tool.coverage.run]
concurrency = ["gevent"]
[tool.poetry.group.runtime.dependencies]
jupyterlab = "*"
notebook = "*"
@@ -129,6 +130,7 @@ ignore = ["D1"]
[tool.ruff.lint.pydocstyle]
convention = "google"
[tool.poetry.group.evaluation.dependencies]
streamlit = "*"
whatthepatch = "*"

View File

@@ -0,0 +1,138 @@
from unittest.mock import MagicMock, patch
from openhands.core.config import LLMConfig
from openhands.resolver.github_issue import GithubIssue
from openhands.resolver.issue_definitions import PRHandler
@patch('requests.post')
def test_pr_status_in_basic_followup_template(mock_post):
"""Test that PR status is included in the basic-followup template."""
# Mock the GitHub API response for PR status
mock_response = MagicMock()
mock_response.json.return_value = {
'data': {
'repository': {
'pullRequest': {
'mergeable': 'MERGEABLE',
'commits': {
'nodes': [
{
'commit': {
'statusCheckRollup': {
'contexts': {
'nodes': [
{
'name': 'lint',
'conclusion': 'FAILURE',
'text': 'ESLint found issues',
}
]
}
}
}
}
]
},
}
}
}
}
mock_post.return_value = mock_response
# Create a PR handler instance
llm_config = LLMConfig(model='test-model')
handler = PRHandler('test-owner', 'test-repo', 'test-token', llm_config)
# Create a mock issue
issue = GithubIssue(
owner='test-owner',
repo='test-repo',
number=123,
title='Test PR',
body='Test body',
has_merge_conflicts=False,
failed_checks=[{'name': 'lint', 'description': 'ESLint found issues'}],
)
# Use the basic-followup template
with open('openhands/resolver/prompts/resolve/basic-followup.jinja', 'r') as f:
template = f.read()
# Generate instruction
instruction, _ = handler.get_instruction(issue, template)
# Check that PR status information is included
assert 'The following CI checks have failed:' in instruction
assert 'lint: ESLint found issues' in instruction
assert 'Please examine the GitHub workflow files' in instruction
assert 'Please run the failing checks locally to fix the issues.' in instruction
@patch('requests.post')
def test_pr_status_in_custom_template(mock_post):
"""Test that PR status is included when using a custom template."""
# Mock the GitHub API response for PR status
mock_response = MagicMock()
mock_response.json.return_value = {
'data': {
'repository': {
'pullRequest': {
'mergeable': 'MERGEABLE',
'commits': {
'nodes': [
{
'commit': {
'statusCheckRollup': {
'contexts': {
'nodes': [
{
'name': 'lint',
'conclusion': 'FAILURE',
'text': 'ESLint found issues',
}
]
}
}
}
}
]
},
}
}
}
}
mock_post.return_value = mock_response
# Create a PR handler instance
llm_config = LLMConfig(model='test-model')
handler = PRHandler('test-owner', 'test-repo', 'test-token', llm_config)
# Create a mock issue
issue = GithubIssue(
owner='test-owner',
repo='test-repo',
number=123,
title='Test PR',
body='Test body',
has_merge_conflicts=False,
failed_checks=[{'name': 'lint', 'description': 'ESLint found issues'}],
)
# Use a custom template that includes pr_status
template = """
# PR Status
{{ pr_status }}
# Problem Statement
{{ body }}
"""
# Generate instruction
instruction, _ = handler.get_instruction(issue, template)
# Check that PR status information is included
assert 'The following CI checks have failed:' in instruction
assert 'lint: ESLint found issues' in instruction
assert 'Please examine the GitHub workflow files' in instruction
assert 'Please run the failing checks locally to fix the issues.' in instruction

View File

@@ -0,0 +1,112 @@
from unittest.mock import MagicMock, patch
import pytest
from openhands.core.config import LLMConfig
from openhands.resolver.github_issue import GithubIssue
from openhands.resolver.issue_definitions import PRHandler
@pytest.fixture
def pr_handler():
llm_config = LLMConfig(model='test-model')
return PRHandler('test-owner', 'test-repo', 'test-token', llm_config)
def test_get_pr_status(pr_handler):
# Mock the response from GitHub API
mock_response = MagicMock()
mock_response.json.return_value = {
'data': {
'repository': {
'pullRequest': {
'mergeable': 'CONFLICTING',
'commits': {
'nodes': [
{
'commit': {
'statusCheckRollup': {
'contexts': {
'nodes': [
{
'context': 'test-ci',
'state': 'FAILURE',
'description': 'Tests failed',
},
{
'name': 'build',
'conclusion': 'FAILURE',
'text': 'Build failed',
},
]
}
}
}
}
]
},
}
}
}
}
with patch('requests.post', return_value=mock_response):
has_conflicts, failed_checks = pr_handler._PRHandler__get_pr_status(123)
assert has_conflicts is True
assert len(failed_checks) == 2
assert failed_checks[0] == {'name': 'test-ci', 'description': 'Tests failed'}
assert failed_checks[1] == {'name': 'build', 'description': 'Build failed'}
def test_get_instruction_with_pr_status(pr_handler):
# Create a test issue with merge conflicts and failed checks
issue = GithubIssue(
owner='test-owner',
repo='test-repo',
number=123,
title='Test PR',
body='Test body',
has_merge_conflicts=True,
failed_checks=[
{'name': 'test-ci', 'description': 'Tests failed'},
{'name': 'build', 'description': 'Build failed'},
],
)
# Test template that includes pr_status
template = '{{ pr_status }}'
instruction, _ = pr_handler.get_instruction(issue, template)
# Verify the instruction includes merge conflict and CI failure info
assert 'merge conflicts that need to be resolved' in instruction
assert 'The following CI checks have failed:' in instruction
assert 'test-ci: Tests failed' in instruction
assert 'build: Build failed' in instruction
assert 'examine the GitHub workflow files' in instruction
def test_get_instruction_without_pr_status(pr_handler):
# Create a test issue without merge conflicts or failed checks
issue = GithubIssue(
owner='test-owner',
repo='test-repo',
number=123,
title='Test PR',
body='Test body',
has_merge_conflicts=False,
failed_checks=[],
)
# Test template that includes pr_status
template = '{{ pr_status }}'
instruction, _ = pr_handler.get_instruction(issue, template)
# Verify the instruction includes the message for no merge conflicts and passed CI checks
assert (
'This PR has no merge conflicts and all CI checks have passed.' in instruction
)
assert 'merge conflicts that need to be resolved' not in instruction
assert 'CI checks have failed' not in instruction

View File

@@ -0,0 +1,195 @@
from unittest.mock import MagicMock, patch
import pytest
from openhands.core.config import LLMConfig
from openhands.resolver.github_issue import GithubIssue
from openhands.resolver.issue_definitions import PRHandler
@pytest.fixture
def pr_handler():
llm_config = LLMConfig(model='test-model')
return PRHandler('test-owner', 'test-repo', 'test-token', llm_config)
@patch('requests.post')
def test_get_pr_status_pending(mock_post, pr_handler):
mock_response = MagicMock()
mock_response.json.return_value = {
'data': {
'repository': {
'pullRequest': {
'mergeable': 'UNKNOWN',
'commits': {
'nodes': [
{
'commit': {
'statusCheckRollup': {'contexts': {'nodes': []}}
}
}
]
},
}
}
}
}
mock_post.return_value = mock_response
has_conflicts, failed_checks = pr_handler._PRHandler__get_pr_status(123)
assert has_conflicts is None
assert failed_checks == [] # Changed from 'is None' to '== []'
@patch('requests.post')
def test_get_instruction_pending_status(mock_post, pr_handler):
mock_response = MagicMock()
mock_response.json.return_value = {
'data': {
'repository': {
'pullRequest': {
'mergeable': 'UNKNOWN',
'commits': {
'nodes': [
{
'commit': {
'statusCheckRollup': {'contexts': {'nodes': []}}
}
}
]
},
}
}
}
}
mock_post.return_value = mock_response
issue = GithubIssue(
owner='test-owner',
repo='test-repo',
number=123,
title='Test PR',
body='Test body',
has_merge_conflicts=None,
failed_checks=None,
)
template = '{{ pr_status }}'
instruction, _ = pr_handler.get_instruction(issue, template)
assert 'The merge status of this PR is currently unknown or pending.' in instruction
assert 'The CI check status is currently unknown or pending.' in instruction
@patch('requests.post')
def test_get_instruction_with_linting_issues(mock_post, pr_handler):
mock_response = MagicMock()
mock_response.json.return_value = {
'data': {
'repository': {
'pullRequest': {
'mergeable': 'MERGEABLE',
'commits': {
'nodes': [
{
'commit': {
'statusCheckRollup': {
'contexts': {
'nodes': [
{
'name': 'lint',
'conclusion': 'FAILURE',
'text': 'ESLint found 2 errors and 1 warning',
}
]
}
}
}
}
]
},
}
}
}
}
mock_post.return_value = mock_response
issue = GithubIssue(
owner='test-owner',
repo='test-repo',
number=123,
title='Test PR',
body='Test body',
has_merge_conflicts=False,
failed_checks=[
{'name': 'lint', 'description': 'ESLint found 2 errors and 1 warning'}
],
)
template = '{{ pr_status }}'
instruction, _ = pr_handler.get_instruction(issue, template)
assert 'The following CI checks have failed:' in instruction
assert 'lint: ESLint found 2 errors and 1 warning' in instruction
assert 'Please examine the GitHub workflow files' in instruction
assert 'Please run the failing checks locally to fix the issues.' in instruction
@patch('requests.post')
def test_get_instruction_with_failed_lint_check(mock_post, pr_handler, capsys):
# Mocking the response from GitHub API for PR #14
mock_response = MagicMock()
mock_response.json.return_value = {
'data': {
'repository': {
'pullRequest': {
'mergeable': 'MERGEABLE',
'commits': {
'nodes': [
{
'commit': {
'statusCheckRollup': {
'contexts': {
'nodes': [
{
'name': 'Lint',
'conclusion': 'FAILURE',
'text': 'ESLint found issues',
}
]
}
}
}
}
]
},
}
}
}
}
mock_post.return_value = mock_response
# Create a GithubIssue object simulating PR #14
issue = GithubIssue(
owner='neubig',
repo='pr-viewer',
number=14,
title='Fix issue #13: Add the option to sort PRs',
body='This pull request fixes #13.\n\nThe issue has been successfully resolved. The AI implemented a complete sorting functionality for pull requests that addresses the original request.',
has_merge_conflicts=False,
failed_checks=[{'name': 'Lint', 'description': 'ESLint found issues'}],
)
template = '{{ pr_status }}'
instruction, _ = pr_handler.get_instruction(issue, template)
print(f'\nGenerated instruction for PR #14:\n{instruction}')
assert 'The following CI checks have failed:' in instruction
assert 'Lint: ESLint found issues' in instruction
assert 'Please examine the GitHub workflow files' in instruction
assert 'Please run the failing checks locally to fix the issues.' in instruction