diff --git a/pr_agent/algo/utils.py b/pr_agent/algo/utils.py index f38bb5e8..32a1965d 100644 --- a/pr_agent/algo/utils.py +++ b/pr_agent/algo/utils.py @@ -11,6 +11,7 @@ from enum import Enum from typing import Any, List, Tuple import yaml +from pydantic import BaseModel from starlette_context import context from pr_agent.algo import MAX_TOKENS @@ -19,6 +20,12 @@ from pr_agent.config_loader import get_settings, global_settings from pr_agent.algo.types import FilePatchInfo from pr_agent.log import get_logger +class Range(BaseModel): + line_start: int # should be 0-indexed + line_end: int + column_start: int = -1 + column_end: int = -1 + class ModelType(str, Enum): REGULAR = "regular" TURBO = "turbo" diff --git a/pr_agent/git_providers/git_provider.py b/pr_agent/git_providers/git_provider.py index 746e9e33..2e514ac8 100644 --- a/pr_agent/git_providers/git_provider.py +++ b/pr_agent/git_providers/git_provider.py @@ -3,6 +3,7 @@ from abc import ABC, abstractmethod # enum EDIT_TYPE (ADDED, DELETED, MODIFIED, RENAMED) from typing import Optional +from pr_agent.algo.utils import Range from pr_agent.config_loader import get_settings from pr_agent.algo.types import FilePatchInfo from pr_agent.log import get_logger diff --git a/pr_agent/git_providers/github_provider.py b/pr_agent/git_providers/github_provider.py index 8cec5564..3136ea7d 100644 --- a/pr_agent/git_providers/github_provider.py +++ b/pr_agent/git_providers/github_provider.py @@ -11,7 +11,7 @@ from starlette_context import context from ..algo.file_filter import filter_ignored from ..algo.language_handler import is_valid_file -from ..algo.utils import PRReviewHeader, load_large_diff, clip_tokens, find_line_number_of_relevant_line_in_file +from ..algo.utils import PRReviewHeader, load_large_diff, clip_tokens, find_line_number_of_relevant_line_in_file, Range from ..config_loader import get_settings from ..log import get_logger from ..servers.utils import RateLimitExceeded diff --git a/pr_agent/git_providers/gitlab_provider.py b/pr_agent/git_providers/gitlab_provider.py index 0461535b..d414f9d3 100644 --- a/pr_agent/git_providers/gitlab_provider.py +++ b/pr_agent/git_providers/gitlab_provider.py @@ -4,13 +4,14 @@ from typing import Optional, Tuple from urllib.parse import urlparse import gitlab +import requests from gitlab import GitlabGetError from ..algo.file_filter import filter_ignored from ..algo.language_handler import is_valid_file from ..algo.utils import load_large_diff, clip_tokens, find_line_number_of_relevant_line_in_file from ..config_loader import get_settings -from .git_provider import GitProvider +from .git_provider import GitProvider, MAX_FILES_ALLOWED_FULL from pr_agent.algo.types import EDIT_TYPE, FilePatchInfo from ..log import get_logger @@ -45,7 +46,8 @@ class GitLabProvider(GitProvider): self.incremental = incremental def is_supported(self, capability: str) -> bool: - if capability in ['get_issue_comments', 'create_inline_comment', 'publish_inline_comments']: # gfm_markdown is supported in gitlab ! + if capability in ['get_issue_comments', 'create_inline_comment', 'publish_inline_comments', + 'publish_file_comments']: # gfm_markdown is supported in gitlab ! return False return True @@ -101,13 +103,23 @@ class GitLabProvider(GitProvider): diff_files = [] invalid_files_names = [] + counter_valid = 0 for diff in diffs: if not is_valid_file(diff['new_path']): invalid_files_names.append(diff['new_path']) continue - original_file_content_str = self.get_pr_file_content(diff['old_path'], self.mr.diff_refs['base_sha']) - new_file_content_str = self.get_pr_file_content(diff['new_path'], self.mr.diff_refs['head_sha']) + # allow only a limited number of files to be fully loaded. We can manage the rest with diffs only + counter_valid += 1 + if counter_valid < MAX_FILES_ALLOWED_FULL or not diff['diff']: + original_file_content_str = self.get_pr_file_content(diff['old_path'], self.mr.diff_refs['base_sha']) + new_file_content_str = self.get_pr_file_content(diff['new_path'], self.mr.diff_refs['head_sha']) + else: + if counter_valid == MAX_FILES_ALLOWED_FULL: + get_logger().info(f"Too many files in PR, will avoid loading full content for rest of files") + original_file_content_str = '' + new_file_content_str = '' + try: if isinstance(original_file_content_str, bytes): original_file_content_str = bytes.decode(original_file_content_str, 'utf-8') @@ -206,11 +218,11 @@ class GitLabProvider(GitProvider): raise NotImplementedError("Gitlab provider does not support publishing inline comments yet") def get_comment_body_from_comment_id(self, comment_id: int): - comment = self.mr.notes.get(comment_id) + comment = self.mr.notes.get(comment_id).body return comment def send_inline_comment(self,body: str,edit_type: str,found: bool,relevant_file: str,relevant_line_in_file: int, - source_line_no: int, target_file: str,target_line_no: int) -> None: + source_line_no: int, target_file: str,target_line_no: int, original_suggestion) -> None: if not found: get_logger().info(f"Could not find position for {relevant_file} {relevant_line_in_file}") else: @@ -230,12 +242,46 @@ class GitLabProvider(GitProvider): else: pos_obj['new_line'] = target_line_no - 1 pos_obj['old_line'] = source_line_no - 1 - get_logger().debug(f"Creating comment in {self.id_mr} with body {body} and position {pos_obj}") + get_logger().debug(f"Creating comment in MR {self.id_mr} with body {body} and position {pos_obj}") try: self.mr.discussions.create({'body': body, 'position': pos_obj}) except Exception as e: - get_logger().debug( - f"Failed to create comment in {self.id_mr} with position {pos_obj} (probably not a '+' line)") + try: + # fallback - create a general note on the file in the MR + line_start = original_suggestion['suggestion_orig_location']['start_line'] + line_end = original_suggestion['suggestion_orig_location']['end_line'] + old_code_snippet = original_suggestion['prev_code_snippet'] + new_code_snippet = original_suggestion['new_code_snippet'] + content = original_suggestion['suggestion_summary'] + label = original_suggestion['category'] + score = original_suggestion['score'] + + if hasattr(self, 'main_language'): + language = self.main_language + else: + language = '' + link = self.get_line_link(relevant_file, line_start, line_end) + body_fallback =f"**Suggestion:** {content} [{label}, importance: {score}]\n___\n" + body_fallback +=f"\n\nReplace lines ([{line_start}-{line_end}]({link}))\n\n```{language}\n{old_code_snippet}\n````\n\n" + body_fallback +=f"with\n\n```{language}\n{new_code_snippet}\n````" + body_fallback += f"\n\n___\n\n`(Cannot implement this suggestion directly, as gitlab API does not enable committing to a non -+ line in a PR)`" + + # Create a general note on the file in the MR + self.mr.notes.create({ + 'body': body_fallback, + 'position': { + 'base_sha': diff.base_commit_sha, + 'start_sha': diff.start_commit_sha, + 'head_sha': diff.head_commit_sha, + 'position_type': 'text', + 'file_path': f'{target_file.filename}', + } + }) + + # get_logger().debug( + # f"Failed to create comment in MR {self.id_mr} with position {pos_obj} (probably not a '+' line)") + except Exception as e: + get_logger().exception(f"Failed to create comment in MR {self.id_mr} with position {pos_obj}: {e}") def get_relevant_diff(self, relevant_file: str, relevant_line_in_file: int) -> Optional[dict]: changes = self.mr.changes() # Retrieve the changes for the merge request once @@ -257,6 +303,7 @@ class GitLabProvider(GitProvider): def publish_code_suggestions(self, code_suggestions: list) -> bool: for suggestion in code_suggestions: try: + original_suggestion = suggestion['original_suggestion'] body = suggestion['body'] relevant_file = suggestion['relevant_file'] relevant_lines_start = suggestion['relevant_lines_start'] @@ -283,13 +330,16 @@ class GitLabProvider(GitProvider): edit_type = 'addition' self.send_inline_comment(body, edit_type, found, relevant_file, relevant_line_in_file, source_line_no, - target_file, target_line_no) + target_file, target_line_no, original_suggestion) except Exception as e: get_logger().exception(f"Could not publish code suggestion:\nsuggestion: {suggestion}\nerror: {e}") # note that we publish suggestions one-by-one. so, if one fails, the rest will still be published return True + def publish_file_comments(self, file_comments: list) -> bool: + pass + def search_line(self, relevant_file, relevant_line_in_file): target_file = None @@ -380,6 +430,9 @@ class GitLabProvider(GitProvider): except Exception: return "" + def get_workspace_name(self): + return self.id_project.split('/')[0] + def add_eyes_reaction(self, issue_comment_id: int, disable_eyes: bool = False) -> Optional[int]: return True diff --git a/pr_agent/tools/pr_code_suggestions.py b/pr_agent/tools/pr_code_suggestions.py index 1f591d1a..ddad36c6 100644 --- a/pr_agent/tools/pr_code_suggestions.py +++ b/pr_agent/tools/pr_code_suggestions.py @@ -432,7 +432,8 @@ class PRCodeSuggestions: body = f"**Suggestion:** {content} [{label}]\n```suggestion\n" + new_code_snippet + "\n```" code_suggestions.append({'body': body, 'relevant_file': relevant_file, 'relevant_lines_start': relevant_lines_start, - 'relevant_lines_end': relevant_lines_end}) + 'relevant_lines_end': relevant_lines_end, + 'original_suggestion': d}) except Exception: get_logger().info(f"Could not parse suggestion: {d}")