test: add failing test for openrouter adapter issue#315
test: add failing test for openrouter adapter issue#315tombeckenham wants to merge 1 commit intoTanStack:mainfrom
Conversation
📝 WalkthroughWalkthroughThe change adds comprehensive test coverage for OpenRouter adapter parameter serialization, verifying that outbound parameters are correctly converted from camelCase to snake_case format before transmission to the endpoint. The tests reference an existing outbound schema and validate serialization of core parameters and model options without modifying any runtime logic. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=test:sherif,test:knip,tes... |
❌ Failed | 10s | View ↗ |
nx run-many --targets=build --exclude=examples/** |
✅ Succeeded | 2s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-02-25 02:52:26 UTC
@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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/typescript/ai-openrouter/tests/openrouter-adapter.test.ts (2)
142-143: The "false positive" assertion muddies the test signal.Line 143 checks raw pre-serialization params, where
tool_choiceis trivially present regardless of whether the underlying adapter bug exists. With the corrective assertion on Line 162 (serializedhavingtool_choice) now in place, keeping this assertion as a labeled "false positive" is misleading — it will continue to pass even after the bug is fixed, which obscures whether the test is correctly tracking the issue.Consider removing this assertion and relying solely on the post-serialization check on Line 162 to keep the test's intent unambiguous.
♻️ Suggested cleanup
- // This is the false postive - as it never gets passed to the openrouter endpoint - expect(params.tool_choice).toBe('auto') - expect(params.messages).toBeDefined()🤖 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 142 - 143, Remove the misleading pre-serialization assertion that checks params.tool_choice (the line with expect(params.tool_choice).toBe('auto')) because it always passes and obscures the real check; instead, delete that assertion and rely solely on the post-serialization assertion that inspects serialized (the existing check around serialized.tool_choice) to verify the adapter behavior.
873-874: RedundantrawParamsalias — use the destructuring pattern used elsewhere.
rawParamsis assigned and immediately aliased toparamswith no transformation. Lines 135 and 366 in the same file use the direct destructuring form.♻️ Proposed simplification
- const [rawParams] = mockSend.mock.calls[0]! - const params = rawParams + const [params] = mockSend.mock.calls[0]!🤖 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 873 - 874, The test creates an unnecessary alias rawParams before assigning it to params; replace that two-step assignment with the direct destructuring pattern used elsewhere by extracting params directly from mockSend.mock.calls[0] (i.e., destructure the first element into params instead of creating rawParams then assigning it to params) so the test uses a single const [params] = mockSend.mock.calls[0]! like the other examples.
🤖 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/tests/openrouter-adapter.test.ts`:
- Around line 142-143: Remove the misleading pre-serialization assertion that
checks params.tool_choice (the line with
expect(params.tool_choice).toBe('auto')) because it always passes and obscures
the real check; instead, delete that assertion and rely solely on the
post-serialization assertion that inspects serialized (the existing check around
serialized.tool_choice) to verify the adapter behavior.
- Around line 873-874: The test creates an unnecessary alias rawParams before
assigning it to params; replace that two-step assignment with the direct
destructuring pattern used elsewhere by extracting params directly from
mockSend.mock.calls[0] (i.e., destructure the first element into params instead
of creating rawParams then assigning it to params) so the test uses a single
const [params] = mockSend.mock.calls[0]! like the other examples.
1 participant
🎯 Changes
This PR includes a failing test that demonstrates modelOptions not passing though to openrouter.
Summary by CodeRabbit