fix: openAi model names and structure parsing #188
Conversation
📝 WalkthroughWalkthroughThis PR renames OpenAI chat model public names, exports OPENAI_CHAT_MODELS, extends OpenAI structured-output schema conversion to handle unions (anyOf) with explicit errors for unsupported oneOf, adds unit tests for the converter, relaxes the JSONSchema index signature for assignability, removes "TanStack Start Integration" sections from multiple READMEs, and adds changeset files for patch bumps. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
packages/typescript/*/src/index.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (4)📚 Learning: 2025-12-27T20:22:51.232ZApplied to files:
📚 Learning: 2025-12-13T17:09:09.794ZApplied to files:
📚 Learning: 2025-12-13T17:09:09.794ZApplied to files:
📚 Learning: 2025-12-27T21:39:29.563ZApplied to files:
🔇 Additional comments (1)
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 |
|
View your CI Pipeline Execution ↗ for commit f9239fb
☁️ Nx Cloud last updated this comment at |
@tanstack/ai
@tanstack/ai-anthropic
@tanstack/ai-client
@tanstack/ai-devtools-core
@tanstack/ai-gemini
@tanstack/ai-ollama
@tanstack/ai-openai
@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/react-ai-devtools
@tanstack/solid-ai-devtools
commit: |
| prop.items.required || [], | ||
| ), | ||
| } | ||
| } else if (prop.anyOf || prop.oneOf) { |
There was a problem hiding this comment.
Just to notice, oneOf is not part of the open ai supported types
There was a problem hiding this comment.
ah you're right, thank you! Wanted to cover both while I was at it 😓
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.changeset/full-stars-drop.mdpackages/typescript/ai-openai/src/model-meta.tspackages/typescript/ai-openai/src/utils/schema-converter.tspackages/typescript/ai-openai/tests/schema-converter.test.tspackages/typescript/ai/src/types.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use tree-shakeable adapter architecture for provider implementations - export specialized adapters (text, embedding, summarize, image) as separate imports from/adapterssubpath rather than monolithic adapters
Use Zod for runtime schema validation and type inference, particularly for tool input/output definitions withtoolDefinition()and Zod schema inference
Implement isomorphic tool system usingtoolDefinition()with.server()and.client()implementations for dual-environment execution
Use type-safe per-model configuration with provider options typed based on selected model to ensure compile-time safety
Implement stream processing with StreamProcessor for handling chunked responses and support partial JSON parsing for streaming AI responses
Files:
packages/typescript/ai/src/types.tspackages/typescript/ai-openai/src/model-meta.tspackages/typescript/ai-openai/src/utils/schema-converter.tspackages/typescript/ai-openai/tests/schema-converter.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for function and variable names throughout the codebase
Files:
packages/typescript/ai/src/types.tspackages/typescript/ai-openai/src/model-meta.tspackages/typescript/ai-openai/src/utils/schema-converter.tspackages/typescript/ai-openai/tests/schema-converter.test.ts
packages/typescript/*/src/model-meta.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain model metadata files that define provider options and capabilities per model for per-model type safety
Files:
packages/typescript/ai-openai/src/model-meta.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Write unit tests using Vitest alongside source files with
.test.tsnaming convention
Files:
packages/typescript/ai-openai/tests/schema-converter.test.ts
🧠 Learnings (3)
📚 Learning: 2025-12-13T17:09:09.794Z
Learnt from: CR
Repo: TanStack/ai PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T17:09:09.794Z
Learning: Applies to packages/typescript/*/src/index.ts : Export tree-shakeable adapters with clear subpath exports in package.json (e.g., `tanstack/ai/adapters`, `tanstack/ai-openai/adapters`) to minimize bundle size
Applied to files:
packages/typescript/ai/src/types.ts
📚 Learning: 2025-12-13T17:09:09.794Z
Learnt from: CR
Repo: TanStack/ai PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T17:09:09.794Z
Learning: Applies to **/*.{ts,tsx} : Use Zod for runtime schema validation and type inference, particularly for tool input/output definitions with `toolDefinition()` and Zod schema inference
Applied to files:
packages/typescript/ai/src/types.tspackages/typescript/ai-openai/src/utils/schema-converter.ts
📚 Learning: 2025-12-13T17:09:09.794Z
Learnt from: CR
Repo: TanStack/ai PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T17:09:09.794Z
Learning: Applies to packages/typescript/*/src/model-meta.ts : Maintain model metadata files that define provider options and capabilities per model for per-model type safety
Applied to files:
packages/typescript/ai/src/types.tspackages/typescript/ai-openai/src/model-meta.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Preview
🔇 Additional comments (7)
packages/typescript/ai/src/types.ts (1)
63-63: LGTM: Index signature change improves type compatibility.Changing from
unknowntoanyresolves type assignment issues when using custom JSON Schema definitions withtoolDefinition(), as noted in issue #178. While this reduces type safety, it's appropriate for JSON Schema's dynamic nature and aligns with the extensibility goal noted in the comment.packages/typescript/ai-openai/src/utils/schema-converter.ts (3)
64-83: LGTM: Property-level union handling correctly added.The new logic recursively transforms
anyOfandoneOfproperties, ensuring each union variant receives proper OpenAI structured output compatibility (includingadditionalProperties: false). This addresses issue #186.
115-120: LGTM: Schema-level anyOf handling implemented correctly.Each variant in
anyOfunions is recursively transformed with its ownrequiredarray, ensuring OpenAI compatibility for union schemas.
122-127: LGTM: Schema-level oneOf handling mirrors anyOf approach.The
oneOfhandling correctly applies the same recursive transformation pattern asanyOf, ensuring discriminated union schemas are compatible with OpenAI's requirements.packages/typescript/ai-openai/tests/schema-converter.test.ts (2)
7-29: LGTM: Comprehensive tests for transformNullsToUndefined.The test suite covers null-to-undefined conversion, nested object pruning, array handling, and value preservation. All test cases are well-structured and validate the expected behavior.
31-183: LGTM: Thorough test coverage for OpenAI schema transformation.The test suite validates all key behaviors:
additionalProperties: falseenforcement- Required property augmentation
- Nullable optional fields
- Union type handling (anyOf/oneOf) with recursive transformation
- Nested structures within unions
- Array item schemas
All tests are well-structured and align with the implementation changes for structured output union support.
packages/typescript/ai-openai/src/model-meta.ts (1)
136-136: Verify updated model names match OpenAI's API.The model name updates to
gpt-5.2-chat-latest,gpt-5.1-chat-latest, andchatgpt-4o-latestalign with valid OpenAI API identifiers as of December 2025. However, verify that any hardcoded references to the old model names in examples, tests, or documentation are also updated.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/typescript/ai-openai/tests/schema-converter.test.ts (2)
7-29: Consider adding edge case tests for deeply nested structures.The current test coverage is solid for the core functionality. However, consider adding tests for:
- Deeply nested structures (3+ levels of nesting)
- Empty objects and arrays
- Mixed arrays containing objects, arrays, and primitives
These additions would strengthen confidence in the recursive transformation logic.
💡 Example additional test cases
+ it('should handle deeply nested structures', () => { + const input = { + level1: { + level2: { + level3: { value: null, keep: 'this' } + } + } + } + const result = transformNullsToUndefined(input) + expect(result).toEqual({ level1: { level2: { level3: { keep: 'this' } } } }) + }) + + it('should handle empty objects and arrays', () => { + expect(transformNullsToUndefined({})).toEqual({}) + expect(transformNullsToUndefined([])).toEqual([]) + })
31-154: Add test coverage for root-level anyOf and additional type scenarios.The implementation (per relevant code snippets) handles root-level anyOf schemas where the schema itself contains an anyOf array, not just anyOf in properties. This important case is not tested.
Additionally, the optional-fields-nullable test (lines 59-72) only covers string types. Other primitive types and edge cases should be verified.
🔎 Suggested additional test cases
it('should handle root-level anyOf schemas', () => { const schema = { anyOf: [ { type: 'object', properties: { a: { type: 'string' } }, required: ['a'], }, { type: 'object', properties: { b: { type: 'number' } }, required: ['b'], }, ], } const result = makeOpenAIStructuredOutputCompatible(schema) // Each root-level anyOf variant should have additionalProperties: false expect(result.anyOf[0].additionalProperties).toBe(false) expect(result.anyOf[1].additionalProperties).toBe(false) expect(result.anyOf).toHaveLength(2) }) it('should handle nullable optional fields of different types', () => { const schema = { type: 'object', properties: { optString: { type: 'string' }, optNumber: { type: 'number' }, optBoolean: { type: 'boolean' }, optArray: { type: 'array', items: { type: 'string' } }, }, required: [], } const result = makeOpenAIStructuredOutputCompatible(schema, []) expect(result.properties.optString.type).toEqual(['string', 'null']) expect(result.properties.optNumber.type).toEqual(['number', 'null']) expect(result.properties.optBoolean.type).toEqual(['boolean', 'null']) expect(result.properties.optArray.type).toEqual(['array', 'null']) })Based on relevant code snippets showing that
makeOpenAIStructuredOutputCompatiblehandles root-level anyOf schemas.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.changeset/mighty-gifts-enter.mdREADME.mdpackages/typescript/ai-client/README.mdpackages/typescript/ai-devtools/README.mdpackages/typescript/ai-gemini/README.mdpackages/typescript/ai-ollama/README.mdpackages/typescript/ai-openai/README.mdpackages/typescript/ai-openai/src/utils/schema-converter.tspackages/typescript/ai-openai/tests/schema-converter.test.tspackages/typescript/ai-react-ui/README.mdpackages/typescript/ai-react/README.mdpackages/typescript/ai/README.mdpackages/typescript/react-ai-devtools/README.mdpackages/typescript/solid-ai-devtools/README.md
💤 Files with no reviewable changes (11)
- packages/typescript/ai-client/README.md
- README.md
- packages/typescript/solid-ai-devtools/README.md
- packages/typescript/ai-openai/README.md
- packages/typescript/ai-react/README.md
- packages/typescript/ai-gemini/README.md
- packages/typescript/ai-react-ui/README.md
- packages/typescript/react-ai-devtools/README.md
- packages/typescript/ai-ollama/README.md
- packages/typescript/ai-devtools/README.md
- packages/typescript/ai/README.md
✅ Files skipped from review due to trivial changes (1)
- .changeset/mighty-gifts-enter.md
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/typescript/ai-openai/src/utils/schema-converter.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use tree-shakeable adapter architecture for provider implementations - export specialized adapters (text, embedding, summarize, image) as separate imports from/adapterssubpath rather than monolithic adapters
Use Zod for runtime schema validation and type inference, particularly for tool input/output definitions withtoolDefinition()and Zod schema inference
Implement isomorphic tool system usingtoolDefinition()with.server()and.client()implementations for dual-environment execution
Use type-safe per-model configuration with provider options typed based on selected model to ensure compile-time safety
Implement stream processing with StreamProcessor for handling chunked responses and support partial JSON parsing for streaming AI responses
Files:
packages/typescript/ai-openai/tests/schema-converter.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Write unit tests using Vitest alongside source files with
.test.tsnaming convention
Files:
packages/typescript/ai-openai/tests/schema-converter.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for function and variable names throughout the codebase
Files:
packages/typescript/ai-openai/tests/schema-converter.test.ts
🧠 Learnings (1)
📚 Learning: 2025-12-13T17:09:09.794Z
Learnt from: CR
Repo: TanStack/ai PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T17:09:09.794Z
Learning: Applies to **/*.{ts,tsx} : Use Zod for runtime schema validation and type inference, particularly for tool input/output definitions with `toolDefinition()` and Zod schema inference
Applied to files:
packages/typescript/ai-openai/tests/schema-converter.test.ts
🧬 Code graph analysis (1)
packages/typescript/ai-openai/tests/schema-converter.test.ts (1)
packages/typescript/ai-openai/src/utils/schema-converter.ts (2)
transformNullsToUndefined(12-35)makeOpenAIStructuredOutputCompatible(48-123)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Preview
🔇 Additional comments (2)
packages/typescript/ai-openai/tests/schema-converter.test.ts (2)
1-5: LGTM! Clean imports and proper test setup.The imports follow the coding guidelines (Vitest for unit tests with
.test.tsnaming) and correctly reference the source functions.
103-133: Good coverage of nested object transformation.This test effectively verifies the recursive transformation of nested objects within anyOf variants. The assertions check both the variant level and the nested object level, which is excellent.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…thub.com/TanStack/ai into 173-gpt-5-chat-model-names-are-incorrect
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/typescript/ai-openai/tests/schema-converter.test.ts (2)
74-140: LGTM! anyOf handling is thoroughly tested.The anyOf tests comprehensively verify:
- Recursive transformation of union variants
additionalProperties: falseon each variant- Preservation of required fields through transformation
- Deep nesting within union types
Consider adding an assertion for the nested object's required array in the second test for complete verification:
Optional enhancement
const result = makeOpenAIStructuredOutputCompatible(schema, ['data']) // The nested object inside anyOf variant should also have additionalProperties: false expect(result.properties.data.anyOf[0].additionalProperties).toBe(false) expect( result.properties.data.anyOf[0].properties.nested.additionalProperties, ).toBe(false) + expect( + result.properties.data.anyOf[0].properties.nested.required, + ).toEqual(['x'])
31-187: Consider adding tests for additional edge cases.The current test suite covers the main functionality well. For more comprehensive coverage, consider adding tests for:
- Root-level unions: anyOf/oneOf at the schema root rather than within properties
- Type arrays: Fields with
type: ['string', 'number']or types that already include'null'- Optional fields within anyOf variants: Verify nullable transformation applies recursively
Example tests for edge cases
it('should handle anyOf at root level', () => { const schema = { anyOf: [ { type: 'object', properties: { a: { type: 'string' } }, required: ['a'], }, { type: 'object', properties: { b: { type: 'number' } }, required: ['b'], }, ], } const result = makeOpenAIStructuredOutputCompatible(schema, []) expect(result.anyOf[0].additionalProperties).toBe(false) expect(result.anyOf[1].additionalProperties).toBe(false) }) it('should handle fields with existing type arrays', () => { const schema = { type: 'object', properties: { multiType: { type: ['string', 'number'] }, alreadyNullable: { type: ['string', 'null'] }, }, required: [], } const result = makeOpenAIStructuredOutputCompatible(schema, []) expect(result.properties.multiType.type).toEqual(['string', 'number', 'null']) expect(result.properties.alreadyNullable.type).toEqual(['string', 'null']) })
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/typescript/ai-openai/src/utils/schema-converter.tspackages/typescript/ai-openai/tests/schema-converter.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/typescript/ai-openai/src/utils/schema-converter.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use tree-shakeable adapter architecture for provider implementations - export specialized adapters (text, embedding, summarize, image) as separate imports from/adapterssubpath rather than monolithic adapters
Use Zod for runtime schema validation and type inference, particularly for tool input/output definitions withtoolDefinition()and Zod schema inference
Implement isomorphic tool system usingtoolDefinition()with.server()and.client()implementations for dual-environment execution
Use type-safe per-model configuration with provider options typed based on selected model to ensure compile-time safety
Implement stream processing with StreamProcessor for handling chunked responses and support partial JSON parsing for streaming AI responses
Files:
packages/typescript/ai-openai/tests/schema-converter.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Write unit tests using Vitest alongside source files with
.test.tsnaming convention
Files:
packages/typescript/ai-openai/tests/schema-converter.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for function and variable names throughout the codebase
Files:
packages/typescript/ai-openai/tests/schema-converter.test.ts
🧠 Learnings (1)
📚 Learning: 2025-12-13T17:09:09.794Z
Learnt from: CR
Repo: TanStack/ai PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T17:09:09.794Z
Learning: Applies to **/*.{ts,tsx} : Use Zod for runtime schema validation and type inference, particularly for tool input/output definitions with `toolDefinition()` and Zod schema inference
Applied to files:
packages/typescript/ai-openai/tests/schema-converter.test.ts
🧬 Code graph analysis (1)
packages/typescript/ai-openai/tests/schema-converter.test.ts (1)
packages/typescript/ai-openai/src/utils/schema-converter.ts (2)
transformNullsToUndefined(12-35)makeOpenAIStructuredOutputCompatible(48-134)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Preview
- GitHub Check: Test
🔇 Additional comments (3)
packages/typescript/ai-openai/tests/schema-converter.test.ts (3)
7-29: LGTM! Comprehensive coverage of null transformation.The test suite thoroughly covers the key behaviors of
transformNullsToUndefined:
- Primitive null conversion to undefined
- Field pruning for null values in objects (absent rather than undefined)
- Array element handling (null becomes undefined in place)
- Preservation of non-null values
31-72: LGTM! Core transformation behaviors are well tested.These tests effectively verify the fundamental OpenAI schema compatibility transformations:
- Enforcement of
additionalProperties: false- All properties made required (previously optional fields added to required array)
- Optional fields made nullable via union types
142-187: LGTM! Array and oneOf restriction tests are correct.These tests properly verify:
- Recursive transformation of array item schemas
- Explicit error on oneOf usage (unsupported by OpenAI)
The error message assertion is sufficient to identify the correct error case.
|
would it be possible to tack on exporting the chat model names in the main index file? |
|
@stephansama mind opening up an issue? Im off this week and I'll forget this by the time I'm back 😅 |
1 similar comment
|
@stephansama mind opening up an issue? Im off this week and I'll forget this by the time I'm back 😅 |
Of course! I can also make the changes if that is permissible or just create the issue either or both |
|
@stephansama added the export |
harry-whorlow
changed the title
🎯 Changes
Fixes #173
Fixes #178
Fixes #186
Fixes #187
Fixes #170
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.