diff --git a/investigate_pr_14.py b/investigate_pr_14.py new file mode 100644 index 000000000000..6bea26b37ae3 --- /dev/null +++ b/investigate_pr_14.py @@ -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()) \ No newline at end of file diff --git a/openhands/resolver/github_issue.py b/openhands/resolver/github_issue.py index 9b8f58d589a4..d7e1f3197de8 100644 --- a/openhands/resolver/github_issue.py +++ b/openhands/resolver/github_issue.py @@ -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 diff --git a/openhands/resolver/issue_definitions.py b/openhands/resolver/issue_definitions.py index f36bb7a61527..a02fc725cb1f 100644 --- a/openhands/resolver/issue_definitions.py +++ b/openhands/resolver/issue_definitions.py @@ -296,6 +296,129 @@ def __init__(self, owner: str, repo: str, token: str, llm_config: LLMConfig): 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 @@ def get_converted_issues( 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 @@ def get_converted_issues( 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 @@ def get_instruction( 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 @@ def get_instruction( files=review_thread_file_str, thread_context=thread_context, repo_instruction=repo_instruction, + pr_status=pr_status, ) return instruction, images diff --git a/openhands/resolver/prompts/resolve/basic-followup.jinja b/openhands/resolver/prompts/resolve/basic-followup.jinja index b7bdc2ae9e18..839b80422337 100644 --- a/openhands/resolver/prompts/resolve/basic-followup.jinja +++ b/openhands/resolver/prompts/resolve/basic-followup.jinja @@ -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 }} diff --git a/pyproject.toml b/pyproject.toml index 4a4131e325b5..155f62b8c764 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -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 = "*" diff --git a/tests/unit/resolver/test_pr_status_in_prompt.py b/tests/unit/resolver/test_pr_status_in_prompt.py new file mode 100644 index 000000000000..2461e2dda8cf --- /dev/null +++ b/tests/unit/resolver/test_pr_status_in_prompt.py @@ -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 diff --git a/tests/unit/test_issue_definitions.py b/tests/unit/test_issue_definitions.py new file mode 100644 index 000000000000..f1473b878399 --- /dev/null +++ b/tests/unit/test_issue_definitions.py @@ -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 diff --git a/tests/unit/test_pr_status.py b/tests/unit/test_pr_status.py new file mode 100644 index 000000000000..23dadd2162ab --- /dev/null +++ b/tests/unit/test_pr_status.py @@ -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