feat(provider): add azure openai#62
Conversation
|
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 Azure OpenAI provider support across backend and frontend: JSON schema, Rust config enum, new Azure provider implementation and transforms, proxy auth/base-url handling with api-version normalization, path-segment percent-encoding helper, UI form/types/i18n, dependency, and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as User UI
participant API as Backend Proxy
participant Registry as Provider Registry
participant Gateway as AzureDef
participant Azure as Azure OpenAI API
UI->>API: submit request (provider="azure", model, payload)
API->>Registry: resolve provider "azure"
Registry->>Gateway: provide AzureDef + config
API->>Gateway: forward request metadata (model, body)
Gateway->>Gateway: transform body (remove model, inject stream_options.include_usage)
Gateway->>Gateway: encode deployment, build endpoint with api-version
Gateway->>Azure: send HTTP request with `api-key` header and transformed body
Azure-->>Gateway: respond (completion/embedding)
Gateway-->>API: normalized response
API-->>UI: return response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
ui/src/components/providers/provider-form.tsx (1)
204-229: Consider resetting Azure-specific fields when switching provider types.When a user fills in
api_versionfor Azure and then switches to another provider type (e.g., OpenAI), theapi_versionvalue remains in form state even though it's not submitted. This is harmless but could be confusing if they switch back to Azure later.♻️ Optional: Reset api_version on type change
<Select value={field.state.value} onValueChange={(next) => { setClientError(undefined); field.handleChange(next as ProviderType); + // Reset Azure-specific field when switching away + if (field.state.value === 'azure' && next !== 'azure') { + form.setFieldValue('api_version', ''); + } }} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/providers/provider-form.tsx` around lines 204 - 229, When the provider type select (form.Field name="type") changes, clear any Azure-specific form state (the api_version field) so it doesn't persist when switching away from Azure; update the onValueChange handler (where setClientError and field.handleChange are called) to detect the new ProviderType and, if it's not Azure, reset/clear the 'api_version' form field (e.g. via the form's resetField or setValue API) so the Azure-only value is removed from form state.src/gateway/providers/azure.rs (2)
190-195: Consider usingassert_matches!for enum variant assertions.The coding guidelines recommend using
assert_matches::assert_matchesfor better test output. This would simplify the match statement and provide clearer error messages on failure.♻️ Suggested refactor
+ use assert_matches::assert_matches; + #[test] fn azure_def_transforms_embeddings_request_without_model() { let provider = AzureDef; let request = serde_json::from_value(json!({ "model": "text-embedding-3-large", "input": ["hello", "world"] })) .unwrap(); let body = provider.transform_embeddings_request(&request).unwrap(); - match body { - EmbedRequestBody::Json(value) => { - assert_eq!(value["input"][0], "hello"); - assert_eq!(value.get("model"), None); - } - } + assert_matches!(body, EmbedRequestBody::Json(value) => { + assert_eq!(value["input"][0], "hello"); + assert_eq!(value.get("model"), None); + }); }As per coding guidelines: "Use pretty_assertions::assert_eq and assert_matches::assert_matches for better test output".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gateway/providers/azure.rs` around lines 190 - 195, Replace the manual match on EmbedRequestBody with assert_matches::assert_matches to get better failure output: assert that body matches EmbedRequestBody::Json(value) using assert_matches!, then perform the inner assertions using pretty_assertions::assert_eq for the two checks (value["input"][0] == "hello" and value.get("model") == None); update imports to bring assert_matches::assert_matches and pretty_assertions::assert_eq into scope and remove the explicit match block around EmbedRequestBody::Json.
21-30: Consider adding doc comments to public items.The coding guidelines specify using
///for doc comments on public items. Adding brief documentation toAzureProviderConfigandAzureDefwould improve API discoverability.📝 Suggested documentation
+/// Configuration for the Azure OpenAI provider. #[derive(Debug, Clone, Serialize, Deserialize, utoipa::ToSchema)] pub struct AzureProviderConfig { + /// The API key for authenticating with Azure OpenAI. pub api_key: String, + /// The base URL for the Azure OpenAI resource (e.g., `https://<resource>.openai.azure.com`). pub api_base: String, + /// Optional API version override. Defaults to `2024-10-21`. #[serde(skip_serializing_if = "Option::is_none")] pub api_version: Option<String>, } +/// Azure OpenAI provider definition implementing chat and embedding transformations. pub struct AzureDef;As per coding guidelines: "Use /// for doc comments on public items in Rust".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gateway/providers/azure.rs` around lines 21 - 30, Add doc comments for the public types to follow the Rust coding guidelines: add a brief /// comment above AzureProviderConfig describing that it holds configuration for the Azure provider (include a short note about api_key, api_base, and optional api_version) and add a /// comment above AzureDef describing its role (e.g., marker/type representing the Azure provider implementation). Ensure comments are concise and appear immediately before the pub struct declarations for AzureProviderConfig and AzureDef.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/gateway/providers/azure.rs`:
- Around line 190-195: Replace the manual match on EmbedRequestBody with
assert_matches::assert_matches to get better failure output: assert that body
matches EmbedRequestBody::Json(value) using assert_matches!, then perform the
inner assertions using pretty_assertions::assert_eq for the two checks
(value["input"][0] == "hello" and value.get("model") == None); update imports to
bring assert_matches::assert_matches and pretty_assertions::assert_eq into scope
and remove the explicit match block around EmbedRequestBody::Json.
- Around line 21-30: Add doc comments for the public types to follow the Rust
coding guidelines: add a brief /// comment above AzureProviderConfig describing
that it holds configuration for the Azure provider (include a short note about
api_key, api_base, and optional api_version) and add a /// comment above
AzureDef describing its role (e.g., marker/type representing the Azure provider
implementation). Ensure comments are concise and appear immediately before the
pub struct declarations for AzureProviderConfig and AzureDef.
In `@ui/src/components/providers/provider-form.tsx`:
- Around line 204-229: When the provider type select (form.Field name="type")
changes, clear any Azure-specific form state (the api_version field) so it
doesn't persist when switching away from Azure; update the onValueChange handler
(where setClientError and field.handleChange are called) to detect the new
ProviderType and, if it's not Azure, reset/clear the 'api_version' form field
(e.g. via the form's resetField or setValue API) so the Azure-only value is
removed from form state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d5940aff-df5f-47b5-b592-98a87373c143
📒 Files selected for processing (9)
src/config/entities/providers-schema.jsonsrc/config/entities/providers.rssrc/gateway/providers/azure.rssrc/gateway/providers/mod.rssrc/proxy/provider.rsui/src/components/providers/provider-form.tsxui/src/i18n/locales/en.jsonui/src/i18n/locales/zh-CN.jsonui/src/lib/api/types.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/gateway/providers/azure.rs (1)
17-19: Add///docs for exported constants.
IDENTIFIERandDEFAULT_API_VERSIONare public but undocumented.✍️ Suggested update
+/// Provider identifier used in registry/config. pub const IDENTIFIER: &str = "azure"; +/// Default Azure OpenAI REST API version used by this provider. pub const DEFAULT_API_VERSION: &str = "2024-10-21";As per coding guidelines, "Use /// for doc comments on public items in Rust".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gateway/providers/azure.rs` around lines 17 - 19, Add triple-slash documentation comments for the public constants IDENTIFIER and DEFAULT_API_VERSION in azure.rs: describe what each constant represents and any important usage notes (e.g., IDENTIFIER is the provider key used across the gateway, DEFAULT_API_VERSION is the Azure API version used by default). Place the /// comments immediately above the respective const declarations (IDENTIFIER and DEFAULT_API_VERSION) and keep DEFAULT_BASE_URL unchanged since it is private.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/gateway/providers/azure.rs`:
- Around line 46-50: The string substitution model.replace('/', "%2F") only
escapes slashes; instead percent-encode the deployment segment properly using
the url crate's percent-encoding utilities (e.g. utf8_percent_encode with an
appropriate path-segment encode set) so reserved characters like ?, #, %, and
spaces are escaped; update the chat_endpoint_path function in azure.rs to use
that percent-encoding for the model/deployment value and apply the same change
to the Bedrock provider's equivalent endpoint builder (the function around the
noted line ~81) so both providers safely encode the deployment path segment.
---
Nitpick comments:
In `@src/gateway/providers/azure.rs`:
- Around line 17-19: Add triple-slash documentation comments for the public
constants IDENTIFIER and DEFAULT_API_VERSION in azure.rs: describe what each
constant represents and any important usage notes (e.g., IDENTIFIER is the
provider key used across the gateway, DEFAULT_API_VERSION is the Azure API
version used by default). Place the /// comments immediately above the
respective const declarations (IDENTIFIER and DEFAULT_API_VERSION) and keep
DEFAULT_BASE_URL unchanged since it is private.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 37bb300b-42e7-48c9-afd5-8f3fa95d9fe2
📒 Files selected for processing (1)
src/gateway/providers/azure.rs
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/gateway/traits/provider.rs (1)
44-46: Add trace/doc annotations to the new crate-visible helper.
encode_path_segmentis crate-visible but lacks tracing and rustdoc annotations expected by this repository's Rust conventions.♻️ Suggested patch
+/// Percent-encode a dynamic URL path segment for safe URL construction. +#[fastrace::trace] pub(crate) fn encode_path_segment(segment: &str) -> String { utf8_percent_encode(segment, PATH_SEGMENT_ENCODE_SET).to_string() }As per coding guidelines, "
src/**/*.rs: Use#[fastrace::trace]decorator for distributed tracing on public functions" and "**/*.rs: Use///for doc comments on public items in Rust".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/gateway/traits/provider.rs` around lines 44 - 46, The helper function encode_path_segment is crate-visible and missing the repository's required tracing and documentation annotations; add a doc comment (///) describing what encode_path_segment does (percent-encodes a path segment using PATH_SEGMENT_ENCODE_SET) and decorate the function with the tracing attribute #[fastrace::trace] so distributed tracing is enabled for encode_path_segment; update the function signature to include these annotations (keeping the existing name and behavior) and ensure the doc comment follows Rustdoc conventions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/gateway/providers/azure.rs`:
- Around line 20-21: Add Rustdoc comments for the two exported constants by
placing /// comments directly above IDENTIFIER and DEFAULT_API_VERSION; describe
what each constant represents (e.g., IDENTIFIER is the provider string "azure"
used to identify this gateway provider, DEFAULT_API_VERSION is the Azure REST
API version used by requests) and include any usage notes or expected format.
Ensure the comments use triple-slash `///` so they appear in generated docs and
keep them concise and idiomatic for public items.
- Around line 28-35: The derived Debug for AzureProviderConfig exposes sensitive
api_key; replace the auto-derived Debug with a custom implementation that
redacts or masks api_key in any Debug output (keep
Serialize/Deserialize/ToSchema derives), i.e., remove Debug from the derive list
and implement std::fmt::Debug for AzureProviderConfig to print other fields
normally while showing a fixed redacted value (or masked form) for the api_key
field to avoid leaking credentials.
---
Nitpick comments:
In `@src/gateway/traits/provider.rs`:
- Around line 44-46: The helper function encode_path_segment is crate-visible
and missing the repository's required tracing and documentation annotations; add
a doc comment (///) describing what encode_path_segment does (percent-encodes a
path segment using PATH_SEGMENT_ENCODE_SET) and decorate the function with the
tracing attribute #[fastrace::trace] so distributed tracing is enabled for
encode_path_segment; update the function signature to include these annotations
(keeping the existing name and behavior) and ensure the doc comment follows
Rustdoc conventions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 13461d36-8927-4f87-afb4-dbf1bf2dd3c8
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
Cargo.tomlsrc/gateway/providers/azure.rssrc/gateway/providers/bedrock.rssrc/gateway/traits/provider.rs
✅ Files skipped from review due to trivial changes (1)
- Cargo.toml
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 `@src/gateway/providers/azure.rs`:
- Around line 167-172: The test hardcodes "api-version=2024-10-21" for
chat_url.query() and embed_url.query(), which will break when
DEFAULT_API_VERSION changes; update the assertions to use the
DEFAULT_API_VERSION constant as the single source of truth (e.g., compare
chat_url.query() and embed_url.query() to a string built from
DEFAULT_API_VERSION) so the test uses the same constant used by the code—locate
DEFAULT_API_VERSION and the assertions around chat_url and embed_url in azure.rs
and replace the literal query expectations with a value derived from
DEFAULT_API_VERSION.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: be9429ff-220d-4103-b500-fbc4fcb3c52b
📒 Files selected for processing (1)
src/gateway/providers/azure.rs
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/proxy/provider.rs (1)
161-176: Usepretty_assertions::assert_eqfor better test output.The test imports
pretty_assertionsusage elsewhere in the codebase. Consider usingpretty_assertions::assert_eqfor the URL comparison assertions to get more readable diffs on failure.As per coding guidelines: "
{tests,src}/**/*.rs: Use pretty_assertions::assert_eq and assert_matches::assert_matches for better test output".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/proxy/provider.rs` around lines 161 - 176, The test provider_auth_and_base_url_returns_azure_api_key_and_versioned_base_url uses std assert_eq for comparing the returned URL; replace those assertions with pretty_assertions::assert_eq to get better diff output on failure. Update the two assert_eq calls in this test (the one checking auth.api_key_for and the one checking base_url_override) to use pretty_assertions::assert_eq, ensuring pretty_assertions is imported or referenced (e.g., use pretty_assertions::assert_eq) and leave provider_auth_and_base_url and AzureProviderConfig usage unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/proxy/provider.rs`:
- Around line 161-176: The test
provider_auth_and_base_url_returns_azure_api_key_and_versioned_base_url uses std
assert_eq for comparing the returned URL; replace those assertions with
pretty_assertions::assert_eq to get better diff output on failure. Update the
two assert_eq calls in this test (the one checking auth.api_key_for and the one
checking base_url_override) to use pretty_assertions::assert_eq, ensuring
pretty_assertions is imported or referenced (e.g., use
pretty_assertions::assert_eq) and leave provider_auth_and_base_url and
AzureProviderConfig usage unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e0896c36-9e84-4964-9417-19812d754d00
📒 Files selected for processing (2)
src/gateway/providers/azure.rssrc/proxy/provider.rs
Summary by CodeRabbit
New Features
Bug Fixes
Localization