-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: improve Azure OpenAI detection and update API version for o3 models #7499
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
Conversation
- Updated default Azure API version from 2024-08-01-preview to 2025-03-01-preview to support newer models like o3-mini - Enhanced Azure OpenAI detection logic to check for .openai.azure.com domains and /openai/deployments/ paths - Added comprehensive tests for Azure OpenAI detection scenarios - Fixed test mocks to properly support AzureOpenAI client Fixes #7498
| }) | ||
| }) | ||
|
|
||
| describe("Azure OpenAI Detection", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new Azure OpenAI detection tests are a great addition. For even stronger validation, consider asserting that the underlying client is an instance of AzureOpenAI (or that the 'apiVersion' parameter is set to the updated value) when an Azure endpoint is detected rather than only checking the handler instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own code is like debugging in production - technically possible but morally questionable.
| urlHost.endsWith(".azure.com") || | ||
| urlHost.endsWith(".openai.azure.com") || | ||
| // Check if URL contains Azure OpenAI specific paths | ||
| baseUrl.includes("/openai/deployments/") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The URL pattern check for /openai/deployments/ could potentially match non-Azure URLs. Is this intentional, or should we make the detection more specific to avoid false positives?
For example, a custom OpenAI-compatible API at https://custom-api.com/openai/deployments/model would be incorrectly detected as Azure.
| return urlHost.endsWith(".services.ai.azure.com") | ||
| } | ||
|
|
||
| private _isAzureOpenAi(baseUrl?: string): boolean { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding JSDoc comments to document this method's purpose and the URL patterns it detects:
| private _isAzureOpenAi(baseUrl?: string): boolean { | |
| /** | |
| * Detects if the provided base URL is an Azure OpenAI endpoint | |
| * @param baseUrl - The API base URL to check | |
| * @returns true if the URL matches Azure OpenAI patterns: | |
| * - Domains ending with .openai.azure.com | |
| * - URLs containing /openai/deployments/ path | |
| * - Domains ending with .azure.com | |
| */ | |
| private _isAzureOpenAi(baseUrl?: string): boolean { |
| // https://learn.microsoft.com/en-us/azure/ai-services/openai/api-version-deprecation | ||
| // https://learn.microsoft.com/en-us/azure/ai-services/openai/reference#api-specs | ||
| export const azureOpenAiDefaultApiVersion = "2024-08-01-preview" | ||
| // Updated to support newer models like o3-mini |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we expand this comment to explain why 2025-03-01-preview specifically enables o3 model support? This would help future maintainers understand when to update the API version.
| }) | ||
| expect(azureHandler).toBeInstanceOf(OpenAiHandler) | ||
| // The handler should use the updated API version (2025-03-01-preview) | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a test case for the potential edge case where a non-Azure URL contains /openai/deployments/. This would help ensure the detection logic handles all scenarios correctly:
| }) | |
| it("should not incorrectly detect non-Azure URLs with /openai/deployments/ path", () => { | |
| const nonAzureHandler = new OpenAiHandler({ | |
| ...mockOptions, | |
| openAiBaseUrl: "https://custom-api.com/openai/deployments/model", | |
| }) | |
| // Verify behavior - should it detect as Azure or not? | |
| expect(nonAzureHandler).toBeInstanceOf(OpenAiHandler) | |
| }) |
|
I'm not sure if this will deal with the issue if the user is providing a custom URL |
Summary
This PR attempts to address Issue #7498 where Azure OpenAI returns 404 errors when using newer models like o3-mini through the OpenAI Compatible provider.
Problem
Users were experiencing 404 errors when trying to use Azure OpenAI with newer models (like o3-mini) through Roo-Code's OpenAI Compatible provider, even though the same configuration worked in Python code using the Azure OpenAI SDK.
Root Cause
Solution
1. Updated Azure API Version
2024-08-01-previewto2025-03-01-preview2. Enhanced Azure Detection Logic
_isAzureOpenAi()method with improved URL pattern detection.openai.azure.comdomains/openai/deployments/pathsopenAiUseAzureflag3. Added Comprehensive Tests
Testing
Related Issues
Fixes #7498
Feedback and guidance are welcome!
Important
Updated Azure OpenAI API version and detection logic to support newer models and added comprehensive tests.
2024-08-01-previewto2025-03-01-previewinopenai.tsto support newer models likeo3-mini.OpenAiHandlerby adding_isAzureOpenAi()method to detect.openai.azure.comdomains, URLs with/openai/deployments/, andopenAiUseAzureflag.openai.spec.tsfor Azure OpenAI detection scenarios.This description was created by
for 4f7a858. You can customize this summary. It will automatically update as commits are pushed.