fix: patch openrouter snake_case options to map to camelCase#316
Open
tombeckenham wants to merge 3 commits intoTanStack:mainfrom
Open
fix: patch openrouter snake_case options to map to camelCase#316tombeckenham wants to merge 3 commits intoTanStack:mainfrom
tombeckenham wants to merge 3 commits intoTanStack:mainfrom
Conversation
Contributor
📝 WalkthroughWalkthroughIntroduces a utility function to convert snake_case keys to camelCase in model options and applies this transformation in the OpenRouter text adapter to ensure proper serialization before passing to the SDK. Updates tests to validate the conversion and wire-format serialization. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
|
View your CI Pipeline Execution ↗ for commit 4a09d2f
☁️ Nx Cloud last updated this comment at |
@tanstack/ai
@tanstack/ai-anthropic
@tanstack/ai-client
@tanstack/ai-devtools-core
@tanstack/ai-fal
@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: |
2 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/typescript/ai-openrouter/src/utils/casing.ts (1)
5-11: Preserve typing insnakeToCamelOptionsreturn value.Returning
Record<string, unknown>weakens compile-time safety for provider option mapping. A generic mapped return type keeps this utility type-safe where it is spread into SDK params.🧩 Suggested typed version
+type SnakeToCamelCase<S extends string> = + S extends `${infer Head}_${infer Tail}` + ? `${Head}${Capitalize<SnakeToCamelCase<Tail>>}` + : S + -export const snakeToCamelOptions = (obj: Record<string, unknown>) => { +export const snakeToCamelOptions = <T extends Record<string, unknown>>( + obj: T, +) => { return Object.fromEntries( Object.entries(obj).map(([key, value]) => { return [snakeToCamel(key), value] }), - ) + ) as { + [K in keyof T as K extends string ? SnakeToCamelCase<K> : K]: T[K] + } }As per coding guidelines, "Use type-safe per-model configuration with provider options typed based on selected model to ensure compile-time safety".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/typescript/ai-openrouter/src/utils/casing.ts` around lines 5 - 11, snakeToCamelOptions currently loses type information by returning a generic Record<string, unknown>; change it to a generic function so the input type is preserved and the output keys are the camel-cased version of the input keys while values keep their original types. Update snakeToCamelOptions to accept a generic type parameter (e.g., T extends Record<string, unknown>) and return a mapped type that remaps each key K in keyof T to the camel-cased key (using the existing snakeToCamel function for the transformation conceptually) with the same value type T[K], so callers retain compile-time safety when spreading into SDK params. Ensure typings and exports are updated accordingly without changing runtime behavior.packages/typescript/ai-openrouter/src/adapters/text.ts (1)
525-534: Excludevariantfrom spread model options.
variantis already baked intomodelon Line 527-528. Spreading all mappedmodelOptionson Line 533 forwardsvariantagain as a separate param, which is redundant and can become ambiguous later.♻️ Suggested refinement
- const modelOptions = options.modelOptions as + const modelOptions = options.modelOptions as | Omit<InternalTextProviderOptions, 'model' | 'messages' | 'tools'> | undefined + const { variant, ...providerOptions } = modelOptions ?? {} @@ - model: - options.model + - (modelOptions?.variant ? `:${modelOptions.variant}` : ''), + model: options.model + (variant ? `:${variant}` : ''), @@ - ...snakeToCamelOptions(modelOptions ?? {}), + ...snakeToCamelOptions(providerOptions),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/typescript/ai-openrouter/src/adapters/text.ts` around lines 525 - 534, The ChatGenerationParams construction currently appends modelOptions?.variant into the model string and then spreads snakeToCamelOptions(modelOptions) (in the request object building code), which re-introduces variant; update the construction so the spread excludes the variant key—e.g., filter out variant from modelOptions before calling snakeToCamelOptions or remove variant from the resulting object—so that only the combined model string (options.model + ':' + variant) is sent and no separate variant property is forwarded.packages/typescript/ai-openrouter/tests/openrouter-adapter.test.ts (1)
874-889: Assert adapter-side camelCase before outbound schema parse.This test currently verifies wire-format snake_case after schema parsing. Add assertions on
paramsfirst (frequencyPenalty,presencePenalty,toolChoice,maxCompletionTokens) so adapter mapping is validated directly, independent of serializer behavior.✅ Suggested assertion additions
const [rawParams] = mockSend.mock.calls[0]! const params = rawParams + + expect(params).toHaveProperty('frequencyPenalty', 0.5) + expect(params).toHaveProperty('presencePenalty', 0.3) + expect(params).toHaveProperty('toolChoice', 'auto') + expect(params).toHaveProperty('maxCompletionTokens', 4096)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/typescript/ai-openrouter/tests/openrouter-adapter.test.ts` around lines 874 - 889, The test currently asserts snake_case keys on the serialized result but should first assert the adapter produced camelCase keys; before calling ChatGenerationParams$outboundSchema.parse(rawParams) add assertions that params (or rawParams) has frequencyPenalty: 0.5, presencePenalty: 0.3, toolChoice: 'auto', and maxCompletionTokens: 4096 so the adapter mapping is validated independently of the outbound schema.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/typescript/ai-openrouter/src/adapters/text.ts`:
- Around line 525-534: The ChatGenerationParams construction currently appends
modelOptions?.variant into the model string and then spreads
snakeToCamelOptions(modelOptions) (in the request object building code), which
re-introduces variant; update the construction so the spread excludes the
variant key—e.g., filter out variant from modelOptions before calling
snakeToCamelOptions or remove variant from the resulting object—so that only the
combined model string (options.model + ':' + variant) is sent and no separate
variant property is forwarded.
In `@packages/typescript/ai-openrouter/src/utils/casing.ts`:
- Around line 5-11: snakeToCamelOptions currently loses type information by
returning a generic Record<string, unknown>; change it to a generic function so
the input type is preserved and the output keys are the camel-cased version of
the input keys while values keep their original types. Update
snakeToCamelOptions to accept a generic type parameter (e.g., T extends
Record<string, unknown>) and return a mapped type that remaps each key K in
keyof T to the camel-cased key (using the existing snakeToCamel function for the
transformation conceptually) with the same value type T[K], so callers retain
compile-time safety when spreading into SDK params. Ensure typings and exports
are updated accordingly without changing runtime behavior.
In `@packages/typescript/ai-openrouter/tests/openrouter-adapter.test.ts`:
- Around line 874-889: The test currently asserts snake_case keys on the
serialized result but should first assert the adapter produced camelCase keys;
before calling ChatGenerationParams$outboundSchema.parse(rawParams) add
assertions that params (or rawParams) has frequencyPenalty: 0.5,
presencePenalty: 0.3, toolChoice: 'auto', and maxCompletionTokens: 4096 so the
adapter mapping is validated independently of the outbound schema.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.changeset/bright-geese-poke.mdpackages/typescript/ai-openrouter/src/adapters/text.tspackages/typescript/ai-openrouter/src/utils/casing.tspackages/typescript/ai-openrouter/src/utils/index.tspackages/typescript/ai-openrouter/tests/openrouter-adapter.test.ts
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.
Fixes #314
🎯 Changes
Transforms all snake_case model options to camelCase in the open router adapter to ensure optins are passed through the SDK to openrouter
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
New Features
Tests