fix(openai): expose azure_deployment on LLM and fix model property fallback#6221
fix(openai): expose azure_deployment on LLM and fix model property fallback#6221tsushanth wants to merge 1 commit into
Conversation
β¦llback When using LLM.with_azure() without an explicit model, the model property returned "unknown" rather than reflecting the configured azure_deployment. This adds azure_deployment to _LLMOptions, stores it from with_azure(), exposes it as a public property, and updates the model property to fall back to azure_deployment when model is None. Fixes livekit#6209
There was a problem hiding this comment.
π΄ Passing None model to LLMStream causes TypeError crash in Azure path
When LLM.with_azure() is called without specifying model (which now defaults to None), the chat() method at line 1031 passes self._opts.model (which is None) directly to LLMStream. This propagates through the openai plugin's LLMStream.__init__ into the inference LLMStream.__init__ (livekit-agents/livekit/agents/inference/llm.py:378), which calls drop_unsupported_params(model, ...). Inside that function (livekit-agents/livekit/agents/inference/llm.py:85), the expression "/" in model raises TypeError: argument of type 'NoneType' is not iterable because model is None.
The model property (line 186) already handles the None case correctly by falling back to azure_deployment or "unknown", but the chat() method bypasses the property and reads self._opts.model directly.
(Refers to line 1031)
Was this helpful? React with π or π to provide feedback.
| def with_azure( | ||
| *, | ||
| model: str | ChatModels = "gpt-4o", | ||
| model: str | ChatModels | None = None, |
There was a problem hiding this comment.
π© Breaking change: with_azure model default changed from "gpt-4o" to None
The model parameter default in with_azure changed from "gpt-4o" to None (line 199). This is a behavioral change for existing callers who relied on the implicit default model. Previously, LLM.with_azure(azure_deployment="my-deployment") would still send model="gpt-4o" to the API. Now it sends None (or after the bug fix, sends the azure_deployment value). This is likely the intended behavior since Azure users typically specify deployment names rather than model names, but it's a breaking change for users who relied on the old default.
Was this helpful? React with π or π to provide feedback.
| verbosity=verbosity, | ||
| max_completion_tokens=max_completion_tokens, | ||
| ) | ||
| llm._opts.azure_deployment = azure_deployment |
There was a problem hiding this comment.
π© Setting azure_deployment after construction bypasses validation
At line 261, azure_deployment is set directly on _opts after the LLM instance is already constructed (llm._opts.azure_deployment = azure_deployment). This means the __init__ method doesn't know about the deployment during construction (it sees azure_deployment=None). This pattern works but is fragile β any future logic in __init__ that needs to consider azure_deployment would be bypassed. Additionally, azure_deployment could itself be None if the caller doesn't pass it, making both model and azure_deployment None, resulting in the model property returning "unknown".
Was this helpful? React with π or π to provide feedback.
Closes #6209
When calling
LLM.with_azure()without an explicitmodelargument, themodelproperty on the returned instance returned"unknown"even whenazure_deploymentwas set. This happened because the model fallback inwith_azure()previously hard-coded a default and themodelproperty had no awareness ofazure_deployment.This change adds
azure_deploymentto_LLMOptionsso it is tracked independently ofmodel. Thewith_azure()factory now stores theazure_deploymentvalue in opts rather than using it to silently fill inmodel. Themodelproperty is updated to fall back toazure_deploymentwhenmodelisNone, and a newazure_deploymentproperty is exposed so callers can inspect the value directly.The
LLM.__init__signature is updated to acceptNoneformodelto support the Azure path cleanly without overloading the default.