Skip to content
Open
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
42 changes: 42 additions & 0 deletions pr_agent/algo/pr_processing.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,34 @@ def cap_and_log_extra_lines(value, direction) -> int:
return value


def record_starved_diff(token_handler: TokenHandler, model: str, skipped_files: list,
diff_and_prompt_tokens: int = 0) -> None:
"""Record that pruning left the caller with no diff at all, and say so in the log.

The tools read `token_handler.starved_diff` to report the outcome on the pull request. Before
this existed, /review published nothing and /improve published "No code suggestions found",
both of which read as a clean review of code that never reached the model; ten wkda/admin pull
requests merged that way (OPS-25871). Logged at error, not warning, because the run produced
no review: the remedy is a repository setting somebody has to change.

This is a report, not a remedy. The token limit is unchanged and the run still ends empty.
"""
prompt_tokens = getattr(token_handler, "prompt_tokens", 0)
max_tokens = get_max_tokens(model)
token_handler.starved_diff = {
"model": model,
"max_tokens": max_tokens,
"prompt_tokens": prompt_tokens,
"diff_and_prompt_tokens": diff_and_prompt_tokens,
"skipped_files": list(skipped_files),
}
get_logger().error(
f"Diff pruned to nothing: the prompt needs {prompt_tokens} of {max_tokens} tokens, so none "
f"of the {len(skipped_files)} changed files could be included",
artifact=token_handler.starved_diff,
)


def get_pr_diff(git_provider: GitProvider, token_handler: TokenHandler,
model: str,
add_line_numbers_to_hunks: bool = False,
Expand Down Expand Up @@ -90,6 +118,12 @@ def get_pr_diff(git_provider: GitProvider, token_handler: TokenHandler,
total_tokens_new = total_tokens_list[0]
files_in_patch = files_in_patches_list[0]

# Pruning kept NO file at all. The prompt overhead -- system, user, repo context, ticket -- has
# filled the model budget on its own, so the tool is about to run on an empty diff and return
# nothing.
if not files_in_patch and file_dict:
record_starved_diff(token_handler, model, sorted(file_dict.keys()), total_tokens)

# Insert additional information about added, modified, and deleted files if there is enough space
max_tokens = get_max_tokens(model) - OUTPUT_BUFFER_TOKENS_HARD_THRESHOLD
curr_token = total_tokens_new # == token_handler.count_tokens(final_diff)+token_handler.prompt_tokens
Expand Down Expand Up @@ -497,6 +531,14 @@ def get_pr_multi_diffs(git_provider: GitProvider,
final_diff = "\n".join(patches)
final_diff_list.append(final_diff.strip())

# Same starvation as in get_pr_diff(): every candidate file was skipped or clipped away, so the
# caller is about to run on nothing. This path is the one /improve takes in extended mode, and
# its "No code suggestions found for the PR" message is a verdict on code it never saw.
if not final_diff_list:
candidate_files = sorted(f.filename.strip() for f in sorted_files if f.patch)
if candidate_files:
record_starved_diff(token_handler, model, candidate_files)

return final_diff_list


Expand Down
7 changes: 6 additions & 1 deletion pr_agent/algo/token_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,12 @@ def __init__(self, pr=None, vars: dict = {}, system="", user=""):
- user: The user string.
"""
self.encoder = TokenEncoder.get_token_encoder()


# Set by get_pr_diff() when pruning removed EVERY changed file, i.e. the prompt overhead
# alone filled the model budget and the tool received no diff at all. The tools read it to
# say so on the pull request; None means it did not happen. See OPS-25871.
self.starved_diff = None

if pr is not None:
self.prompt_tokens = self._get_system_user_tokens(pr, self.encoder, vars, system, user)

Expand Down
44 changes: 44 additions & 0 deletions pr_agent/algo/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,50 @@ class PRReviewHeader(str, Enum):
INCREMENTAL = "## Incremental PR Reviewer Guide"


STARVED_DIFF_HEADER = "## PR-Agent could not read this diff"

# A GitHub issue comment is capped at 65536 characters, and this list is one line per changed
# file, so a large pull request would push the body past the cap and lose the whole comment --
# the failure this notice exists to make visible. The count in the sentence above the list stays
# exact either way, so truncating costs the reader nothing they need in order to act.
STARVED_DIFF_MAX_LISTED_FILES = 50


def starved_diff_comment(report: dict, tool: str) -> str:
"""Build the pull-request comment for a diff that pruning emptied.

The point of this comment is that NOTHING was reviewed. Say that first and say it plainly: the
silent version of this outcome was indistinguishable from a clean review, and a reader who
takes "no findings" for "no problems" merges unreviewed code. Name the numbers, because the
remedy is a number the repository owner controls -- the size of `repo_context_files`.
"""
max_tokens = report.get("max_tokens") or 0
prompt_tokens = report.get("prompt_tokens") or 0
skipped = report.get("skipped_files") or []
listed = skipped[:STARVED_DIFF_MAX_LISTED_FILES]
# A path may legitimately contain a backtick, which would end the inline code span and let the
# rest of the name render as markup. Neutralise it rather than trusting the filename.
safe_names = [str(name).replace("`", "'") for name in listed]
file_list = "\n".join("- `{}`".format(name) for name in safe_names)
if len(skipped) > len(listed):
file_list += f"\n- ...and {len(skipped) - len(listed)} more"
overhead = f"{prompt_tokens} of {max_tokens}" if max_tokens else str(prompt_tokens)
share = f" ({prompt_tokens * 100 // max_tokens} %)" if max_tokens else ""

return (
f"{STARVED_DIFF_HEADER}\n\n"
f"**`{tool}` produced no output, and it did not review any code.** This is not a clean "
f"result. The prompt for this repository needs {overhead} available tokens{share} before "
f"any of the diff is added, so all "
f"{len(skipped)} changed file(s) were dropped and the model saw an empty diff:\n\n"
f"{file_list}\n\n"
f"**What to do:** shrink `config.repo_context_files` in this repository's `.pr_agent.toml`, "
f"on the default branch — the bot reads that setting from the default branch, so a change "
f"on this branch will not take effect here. Until the context fits, `/review` and "
f"`/improve` cannot see this pull request, however small the diff is.\n"
)


class ReasoningEffort(str, Enum):
XHIGH = "xhigh"
HIGH = "high"
Expand Down
13 changes: 11 additions & 2 deletions pr_agent/tools/pr_code_suggestions.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
format_code_suggestion_metadata, get_max_tokens,
get_model, load_yaml,
replace_code_tags,
show_relevant_configurations)
show_relevant_configurations,
starved_diff_comment)
from pr_agent.config_loader import get_settings
from pr_agent.git_providers import (AzureDevopsProvider, GithubProvider,
GitLabProvider, get_git_provider,
Expand Down Expand Up @@ -224,7 +225,15 @@ async def add_self_review_text(self, pr_body):
return pr_body

async def publish_no_suggestions(self):
pr_body = "## PR Code Suggestions ✨\n\nNo code suggestions found for the PR."
# "No code suggestions found" is a verdict on the code. It must not be published when the
# tool never saw the code: pruning can empty the diff outright, and then this message reads
# as a clean bill of health for a pull request nothing looked at. Say what happened instead.
# See OPS-25871.
starved = getattr(self.token_handler, "starved_diff", None)
if starved:
pr_body = starved_diff_comment(starved, "/improve")
else:
pr_body = "## PR Code Suggestions ✨\n\nNo code suggestions found for the PR."
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.')
Expand Down
30 changes: 29 additions & 1 deletion pr_agent/tools/pr_reviewer.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@
from pr_agent.algo.repo_context import build_repo_context
from pr_agent.algo.token_handler import TokenHandler
from pr_agent.algo.utils import (ModelType, PRReviewHeader,
STARVED_DIFF_HEADER,
convert_to_markdown_v2, get_max_tokens,
github_action_output,
load_yaml, parse_requirement_items,
show_relevant_configurations)
show_relevant_configurations,
starved_diff_comment)
from pr_agent.config_loader import get_settings
from pr_agent.git_providers import (get_git_provider,
get_git_provider_with_context)
Expand Down Expand Up @@ -214,6 +216,7 @@ async def run(self) -> None:
await retry_with_fallback_models(self._prepare_prediction, model_type=ModelType.REGULAR)
if not self.prediction:
self.git_provider.remove_initial_comment()
self._publish_starved_diff_notice()
return None

pr_review = self._prepare_pr_review()
Expand Down Expand Up @@ -244,6 +247,31 @@ async def run(self) -> None:
if get_settings().config.get("propagate_tool_errors", False):
raise

def _publish_starved_diff_notice(self) -> None:
"""Say on the pull request that the diff never reached the model.

Only fires when pruning emptied the diff, which the token handler records. An empty
prediction has other causes -- a model that answered nothing, a parse failure -- and those
are not this message, so they keep the existing behaviour of publishing nothing.
"""
report = getattr(self.token_handler, "starved_diff", None)
if not report:
return
if not get_settings().config.publish_output:
return

try:
self.git_provider.publish_persistent_comment(
starved_diff_comment(report, "/review"),
initial_header=STARVED_DIFF_HEADER,
update_header=False,
final_update_message=False,
)
except Exception as e:
# The notice is a courtesy on top of an already-failed run. Losing it must not turn a
# missing review into an exception the caller has to handle.
get_logger().error(f"Failed to publish the starved-diff notice: {e}")

def _should_publish_review_no_suggestions(self, pr_review: str) -> bool:
return get_settings().pr_reviewer.get('publish_output_no_suggestions', True) or "No major issues detected" not in pr_review

Expand Down
Loading
Loading