Skip to content

Commit

Permalink
Improve PR file content retrieval and logging verbosity handling
Browse files Browse the repository at this point in the history
  • Loading branch information
@mrT23
mrT23 committed Nov 12, 2024
1 parent 9c82047 commit 0657770
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 28 deletions.
62 changes: 34 additions & 28 deletions pr_agent/git_providers/github_provider.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,24 @@ def get_diff_files(self) -> list[FilePatchInfo]:
if avoid_load:
original_file_content_str = ""
else:
original_file_content_str = self._get_pr_file_content(file, self.pr.base.sha)
# The base.sha will point to the current state of the base branch (including parallel merges), not the original base commit when the PR was created
# We can fix this by finding the merge base commit between the PR head and base branches
# Note that The pr.head.sha is actually correct as is - it points to the latest commit in your PR branch.
# This SHA isn't affected by parallel merges to the base branch since it's specific to your PR's branch.
repo = self.repo_obj
pr = self.pr
try:
compare = repo.compare(pr.base.sha, pr.head.sha)
merge_base_commit = compare.merge_base_commit
except Exception as e:
get_logger().error(f"Failed to get merge base commit: {e}")
merge_base_commit = pr.base
if merge_base_commit.sha != pr.base.sha:
get_logger().info(
f"Using merge base commit {merge_base_commit.sha} instead of base commit "
f"{pr.base.sha} for {file.filename}")
original_file_content_str = self._get_pr_file_content(file, merge_base_commit.sha)

if not patch:
patch = load_large_diff(file.filename, new_file_content_str, original_file_content_str)

Expand Down Expand Up @@ -283,8 +300,7 @@ def create_inline_comment(self, body: str, relevant_file: str, relevant_line_in_
relevant_line_in_file,
absolute_position)
if position == -1:
if get_settings().config.verbosity_level >= 2:
get_logger().info(f"Could not find position for {relevant_file} {relevant_line_in_file}")
get_logger().info(f"Could not find position for {relevant_file} {relevant_line_in_file}")
subject_type = "FILE"
else:
subject_type = "LINE"
Expand All @@ -296,20 +312,17 @@ def publish_inline_comments(self, comments: list[dict], disable_fallback: bool =
# publish all comments in a single message
self.pr.create_review(commit=self.last_commit_id, comments=comments)
except Exception as e:
if get_settings().config.verbosity_level >= 2:
get_logger().error(f"Failed to publish inline comments")
get_logger().info(f"Initially failed to publish inline comments as committable")

if (getattr(e, "status", None) == 422
and get_settings().github.publish_inline_comments_fallback_with_verification and not disable_fallback):
if (getattr(e, "status", None) == 422 and not disable_fallback):
pass # continue to try _publish_inline_comments_fallback_with_verification
else:
raise e # will end up with publishing the comments one by one

try:
self._publish_inline_comments_fallback_with_verification(comments)
except Exception as e:
if get_settings().config.verbosity_level >= 2:
get_logger().error(f"Failed to publish inline code comments fallback, error: {e}")
get_logger().error(f"Failed to publish inline code comments fallback, error: {e}")
raise e

def _publish_inline_comments_fallback_with_verification(self, comments: list[dict]):
Expand All @@ -334,11 +347,9 @@ def _publish_inline_comments_fallback_with_verification(self, comments: list[dic
for comment in fixed_comments_as_one_liner:
try:
self.publish_inline_comments([comment], disable_fallback=True)
if get_settings().config.verbosity_level >= 2:
get_logger().info(f"Published invalid comment as a single line comment: {comment}")
get_logger().info(f"Published invalid comment as a single line comment: {comment}")
except:
if get_settings().config.verbosity_level >= 2:
get_logger().error(f"Failed to publish invalid comment as a single line comment: {comment}")
get_logger().error(f"Failed to publish invalid comment as a single line comment: {comment}")

def _verify_code_comment(self, comment: dict):
is_verified = False
Expand Down Expand Up @@ -396,8 +407,7 @@ def _try_fix_invalid_inline_comments(self, invalid_comments: list[dict]) -> list
if fixed_comment != comment:
fixed_comments.append(fixed_comment)
except Exception as e:
if get_settings().config.verbosity_level >= 2:
get_logger().error(f"Failed to fix inline comment, error: {e}")
get_logger().error(f"Failed to fix inline comment, error: {e}")
return fixed_comments

def publish_code_suggestions(self, code_suggestions: list) -> bool:
Expand All @@ -412,16 +422,14 @@ def publish_code_suggestions(self, code_suggestions: list) -> bool:
relevant_lines_end = suggestion['relevant_lines_end']

if not relevant_lines_start or relevant_lines_start == -1:
if get_settings().config.verbosity_level >= 2:
get_logger().exception(
f"Failed to publish code suggestion, relevant_lines_start is {relevant_lines_start}")
get_logger().exception(
f"Failed to publish code suggestion, relevant_lines_start is {relevant_lines_start}")
continue

if relevant_lines_end < relevant_lines_start:
if get_settings().config.verbosity_level >= 2:
get_logger().exception(f"Failed to publish code suggestion, "
f"relevant_lines_end is {relevant_lines_end} and "
f"relevant_lines_start is {relevant_lines_start}")
get_logger().exception(f"Failed to publish code suggestion, "
f"relevant_lines_end is {relevant_lines_end} and "
f"relevant_lines_start is {relevant_lines_start}")
continue

if relevant_lines_end > relevant_lines_start:
Expand All @@ -445,8 +453,7 @@ def publish_code_suggestions(self, code_suggestions: list) -> bool:
self.publish_inline_comments(post_parameters_list)
return True
except Exception as e:
if get_settings().config.verbosity_level >= 2:
get_logger().error(f"Failed to publish code suggestion, error: {e}")
get_logger().error(f"Failed to publish code suggestion, error: {e}")
return False

def edit_comment(self, comment, body: str):
Expand Down Expand Up @@ -505,6 +512,7 @@ def publish_file_comments(self, file_comments: list) -> bool:
elif self.deployment_type == 'user':
same_comment_creator = self.github_user_id == existing_comment['user']['login']
if existing_comment['subject_type'] == 'file' and comment['path'] == existing_comment['path'] and same_comment_creator:

headers, data_patch = self.pr._requester.requestJsonAndCheck(
"PATCH", f"{self.base_url}/repos/{self.repo}/pulls/comments/{existing_comment['id']}", input={"body":comment['body']}
)
Expand All @@ -516,8 +524,7 @@ def publish_file_comments(self, file_comments: list) -> bool:
)
return True
except Exception as e:
if get_settings().config.verbosity_level >= 2:
get_logger().error(f"Failed to publish diffview file summary, error: {e}")
get_logger().error(f"Failed to publish diffview file summary, error: {e}")
return False

def remove_initial_comment(self):
Expand Down Expand Up @@ -805,8 +812,7 @@ def generate_link_to_relevant_line_number(self, suggestion) -> str:
link = f"{self.base_url_html}/{self.repo}/pull/{self.pr_num}/files#diff-{sha_file}R{absolute_position}"
return link
except Exception as e:
if get_settings().config.verbosity_level >= 2:
get_logger().info(f"Failed adding line link, error: {e}")
get_logger().info(f"Failed adding line link, error: {e}")

return ""

Expand Down
2 changes: 2 additions & 0 deletions pr_agent/tools/pr_code_suggestions.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,8 @@ async def _prepare_prediction(self, model: str) -> dict:
model,
add_line_numbers_to_hunks=True,
disable_extra_lines=False)
self.patches_diff_list = [self.patches_diff]
self.patches_diff_no_line_number = self.remove_line_numbers([self.patches_diff])[0]

if self.patches_diff:
get_logger().debug(f"PR diff", artifact=self.patches_diff)
Expand Down

0 comments on commit 0657770

Please sign in to comment.