feat: support OpenRouter reasoning options#810
Conversation
|
@lukeaus is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds support for OpenRouter reasoning-capable models by allowing users to configure reasoning effort and enable reasoning token inclusion via environment variables, building OpenRouter-specific request fields conditionally, and parsing reasoning output when message content is empty. ChangesOpenRouter Reasoning Models Support
Sequence Diagram(s)sequenceDiagram
participant Caller
participant OpenRouterProvider
participant OpenRouterAPI as OpenRouter API
Caller->>OpenRouterProvider: call(prompt)
OpenRouterProvider->>OpenRouterProvider: read reasoningEffort and includeReasoning
OpenRouterProvider->>OpenRouterProvider: build OpenRouterRequestBody with optional reasoning fields
OpenRouterProvider->>OpenRouterAPI: POST /chat with JSON body (includes reasoning.* when set)
OpenRouterAPI-->>OpenRouterProvider: response with choices[0].message
alt message.content present
OpenRouterProvider->>Caller: return message.content
else content empty
OpenRouterProvider->>Caller: return message.reasoning or message.reasoning_content
else neither present
OpenRouterProvider->>Caller: throw error (truncated response JSON)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
|
Linked issue: #808 |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds OpenRouter “reasoning” request/response support and documents/configures how to enable it, plus introduces a none embedding-provider override to disable embeddings even when auto-detected.
Changes:
- Add OpenRouter request flags (
reasoning.effort,include_reasoning) controlled by env vars, and parse reasoning-only responses whencontentis empty. - Allow
EMBEDDING_PROVIDER=noneto explicitly disable embedding provider auto-detection. - Add tests and documentation for the new configuration options.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/providers/openrouter.ts |
Adds env-controlled OpenRouter reasoning request fields and reasoning-only response handling. |
src/config.ts |
Adds EMBEDDING_PROVIDER=none support and trims forced provider value. |
test/fetch-timeout.test.ts |
Adds coverage for OpenRouter reasoning options and response parsing; cleans env vars between tests. |
test/embedding-provider.test.ts |
Adds a test ensuring EMBEDDING_PROVIDER=none returns null. |
README.md |
Documents OpenRouter reasoning env vars and adds none to embedding provider list. |
.env.example |
Documents OpenRouter reasoning env vars and adds none to embedding provider list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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
🧹 Nitpick comments (1)
test/fetch-timeout.test.ts (1)
195-207: ⚡ Quick winAdd a regression test for empty
reasoningwith populatedreasoning_content.Current coverage checks empty
content, but not the case wherereasoningis""andreasoning_contenthas the usable output. That case would catch the fallback edge at Line 96 in the provider.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/fetch-timeout.test.ts` around lines 195 - 207, Add a regression test for the case where the OpenRouter API returns an empty "reasoning" but a populated "reasoning_content": update the test in fetch-timeout.test.ts to call mockChatResponse with reasoning: "" and reasoning_content: "reasoning output" (while content may be empty), instantiate OpenRouterProvider as before, and assert that OpenRouterProvider.compress("system", "user") resolves to the reasoning_content string; this will exercise the fallback branch in the provider (the logic referenced around Line 96) that should prefer reasoning_content when reasoning is empty.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/providers/openrouter.ts`:
- Around line 96-98: The current assignment const reasoning = message?.reasoning
?? message?.reasoning_content treats an empty string as a valid value and
prevents falling back; change the fallback logic so empty strings from
message?.reasoning fall through to message?.reasoning_content (e.g., replace the
nullish-coalescing with a falsy check or explicit empty-string check on
message?.reasoning) so the subsequent if (reasoning) behaves correctly and
non-empty reasoning_content isn't ignored.
---
Nitpick comments:
In `@test/fetch-timeout.test.ts`:
- Around line 195-207: Add a regression test for the case where the OpenRouter
API returns an empty "reasoning" but a populated "reasoning_content": update the
test in fetch-timeout.test.ts to call mockChatResponse with reasoning: "" and
reasoning_content: "reasoning output" (while content may be empty), instantiate
OpenRouterProvider as before, and assert that
OpenRouterProvider.compress("system", "user") resolves to the reasoning_content
string; this will exercise the fallback branch in the provider (the logic
referenced around Line 96) that should prefer reasoning_content when reasoning
is empty.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4302bc4f-1dd4-4442-a6c9-af1a5d2e4bc3
📒 Files selected for processing (6)
.env.exampleREADME.mdsrc/config.tssrc/providers/openrouter.tstest/embedding-provider.test.tstest/fetch-timeout.test.ts
Signed-off-by: Luke Scott <luke.scott@investflo.ai>
lukeaus
force-pushed
the
fix/openrouter-reasoning-options
branch
from
562e147 to
dad2d69
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/fetch-timeout.test.ts (1)
195-225: ⚡ Quick winAdd an explicit precedence test for
reasoningoverreasoning_content.The fallback contract is
message.reasoning ?? message.reasoning_content; current tests cover emptyreasoningfallback, but not non-emptyreasoningpriority.➕ Suggested test addition
describe("OpenRouterProvider reasoning options", () => { @@ it("falls through empty reasoning to reasoning_content", async () => { @@ }); + + it("prefers reasoning over reasoning_content when both are present", async () => { + mockChatResponse({ + content: "", + reasoning: "primary reasoning", + reasoning_content: "secondary reasoning content", + }); + const provider = new OpenRouterProvider( + "test-key", + "moonshotai/kimi-k2.6", + 1024, + "https://openrouter.ai/api/v1/chat/completions", + ); + + await expect(provider.compress("system", "user")).resolves.toBe( + "primary reasoning", + ); + }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/fetch-timeout.test.ts` around lines 195 - 225, Add a test in test/fetch-timeout.test.ts that verifies OpenRouterProvider.compress prefers message.reasoning over message.reasoning_content: mockChatResponse with content: "", reasoning: "explicit reasoning", reasoning_content: "fallback reasoning", instantiate OpenRouterProvider as in the other tests, and assert await provider.compress("system","user") resolves to "explicit reasoning"; this ensures the precedence contract (message.reasoning ?? message.reasoning_content) is enforced.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/fetch-timeout.test.ts`:
- Around line 195-225: Add a test in test/fetch-timeout.test.ts that verifies
OpenRouterProvider.compress prefers message.reasoning over
message.reasoning_content: mockChatResponse with content: "", reasoning:
"explicit reasoning", reasoning_content: "fallback reasoning", instantiate
OpenRouterProvider as in the other tests, and assert await
provider.compress("system","user") resolves to "explicit reasoning"; this
ensures the precedence contract (message.reasoning ?? message.reasoning_content)
is enforced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ea45c7f3-7af0-42d5-b8a9-4ae3c38ac879
📒 Files selected for processing (4)
.env.exampleREADME.mdsrc/providers/openrouter.tstest/fetch-timeout.test.ts
✅ Files skipped from review due to trivial changes (1)
- README.md
|
Status update after review pass:
|
|
Reviewed the latest CodeRabbit comments. There are no outstanding actionable CodeRabbit review threads on this PR. The prior actionable reasoning fallback/test feedback was addressed in the latest head ( |
Summary
Adds OpenRouter-specific opt-in reasoning controls and response fallback handling for reasoning-capable OpenRouter models.
Fixes #808.
Changes
OPENROUTER_REASONING_EFFORTandOPENROUTER_INCLUDE_REASONINGsupport for OpenRouter chat requests.message.reasoning/message.reasoning_contentwhenmessage.contentis empty.Verification
HOME=$(mktemp -d) npx vitest run test/fetch-timeout.test.ts test/embedding-provider.test.tsnpm run buildSummary by CodeRabbit
New Features
Documentation
Tests