Skip to content

fix(openai): default Azure LLM model to deployment#6266

Open
nightcityblade wants to merge 3 commits into
livekit:mainfrom
nightcityblade:fix/issue-6209
Open

fix(openai): default Azure LLM model to deployment#6266
nightcityblade wants to merge 3 commits into
livekit:mainfrom
nightcityblade:fix/issue-6209

Conversation

@nightcityblade

Copy link
Copy Markdown
Contributor

Fixes #6209

Summary

  • stop LLM.with_azure() from exposing the unrelated gpt-4o default when Azure callers only set azure_deployment
  • use the Azure deployment name as the fallback model identifier so telemetry and llm.model reflect the configured deployment
  • add a regression test covering the Azure fallback behavior

Testing

  • uv run ruff check livekit-plugins/livekit-plugins-openai/livekit/plugins/openai/llm.py tests/test_config.py
  • uv run pytest tests/test_config.py

@nightcityblade nightcityblade requested a review from a team as a code owner June 28, 2026 15:10
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


nightcityblade seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

βœ… Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@@ -238,8 +238,10 @@ def with_azure(
else httpx.Timeout(connect=15.0, read=5.0, write=5.0, pool=5.0),
) # type: ignore

resolved_model = model if model is not None else azure_deployment or "gpt-4o"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can simply use unknown as the last resort, as the model parameter is only used for metric/label.

@nightcityblade

Copy link
Copy Markdown
Contributor Author

Addressed the review note in 4c0ede6. LLM.with_azure() now falls back to "unknown" when neither an explicit model nor an Azure deployment is provided, so we keep the metrics label neutral instead of defaulting to a specific model family. I also added a focused regression test for the no-model/no-deployment case.

Validation:

  • uv run pytest tests/test_config.py
  • uv run ruff check livekit-plugins/livekit-plugins-openai/livekit/plugins/openai/llm.py tests/test_config.py

@nightcityblade

Copy link
Copy Markdown
Contributor Author

I pushed a follow-up commit to align with the review suggestion: Azure now keeps as the fallback model label unless the caller explicitly provides , and I updated the config expectation to match.\n\nI also rechecked the simple constructor path locally against the new behavior.

@nightcityblade

Copy link
Copy Markdown
Contributor Author

I pushed a follow-up commit to align with the review suggestion: Azure now keeps "unknown" as the fallback model label unless the caller explicitly provides model, and I updated the config expectation to match.

I also rechecked the simple LLM.with_azure(...) constructor path locally against the new behavior.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

Open in Devin Review

Comment on lines +241 to +243
# For Azure, the model name is only used as a metric label, so keep the
# fallback neutral unless the caller provided an explicit model name.
resolved_model = model if model is not None else "unknown"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Model value "unknown" is sent in the API request body to Azure

When model=None is passed to with_azure and no azure_deployment is provided either (as in test_llm_with_azure_uses_unknown_without_model_or_deployment), the string "unknown" will be sent as the model parameter in chat.completions.create() at livekit-agents/livekit/agents/inference/llm.py:351. With AsyncAzureOpenAI, if azure_deployment is None, the client may attempt to use the model value as the deployment name, which would cause a runtime 404 from the Azure API. This is arguably fine β€” the user failed to provide any routing info β€” but it's a change from the old default of "gpt-4o" which at least could match a real deployment name. The comment at llm.py:241-242 says the model is only used as a metric label, which is true when azure_deployment is set but not when it's absent.

Open in Devin Review

Was this helpful? React with πŸ‘ or πŸ‘Ž to provide feedback.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. The intent of the change was to avoid reporting a misleading default metric label when Azure callers provide a deployment but omit model, not to make the no-deployment path any more valid than it already is.

If maintainers want, I can follow up by narrowing the fallback so we only use "unknown" when azure_deployment is set, and otherwise keep the previous behavior for the invalid no-deployment case. That would preserve the neutral label improvement without changing the request-routing fallback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove default for "model" parameter of Azure OpenAI LLM

4 participants