-
-
Notifications
You must be signed in to change notification settings - Fork 116
feat(ai-openai): allow passing all open ai configuration options #245
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
feat(ai-openai): allow passing all open ai configuration options #245
Conversation
📝 WalkthroughWalkthroughModel name unions moved into Changes
Sequence Diagram(s)(omitted — changes are type/import refactors without new multi-component control flow) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 2
🤖 Fix all issues with AI agents
In @.changeset/olive-wolves-love.md:
- Line 5: Update the changeset copy that currently reads "allows additional
configuration options when creating an openAI client" to use the correct brand
casing "OpenAI" (i.e., change "openAI" → "OpenAI"), and scan the same .changeset
entry for any other occurrences of "openAI" to correct them for consistency.
In `@packages/typescript/ai-openai/src/utils/client.ts`:
- Around line 2-6: OpenAIClientConfig currently forces apiKey as required which
conflicts with openai's ClientOptions where apiKey is optional; change
OpenAIClientConfig to make apiKey optional (apiKey?: string) so it extends
ClientOptions' fallback behavior, or alternatively document why a required
apiKey is intentional—update the OpenAIClientConfig interface declaration
(symbol: OpenAIClientConfig) and ensure it aligns with ClientOptions from the
openai SDK (symbol: ClientOptions) so callers can rely on
process.env.OPENAI_API_KEY when not supplying a key.
| '@tanstack/ai-openai': minor | ||
| --- | ||
|
|
||
| allows additional configuration options when creating an openAI client |
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.
Capitalize “OpenAI” in the changeset text.
Minor copy edit for brand casing.
✏️ Suggested fix
-allows additional configuration options when creating an openAI client
+allows additional configuration options when creating an OpenAI client📝 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.
| allows additional configuration options when creating an openAI client | |
| allows additional configuration options when creating an OpenAI client |
🤖 Prompt for AI Agents
In @.changeset/olive-wolves-love.md at line 5, Update the changeset copy that
currently reads "allows additional configuration options when creating an openAI
client" to use the correct brand casing "OpenAI" (i.e., change "openAI" →
"OpenAI"), and scan the same .changeset entry for any other occurrences of
"openAI" to correct them for consistency.
| import type { ClientOptions } from 'openai'; | ||
|
|
||
| export interface OpenAIClientConfig { | ||
| apiKey: string | ||
| organization?: string | ||
| baseURL?: string | ||
| export interface OpenAIClientConfig extends ClientOptions { | ||
| apiKey: string; | ||
| } |
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.
🧩 Analysis chain
🏁 Script executed:
find . -path "*/packages/typescript/ai-openai/package.json" -o -path "*/packages/typescript/ai-openai/**/package.json" | head -5Repository: TanStack/ai
Length of output: 155
🏁 Script executed:
cat packages/typescript/ai-openai/package.json | grep -A 5 -B 5 "openai"Repository: TanStack/ai
Length of output: 743
🌐 Web query:
OpenAI JavaScript SDK ClientOptions interface apiKey required github.com/openai/openai-node
💡 Result:
In openai/openai-node, ClientOptions.apiKey is not required in the TypeScript type—it’s declared as optional (apiKey?: string). [1]
However, the SDK still needs an API key at runtime: if you don’t pass apiKey, it will use process.env.OPENAI_API_KEY (so you can omit apiKey only if that env var is set). [2]
Sources: [1] [2]
Remove apiKey requirement from OpenAIClientConfig to match SDK flexibility.
The OpenAI SDK v6.9.1+ declares ClientOptions.apiKey as optional (apiKey?: string), allowing the SDK to fall back to process.env.OPENAI_API_KEY if the key is not explicitly passed. Making apiKey: string required in OpenAIClientConfig adds an unnecessary constraint and prevents developers from leveraging the SDK's environment variable fallback. Either make apiKey optional in the interface or document why this stricter requirement is intentional for your use case.
🤖 Prompt for AI Agents
In `@packages/typescript/ai-openai/src/utils/client.ts` around lines 2 - 6,
OpenAIClientConfig currently forces apiKey as required which conflicts with
openai's ClientOptions where apiKey is optional; change OpenAIClientConfig to
make apiKey optional (apiKey?: string) so it extends ClientOptions' fallback
behavior, or alternatively document why a required apiKey is intentional—update
the OpenAIClientConfig interface declaration (symbol: OpenAIClientConfig) and
ensure it aligns with ClientOptions from the openai SDK (symbol: ClientOptions)
so callers can rely on process.env.OPENAI_API_KEY when not supplying a key.
|
View your CI Pipeline Execution ↗ for commit b9d43a0
☁️ Nx Cloud last updated this comment at |
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: 1
🤖 Fix all issues with AI agents
In `@packages/typescript/ai-openai/src/adapters/summarize.ts`:
- Around line 4-10: You removed the backwards-compatible type alias
OpenAISummarizeModel by constraining generics to OpenAIChatModel; restore a
compatibility export by reintroducing a deprecated alias such as "export type
OpenAISummarizeModel = OpenAIChatModel" (add a JSDoc `@deprecated` note) near the
imports or the top of summarize.ts and replicate the same alias in the other
modules where the alias was removed so existing consumer imports keep working
(look for places referencing OpenAIChatModel generics around the summarize
adapter and the other spots mentioned).
AlemTuzlak
left a 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.
Thank you for this!
@tanstack/ai
@tanstack/ai-anthropic
@tanstack/ai-client
@tanstack/ai-devtools-core
@tanstack/ai-gemini
@tanstack/ai-grok
@tanstack/ai-ollama
@tanstack/ai-openai
@tanstack/ai-openrouter
@tanstack/ai-preact
@tanstack/ai-react
@tanstack/ai-react-ui
@tanstack/ai-solid
@tanstack/ai-solid-ui
@tanstack/ai-svelte
@tanstack/ai-vue
@tanstack/ai-vue-ui
@tanstack/preact-ai-devtools
@tanstack/react-ai-devtools
@tanstack/solid-ai-devtools
commit: |
🎯 Changes
Allows passing in properties into openAI configuration like dangerouslyAllowBrowser.
Resolved the discussion here #197
Note: may want to consider if we want to expose all options all at once or limit to a smaller subset
✅ Checklist
--> contribution guidelines are no longer part of the repo?
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
New Features
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.