Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 16 additions & 9 deletions docs/docs/tools/improve.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@ num_code_suggestions_per_chunk = ...

### Table vs Committable code comments

PR-Agent supports two modes for presenting code suggestions:
PR-Agent supports two modes for presenting code suggestions:

1) [Table](https://codium.ai/images/pr_agent/code_suggestions_as_comment_closed.png) mode
1) [Table](https://codium.ai/images/pr_agent/code_suggestions_as_comment_closed.png) mode

2) [Inline Committable](https://codium.ai/images/pr_agent/improve.png) code comments mode.

Expand All @@ -74,7 +74,7 @@ The table format offers several key advantages:
- **Centralized tracking**: Shows suggestion implementation status in one place
- **IDE integration**: Allows applying suggestions directly in your IDE via the CLI tool

Table mode is the default of PR-Agent, and is recommended approach for most users due to these benefits.
Table mode is the default of PR-Agent, and is recommended approach for most users due to these benefits.

![code_suggestions_as_comment_closed.png](https://codium.ai/images/pr_agent/code_suggestions_as_comment_closed.png){width=512}

Expand Down Expand Up @@ -113,9 +113,9 @@ Use triple quotes to write multi-line instructions. Use bullet points or numbers
PR-Agent supports both simple and hierarchical best practices configurations to provide guidance to the AI model for generating relevant code suggestions.

???- tip "Writing effective best practices files"

The following guidelines apply to all best practices files:

- Write clearly and concisely
- Include brief code examples when helpful with before/after patterns
- Focus on project-specific guidelines that will result in relevant suggestions you actually want to get
Expand All @@ -126,9 +126,9 @@ PR-Agent supports both simple and hierarchical best practices configurations to
- Use pattern-based structure rather than simple bullet points for better clarity

???- tip "Example of a best practices file"

Pattern 1: Add proper error handling with try-except blocks around external function calls.

Example code before:

```python
Expand All @@ -147,7 +147,7 @@ PR-Agent supports both simple and hierarchical best practices configurations to
```

Pattern 2: Add defensive null/empty checks before accessing object properties or performing operations on potentially null variables to prevent runtime errors.

Example code before:

```python
Expand Down Expand Up @@ -315,7 +315,7 @@ Note: Chunking is primarily relevant for large PRs. For most PRs (up to 600 line
</tr>
<tr>
<td><b>focus_only_on_problems</b></td>
<td>If set to true, suggestions will focus primarily on identifying and fixing code problems, and less on style considerations like best practices, maintainability, or readability. Default is true.</td>
<td>If set to true, suggestions will focus primarily on identifying and fixing code problems, and less on style considerations like best practices, maintainability, or readability. Default is true.</td>
</tr>
<tr>
<td><b>persistent_comment</b></td>
Expand All @@ -337,6 +337,13 @@ Note: Chunking is primarily relevant for large PRs. For most PRs (up to 600 line
<td><b>publish_output_no_suggestions</b></td>
<td>If set to true, the tool will publish a comment even if no suggestions were found. Default is true.</td>
</tr>
<tr>
<td><b>enable_suggestions_coverage_footer</b></td>
<td>
If set to true, the tool will display a coverage notice when failed analysis chunks make the
suggestions incomplete. Default is true.
</td>
</tr>
</table>

???+ example "Params for number of suggestions and AI calls"
Expand Down
16 changes: 14 additions & 2 deletions pr_agent/git_providers/git_provider.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
from abc import ABC, abstractmethod
# enum EDIT_TYPE (ADDED, DELETED, MODIFIED, RENAMED)
import os
import shutil
import subprocess
import time
from abc import ABC, abstractmethod
from typing import Optional, Tuple

from pr_agent.algo.types import FilePatchInfo
from pr_agent.algo.utils import Range, add_pr_review_identity, comment_matches_identity, process_description
from pr_agent.algo.utils import (Range, add_pr_review_identity,
comment_matches_identity, process_description)
from pr_agent.config_loader import get_settings
from pr_agent.log import get_logger

Expand Down Expand Up @@ -130,6 +131,17 @@ def supports_code_suggestions_artifact(self) -> bool:
"""Return whether `publish_code_suggestions()` writes a standalone output artifact."""
return False

def publish_code_suggestions_artifact(
self, code_suggestions: list, artifact_footer: str = "",
no_suggestions_message: str = "No code suggestions found for the PR.") -> bool:
"""Publish suggestions to a standalone artifact, optionally with additional context.

Providers that return True from `supports_code_suggestions_artifact()` should override
this method when they can preserve the footer in the same artifact. The default keeps
backward compatibility for providers that only implement `publish_code_suggestions()`.
"""
return self.publish_code_suggestions(code_suggestions)

#Given a url (issues or PR/MR) - get the .git repo url to which they belong. Needs to be implemented by the provider.
def get_git_repo_url(self, issues_or_pr_url: str) -> str:
get_logger().warning("Not implemented! Returning empty url")
Expand Down
8 changes: 7 additions & 1 deletion pr_agent/git_providers/local_git_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,11 @@ def publish_code_suggestion(self, body: str, relevant_file: str,
raise NotImplementedError('Publishing code suggestions is not implemented for the local git provider')

def publish_code_suggestions(self, code_suggestions: list) -> bool:
return self.publish_code_suggestions_artifact(code_suggestions)

def publish_code_suggestions_artifact(
self, code_suggestions: list, artifact_footer: str = "",
no_suggestions_message: str = "No code suggestions found for the PR.") -> bool:
"""
Write /improve output to a file (improve.md by default).

Expand All @@ -167,7 +172,8 @@ def publish_code_suggestions(self, code_suggestions: list) -> bool:
sections.append(f"{header}\n\n{suggestion.get('body', '').strip()}")
header = format_pr_code_suggestions_header(markdown_level=1)
pr_body = f"{header}\n\n" + "\n\n".join(sections) if sections \
else f"{header}\n\nNo code suggestions found for the PR."
else f"{header}\n\n{no_suggestions_message}"
pr_body += artifact_footer
if not sections and get_settings().get("config.output_run_details", False):
pr_body += show_run_details(False)
with open(self.improve_path, "w", encoding="utf-8") as file:
Expand Down
1 change: 1 addition & 0 deletions pr_agent/settings/configuration.toml
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ enable_chat_text=false
persistent_comment=true
max_history_len=4
publish_output_no_suggestions=true
enable_suggestions_coverage_footer=true # show when failed analysis chunks make the suggestions incomplete
# suggestions scoring
suggestions_score_threshold=0 # [0-10]| recommend not to set this value above 8, since above it may clip highly relevant suggestions
new_score_mechanism=true
Expand Down
57 changes: 47 additions & 10 deletions pr_agent/tools/pr_code_suggestions.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@
format_pr_code_suggestions_header,
get_max_tokens, get_model, load_yaml,
replace_code_tags,
show_relevant_configurations, show_run_details)
show_relevant_configurations,
show_run_details)
from pr_agent.config_loader import get_settings
from pr_agent.git_providers import (AzureDevopsProvider, GithubProvider,
GitLabProvider, get_git_provider,
Expand Down Expand Up @@ -207,6 +208,7 @@

# generate summarized suggestions
pr_body = self.generate_summarized_suggestions(data)
pr_body += self._get_suggestions_coverage_footer()
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
get_logger().debug(f"PR output", artifact=pr_body)

# require self-review
Expand Down Expand Up @@ -266,6 +268,7 @@
else:
get_logger().info('Code suggestions generated for PR, but not published since publish_output is False.')
pr_body = self.generate_summarized_suggestions(data)
pr_body += self._get_suggestions_coverage_footer()
get_settings().data = {"artifact": pr_body}
return
except Exception as e:
Expand Down Expand Up @@ -296,13 +299,30 @@
pr_body += ' <!-- approve and fold suggestions self-review -->'
return pr_body

def _get_suggestions_coverage_footer(self, suggestions_present: bool = True) -> str:
failed_chunk_count = getattr(self, "failed_chunk_count", 0)
if (not failed_chunk_count or
not get_settings().pr_code_suggestions.get("enable_suggestions_coverage_footer", True)):
return ""
total_chunk_count = getattr(self, "total_chunk_count", failed_chunk_count)
coverage_detail = ("the suggestions above are based on the successful chunks only."
if suggestions_present else
"no suggestions were found in the successful chunks; failed chunks could not be analyzed.")
return (f"\n\n⚠️ **Suggestion coverage:** {failed_chunk_count} of {total_chunk_count} "
"analysis chunks failed; "
f"{coverage_detail}")

async def publish_no_suggestions(self):
pr_body = f"{format_pr_code_suggestions_header()}\n\nNo code suggestions found for the PR."
coverage_footer = self._get_suggestions_coverage_footer(suggestions_present=False)
no_suggestions_message = ("No code suggestions found in the successfully analyzed chunks."
if coverage_footer else "No code suggestions found for the PR.")
pr_body = f"{format_pr_code_suggestions_header()}\n\n{no_suggestions_message}{coverage_footer}"
if (get_settings().config.publish_output and
get_settings().pr_code_suggestions.get('publish_output_no_suggestions', True)):
get_logger().warning("No code suggestions found for the PR.")
if self.git_provider.supports_code_suggestions_artifact():
self.git_provider.publish_code_suggestions([])
if self.git_provider.supports_code_suggestions_artifact() is True:
self.git_provider.publish_code_suggestions_artifact(
[], artifact_footer=coverage_footer, no_suggestions_message=no_suggestions_message)
return
pr_body = add_comment_identity(
pr_body,
Expand All @@ -318,7 +338,7 @@
else:
self.git_provider.publish_comment(pr_body)
else:
get_settings().data = {"artifact": ""}
get_settings().data = {"artifact": pr_body if coverage_footer else ""}
if self.progress_response:
self.git_provider.remove_comment(self.progress_response)

Expand All @@ -337,7 +357,7 @@
if data_above_threshold['code_suggestions']:
get_logger().info(
f"Publishing {len(data_above_threshold['code_suggestions'])} suggestions in dual publishing mode")
await self.push_inline_code_suggestions(data_above_threshold)
await self.push_inline_code_suggestions(data_above_threshold, include_coverage_footer=False)
except Exception as e:
get_logger().error(f"Failed to publish dual publishing suggestions, error: {e}")

Expand Down Expand Up @@ -740,17 +760,24 @@

return data

async def push_inline_code_suggestions(self, data):
async def push_inline_code_suggestions(self, data, include_coverage_footer: bool = True):
code_suggestions = []
fallback_comments = []
coverage_footer = self._get_suggestions_coverage_footer() if include_coverage_footer else ""
supports_suggestions_artifact = self.git_provider.supports_code_suggestions_artifact() is True

if not data['code_suggestions']:
get_logger().info('No suggestions found to improve this PR.')
empty_coverage_footer = (self._get_suggestions_coverage_footer(suggestions_present=False)
if include_coverage_footer else "")
no_suggestions_message = ("No suggestions found in the successfully analyzed chunks."
if empty_coverage_footer else "No suggestions found to improve this PR.")
pr_body = no_suggestions_message + empty_coverage_footer
if self.progress_response:
return self.git_provider.edit_comment(self.progress_response,
body='No suggestions found to improve this PR.')
body=pr_body)
else:
return self.git_provider.publish_comment('No suggestions found to improve this PR.')
return self.git_provider.publish_comment(pr_body)

for d in data['code_suggestions']:
try:
Expand Down Expand Up @@ -796,11 +823,17 @@
'original_suggestion': d})

if code_suggestions:
is_successful = self.git_provider.publish_code_suggestions(code_suggestions)
if supports_suggestions_artifact:
is_successful = self.git_provider.publish_code_suggestions_artifact(
code_suggestions, artifact_footer=coverage_footer)
else:
is_successful = self.git_provider.publish_code_suggestions(code_suggestions)
if not is_successful:
get_logger().info("Failed to publish code suggestions, trying to publish each suggestion separately")
for code_suggestion in code_suggestions:
self.git_provider.publish_code_suggestions([code_suggestion])
if coverage_footer and not supports_suggestions_artifact:
fallback_comments.append(coverage_footer.strip())
if fallback_comments:
self.git_provider.publish_comment("\n\n---\n\n".join(fallback_comments))

Expand Down Expand Up @@ -1112,6 +1145,8 @@
return patches_diff_list

async def prepare_prediction_main(self, model: str) -> dict:
self.failed_chunk_count = 0
self.total_chunk_count = 0
# get PR diff
if get_settings().pr_code_suggestions.decouple_hunks:
self.patches_diff_list = get_pr_multi_diffs(self.git_provider,
Expand Down Expand Up @@ -1145,6 +1180,7 @@
prediction_list = []
chunk_errors = []
chunk_pairs = list(zip(self.patches_diff_list, self.patches_diff_list_no_line_numbers))
self.total_chunk_count = len(chunk_pairs)

# parallelize calls to AI:
if get_settings().pr_code_suggestions.parallel_calls:
Expand Down Expand Up @@ -1179,6 +1215,7 @@
else:
prediction_list.append(prediction)

self.failed_chunk_count = len(chunk_errors)
if chunk_errors and not prediction_list:
raise chunk_errors[0]
self.prediction_list = prediction_list
Expand Down
16 changes: 16 additions & 0 deletions tests/unittest/test_local_git_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,22 @@ def test_publish_code_suggestions_no_suggestions(tmp_path):
assert "No code suggestions found" in improve_path.read_text()


def test_publish_code_suggestions_artifact_includes_partial_coverage(tmp_path):
improve_path = tmp_path / "improve.md"
provider = object.__new__(LocalGitProvider)
provider.improve_path = improve_path

assert provider.publish_code_suggestions_artifact(
[],
artifact_footer="\n\n⚠️ **Suggestion coverage:** 1 of 2 analysis chunks failed.",
no_suggestions_message="No code suggestions found in the successfully analyzed chunks.",
) is True

content = improve_path.read_text()
assert "No code suggestions found in the successfully analyzed chunks." in content
assert "1 of 2 analysis chunks failed" in content


def test_publish_code_suggestions_uses_custom_heading_without_identity(tmp_path):
snapshot = snapshot_settings(["pr_code_suggestions.suggestions_heading"])
improve_path = tmp_path / "improve.md"
Expand Down
Loading
Loading