feat: add support for O1 model by combining system and user prompt

[mrT23]

User description

fixes #1278

---

PR Type

enhancement, bug_fix

---

Description

• Added support for the O1 model in the litellm_ai_handler by combining system and user prompts into a single user prompt, as the O1 model does not support separate roles.
• Adjusted the message construction logic to ensure compatibility with the O1 model, preventing errors related to unsupported roles.
• Updated the handling of image URLs within user messages to ensure proper message formatting.
• Included logging to inform when the system and user prompts are combined for O1 models.

---

Changes walkthrough 📝

Relevant files

Enhancement

litellm_ai_handler.py Add support for O1 model in `litellm_ai_handler`                  pr_agent/algo/ai_handlers/litellm_ai_handler.py Added support for O1 model by combining system and user prompts. Modified message construction for O1 models to avoid unsupported roles. Adjusted logic to handle image URLs in user messages. Updated logging to inform about the combination of prompts for O1 models. +24/-8

---

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

[qodo-merge-pro]

PR Code Suggestions ✨

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

---

Previous suggestions

✅ Suggestions up to commit 727b08f

Category	Suggestion	Score
Possible issue	Add input validation to handle potential empty system or user prompts Consider adding a check for empty system and user inputs before combining them for the O1 model. This would prevent potential issues with empty strings and improve the robustness of the code. pr_agent/algo/ai_handlers/litellm_ai_handler.py [190-194] if model.startswith('o1-'): - user = f"{system}\n\n\n{user}" + if system.strip() and user.strip(): + user = f"{system.strip()}\n\n\n{user.strip()}" + elif system.strip(): + user = system.strip() + elif not user.strip(): + user = "No input provided" system = "" get_logger().info(f"Using O1 model, combining system and user prompts") messages = [{"role": "user", "content": user}] Suggestion importance[1-10]: 9 Why: Adding checks for empty inputs before combining them for the O1 model improves robustness by preventing issues with empty strings. This is a critical enhancement for ensuring the code handles edge cases gracefully.	9
Enhancement	Reduce code duplication by extracting common parameters and using dictionary unpacking Consider extracting the common kwargs into a separate dictionary and then updating it based on the model type. This would reduce code duplication and make it easier to manage common parameters. pr_agent/algo/ai_handlers/litellm_ai_handler.py [190-210] +common_kwargs = { + "model": model, + "deployment_id": deployment_id, + "timeout": get_settings().config.ai_timeout, + "api_base": self.api_base, +} + if model.startswith('o1-'): user = f"{system}\n\n\n{user}" system = "" get_logger().info(f"Using O1 model, combining system and user prompts") messages = [{"role": "user", "content": user}] - kwargs = { - "model": model, - "deployment_id": deployment_id, - "messages": messages, - "timeout": get_settings().config.ai_timeout, - "api_base": self.api_base, - } + kwargs = {**common_kwargs, "messages": messages} else: - kwargs = { - "model": model, - "deployment_id": deployment_id, - "messages": messages, - "temperature": temperature, - "timeout": get_settings().config.ai_timeout, - "api_base": self.api_base, - } + kwargs = {**common_kwargs, "messages": messages, "temperature": temperature} Suggestion importance[1-10]: 8 Why: Extracting common parameters into a separate dictionary reduces code duplication and improves readability. It also simplifies future modifications to shared parameters, enhancing maintainability.	8
Maintainability	✅ Use a constant for the model prefix to improve code maintainability Consider using a constant or configuration value for the O1 model prefix ('o1-') instead of hardcoding it in the condition. This would make the code more maintainable and easier to update if the prefix changes in the future. pr_agent/algo/ai_handlers/litellm_ai_handler.py [190-194] -if model.startswith('o1-'): +O1_MODEL_PREFIX = 'o1-' +if model.startswith(O1_MODEL_PREFIX): user = f"{system}\n\n\n{user}" system = "" get_logger().info(f"Using O1 model, combining system and user prompts") messages = [{"role": "user", "content": user}] [Suggestion has been applied] Suggestion importance[1-10]: 7 Why: Using a constant for the model prefix enhances maintainability by making it easier to update the prefix in the future. This is a good practice for reducing hardcoded values in the code.	7

[mrT23]

PR Reviewer Guide 🔍

(Review updated until commit e6c56c7)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ 1278 - Fully compliant Fully compliant requirements: Support new OpenAI models (o1-mini/o1-preview) Fix the error related to unsupported 'system' role in messages for O1 models

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

🏅 Score: 95

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling The PR adds new logic for O1 models, but doesn't include specific error handling for potential issues that might arise from this change. Consider adding try-except blocks or additional error checks. Logging Improvement The PR adds a log message for O1 models, but it might be beneficial to add more detailed logging throughout the new logic to aid in debugging and monitoring.

[mrT23]

Persistent review updated to latest commit e6c56c7

[mrT23]

PR Description updated to latest commit (e6c56c7)

• Copy walkthrough table to "Files Changed" Tab

[mrT23]

PR Description updated to latest commit (e6c56c7)

• Copy walkthrough table to "Files Changed" Tab

[mrT23]

PR Description updated to latest commit (e6c56c7)

• Copy walkthrough table to "Files Changed" Tab

[mrT23]

/review

[qodo-merge-pro]

Persistent review updated to latest commit e6c56c7

[mrT23]

/review

[qodo-merge-pro]

Persistent review updated to latest commit e6c56c7