-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
feat(openai): add Azure OpenAI support with configuration options #2382
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughAdds Azure OpenAI support across client settings, store, and OpenAI client initialization, and introduces a DEFAULT_AZURE_API_VERSION constant ("2024-10-21"). The settings UI includes an Azure toggle, resource URL, deployment (model) and API version inputs; the store persists Azure flags/version; the OpenAI client builds Azure deployment URLs with api-version when enabled. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant SettingsUI as Settings UI
participant Store as OpenAI Store
participant Client as OpenAI Client
User->>SettingsUI: Toggle "Use Azure OpenAI", enter Resource URL, Deployment, API Version
SettingsUI->>Store: setIsAzure / setBaseURL / setModel / setAzureApiVersion
Note over Store: Persist isAzure (false/true) and\nazureApiVersion (default "2024-10-21")
User->>SettingsUI: Trigger LLM request
SettingsUI->>Client: getClient()
alt isAzure = true
Client->>Store: read apiKey, baseURL, model, azureApiVersion
Client->>Client: construct URL: {baseURL}/openai/deployments/{model}
Client-->>SettingsUI: OpenAI client configured with baseURL and api-version
else isAzure = false
Client->>Store: read apiKey, baseURL
Client-->>SettingsUI: OpenAI client (standard/baseURL/ollama)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/client/src/pages/dashboard/settings/_sections/openai.tsx (1)
22-38: Enforce Azure-specific required fields and tighten validation.Right now Azure fields aren’t required when Azure is enabled, and maxTokens can become NaN. Add conditional rules, trim inputs, and coerce numbers.
-const formSchema = z.object({ - apiKey: z - .string() - // eslint-disable-next-line lingui/no-unlocalized-strings - .min(1, "API key cannot be empty.") - .default(""), - baseURL: z - .string() - // eslint-disable-next-line lingui/no-unlocalized-strings - .regex(/^https?:\/\/\S+$/, "That doesn't look like a valid URL") - .or(z.literal("")) - .default(""), - model: z.string().default(DEFAULT_MODEL), - maxTokens: z.number().default(DEFAULT_MAX_TOKENS), - isAzure: z.boolean().default(false), - azureApiVersion: z.string().default(DEFAULT_AZURE_API_VERSION), -}); +const formSchema = z + .object({ + apiKey: z.string().min(1, "API key cannot be empty.").default(""), + baseURL: z + .string() + .trim() + .regex(/^https?:\/\/\S+$/, "That doesn't look like a valid URL") + .or(z.literal("")) + .default(""), + model: z.string().default(DEFAULT_MODEL), + maxTokens: z.coerce.number().int().positive().default(DEFAULT_MAX_TOKENS), + isAzure: z.boolean().default(false), + azureApiVersion: z + .string() + .trim() + .regex(/^\d{4}-\d{2}-\d{2}(-preview)?$/, "Invalid Azure API version.") + .default(DEFAULT_AZURE_API_VERSION), + }) + .superRefine((values, ctx) => { + if (values.isAzure) { + if (!values.baseURL) { + ctx.addIssue({ code: z.ZodIssueCode.custom, path: ["baseURL"], message: "Resource URL is required for Azure OpenAI." }); + } + if (!values.model || values.model === DEFAULT_MODEL) { + ctx.addIssue({ code: z.ZodIssueCode.custom, path: ["model"], message: "Deployment Name is required for Azure OpenAI." }); + } + if (!values.azureApiVersion) { + ctx.addIssue({ code: z.ZodIssueCode.custom, path: ["azureApiVersion"], message: "Azure API Version is required." }); + } + } + });
🧹 Nitpick comments (12)
apps/client/src/stores/openai.ts (3)
4-4: Sort imports to satisfy lint.ESLint flagged import ordering. Run the project’s autofix to avoid churn.
pnpm lint --fix
15-18: Make azureApiVersion non-nullable to reflect the runtime requirement.Azure requires an api-version; the client throws without it. Simplify types and avoid nulls in state.
- azureApiVersion: string | null; - setAzureApiVersion: (apiVersion: string | null) => void; + azureApiVersion: string; + setAzureApiVersion: (apiVersion: string) => void; ... - azureApiVersion: DEFAULT_AZURE_API_VERSION, - setAzureApiVersion: (azureApiVersion: string | null) => { + azureApiVersion: DEFAULT_AZURE_API_VERSION, + setAzureApiVersion: (azureApiVersion: string) => { set({ azureApiVersion }); },Also applies to: 45-47
40-43: Avoid accidental misuse of “model” when switching to Azure by clearing or relabeling it.When toggling Azure on, keeping the OpenAI model (e.g., gpt-3.5-turbo) as the “deployment name” often 404s. Either clear it or set a UX hint.
- setIsAzure: (isAzure: boolean) => { - set({ isAzure }); - }, + setIsAzure: (isAzure: boolean) => { + set((state) => ({ + isAzure, + // Clear model on Azure enable to prompt entering a deployment name + model: isAzure ? null : state.model, + })); + },apps/client/src/services/openai/client.ts (1)
35-36: Harden baseURL normalization to avoid double “/openai”.Users sometimes paste the resource URL with “/openai”. Guard against ending up with
/openai/openai/deployments/....- const azureBaseURL = baseURL.endsWith('/') ? baseURL.slice(0, -1) : baseURL; + const trimmed = baseURL.replace(/\/+$/, ""); + const azureBaseURL = trimmed.endsWith("/openai") + ? trimmed.slice(0, -"/openai".length) + : trimmed;apps/client/src/pages/dashboard/settings/_sections/openai.tsx (8)
54-64: Defaults are sensible. Consider empty model when Azure is enabled.Minor: defaulting model to an empty string when Azure is on nudges users to enter their Deployment Name rather than keeping the OpenAI default.
66-81: Sanitize Base URL and scope Azure version persistence.Trim and strip trailing slashes; only persist Azure API version when Azure is enabled.
-const onSubmit = ({ apiKey, baseURL, model, maxTokens, isAzure, azureApiVersion }: FormValues) => { +const onSubmit = ({ apiKey, baseURL, model, maxTokens, isAzure, azureApiVersion }: FormValues) => { setApiKey(apiKey); setIsAzure(isAzure); - if (baseURL) { - setBaseURL(baseURL); - } + const sanitizedBaseURL = baseURL?.trim().replace(/\/+$/, ""); + if (sanitizedBaseURL) setBaseURL(sanitizedBaseURL); if (model) { setModel(model); } if (maxTokens) { setMaxTokens(maxTokens); } - if (azureApiVersion) { - setAzureApiVersion(azureApiVersion); - } + setAzureApiVersion(isAzure ? azureApiVersion.trim() : DEFAULT_AZURE_API_VERSION); };
137-141: Avoid backticks in i18n here too.Mirror the
approach for Ollama examples to prevent future lint failures.- You can also integrate with Ollama simply by setting the API key to - `sk-1234567890abcdef` and the Base URL to your Ollama URL, i.e. - `http://localhost:11434/v1`. You can also pick and choose models and set the max tokens + You can also integrate with Ollama simply by setting the API key to + <code>sk-1234567890abcdef</code> and the Base URL to your Ollama URL, i.e. + <code>http://localhost:11434/v1</code>. You can also pick and choose models and set the max tokens as per your preference.
165-183: Base URL UX: clarify placeholder for non-Azure.Make it explicit that OpenAI doesn’t require a Base URL; Ollama does.
- <Input + <Input type="text" placeholder={ form.watch("isAzure") ? "https://your-resource.openai.azure.com" - : "http://localhost:11434/v1" + : "Leave blank for OpenAI • e.g. http://localhost:11434/v1 for Ollama" } {...field} />
191-199: Deployment name placeholder when Azure is on.Helps avoid confusion with OpenAI model IDs.
- <Input type="text" placeholder={DEFAULT_MODEL} {...field} /> + <Input + type="text" + placeholder={form.watch("isAzure") ? "my-deployment" : DEFAULT_MODEL} + {...field} + />
243-259: Gate and validate Azure API version at the field level too.Keep it disabled when not Azure (done) and add an HTML pattern for quick feedback.
<Input type="text" placeholder={DEFAULT_AZURE_API_VERSION} disabled={!form.watch("isAzure")} + pattern="^\d{4}-\d{2}-\d{2}(-preview)?$" + inputMode="text" {...field} />
276-284: Generalize the storage disclosure for Azure/Ollama.This text mentions “OpenAI official SDK” only; broaden to cover Azure and Ollama paths.
- Your API key is securely stored in the browser's local storage and is only utilized when - making requests to OpenAI via their official SDK. Rest assured that your key is not - transmitted to any external server except when interacting with OpenAI's services. + Your API key is securely stored in the browser's local storage and used only when + making requests from the app to your configured provider (OpenAI, Azure OpenAI, or Ollama). + It is not transmitted to any external server other than the selected provider.
19-19: Nit: import order.Keep constant imports alphabetized if the repo enforces ordering.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between b995a6b6c0e518f7841d16ce171aee750ce31981 and 098d67cd8ce6631993bd9e51404961ebba13b3a1.
📒 Files selected for processing (4)
apps/client/src/constants/llm.ts (1 hunks)
apps/client/src/pages/dashboard/settings/_sections/openai.tsx (8 hunks)
apps/client/src/services/openai/client.ts (1 hunks)
apps/client/src/stores/openai.ts (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
apps/client/src/stores/openai.ts (1)
apps/client/src/constants/llm.ts (1)
DEFAULT_AZURE_API_VERSION (3-3)
apps/client/src/services/openai/client.ts (1)
apps/client/src/stores/openai.ts (1)
useOpenAiStore (21-51)
apps/client/src/pages/dashboard/settings/_sections/openai.tsx (3)
apps/client/src/constants/llm.ts (3)
DEFAULT_AZURE_API_VERSION (3-3)
DEFAULT_MODEL (1-1)
DEFAULT_MAX_TOKENS (2-2)
apps/client/src/stores/openai.ts (1)
useOpenAiStore (21-51)
libs/ui/src/components/checkbox.tsx (1)
Checkbox (6-22)
🪛 ESLint
apps/client/src/stores/openai.ts
[error] 1-4: Run autofix to sort these imports!
(simple-import-sort/imports)
apps/client/src/pages/dashboard/settings/_sections/openai.tsx
[error] 127-132: This string contains forbidden characters
(lingui/text-restrictions)
🪛 Biome (2.1.2)
apps/client/src/services/openai/client.ts
[error] 27-31: This code will never be reached ...
... because either this statement, ...
... this statement, ...
... or this statement will throw an exception beforehand
(lint/correctness/noUnreachable)
🔇 Additional comments (5)
apps/client/src/pages/dashboard/settings/_sections/openai.tsx (5)
42-51: Store wiring looks good.
83-98: Reset flow is thorough.
100-107: Copy looks good and clearly explains the options.
Also applies to: 286-311
22-38: I’ve located the definition of DEFAULT_AZURE_API_VERSION. Reviewing its current value against the latest GA version (2024-10-21) and latest preview (2025-04-01-preview) will confirm if it needs updating. Please share the script output.
66-81: Confirmed Azure URL construction: client.ts strips any trailing slash from baseURL, appends /openai/deployments/${model}, and passes { "api-version": azureApiVersion } via defaultQuery, resulting in {baseURL}/openai/deployments/{deployment}?api-version={azureApiVersion} as intended.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/client/src/pages/dashboard/settings/_sections/openai.tsx (2)
126-133: i18n text restriction fix confirmed; keep an eye on linter.Switch to
resolves the backtick restriction.Run the client lint to confirm no lingering lingui/text-restrictions issues:
#!/bin/bash pnpm -w -F apps/client lint
224-243: Checkbox boolean coercion: perfect.Prevents "indeterminate" from leaking into RHF state.
🧹 Nitpick comments (5)
apps/client/src/pages/dashboard/settings/_sections/openai.tsx (5)
36-38: Make azureApiVersion conditionally required and trimmed; add light format check.
Prevents empty/whitespace versions when Azure is enabled and reduces user error.
Apply this diff to strengthen the schema:
-const formSchema = z.object({
+const formSchema = z
+ .object({
apiKey: z
.string()
// eslint-disable-next-line lingui/no-unlocalized-strings
.min(1, "API key cannot be empty.")
.default(""),
baseURL: z
.string()
// eslint-disable-next-line lingui/no-unlocalized-strings
.regex(/^https?:\/\/\S+$/, "That doesn't look like a valid URL")
.or(z.literal(""))
.default(""),
model: z.string().default(DEFAULT_MODEL),
maxTokens: z.number().default(DEFAULT_MAX_TOKENS),
- isAzure: z.boolean().default(false),
- azureApiVersion: z.string().default(DEFAULT_AZURE_API_VERSION),
-});
+ isAzure: z.boolean().default(false),
+ azureApiVersion: z
+ .string()
+ .trim()
+ // eslint-disable-next-line lingui/no-unlocalized-strings
+ .regex(/^\d{4}-\d{2}-\d{2}(?:-preview)?$/, "Use format YYYY-MM-DD or YYYY-MM-DD-preview")
+ .default(DEFAULT_AZURE_API_VERSION),
+ })
+ .superRefine((vals, ctx) => {
+ if (vals.isAzure && !vals.azureApiVersion.trim()) {
+ ctx.addIssue({
+ code: z.ZodIssueCode.custom,
+ path: ["azureApiVersion"],
+ // eslint-disable-next-line lingui/no-unlocalized-strings
+ message: "Azure API Version is required when Azure is enabled.",
+ });
+ }
+ });
66-81: Persist clears and avoid truthy guards; normalize Base URL.
Currently, clearing fields (empty string) won’t update the store, and maxTokens=0 cannot be saved. Normalize and always set.
Apply this diff:
-const onSubmit = ({ apiKey, baseURL, model, maxTokens, isAzure, azureApiVersion }: FormValues) => {
- setApiKey(apiKey);
- setIsAzure(isAzure);
- if (baseURL) {
- setBaseURL(baseURL);
- }
- if (model) {
- setModel(model);
- }
- if (maxTokens) {
- setMaxTokens(maxTokens);
- }
- if (azureApiVersion) {
- setAzureApiVersion(azureApiVersion);
- }
-};
+const onSubmit = ({ apiKey, baseURL, model, maxTokens, isAzure, azureApiVersion }: FormValues) => {
+ setApiKey(apiKey.trim());
+ setIsAzure(isAzure);
+ const normalizedBase = baseURL.trim().replace(/\/+$/, "");
+ // empty => null to clearly clear persisted value
+ setBaseURL(normalizedBase || null);
+ // fallback to DEFAULT_MODEL if user submits empty
+ setModel((model ?? "").trim() || DEFAULT_MODEL);
+ // always persist numeric value; schema guards validity
+ setMaxTokens(maxTokens);
+ setAzureApiVersion(azureApiVersion.trim());
+};
165-180: Minor UX: prefer type="url" for Base/Resource URL input.
Improves native validation and mobile keyboard.
Apply this diff:
- <Input
- type="text"
+ <Input
+ type="url"
placeholder={
form.watch("isAzure")
? "https://your-resource.openai.azure.com"
: "http://localhost:11434/v1"
}
{...field}
/>
191-199: Use Azure-specific placeholder for deployment name.
Avoids confusion with model names when Azure is enabled.
Apply this diff:
<FormControl>
- <Input type="text" placeholder={DEFAULT_MODEL} {...field} />
+ <Input
+ type="text"
+ placeholder={form.watch("isAzure") ? "my-deployment" : DEFAULT_MODEL}
+ {...field}
+ />
</FormControl>
245-261: Azure API Version field gating is good; consider helper text.
Optional: add a brief hint like “e.g. 2024-10-21 or 2025-01-01-preview,” consistent with the schema suggestion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 098d67cd8ce6631993bd9e51404961ebba13b3a1 and 35c01777294f3098f6e2ec6cad41c848b980dfde.
📒 Files selected for processing (3)
apps/client/src/constants/llm.ts (1 hunks)
apps/client/src/pages/dashboard/settings/_sections/openai.tsx (8 hunks)
apps/client/src/services/openai/client.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/client/src/services/openai/client.ts
- apps/client/src/constants/llm.ts
🧰 Additional context used
🧬 Code graph analysis (1)
apps/client/src/pages/dashboard/settings/_sections/openai.tsx (3)
apps/client/src/constants/llm.ts (3)
DEFAULT_AZURE_API_VERSION (3-3)
DEFAULT_MODEL (1-1)
DEFAULT_MAX_TOKENS (2-2)
apps/client/src/stores/openai.ts (1)
useOpenAiStore (21-51)
libs/ui/src/components/checkbox.tsx (1)
Checkbox (6-22)
🪛 ESLint
apps/client/src/pages/dashboard/settings/_sections/openai.tsx
[error] 127-130: This string contains forbidden characters
(lingui/text-restrictions)
🔇 Additional comments (5)
apps/client/src/pages/dashboard/settings/_sections/openai.tsx (5)
14-15: Import of Checkbox looks good.
Matches the UI component API.
43-50: Store wiring and defaults for Azure fields look correct.
Names match the store and constants; defaults align with persisted values.
Consider confirming that persisted localStorage schema migrations are not needed (key: "openai"), since new fields have safe defaults.
Also applies to: 61-63
88-97: Reset flow is consistent.
Resets store and form state, including Azure-specific fields.
103-106: Heading and intro copy: clear and inclusive.
Covers OpenAI, Azure OpenAI, and Ollama succinctly.
19-19: DEFAULT_AZURE_API_VERSION = "2024-10-21" — current GA, no change required
Azure OpenAI docs list 2024-10-21 as the latest GA (data-plane) API version; keep DEFAULT_AZURE_API_VERSION as-is.
Fixes #2381 [Feature] Add support for Azure OpenAi
Summary by CodeRabbit
New Features
Documentation
Chores