add gpt4o mini support #666
Conversation
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to 087e962 in 58 seconds
More details
- Looked at
152lines of code in4files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_OizGxUn8h4bA1IB0
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| @@ -158,6 +160,8 @@ async def llm_api_handler( | |||
| parameters = LLMAPIHandlerFactory.get_api_parameters() | |||
|
|
|||
| active_parameters.update(parameters) | |||
| if llm_config.litellm_params: | |||
There was a problem hiding this comment.
The code assumes litellm_params is always a dictionary, but it's defined as optional and can be None. This can lead to a TypeError when attempting to update active_parameters with None. Consider adding a check to ensure litellm_params is not None before updating.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on e54cadb in 57 seconds
More details
- Looked at
33lines of code in2files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. skyvern/forge/sdk/api/llm/api_handler_factory.py:163
- Draft comment:
Using# type: ignorecan lead to potential runtime errors iflitellm_paramsare not structured as expected. Consider implementing proper type checks or conversions to ensure compatibility.
if llm_config.litellm_params is not None:
active_parameters.update(llm_config.litellm_params)
- Reason this comment was not posted:
Confidence of 20% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_CvtgcetOCuPG1iXA
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
LawyZheng
force-pushed
the
add-azure-gpt4o-mini
branch
from
e54cadb to
ae7a6d9
Compare
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on ae7a6d9 in 53 seconds
More details
- Looked at
33lines of code in2files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. skyvern/forge/sdk/api/llm/api_handler_factory.py:163
- Draft comment:
Usingtype: ignorecan hide potential issues. Consider handling the optional nature oflitellm_paramsexplicitly to avoid type mismatches and improve code safety.
if llm_config.litellm_params is not None:
active_parameters.update(llm_config.litellm_params)
- Reason this comment was not posted:
Confidence of 20% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_Lo6xhtyIeTjjSxIz
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
Summary:
Added support for Azure GPT-4o mini by updating configuration settings, API handler factory, and LLM configuration registry.
Key points:
ENABLE_AZURE_GPT4O_MINIflag inskyvern/config.py.skyvern/config.py.LLMConfigandLLMConfigBaseinskyvern/forge/sdk/api/llm/models.pyto includelitellm_params.LLMAPIHandlerFactory.get_llm_api_handlerinskyvern/forge/sdk/api/llm/api_handler_factory.pyto handlelitellm_params.skyvern/forge/sdk/api/llm/config_registry.py.Generated with ❤️ by ellipsis.dev