You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Add Azure OpenAI as a first-class LLM and embedding provider
Both factories advertised multi-provider support behind real ABCs but only ever
constructed public-OpenAI clients. This adds azure_openai to each.
Approach: target Azure's v1 OpenAI-compatible surface (<endpoint>/openai/v1) and reuse
the standard AsyncOpenAI client rather than AsyncAzureOpenAI, whose static types the
openai SDK's own README warns "can be incorrect". So the Azure branch is a different
base_url and a deployment name, not a second client implementation.
- config.py: azure_openai_endpoint / _api_key / _deployment / _embedding_deployment /
_tokenizer_model, plus azure_openai_base_url() which normalises the endpoint to
/openai/v1 idempotently (accepts bare host, trailing slash, or an already-complete
URL).
- llm/factory.py + embeddings/factory.py: azure_openai branch, failing fast with a
named variable when endpoint / key / deployment are missing.
- openai_llm.py + openai_embeddings.py: api_key widened to str | Callable[[], str] so
an Entra token provider can be passed without either class knowing how the
credential is obtained. Nothing here supplies one yet -- auth is API-key only.
- requirements.txt: openai>=1.106.0, the floor Microsoft documents for the v1 surface
and callable token providers, and the first version exporting the error classes the
health check now discriminates on. Was >=1.12.0.
Two Azure divergences that would otherwise be silent:
- health_check no longer treats a missing /models route as unhealthy. On Azure that
route enumerates *deployments* and some configurations omit it entirely; a 404 means
the endpoint answered, so credentials and networking are fine. 401/403 and unexpected
statuses still fail. Previously /api/health would have reported a working Azure
deployment as degraded.
- openai_embeddings.py takes tokenizer_model separately, because tiktoken resolves an
encoding from a model id and on Azure `model` is a deployment name. Note honestly
that the old bare `except KeyError: cl100k_base` was *accidentally* correct: every
current OpenAI embedding model resolves to cl100k_base anyway. So this is a latent
correctness fix plus a warning where there was silence, not a live bug fix. It would
have mattered on an o200k_base embedding model or a non-OpenAI base_url.
Also, because Azure makes them reachable:
- openai_embedding_dimensions is now configurable and threaded through both factories.
It was hardcoded to 1536 while the model was configurable, so a text-embedding-3-large
deployment (3072) only failed when Chroma rejected the insert.
- embeddings/factory.py's unknown-provider branch now raises instead of falling back to
OpenAI whenever a key happened to be set. That fallback dropped all seven rate-limit,
batching and pacing arguments, so a typo in EMBEDDING_PROVIDER silently produced a
differently-behaving client with no error. llm/factory.py already raised.
docker-compose.yml forwards the six new variables (55 total), and .env.example plus
docker/README.md document deployment-names-not-model-ids, the tokenizer requirement,
the dimensions match, and that Entra is not wired up.
Verified: 22 new unit tests covering URL normalisation, factory wiring, missing-config
errors, tokenizer resolution, dimensions and all five health-check branches; 111 tests
pass (was 89); ruff clean; both providers construct correctly from environment alone
and the default OpenAI path is unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
0 commit comments