Skip to content

fix: avoid rare cases where the AI reviewer merely describes the changes #1863

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

alessio-locatelli
Copy link
Contributor

@alessio-locatelli alessio-locatelli commented Jun 11, 2025

User description

Problem Example

Here's an invalid AI comment that offers no remarks or suggestions and merely repeats the evident, known fact about what was changed: #1745 (comment)


Why?

Describing the changes belongs in the pull request description and commit messages. A reviewer's purpose is to point out bugs and errors and to suggest improvements.


PR Type

Enhancement


Description

• Updated PR reviewer prompt to focus on identifying issues and improvements
• Added guidance to avoid merely describing changes in reviews


Changes diagram

flowchart LR
  A["Original Prompt"] -- "Enhanced with guidance" --> B["Updated Prompt"]
  B --> C["Focus on Issues & Improvements"]
  B --> D["Avoid Describing Changes"]
Loading

Changes walkthrough 📝

Relevant files
Enhancement
pr_reviewer_prompts.toml
Enhanced reviewer prompt for better feedback focus             

pr_agent/settings/pr_reviewer_prompts.toml

• Enhanced system prompt with explicit guidance to focus on
identifying issues and improvements
• Added instruction to avoid
merely describing changes in PR reviews

+1/-1     

Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ No major issues detected

    Copy link
    Contributor

    qodo-merge-for-open-source bot commented Jun 11, 2025

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    @mrT23
    Copy link
    Collaborator

    mrT23 commented Jun 12, 2025

    in the PR you attached, the suggestion was correct

    image

    @mrT23
    Copy link
    Collaborator

    mrT23 commented Jun 12, 2025

    In any case, it is already clearly said:

    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.")

    image

    @alessio-locatelli
    Copy link
    Contributor Author

    alessio-locatelli commented Jun 12, 2025

    in the PR you attached, the suggestion was correct

    @mrT23 That was already fixed by the pull request author. This indicates that the AI merely reiterated an already established fact.

    Consider a scenario where you submit a typo fix, and the AI responds, "Awesome, you fixed a typo!"

    In other words, the AI is stating a fact that has already occurred.


    In my private project, I had a similar case: I added a path.mkdir(exists_ok=True) call to my code, and an AI reviewer replied to me with something like "This merge request adds path.mkdir(exists_ok=True)", which was just an obvious fact about the added change, not a remark.

    However, this AI task misinterpretation happens extremely rarely, approximately once per 100 reviews. The most of the time the AI reviewer understands that it must not just spell out the existing changes.

    @alessio-locatelli alessio-locatelli force-pushed the update_review_prompt_for_correct_replies branch from 8c311ad to af84a4a Compare June 13, 2025 04:54
    @alessio-locatelli
    Copy link
    Contributor Author

    Indeed, my initial contribution was a bit imprecise. Thank you for pointing out the key_issues_to_review field! I've rephrased the field slightly to help the AI understand the instructions correctly. Could you please re-review this? If it still doesn't meet the criteria, I will close this PR.

    @mrT23 By the way, why does someone write yes\\no everywhere in the template (which is grammatically incorrect) instead of yes/no?

    @alessio-locatelli alessio-locatelli changed the title fix: keep AI focused on review, not praising user for changes fix: avoid rare cases where the AI reviewer merely describes the changes Jun 15, 2025
    @mrT23
    Copy link
    Collaborator

    mrT23 commented Jun 17, 2025

    image

    The new formulation is more general. potential problems are almost everywhere. In practice - this kind of change in phrasing will lead to more 'issues', that will be more general and more 'nitpicking'

    @mrT23
    Copy link
    Collaborator

    mrT23 commented Jun 17, 2025

    @alessio-locatelli
    but you are correct about the yes vs no

    image

    and are welcome to transform the PR to resolve this :-)

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants