fix(responses): accept reasoning effort none - #1902
Conversation
Signed-off-by: Aniruddh Krovvidi <224641992+ankrovv@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe Responses protocol adds ChangesReasoning effort none support
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ResponsesRouter
participant responses_to_chat
participant ChatCompletionRequest
Client->>ResponsesRouter: POST /v1/responses with effort "none"
ResponsesRouter->>responses_to_chat: Convert ResponsesRequest
responses_to_chat->>ChatCompletionRequest: Set reasoning_effort to "none"
ChatCompletionRequest-->>ResponsesRouter: Converted request
ResponsesRouter-->>Client: OK response
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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.
Code Review
This pull request introduces support for the None variant in ReasoningEffort for responses. It implements serialization and deserialization logic, maps it correctly in regular chat conversions, and ensures that Harmony requests explicitly reject this option since Harmony models do not support it. Comprehensive unit and integration tests have been added to verify these behaviors. I have no further feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Hi @ankrovv, this PR has merge conflicts that must be resolved before it can be merged. Please rebase your branch: git fetch origin main
git rebase origin/main
# resolve any conflicts, then:
git push --force-with-lease |
|
This pull request has been automatically marked as stale because it has not had any activity within 14 days. It will be automatically closed if no further activity occurs within 16 days. Leave a comment if you feel this pull request should remain open. Thank you! |
Problem
SMG rejects
reasoning.effort:"none"during Responses request parsing, before vLLM receives it.Reproducer
Before: HTTP 400,
unknown variant 'none'. Direct vLLM accepts the same request for non-Harmony models.Fix
nonein the Responses protocol.Tests
Protocol round-trip, HTTP serialization, regular gRPC conversion, Harmony new/continuation rejection, and
/v1/responsesendpoint tests pass. Focused clippy passes. The local all-features clippy run requires system OpenCV/pkg-config; CI covers that configuration.Summary by CodeRabbit
New Features
"none"reasoning effort option in the Responses API."none"now preserve the setting across serialization and conversion to chat requests./v1/responsesendpoint accepts requests withreasoning.effort: "none".Bug Fixes
"none"cannot be represented by Harmony conversations.