fix: Prevent duplicate language instructions in extra_instructions

[MaxnSter]

User description

Currently, when processing a non-default response_language, it appends a language-specific directive to the extra_instructions in the settings. However, if this occurs multiple times (e.g., there are 3 commands when serving as gitlab webhook server ), the directive is duplicated, leading to an unnecessarily long instruction string in system prompts.

below are traces that we catch in langfuse:

• system prompt in /describe
  

• system prompt in /review
  

• system prompt in /improve
  

---

PR Type

Bug fix

---

Description

• Prevents duplicate language instructions in extra_instructions

• Adds check before appending language directive to settings

• Refines logic for appending language-specific instructions

---

Changes walkthrough 📝

	Relevant files
Bug fix	pr_agent.py Prevent duplicate language instruction in settings extra_instructions pr_agent/agent/pr_agent.py Adds logic to check for existing language instruction before appending Prevents duplication of language-specific directive in extra_instructions Refactors string concatenation for clarity and correctness +12/-4

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-merge-pro-for-open-source]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Typo Fix The PR fixes a typo in the language instruction message, changing "local code" to "locale code", which is the correct terminology for language identifiers. lang_instruction_text = f"Your response MUST be written in the language corresponding to locale code: '{response_language}'. This is crucial." separator_text = "\n======\n\nIn addition, "

[qodo-merge-pro-for-open-source]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix type handling issue The current implementation may cause issues if current_extra_instructions is not a string type. Convert current_extra_instructions to string only when needed for the comparison, not when appending to avoid potential type conversion issues. pr_agent/agent/pr_agent.py [92-97] # Check if the specific language instruction is already present to avoid duplication if lang_instruction_text not in str(current_extra_instructions): if current_extra_instructions: # If there's existing text - setting.extra_instructions = str(current_extra_instructions) + separator_text + lang_instruction_text + setting.extra_instructions = current_extra_instructions + separator_text + lang_instruction_text else: # If extra_instructions was None or empty setting.extra_instructions = lang_instruction_text Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential type issue where str(current_extra_instructions) is used for concatenation, which could cause unexpected behavior if current_extra_instructions is already a string object. Removing the redundant string conversion in the concatenation line is a valid improvement for code correctness.	Medium
Possible issue	Handle None value explicitly The code doesn't handle the case where current_extra_instructions is None. When str(None) is evaluated, it becomes 'None', but the string comparison will still work. However, the condition if current_extra_instructions: will evaluate to False for None, causing incorrect flow. Add explicit None check. pr_agent/agent/pr_agent.py [92-97] # Check if the specific language instruction is already present to avoid duplication -if lang_instruction_text not in str(current_extra_instructions): - if current_extra_instructions: # If there's existing text - setting.extra_instructions = str(current_extra_instructions) + separator_text + lang_instruction_text - else: # If extra_instructions was None or empty - setting.extra_instructions = lang_instruction_text +if current_extra_instructions is None: + setting.extra_instructions = lang_instruction_text +elif lang_instruction_text not in str(current_extra_instructions): + setting.extra_instructions = current_extra_instructions + separator_text + lang_instruction_text +# If lang_instruction_text is already present, do nothing. Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential issue with handling None values for current_extra_instructions. The improved code explicitly checks for None first, which is cleaner than relying on the truthiness check. This improves robustness and readability, though it's not addressing a critical bug since if current_extra_instructions: would already handle None correctly in practice.	Medium
More

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

[mrT23]

thanks @MaxnSter !
indeed a bug.