feat(provider): add oracle provider#170
Merged
SantiagoDePolonia merged 5 commits intomainfrom Mar 25, 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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds Oracle as a new OpenAI‑compatible provider across code, configuration, docs, Helm, CLI, provider discovery/factory plumbing, and tests; renames Azure env var Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI / Server
participant Factory as ProviderFactory
participant OracleProv as Oracle Provider
participant Upstream as Oracle/OpenAI-Compatible API
CLI->>Factory: Create provider (type: "oracle", cfg{APIKey, BaseURL, Models})
Factory->>OracleProv: New(providerCfg, opts{Models, Hooks, Resilience})
CLI->>OracleProv: Request ListModels / Chat / Responses
OracleProv->>Upstream: GET /v1/models or POST /v1/responses (BaseURL + Authorization)
alt Upstream returns models
Upstream-->>OracleProv: models list
OracleProv-->>CLI: filtered/merged models or responses
else Upstream fails or models unusable
OracleProv-->>CLI: return configured models (if present) or error (suggest configure providers.<name>.models)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 docstrings
🧪 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: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLAUDE.md`:
- Line 7: Update the provider environment variables section in CLAUDE.md to
include Oracle by adding ORACLE_API_KEY and ORACLE_BASE_URL to the listed
provider env vars so the docs match the new "Oracle" mention on line 7; ensure
the new entries follow the same formatting/style as the other provider variables
in that section and include a short note describing their purpose (API key and
base URL) for consistency.
In `@cmd/recordapi/main.go`:
- Around line 204-208: The code sets a provider-specific default model string
("openai.gpt-oss-120b") when *model is empty and *provider == "oracle" (see the
model/provider handling that populates reqBody["model"]); make this behavior
explicit by either (a) exposing the Oracle default as a configurable
flag/constant (e.g., oracleDefaultModel) and using that when building reqBody,
or (b) at minimum add a concise comment above the conditional explaining why
Oracle uses "openai.gpt-oss-120b" (and how it differs from the gpt-4o-mini
default for other providers) so callers understand the implicit choice. Ensure
the change references the same model/provider check that populates
reqBody["model"] in main.go.
In `@docs/adr/0001-explicit-provider-registration.md`:
- Line 5: Update the stale ADR note in
docs/adr/0001-explicit-provider-registration.md: in the "Negative consequences"
section replace the phrase "6 explicit Register calls" (or any occurrence of the
number 6 referring to provider count) with the correct count reflecting Oracle's
addition (e.g., "7 explicit Register calls") and ensure any surrounding text
still reads correctly after the numeric update.
In `@helm/README.md`:
- Line 62: Update the Helm README to document the required Oracle base URL
configuration: add an entry alongside `providers.oracle.enabled` that documents
`providers.oracle.baseUrl` and the corresponding secret/ENV name
`ORACLE_BASE_URL`, explain it's required when `providers.oracle.enabled` is
true, and show an example value and how to set it in the secrets/values
(matching existing `ORACLE_API_KEY` usage). Also add the same documentation in
the duplicate section referenced (line ~85) so both spots include
`providers.oracle.baseUrl`/`ORACLE_BASE_URL`.
In `@helm/templates/NOTES.txt`:
- Line 42: The Helm NOTES condition currently treats Oracle as enabled if
.Values.providers.oracle.enabled or .Values.providers.oracle.apiKey are set but
doesn't ensure .Values.providers.oracle.baseUrl is present; update the
if-expression that appends to $enabledProviders so it also requires a non-empty
.Values.providers.oracle.baseUrl (e.g., wrap the existing or (...) check inside
an and (...) with .Values.providers.oracle.baseUrl or a non-empty check),
ensuring $enabledProviders only gets "oracle" when baseUrl is provided.
In `@helm/values.schema.json`:
- Around line 182-189: The schema adds properties.providers.oracle but misses an
anyOf rule ensuring at least one provider is configured; add an anyOf entry for
"oracle" in the same anyOf array that validates properties.providers.oracle:
require either an object with properties.apiKey with minLength:1 (for
consistency) or, preferably, a combined check that both properties.apiKey and
properties.baseUrl have minLength:1 if Oracle must supply both—update the anyOf
array to include that new oracle validation entry referencing
properties.providers.oracle.apiKey and properties.providers.oracle.baseUrl.
In `@internal/providers/oracle/oracle_test.go`:
- Around line 147-160: Add a new unit test for normalizeConfiguredModels that
covers the all-empty/whitespace input edge case: create a test named
TestNormalizeConfiguredModels_AllEmpty which calls
normalizeConfiguredModels([]string{"", " ", ""}) and asserts the result is nil
(e.g., if got != nil then t.Fatalf). This ensures the function returns nil when
all entries are empty or whitespace.
In `@internal/providers/oracle/oracle.go`:
- Around line 61-101: In Provider.ListModels, when p.configuredModels is
non-empty and the upstream call fails (err != nil after calling
p.compat.ListModels), the error is currently ignored; update the function to
emit a debug/warn log containing the upstream error and context (e.g., indicate
fallback to configuredModels) before proceeding with the fallback. Locate
references to configuredModels, resp, err, and byID inside ListModels and add a
single log statement using the provider's logger (for example p.logger or p.log)
to include err and a concise message like "upstream ListModels failed, using
configuredModels fallback". Ensure the log is non-fatal and only recorded when
err != nil and configuredModels is non-empty.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 12bb2ac6-892e-4be5-9a68-a30881940283
📒 Files selected for processing (20)
.env.templateCLAUDE.mdGETTING_STARTED.mdREADME.mdcmd/gomodel/main.gocmd/recordapi/main.goconfig/config.example.yamlconfig/config_test.godocs/adr/0001-explicit-provider-registration.mddocs/advanced/configuration.mdxhelm/README.mdhelm/templates/NOTES.txthelm/values.schema.jsonhelm/values.yamlinternal/providers/config.gointernal/providers/config_test.gointernal/providers/factory.gointernal/providers/factory_test.gointernal/providers/oracle/oracle.gointernal/providers/oracle/oracle_test.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/recordapi/main.go`:
- Around line 202-211: The code currently aliases eConfig.requestBody into
reqBody and mutates reqBody["model"], which can mutate shared fixtures
(endpointConfigs); instead, create a shallow copy of the map before modifying it
(e.g., allocate a new map and copy key/value pairs from eConfig.requestBody) so
changes to reqBody do not affect the original; update the block that handles
eConfig.requestBody (where reqBody is assigned and model is overridden) to use
the copied map when writing reqBody["model"] and ensure subsequent use reads
from the copied map.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 662ee72a-8e81-4187-894d-75f855550cf9
📒 Files selected for processing (8)
CLAUDE.mdcmd/recordapi/main.godocs/adr/0001-explicit-provider-registration.mdhelm/README.mdhelm/templates/NOTES.txthelm/values.schema.jsoninternal/providers/oracle/oracle.gointernal/providers/oracle/oracle_test.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/oracle/oracle.go`:
- Around line 63-72: The error returned from Provider.ListModels when
p.compat.ListModels fails must be converted to a core.GatewayError; update the
error path in ListModels (the block that checks len(p.configuredModels) == 0 and
err != nil) to return a core.GatewayError with category "provider_error" (or the
appropriate provider error constant) and include the original err as the
cause/message so clients receive a typed GatewayError instead of fmt.Errorf;
locate this logic in Provider.ListModels referencing p.compat.ListModels and
p.configuredModels and replace the plain fmt.Errorf return with construction of
the core.GatewayError.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 732e5c93-f8c4-42c8-90d4-51e46489390d
📒 Files selected for processing (5)
docs/advanced/configuration.mdxdocs/docs.jsondocs/guides/oracle.mdxinternal/providers/oracle/oracle.gointernal/providers/oracle/oracle_test.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
internal/providers/oracle/oracle.go (1)
67-76:⚠️ Potential issue | 🟡 MinorReturn
core.GatewayErrorfor client-facing consistency.Per coding guidelines, all errors returned to clients must be instances of
core.GatewayError. TheListModelsmethod is called by client-facing endpoints, so this error should usecore.NewProviderErrorinstead offmt.Errorf.🛠️ Suggested fix
func (p *Provider) ListModels(ctx context.Context) (*core.ModelsResponse, error) { resp, err := p.compat.ListModels(ctx) if len(p.configuredModels) == 0 { if err != nil { - return nil, fmt.Errorf( - "oracle ListModels failed: %w; configure providers.<name>.models to use Oracle when upstream /models is unavailable", - err, + return nil, core.NewProviderError( + "oracle", + http.StatusBadGateway, + "ListModels failed; configure providers.<name>.models to use Oracle when upstream /models is unavailable", + err, ) } return resp, nil }As per coding guidelines: "All errors returned to clients must be instances of
core.GatewayError" and "Use typed client-facing error categories:provider_error..."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/providers/oracle/oracle.go` around lines 67 - 76, Replace the raw fmt.Errorf error in Provider.ListModels with a core.GatewayError using core.NewProviderError so clients receive a typed provider_error; specifically, in the ListModels method on type Provider (the block that currently does `if err != nil { return nil, fmt.Errorf(...) }`) construct and return core.NewProviderError("oracle", fmt.Errorf(...)) or equivalent per project helpers, keeping the original error message text/context and wrapping the underlying err so the returned error implements core.GatewayError.
🤖 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/resolved_config.go`:
- Around line 6-11: ResolveBaseURL trims baseURL to check for emptiness but
returns the original untrimmed baseURL; change the function to return the
trimmed value (use strings.TrimSpace(baseURL)) when non-empty so callers get a
normalized URL instead of one with leading/trailing whitespace, keeping fallback
behavior unchanged.
- Around line 14-19: ResolveAPIVersion currently checks for empty apiVersion
using TrimSpace but returns the original apiVersion which may contain
surrounding whitespace; update ResolveAPIVersion to trim apiVersion once (e.g.,
trimmed := strings.TrimSpace(apiVersion)) and if trimmed == "" return fallback
else return trimmed so callers consistently receive a cleaned API version string
(refer to function ResolveAPIVersion and its parameters apiVersion and
fallback).
---
Duplicate comments:
In `@internal/providers/oracle/oracle.go`:
- Around line 67-76: Replace the raw fmt.Errorf error in Provider.ListModels
with a core.GatewayError using core.NewProviderError so clients receive a typed
provider_error; specifically, in the ListModels method on type Provider (the
block that currently does `if err != nil { return nil, fmt.Errorf(...) }`)
construct and return core.NewProviderError("oracle", fmt.Errorf(...)) or
equivalent per project helpers, keeping the original error message text/context
and wrapping the underlying err so the returned error implements
core.GatewayError.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: be2d0289-23e6-4e84-8cdf-27afca2ed16f
📒 Files selected for processing (31)
.env.templateGETTING_STARTED.mdREADME.mdconfig/config.example.yamlconfig/config_test.godocs/adr/0001-explicit-provider-registration.mddocs/advanced/configuration.mdxdocs/getting-started/quickstart.mdxinternal/providers/anthropic/anthropic.gointernal/providers/anthropic/anthropic_test.gointernal/providers/azure/azure.gointernal/providers/config.gointernal/providers/config_test.gointernal/providers/factory.gointernal/providers/factory_test.gointernal/providers/gemini/gemini.gointernal/providers/gemini/gemini_test.gointernal/providers/groq/groq.gointernal/providers/groq/groq_test.gointernal/providers/init.gointernal/providers/ollama/ollama.gointernal/providers/ollama/ollama_test.gointernal/providers/openai/compatible_provider.gointernal/providers/openai/openai.gointernal/providers/openai/openai_test.gointernal/providers/openrouter/openrouter.gointernal/providers/oracle/oracle.gointernal/providers/resolved_config.gointernal/providers/xai/xai.gointernal/providers/xai/xai_test.gotests/integration/setup_test.go
Comment on lines
+6
to
+11
| func ResolveBaseURL(baseURL, fallback string) string { | ||
| if strings.TrimSpace(baseURL) == "" { | ||
| return fallback | ||
| } | ||
| return baseURL | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider returning the trimmed value for consistency.
ResolveBaseURL trims the input to check for emptiness but returns the original untrimmed value. If the input contains leading/trailing whitespace (e.g., " https://api.example.com "), the returned URL will include that whitespace, which could cause issues with HTTP clients.
♻️ Suggested fix to return trimmed value
func ResolveBaseURL(baseURL, fallback string) string {
- if strings.TrimSpace(baseURL) == "" {
+ trimmed := strings.TrimSpace(baseURL)
+ if trimmed == "" {
return fallback
}
- return baseURL
+ return trimmed
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func ResolveBaseURL(baseURL, fallback string) string { | |
| if strings.TrimSpace(baseURL) == "" { | |
| return fallback | |
| } | |
| return baseURL | |
| } | |
| func ResolveBaseURL(baseURL, fallback string) string { | |
| trimmed := strings.TrimSpace(baseURL) | |
| if trimmed == "" { | |
| return fallback | |
| } | |
| return trimmed | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/providers/resolved_config.go` around lines 6 - 11, ResolveBaseURL
trims baseURL to check for emptiness but returns the original untrimmed baseURL;
change the function to return the trimmed value (use strings.TrimSpace(baseURL))
when non-empty so callers get a normalized URL instead of one with
leading/trailing whitespace, keeping fallback behavior unchanged.
Comment on lines
+14
to
+19
| func ResolveAPIVersion(apiVersion, fallback string) string { | ||
| if strings.TrimSpace(apiVersion) == "" { | ||
| return fallback | ||
| } | ||
| return apiVersion | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Same consideration applies to ResolveAPIVersion.
For consistency and robustness, consider returning the trimmed value here as well.
♻️ Suggested fix
func ResolveAPIVersion(apiVersion, fallback string) string {
- if strings.TrimSpace(apiVersion) == "" {
+ trimmed := strings.TrimSpace(apiVersion)
+ if trimmed == "" {
return fallback
}
- return apiVersion
+ return trimmed
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func ResolveAPIVersion(apiVersion, fallback string) string { | |
| if strings.TrimSpace(apiVersion) == "" { | |
| return fallback | |
| } | |
| return apiVersion | |
| } | |
| func ResolveAPIVersion(apiVersion, fallback string) string { | |
| trimmed := strings.TrimSpace(apiVersion) | |
| if trimmed == "" { | |
| return fallback | |
| } | |
| return trimmed | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/providers/resolved_config.go` around lines 14 - 19,
ResolveAPIVersion currently checks for empty apiVersion using TrimSpace but
returns the original apiVersion which may contain surrounding whitespace; update
ResolveAPIVersion to trim apiVersion once (e.g., trimmed :=
strings.TrimSpace(apiVersion)) and if trimmed == "" return fallback else return
trimmed so callers consistently receive a cleaned API version string (refer to
function ResolveAPIVersion and its parameters apiVersion and fallback).
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
ORACLE_API_KEYandORACLE_BASE_URL, and support configuredmodels:as a fallback inventory when/modelsis unavailableScope
Verification
go test ./cmd/... ./internal/providers/... ./config/...Summary by CodeRabbit
New Features
Behavior
Documentation