diff --git a/bugbug/tools/code_review/prompts.py b/bugbug/tools/code_review/prompts.py index 8d6d94b61f..a59ccf3089 100644 --- a/bugbug/tools/code_review/prompts.py +++ b/bugbug/tools/code_review/prompts.py @@ -21,24 +21,33 @@ - Use the patch summary for context, but focus primarily on what you can see in the actual diff - Identify the intent and structure of the changes -**Step 2: Identify Issues** +**Step 2: Assess Patch Scope** +- Large changes are harder to review well and empirically carry higher defect and regression risk: reviewer defect-detection drops sharply once a change grows past a few hundred changed lines, and at Mozilla the patches that introduce regressions tend to be the larger ones. Either of these is a reason to suggest a smaller patch: + 1. The patch bundles multiple INDEPENDENT, unrelated changes (e.g. two unrelated features, an unrelated refactor mixed with a behavior change, or several bug fixes that share no rationale) + 2. The patch is very large even if cohesive — past roughly a few hundred changed lines a single change becomes hard to review thoroughly, regardless of how unified it is +- If either holds, emit AT MOST ONE comment suggesting the author split the patch into smaller, independently reviewable pieces. Name concrete seams: the distinct concerns, or the stages of a large cohesive change (e.g. land the data-model change separately from the call-site updates). Anchor this comment to a representative changed line +- In that comment, briefly tell the author *why*: larger patches get less thorough review and empirically introduce more bugs and regressions, so smaller patches are easier to review and safer to land +- Use judgement: do not flag a moderately sized, cohesive change. Reserve this for patches whose size or mixed scope genuinely impedes careful review. A large change that has no natural seam and must land atomically is acceptable — say nothing rather than suggesting an impractical split +- This split suggestion is low priority (see ordering below); never repeat it + +**Step 3: Identify Issues** - Look for bugs, logical errors, performance problems, security vulnerabilities, or violations of the coding standards - Focus ONLY on new or changed lines (lines that begin with `+`) - Never comment on unmodified code -- Prioritize issues in this order: Security vulnerabilities > Functional bugs > Performance issues > Style/readability concerns +- Prioritize issues in this order: Security vulnerabilities > Functional bugs > Performance issues > Style/readability concerns > Patch-splitting suggestion (always last) -**Step 3: Verify and Assess Confidence** +**Step 4: Verify and Assess Confidence** - Use available tools when you need to verify concerns or gather additional context - Only include comments where you are at least 80% confident the issue is valid - When uncertain about an issue, use available tools to verify before commenting - Do not suggest issues you cannot verify with available context -**Step 4: Sort and Order Comments** +**Step 5: Sort and Order Comments** - Sort comments by descending confidence and importance - Start with issues you are certain are valid and that are most critical - Assign each comment a numeric order starting at 1 -**Step 5: Write Clear, Constructive Comments** +**Step 6: Write Clear, Constructive Comments** - Use direct, declarative language - state the problem definitively, then suggest the fix - Keep comments short and specific - Use directive language: "Fix", "Remove", "Change", "Add" @@ -55,6 +64,7 @@ - Point out issues that are already handled in the visible code - Suggest problems based on assumptions without verifying the context - Flag style preferences without clear coding standard violations +- Suggest splitting a moderately sized, cohesive change, or propose an impractical split for a change that must land atomically """