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: sanitize Ask tool answers to prevent markdown formatting issues #1426

Merged
merged 2 commits into from
Dec 30, 2024

Conversation

mrT23
Copy link
Collaborator

@mrT23 mrT23 commented Dec 30, 2024

PR Type

Bug fix


Description

  • Fixed markdown formatting issues in Ask tool responses by sanitizing model answers
  • Added preprocessing step to ensure no line starts with "/" by adding a space prefix
  • Prevents potential markdown rendering problems in the tool's output

Changes walkthrough 📝

Relevant files
Bug fix
pr_questions.py
Sanitize model answers to prevent markdown formatting       

pr_agent/tools/pr_questions.py

  • Added sanitization of model answers to prevent markdown formatting
    issues
  • Replaced newlines followed by "/" with newlines followed by " /" to
    avoid markdown conflicts
  • +5/-1     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    Enhance text sanitization to handle all potential markdown special characters that could cause formatting issues

    Consider using a more comprehensive sanitization approach. The current fix only
    handles newlines followed by "/", but other markdown special characters could also
    cause formatting issues. Consider sanitizing other potential markdown syntax
    conflicts.

    pr_agent/tools/pr_questions.py [120-122]

     model_answer = self.prediction.strip()
    -# sanitize the answer so that no line will start with "/"
    -model_answer_sanitized = model_answer.replace("\n/", "\n /")
    +# sanitize markdown special characters that could cause formatting issues
    +markdown_chars = ['/', '#', '*', '_', '`', '>', '-']
    +model_answer_sanitized = model_answer
    +for char in markdown_chars:
    +    model_answer_sanitized = model_answer_sanitized.replace(f"\n{char}", f"\n {char}")
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion significantly improves the robustness of the markdown sanitization by handling multiple special characters that could cause formatting issues, not just the '/' character. This is a valuable enhancement for preventing potential markdown rendering problems.

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

    @mrT23 mrT23 merged commit 014b1f2 into main Dec 30, 2024
    2 checks passed
    @mrT23 mrT23 deleted the tr/ask_fix branch December 30, 2024 13:12
    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