Skip to content

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

fix: improve Azure OpenAI detection and update API version for o3 models #7499

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/types/src/providers/openai.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,8 @@ export const openAiModelInfoSaneDefaults: ModelInfo = {

// 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
Copy link
Contributor Author

@roomote roomote bot Aug 28, 2025

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.

export const azureOpenAiDefaultApiVersion = "2025-03-01-preview"

export const OPENAI_NATIVE_DEFAULT_TEMPERATURE = 0
export const GPT5_DEFAULT_TEMPERATURE = 1.0
Expand Down
127 changes: 85 additions & 42 deletions src/api/providers/__tests__/openai.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,43 @@ const mockCreate = vitest.fn()

vitest.mock("openai", () => {
const mockConstructor = vitest.fn()
return {
__esModule: true,
default: mockConstructor.mockImplementation(() => ({
chat: {
completions: {
create: mockCreate.mockImplementation(async (options) => {
if (!options.stream) {
return {
id: "test-completion",
const mockImplementation = () => ({
chat: {
completions: {
create: mockCreate.mockImplementation(async (options) => {
if (!options.stream) {
return {
id: "test-completion",
choices: [
{
message: { role: "assistant", content: "Test response", refusal: null },
finish_reason: "stop",
index: 0,
},
],
usage: {
prompt_tokens: 10,
completion_tokens: 5,
total_tokens: 15,
},
}
}

return {
[Symbol.asyncIterator]: async function* () {
yield {
choices: [
{
delta: { content: "Test response" },
index: 0,
},
],
usage: null,
}
yield {
choices: [
{
message: { role: "assistant", content: "Test response", refusal: null },
finish_reason: "stop",
delta: {},
index: 0,
},
],
Expand All @@ -34,38 +58,17 @@ vitest.mock("openai", () => {
total_tokens: 15,
},
}
}

return {
[Symbol.asyncIterator]: async function* () {
yield {
choices: [
{
delta: { content: "Test response" },
index: 0,
},
],
usage: null,
}
yield {
choices: [
{
delta: {},
index: 0,
},
],
usage: {
prompt_tokens: 10,
completion_tokens: 5,
total_tokens: 15,
},
}
},
}
}),
},
},
}
}),
},
})),
},
})

return {
__esModule: true,
default: mockConstructor.mockImplementation(mockImplementation),
AzureOpenAI: mockConstructor.mockImplementation(mockImplementation),
}
})

Expand Down Expand Up @@ -849,6 +852,46 @@ describe("OpenAiHandler", () => {
)
})
})

describe("Azure OpenAI Detection", () => {
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot Aug 28, 2025

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.

it("should detect Azure OpenAI from .openai.azure.com domain", () => {
const azureHandler = new OpenAiHandler({
...mockOptions,
openAiBaseUrl: "https://myresource.openai.azure.com",
})
expect(azureHandler).toBeInstanceOf(OpenAiHandler)
// The handler should use AzureOpenAI client internally
})

it("should detect Azure OpenAI from URL containing /openai/deployments/", () => {
const azureHandler = new OpenAiHandler({
...mockOptions,
openAiBaseUrl: "https://myresource.openai.azure.com/openai/deployments/mymodel",
})
expect(azureHandler).toBeInstanceOf(OpenAiHandler)
// The handler should use AzureOpenAI client internally
})

it("should detect Azure OpenAI when openAiUseAzure is true", () => {
const azureHandler = new OpenAiHandler({
...mockOptions,
openAiBaseUrl: "https://custom-endpoint.com",
openAiUseAzure: true,
})
expect(azureHandler).toBeInstanceOf(OpenAiHandler)
// The handler should use AzureOpenAI client internally
})

it("should use updated Azure API version for o3 models", () => {
const azureHandler = new OpenAiHandler({
...mockOptions,
openAiBaseUrl: "https://myresource.openai.azure.com",
openAiModelId: "o3-mini",
})
expect(azureHandler).toBeInstanceOf(OpenAiHandler)
// The handler should use the updated API version (2025-03-01-preview)
})
Copy link
Contributor Author

@roomote roomote bot Aug 28, 2025

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:

Suggested change
})
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)
})

})
})

describe("getOpenAiModels", () => {
Expand Down
16 changes: 15 additions & 1 deletion src/api/providers/openai.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl
const apiKey = this.options.openAiApiKey ?? "not-provided"
const isAzureAiInference = this._isAzureAiInference(this.options.openAiBaseUrl)
const urlHost = this._getUrlHost(this.options.openAiBaseUrl)
const isAzureOpenAi = urlHost === "azure.com" || urlHost.endsWith(".azure.com") || options.openAiUseAzure
// Improved Azure detection: check for various Azure OpenAI URL patterns
const isAzureOpenAi = this._isAzureOpenAi(this.options.openAiBaseUrl) || options.openAiUseAzure

const headers = {
...DEFAULT_HEADERS,
Expand Down Expand Up @@ -403,6 +404,19 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl
return urlHost.endsWith(".services.ai.azure.com")
}

private _isAzureOpenAi(baseUrl?: string): boolean {
Copy link
Contributor Author

@roomote roomote bot Aug 28, 2025

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:

Suggested change
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 {

if (!baseUrl) return false
const urlHost = this._getUrlHost(baseUrl)
// Check for various Azure OpenAI URL patterns
return (
urlHost === "azure.com" ||
urlHost.endsWith(".azure.com") ||
urlHost.endsWith(".openai.azure.com") ||
// Check if URL contains Azure OpenAI specific paths
baseUrl.includes("/openai/deployments/")
Copy link
Contributor Author

@roomote roomote bot Aug 28, 2025

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.

)
}

/**
* Adds max_completion_tokens to the request body if needed based on provider configuration
* Note: max_tokens is deprecated in favor of max_completion_tokens as per OpenAI documentation
Expand Down
Loading