feat(providers): add openrouter and azure env discovery#161
Merged
SantiagoDePolonia merged 6 commits intomainfrom Mar 22, 2026
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds OpenRouter and Azure OpenAI provider support: environment templates and docs, config example updates, provider APIVersion support and auto-discovery, a reusable OpenAI-compatible provider abstraction, concrete Azure and OpenRouter provider implementations (with API-version and attribution-header handling), factory APIVersion wiring, extensible file/batch request preparers, and extensive tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Client
participant Factory as ProviderFactory
participant Provider as Provider (openrouter / azure)
participant External as ExternalAPI
Client->>Factory: Build ProviderConfig(type, api_key, base, api_version)
Factory->>Provider: Create(config)
alt cfg.APIVersion non-empty and provider supports it
Factory->>Provider: SetAPIVersion(cfg.APIVersion)
end
Client->>Provider: ChatCompletion request
Provider->>Provider: RequestMutator (inject headers, default base, api-version)
Provider->>External: HTTP POST /chat/completions (with api-key, query/api-version, headers)
External-->>Provider: HTTP response
Provider-->>Client: Parsed ChatCompletion response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/providers/config.go`:
- Around line 106-108: The hardcoded name check (name == "azure") is fragile;
change the logic to consult provider metadata instead — replace the string
comparison with the provider's require-base flag or type field (e.g., use
p.RequireBase or p.Type) and the existing p.BaseURL check: for example, instead
of if name == "azure" && strings.TrimSpace(p.BaseURL) == "" { continue }, use a
metadata-driven condition like if !p.RequireBase && strings.TrimSpace(p.BaseURL)
== "" { continue } (or if p.Type == "azure" when type semantics are preferred)
so the filter works regardless of the provider instance name.
- Line 38: The config registers Azure as providerType "openai" which causes the
OpenAI client (see setHeaders in internal/providers/openai/openai.go and
SetBaseURL()) to use Bearer auth and fail for Azure OpenAI; change the
registration entry in internal/providers/config.go from type "openai" to a
distinct "azure" (or add a separate Azure provider record) and update the OpenAI
provider's setHeaders to detect provider type "azure" (or implement a new azure
provider) to send the required "api-key" header instead of the Bearer
Authorization header so Azure requests authenticate correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 43632a9b-765c-42e3-a210-e4c576f1c10d
📒 Files selected for processing (8)
.env.templateGETTING_STARTED.mdREADME.mdconfig/config.example.yamlconfig/config_test.godocs/advanced/configuration.mdxinternal/providers/config.gointernal/providers/config_test.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
internal/providers/config.go (1)
113-115:⚠️ Potential issue | 🟠 MajorValidate Azure's required base URL by provider type, not map key.
This guard only skips the provider named exactly
azure. The docs in this PR still advertise custom provider names, so a block likemy-azure: { type: azure }bypasses it, and${AZURE_API_BASE}also slips through because it is non-empty. That leaves a broken Azure config in the resolved map and it only fails later at request time.🛠️ Suggested fix
- if name == "azure" && strings.TrimSpace(p.BaseURL) == "" { + if p.Type == "azure" && (strings.TrimSpace(p.BaseURL) == "" || strings.Contains(p.BaseURL, "${")) { continue }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/providers/config.go` around lines 113 - 115, The guard is checking the map key `name` instead of the provider's declared type, so providers named anything other than the literal key "azure" (e.g., "my-azure") bypass the BaseURL validation; change the conditional to check the provider type (e.g., `p.Type == "azure"` or case-insensitive equivalent) and keep the existing strings.TrimSpace(p.BaseURL) check so any Azure-typed provider with an empty BaseURL is skipped/handled. Locate the block that uses `name` and `p.BaseURL` and update the condition to reference the provider type field (e.g., `p.Type`) instead of `name`. Ensure behavior remains the same for non-Azure providers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@config/config.example.yaml`:
- Around line 155-160: Update the example Azure config to use the environment
placeholder ${AZURE_API_BASE} for the azure.base_url field instead of a
hardcoded URL so it matches .env.template and the setup docs; modify the
commented block that defines the azure config (keys: azure, type, base_url,
api_key, api_version) to set base_url: "${AZURE_API_BASE}" while keeping the
rest of the fields unchanged.
In `@internal/providers/azure/azure.go`:
- Around line 21-45: The Azure Provider currently embeds
openai.CompatibleProvider (Provider) but inherits ListModels, CreateBatch,
GetBatch, ListBatches and CancelBatch which call /models and /batches; implement
overrides on Provider for ListModels, CreateBatch, GetBatch, ListBatches and
CancelBatch so they use the Azure REST paths prefixed with /openai/ (e.g.,
/openai/models, /openai/batches/...) instead of the default routes. In each
override, reuse the CompatibleProvider's request machinery (same auth/headers
from setHeaders and mutateRequest) but construct the HTTP path with the /openai/
prefix before delegating to the underlying request/HTTP client; add these
methods near New/NewWithHTTPClient so Provider consistently handles Azure route
differences.
In `@internal/providers/openai/compatible_provider.go`:
- Around line 86-116: The ChatCompletion and StreamChatCompletion methods (and
the other request-taking methods in this file around lines 131-172) currently
dereference req (via chatRequestBody(req), req.WithStreaming(), and reading
req.Model) and will panic on a nil request; add a nil check at the start of each
of these public entry points and return a typed core.GatewayError with category
"invalid_request_error" when req == nil (follow the same guard pattern used by
Passthrough). Ensure the early-return uses the core GatewayError
constructor/util your codebase uses so callers receive a proper provider-facing
invalid_request_error instead of a panic.
---
Duplicate comments:
In `@internal/providers/config.go`:
- Around line 113-115: The guard is checking the map key `name` instead of the
provider's declared type, so providers named anything other than the literal key
"azure" (e.g., "my-azure") bypass the BaseURL validation; change the conditional
to check the provider type (e.g., `p.Type == "azure"` or case-insensitive
equivalent) and keep the existing strings.TrimSpace(p.BaseURL) check so any
Azure-typed provider with an empty BaseURL is skipped/handled. Locate the block
that uses `name` and `p.BaseURL` and update the condition to reference the
provider type field (e.g., `p.Type`) instead of `name`. Ensure behavior remains
the same for non-Azure providers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e198c176-b6dc-4fb6-a03a-db54620104d4
📒 Files selected for processing (16)
.env.templateGETTING_STARTED.mdREADME.mdcmd/gomodel/main.goconfig/config.example.yamlconfig/config.goconfig/config_test.godocs/advanced/configuration.mdxinternal/providers/azure/azure.gointernal/providers/azure/azure_test.gointernal/providers/config.gointernal/providers/config_test.gointernal/providers/factory.gointernal/providers/factory_test.gointernal/providers/openai/compatible_provider.gointernal/providers/openai/openai.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
internal/providers/config.go (1)
113-115:⚠️ Potential issue | 🟠 MajorFilter Azure configs by
type, not by the literal provider name.
name == "azure"only catches the auto-discovered/default entry. A YAML block likemy-azure: { type: azure }with an emptybase_urlsurvives this filter and then runs against the placeholder default base. Usep.Type == "azure"or the same metadata used during discovery here instead.Suggested change
- if name == "azure" && strings.TrimSpace(p.BaseURL) == "" { + if p.Type == "azure" && strings.TrimSpace(p.BaseURL) == "" { continue }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/providers/config.go` around lines 113 - 115, The filter currently checks the literal provider name variable "name" to skip Azure entries with empty BaseURL, which misses configs like "my-azure" that declare azure via the p.Type field; update the condition to check the provider metadata/type (use p.Type == "azure" or the same discovery metadata used elsewhere) instead of name == "azure" so entries where p.Type is "azure" and strings.TrimSpace(p.BaseURL) == "" are correctly skipped; adjust the conditional in the block around p.BaseURL to reference p.Type (or the discovery marker) and leave the rest of the logic unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/providers/azure/azure.go`:
- Around line 50-59: ListModels and the Azure batch methods are using the
deployment-scoped client base (so requests become
.../deployments/<deployment>/openai/...), but Azure expects resource-level
routes like /openai/models and /openai/batches; update Provider.ListModels and
all batch-related methods (the functions calling p.Do with endpoints
"/openai/models" and "/openai/batches...") to compute a resource-root base URL
(strip the "/deployments/<deployment>" segment from the current base or build
the URL from the Azure resource root stored in the provider) and use that
resource-root when calling p.Do so the final request path becomes
<resource-root>/openai/... instead of <deployment-base>/openai/.... Ensure the
change is applied consistently to ListModels and every batch operation
referenced in the file.
In `@internal/providers/config.go`:
- Around line 61-71: The loop over knownProviderEnvs skips processing when both
apiKey and baseURL are empty, which prevents an env-only AZURE_API_VERSION (from
kp.apiVersionEnv) from overriding YAML; change the logic in the loop that
references knownProviderEnvs and kp.apiVersionEnv so that if apiVersion
(os.Getenv(kp.apiVersionEnv)) is non-empty you do not continue early — instead
allow creation/update of the provider entry in the providers map (e.g.,
providers["azure"] / providers.Azure handling) and apply the apiVersion override
even when apiKey and baseURL are blank; ensure the subsequent code that merges
env values into the provider config still runs for the apiVersion case.
In `@internal/providers/openai/compatible_provider.go`:
- Around line 294-333: The helper-based file and batch-result methods
(GetBatchResults, CreateFile, ListFiles, GetFile, DeleteFile, GetFileContent)
currently call providers.* with p.client directly and therefore skip the
provider-specific requestMutator/prepareRequest; update these methods to run
prepareRequest (or otherwise apply requestMutator) before issuing the
request—e.g., build an llmclient.Request via p.prepareRequest or call a client
wrapper that applies the mutator and then pass that prepared request into the
providers.* functions (or modify the providers call site to accept a prepared
request) so Azure/OpenRouter-specific headers (like api-version and attribution)
are always applied.
---
Duplicate comments:
In `@internal/providers/config.go`:
- Around line 113-115: The filter currently checks the literal provider name
variable "name" to skip Azure entries with empty BaseURL, which misses configs
like "my-azure" that declare azure via the p.Type field; update the condition to
check the provider metadata/type (use p.Type == "azure" or the same discovery
metadata used elsewhere) instead of name == "azure" so entries where p.Type is
"azure" and strings.TrimSpace(p.BaseURL) == "" are correctly skipped; adjust the
conditional in the block around p.BaseURL to reference p.Type (or the discovery
marker) and leave the rest of the logic unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5d30b252-d37d-4e78-b506-f6ab56e8a59b
📒 Files selected for processing (15)
.env.templateGETTING_STARTED.mdcmd/gomodel/main.goconfig/config.example.yamlconfig/config_test.godocs/advanced/configuration.mdxinternal/providers/azure/azure.gointernal/providers/azure/azure_test.gointernal/providers/config.gointernal/providers/config_test.gointernal/providers/openai/compatible_provider.gointernal/providers/openai/openai_test.gointernal/providers/openrouter/openrouter.gointernal/providers/openrouter/openrouter_test.gointernal/providers/registry_test.go
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/providers/batch_results_file_adapter.go (1)
45-53:⚠️ Potential issue | 🟠 MajorGuard the raw batch response before reading
Body.
GetOpenAICompatibleFileContentWithPrepareralready treatsclient.DoRaw(...)as nullable on a nil-error path, but this helper dereferencesbatchRaw.Bodyimmediately. If the same nil-success path happens here,GetBatchResultspanics instead of returning a typed provider error.Suggested fix
batchRaw, err := client.DoRaw(ctx, prepareOpenAICompatibleRequest(prepare, llmclient.Request{ Method: http.MethodGet, Endpoint: "/batches/" + url.PathEscape(batchID), })) if err != nil { return nil, err } + if batchRaw == nil { + return nil, core.NewProviderError(providerName, http.StatusBadGateway, "provider returned empty batch metadata response", fmt.Errorf("nil response")) + } outputFileID, status, endpoint := parseBatchFileMetadata(batchRaw.Body)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/providers/batch_results_file_adapter.go` around lines 45 - 53, GetBatchResults dereferences batchRaw.Body immediately after client.DoRaw; guard against a nil batchRaw or nil Body and return a typed provider error instead of panicking. Update the code after the client.DoRaw(...) call in GetBatchResults to check that batchRaw != nil and batchRaw.Body != nil before calling parseBatchFileMetadata, and if either is nil return the same provider error shape used elsewhere in this package (match the typed provider error pattern used by GetOpenAICompatibleFileContentWithPreparer or other helper functions) so callers receive a proper error instead of a panic.internal/providers/config.go (1)
74-87:⚠️ Potential issue | 🟠 MajorReject unresolved Azure base placeholders during filtering.
applyProviderEnvVarscan update onlyAPIKeyand leave the YAMLBaseURLuntouched. WithBaseURL: "${AZURE_API_BASE}"and a real API key from env, this code sees a non-empty base and keeps the provider, so resolution succeeds but every later request uses the literal placeholder as the base URL. Treat${...}the same as missing here.Suggested fix
- if p.Type == "azure" && strings.TrimSpace(p.BaseURL) == "" { + trimmedBaseURL := strings.TrimSpace(p.BaseURL) + if p.Type == "azure" && (trimmedBaseURL == "" || strings.Contains(trimmedBaseURL, "${")) { continue }Also applies to: 113-118
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/providers/config.go` around lines 74 - 87, In applyProviderEnvVars, when deciding whether to keep or set existing.BaseURL treat unresolved placeholders like "${...}" as empty: detect if existing.BaseURL (or explicitBaseURL) matches the pattern ${...} and consider it absent so the branch that preserves a non-empty BaseURL does not keep literal placeholders; if apiKey exists and kp.defaultBase is available, set existing.BaseURL = kp.defaultBase. Apply the same placeholder-check logic to the second similar block that updates result at the later occurrence (the block around the other existing.BaseURL handling).
♻️ Duplicate comments (1)
internal/providers/azure/azure.go (1)
24-27:⚠️ Potential issue | 🟠 MajorOverride Azure
GetBatchResultsas well.This file moves Azure batch CRUD onto
resourceProvider, butGetBatchResultsis still inherited from the embeddedCompatibleProvider. That path delegates to the generic helper that calls/batches/{id}and/files/{id}/contenton the deployment-scoped client, so Azure batch result retrieval still misses the resource-level/openai/...routes. Add an Azure-specific override or parameterize the shared helper for Azure batch/file paths.Also applies to: 63-147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/providers/azure/azure.go` around lines 24 - 27, The Provider struct now uses resourceProvider for Azure batch CRUD but still inherits GetBatchResults from openai.CompatibleProvider which calls deployment-scoped endpoints; add an Azure-specific override of GetBatchResults on the Provider type (or update the shared helper to accept a base path) so it uses resourceProvider and the resource-level /openai/... routes (match how CreateBatch/CreateFile/etc. were moved). Specifically implement Provider.GetBatchResults (method name) to call the same helper used by other Azure resource-level batch/file operations but targeting resourceProvider (or pass an azureBasePath flag) to ensure /openai/batches/{id} and /openai/files/{id}/content are used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@internal/providers/batch_results_file_adapter.go`:
- Around line 45-53: GetBatchResults dereferences batchRaw.Body immediately
after client.DoRaw; guard against a nil batchRaw or nil Body and return a typed
provider error instead of panicking. Update the code after the client.DoRaw(...)
call in GetBatchResults to check that batchRaw != nil and batchRaw.Body != nil
before calling parseBatchFileMetadata, and if either is nil return the same
provider error shape used elsewhere in this package (match the typed provider
error pattern used by GetOpenAICompatibleFileContentWithPreparer or other helper
functions) so callers receive a proper error instead of a panic.
In `@internal/providers/config.go`:
- Around line 74-87: In applyProviderEnvVars, when deciding whether to keep or
set existing.BaseURL treat unresolved placeholders like "${...}" as empty:
detect if existing.BaseURL (or explicitBaseURL) matches the pattern ${...} and
consider it absent so the branch that preserves a non-empty BaseURL does not
keep literal placeholders; if apiKey exists and kp.defaultBase is available, set
existing.BaseURL = kp.defaultBase. Apply the same placeholder-check logic to the
second similar block that updates result at the later occurrence (the block
around the other existing.BaseURL handling).
---
Duplicate comments:
In `@internal/providers/azure/azure.go`:
- Around line 24-27: The Provider struct now uses resourceProvider for Azure
batch CRUD but still inherits GetBatchResults from openai.CompatibleProvider
which calls deployment-scoped endpoints; add an Azure-specific override of
GetBatchResults on the Provider type (or update the shared helper to accept a
base path) so it uses resourceProvider and the resource-level /openai/... routes
(match how CreateBatch/CreateFile/etc. were moved). Specifically implement
Provider.GetBatchResults (method name) to call the same helper used by other
Azure resource-level batch/file operations but targeting resourceProvider (or
pass an azureBasePath flag) to ensure /openai/batches/{id} and
/openai/files/{id}/content are used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 363a5bb0-494c-4e12-bcc8-7397f611da9f
📒 Files selected for processing (8)
internal/providers/azure/azure.gointernal/providers/azure/azure_test.gointernal/providers/batch_results_file_adapter.gointernal/providers/config.gointernal/providers/config_test.gointernal/providers/file_adapter_openai_compat.gointernal/providers/openai/compatible_provider.gointernal/providers/openai/openai_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Tests