Skip to content
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

fix: refine code suggestions prompts and avoid package-related comments #1446

Merged
merged 1 commit into from
Jan 8, 2025

Conversation

mrT23
Copy link
Collaborator

@mrT23 mrT23 commented Jan 8, 2025

User description

aaa


PR Type

Enhancement


Description

  • Added a new date field to the code suggestions context.

  • Updated prompts to avoid package-related suggestions and improve clarity.

  • Refined guidelines for code suggestions to focus on visible changes.


Changes walkthrough 📝

Relevant files
Enhancement
pr_code_suggestions.py
Add `date` field to code suggestions context                         

pr_agent/tools/pr_code_suggestions.py

  • Added a date field to the context dictionary.
  • Used datetime.now to format the current date.
  • +2/-0     
    pr_code_suggestions_prompts.toml
    Refine prompts to avoid package-related suggestions           

    pr_agent/settings/pr_code_suggestions_prompts.toml

  • Updated prompts to avoid package-related suggestions.
  • Improved clarity on suggestions for visible code changes.
  • Refined guidelines for referencing code elements in responses.
  • +8/-5     

    Need help?
  • Type /help how to ... in the comments thread for any question 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: 2 🔵🔵⚪⚪⚪
    🏅 Score: 95
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ No major issues detected

    @mrT23
    Copy link
    Collaborator Author

    mrT23 commented Jan 8, 2025

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    General
    Standardize instruction wording to maintain consistency throughout the documentation

    The new instruction about backticks has inconsistent wording compared to the old
    one. The example should be consistent with the instruction. Consider standardizing
    the wording to avoid confusion.

    pr_agent/settings/pr_code_suggestions_prompts.toml [61]

    -- When mentioning code elements (variables, names, or files) in your response, surround them with backticks (`). For example: "verify that `user_id` is..."
    +- When mentioning code elements (variables, names, or files) in your response, surround them with backticks (`). For example: "ensure that `user_id` is..."
    • Apply this suggestion
    Suggestion importance[1-10]: 3

    Why: The suggestion correctly identifies a minor inconsistency in wording ("verify" vs "ensure"), but this has minimal impact on functionality or understanding of the instructions.

    3
    • Author self-review: I have reviewed the PR code suggestions, and addressed the relevant ones.

    @mrT23 mrT23 merged commit c1c5c35 into main Jan 8, 2025
    2 checks passed
    @mrT23 mrT23 deleted the tr/avoid_packages_comments branch January 8, 2025 20:12
    @mrT23
    Copy link
    Collaborator Author

    mrT23 commented Jan 14, 2025

    /custom_prompt --pr_custom_prompt.prompt="""
    The code suggestions should focus only on the following:

    • look for edge cases when implementing a new function
    • make sure every variable has a meaningful name
    • make sure the code is efficient
      """

    Copy link
    Contributor

    qodo-merge-pro-for-open-source bot commented Jan 14, 2025

    Custom Prompt Suggestions 🎨

    CategorySuggestion                                                                                                                                    Score
    Maintainability
    Use a more descriptive variable name for the date field

    Consider using a more descriptive variable name than date for the datetime string. Since
    this is used in the PR context, a name like pr_review_date or analysis_date would better
    convey its purpose and usage.

    pr_agent/tools/pr_code_suggestions.py [85]

    -"date": datetime.now().strftime('%Y-%m-%d'),
    +"pr_review_date": datetime.now().strftime('%Y-%m-%d'),
     
    Suggestion importance[1-10]: 10

    Why: The suggestion directly addresses the criteria of meaningful variable names, proposing a more descriptive name that better reflects the variable's purpose in the PR context.

    10
    Possible issue
    Add timezone specification to datetime.now() call

    The datetime.now() call without a timezone specification might lead to inconsistencies
    across different environments. Consider using datetime.now(timezone.utc) to ensure
    consistent timezone handling across different systems.

    pr_agent/tools/pr_code_suggestions.py [85]

    -"date": datetime.now().strftime('%Y-%m-%d'),
    +"date": datetime.now(timezone.utc).strftime('%Y-%m-%d'),
     
    Suggestion importance[1-10]: 10

    Why: The suggestion identifies an important edge case where timezone differences could cause inconsistencies, which aligns with the criteria of looking for edge cases in new functions.

    10

    @mrT23
    Copy link
    Collaborator Author

    mrT23 commented Jan 19, 2025

    /describe

    Copy link
    Contributor

    PR Description updated to latest commit (a74fca7)

    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