Support minimal OpenAI reasoning effort#90
Conversation
|
can you check these reasoning levels with our currently supported integrations |
|
I’ll check PR #90 against the repository’s supported provider integrations and report back with any compatibility or validation issues. |
|
Async check complete for PR #90. Bottom lineThe provider implementation looks correctly scoped:
So the code does not accidentally enable Recommended action before mergeI’d request a small docs update before merging.
none, low, medium, high, xhighIt should clarify that:
Optional but useful: add negative tests asserting Anthropic and Groq reject Suggested PR commentChecked this against the currently supported integrations.
The provider code looks correctly scoped:
- OpenAI Chat Completions now accepts `minimal`.
- OpenAI Responses now accepts `minimal`.
- OpenAI Codex inherits the Responses mapper, so it also accepts `minimal`.
- Anthropic still rejects `minimal`.
- Groq still rejects `minimal`.
- Proxy behavior depends on the target provider.
One thing to fix before merge: `README.md` is now stale. It still lists managed reasoning values as only `none`, `low`, `medium`, `high`, and `xhigh`. Can we update the docs to mention that `minimal` is OpenAI/OpenAI Codex-specific, while Anthropic and Groq do not support it?
Optional but helpful: add defensive tests confirming Anthropic and Groq reject `minimal`.Note: the async agent attempted to run the relevant tests locally, but the environment was missing the |
|
I specifically wanted you to go to the provider websites and check what they support i assume that htere is not equal coverage between what the various providers support. the current implementation just adds it for openai but managed options should work no matter the provider so for example if a provider only supports low then we need and user set minimal we need to automatically map it to the closest matching option, our api should support the maximum that any provider offers so we can atleast map to that one and then closest match the rest |
|
Got it — I misunderstood the scope. I’ve kicked off a provider-docs-based check now: it will inspect PR #90 plus the repo’s supported integrations, then verify each provider’s official reasoning/reasoning-effort support and propose a provider-agnostic managed mapping strategy. I’ll report back with a compatibility matrix, doc links, and concrete PR #90 change recommendations once that review is complete. |
|
Something went wrong please try again |
| /qwen3/i => %w[default], | ||
| /gpt-oss/i => %w[low medium high] | ||
| }.freeze | ||
| DEFAULT_REASONING_LEVELS = %w[low medium high].freeze |
There was a problem hiding this comment.
Everywhere else we using default supported reasoning levels but here we use default we should bee the language identical
| ReasoningEffortMapper.closest_supported(effort, supported_levels) | ||
| end | ||
|
|
||
| def supported_reasoning_levels(model) |
There was a problem hiding this comment.
This should be built into reasoning effort mapper
| module Adapters | ||
| module AnthropicOptionMapper | ||
| DEFAULT_MAX_TOKENS = 20_480 | ||
| REASONING_EFFORT_BUDGET_TOKENS = { |
There was a problem hiding this comment.
I feel like this is going to cause a breaking change that i would like to avoid in this pull request
|
|
||
| module LlmGateway | ||
| module Adapters | ||
| module ReasoningEffortMapper |
There was a problem hiding this comment.
this should be included in to the other tasks
| # Official docs checked: | ||
| # OpenAI: none/minimal/low/medium/high/xhigh; Anthropic: low/medium/high/xhigh/max; | ||
| # Groq: model-specific none/default or low/medium/high. | ||
| LEVELS = %w[minimal low medium high xhigh max].freeze |
There was a problem hiding this comment.
when included this will be the default what we currently call supported levels and children will override
|
No action taken. The review notification for PR #90 has an empty review body, so there isn’t any specific feedback or request for me to address. If there are inline comments not included in this notification, send them over and I can follow up. |
|
@managerbot there are comments attached to the review that you should look at |
|
I’ll look into the inline review comments on PR #90 and follow up with any changes or a reply once I’ve inspected them. |
|
Managerbot needs you to OAuth your GitHub account before it can help from GitHub. I sent a GitHub OAuth link to any mapped chat accounts; connect GitHub there and then retry. |
|
Got it — async agent Summary:
Suggested reviewer response:
|
|
Managerbot needs you to OAuth your GitHub account before it can help from GitHub. I sent a GitHub OAuth link to any mapped chat accounts; connect GitHub there and then retry. |
2 similar comments
|
Managerbot needs you to OAuth your GitHub account before it can help from GitHub. I sent a GitHub OAuth link to any mapped chat accounts; connect GitHub there and then retry. |
|
Managerbot needs you to OAuth your GitHub account before it can help from GitHub. I sent a GitHub OAuth link to any mapped chat accounts; connect GitHub there and then retry. |
1 participant
Summary
minimalto the accepted OpenAI reasoning levels for Chat Completions and Responses option mappers.Tests
env -u RUBYOPT -u BUNDLE_GEMFILE -u BUNDLE_LOCKFILE -u BUNDLE_APP_CONFIG -u BUNDLE_BIN_PATH -u BUNDLER_VERSION -u BUNDLER_SETUP ruby -Itest test/unit/options/openai_responses_option_mapper_test.rb test/unit/options/openai_chat_completions_option_mapper_test.rb test/unit/options/openai_codex_option_mapper_test.rb