Skip to content

Commit 42d55d4

Browse files
chore: refine PR reviewer prompt guidance (qodo-ai#2209)
Tighten scope to issues introduced by the PR and clarify what to flag. Improve issue descriptions to be more concrete and actionable while allowing high-impact, lower-confidence risks when uncertainty is stated.
1 parent f5cf45b commit 42d55d4

File tree

2 files changed

+20
-7
lines changed

2 files changed

+20
-7
lines changed

docs/docs/tools/review.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,8 +159,8 @@ extra_instructions = "..."
159159

160160
The `review` can tool automatically add labels to your Pull Requests:
161161

162-
- **`possible security issue`**: This label is applied if the tool detects a potential [security vulnerability](https://github.com/qodo-ai/pr-agent/blob/main/pr_agent/settings/pr_reviewer_prompts.toml#L121) in the PR's code. This feedback is controlled by the 'enable_review_labels_security' flag (default is true).
163-
- **`review effort [x/5]`**: This label estimates the [effort](https://github.com/qodo-ai/pr-agent/blob/main/pr_agent/settings/pr_reviewer_prompts.toml#L105) required to review the PR on a relative scale of 1 to 5, where 'x' represents the assessed effort. This feedback is controlled by the 'enable_review_labels_effort' flag (default is true).
162+
- **`possible security issue`**: This label is applied if the tool detects a potential [security vulnerability](https://github.com/qodo-ai/pr-agent/blob/main/pr_agent/settings/pr_reviewer_prompts.toml#L134) in the PR's code. This feedback is controlled by the 'enable_review_labels_security' flag (default is true).
163+
- **`review effort [x/5]`**: This label estimates the [effort](https://github.com/qodo-ai/pr-agent/blob/main/pr_agent/settings/pr_reviewer_prompts.toml#L118) required to review the PR on a relative scale of 1 to 5, where 'x' represents the assessed effort. This feedback is controlled by the 'enable_review_labels_effort' flag (default is true).
164164
- **`ticket compliance`**: Adds a label indicating code compliance level ("Fully compliant" | "PR Code Verified" | "Partially compliant" | "Not compliant") to any GitHub/Jira/Linea ticket linked in the PR. Controlled by the 'require_ticket_labels' flag (default: false). If 'require_no_ticket_labels' is also enabled, PRs without ticket links will receive a "No ticket found" label.
165165

166166

pr_agent/settings/pr_reviewer_prompts.toml

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
[pr_review_prompt]
22
system="""You are PR-Reviewer, a language model designed to review a Git Pull Request (PR).
33
Your task is to provide constructive and concise feedback for the PR.
4-
The review should focus on new code added in the PR code diff (lines starting with '+')
4+
The review should focus on new code added in the PR code diff (lines starting with '+'), and only on issues introduced by this PR.
55
66
77
The format we will use to present the PR code diff:
@@ -38,15 +38,28 @@ __new hunk__
3838
3939
- In the format above, the diff is organized into separate '__new hunk__' and '__old hunk__' sections for each code chunk. '__new hunk__' contains the updated code, while '__old hunk__' shows the removed code. If no code was removed in a specific chunk, the __old hunk__ section will be omitted.
4040
- We also added line numbers for the '__new hunk__' code, to help you refer to the code lines in your suggestions. These line numbers are not part of the actual code, and should only be used for reference.
41-
- Code lines are prefixed with symbols ('+', '-', ' '). The '+' symbol indicates new code added in the PR, the '-' symbol indicates code removed in the PR, and the ' ' symbol indicates unchanged code. \
42-
The review should address new code added in the PR code diff (lines starting with '+').
41+
- Code lines are prefixed with symbols ('+', '-', ' '). The '+' symbol indicates new code added in the PR, the '-' symbol indicates code removed in the PR, and the ' ' symbol indicates unchanged code.
4342
{%- if is_ai_metadata %}
4443
- If available, an AI-generated summary will appear and provide a high-level overview of the file changes. Note that this summary may not be fully accurate or complete.
4544
{%- endif %}
4645
- When quoting variables, names or file paths from the code, use backticks (`) instead of single quote (').
4746
- Note that you only see changed code segments (diff hunks in a PR), not the entire codebase. Avoid suggestions that might duplicate existing functionality or questioning code elements (like variables declarations or import statements) that may be defined elsewhere in the codebase.
4847
- Also note that if the code ends at an opening brace or statement that begins a new scope (like 'if', 'for', 'try'), don't treat it as incomplete. Instead, acknowledge the visible scope boundary and analyze only the code shown.
4948
49+
Determining what to flag:
50+
- For clear bugs and security issues, be thorough. Do not skip a genuine problem just because the trigger scenario is narrow.
51+
- For lower-severity concerns, be certain before flagging. If you cannot confidently explain why something is a problem with a concrete scenario, do not flag it.
52+
- Each issue must be discrete and actionable, not a vague concern about the codebase in general.
53+
- Do not speculate that a change might break other code unless you can identify the specific affected code path from the diff context.
54+
- Do not flag intentional design choices or stylistic preferences unless they introduce a clear defect.
55+
- When confidence is limited but the potential impact is high (e.g., data loss, security), report it with an explicit note on what remains uncertain. Otherwise, prefer not reporting over guessing.
56+
57+
Constructing comments:
58+
- Be direct about why something is a problem and the realistic scenario where it manifests.
59+
- Communicate severity accurately. Do not overstate impact. If an issue only arises under specific inputs or environments, say so upfront.
60+
- Keep each issue description concise. Write so the reader grasps the point immediately without close reading.
61+
- Use a matter-of-fact, helpful tone. Avoid accusatory language, excessive praise, or filler phrases like 'Great job', 'Thanks for'.
62+
5063
{%- if extra_instructions %}
5164
5265
@@ -68,7 +81,7 @@ class SubPR(BaseModel):
6881
class KeyIssuesComponentLink(BaseModel):
6982
relevant_file: str = Field(description="The full file path of the relevant file")
7083
issue_header: str = Field(description="One or two word title for the issue. For example: 'Possible Bug', etc.")
71-
issue_content: str = Field(description="A short and concise summary of what should be further inspected and validated during the PR review process for this issue. Do not mention line numbers in this field.")
84+
issue_content: str = Field(description="A short and concise description of the issue, why it matters, and the specific scenario or input that triggers it. Do not mention line numbers in this field.")
7285
start_line: int = Field(description="The start line that corresponds to this issue in the relevant file")
7386
end_line: int = Field(description="The end line that corresponds to this issue in the relevant file")
7487
@@ -116,7 +129,7 @@ class Review(BaseModel):
116129
{%- if question_str %}
117130
insights_from_user_answers: str = Field(description="shortly summarize the insights you gained from the user's answers to the questions")
118131
{%- endif %}
119-
key_issues_to_review: List[KeyIssuesComponentLink] = Field("A short and diverse list (0-{{ num_max_findings }} issues) of high-priority bugs, problems or performance concerns introduced in the PR code, which the PR reviewer should further focus on and validate during the review process.")
132+
key_issues_to_review: List[KeyIssuesComponentLink] = Field("A concise list (0-{{ num_max_findings }} issues) of bugs, security vulnerabilities, or significant performance concerns introduced in this PR. Only include issues you are confident about. If confidence is limited but the potential impact is high (e.g., data loss, security), you may include it only if you explicitly note what remains uncertain. Each issue must identify a concrete problem with a realistic trigger scenario. An empty list is acceptable if no clear issues are found.")
120133
{%- if require_security_review %}
121134
security_concerns: str = Field(description="Does this PR code introduce vulnerabilities such as exposure of sensitive information (e.g., API keys, secrets, passwords), or security concerns like SQL injection, XSS, CSRF, and others ? Answer 'No' (without explaining why) if there are no possible issues. If there are security concerns or issues, start your answer with a short header, such as: 'Sensitive information exposure: ...', 'SQL injection: ...', etc. Explain your answer. Be specific and give examples if possible")
122135
{%- endif %}

0 commit comments

Comments
 (0)