fix: make top_p parameter optional for Azure AI models #347
Conversation
Azure OpenAI models don't support the top_p parameter, which was causing API errors. This change adds conditional logic to only include top_p in model_kwargs when it's defined in the model configuration. Tested with Azure AI Foundry. - Add conditional checks for top_p in simple_chat.py and websocket_wiki.py - Azure models in generator.json already omit top_p (only have temperature) - Other providers continue to use their configured top_p values - Backwards compatible with existing configurations
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @r0kk, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request resolves an API incompatibility issue with Azure OpenAI models by making the top_p parameter optional. By conditionally including top_p based on its presence in the model configuration, the system can now seamlessly interact with Azure models that do not support this parameter, while maintaining functionality for other providers.
Highlights
- Azure AI Model Compatibility: Addresses an issue where Azure OpenAI models do not support the top_p parameter, which previously caused API errors.
- Conditional top_p Inclusion: Implements conditional logic to include the top_p parameter in model_kwargs only if it is explicitly defined in the model configuration.
- Backwards Compatibility: Ensures that the change is backwards compatible with existing configurations and other model providers continue to use their configured top_p values.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request correctly addresses an API error with Azure OpenAI models by making the top_p parameter optional. The fix is applied consistently in both simple_chat.py and websocket_wiki.py. My review focuses on a maintainability improvement. The implemented logic, while correct, is now duplicated across three different provider blocks (openrouter, openai, azure) and across two files. I've added comments suggesting a refactoring to extract this common logic into a shared helper function. This will make the codebase cleaner, more maintainable, and less prone to future inconsistencies.
| model_kwargs = { | ||
| "model": request.model, | ||
| "stream": True, | ||
| "temperature": model_config["temperature"], | ||
| "top_p": model_config["top_p"] | ||
| "temperature": model_config["temperature"] | ||
| } | ||
| # Only add top_p if it exists in the model config | ||
| if "top_p" in model_config: | ||
| model_kwargs["top_p"] = model_config["top_p"] |
There was a problem hiding this comment.
While this change correctly makes top_p optional for the Azure provider, it highlights a code duplication issue. The logic for constructing model_kwargs is now identical for the openrouter, openai, and azure providers. To improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle, consider extracting this common logic into a helper function. This would centralize the logic and make future modifications easier.
For example:
def _prepare_common_model_kwargs(model_config, request_model):
model_kwargs = {
"model": request_model,
"stream": True,
"temperature": model_config["temperature"],
}
if "top_p" in model_config:
model_kwargs["top_p"] = model_config["top_p"]
return model_kwargs| model_kwargs = { | ||
| "model": request.model, | ||
| "stream": True, | ||
| "temperature": model_config["temperature"], | ||
| "top_p": model_config["top_p"] | ||
| "temperature": model_config["temperature"] | ||
| } | ||
| # Only add top_p if it exists in the model config | ||
| if "top_p" in model_config: | ||
| model_kwargs["top_p"] = model_config["top_p"] |
There was a problem hiding this comment.
Similar to the change in simple_chat.py, this logic for constructing model_kwargs is duplicated. It's identical for openrouter, openai, and azure providers. Refactoring this repeated code into a shared utility function would significantly improve code maintainability and reduce redundancy across both websocket_wiki.py and simple_chat.py.
Azure OpenAI models don't support the top_p parameter, which was causing API errors. This change adds conditional logic to only include top_p in model_kwargs when it's defined in the model configuration.
Tested with Azure AI Foundry.