-
Notifications
You must be signed in to change notification settings - Fork 627
fix: extend reasoningEffort support to all reasoning models #749
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -563,13 +563,13 @@ export class OpenAICompatibleProvider extends BaseLLMProvider { | |||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| if (modelId.startsWith('gpt-5')) { | ||||||||||||||||||||||||||||||||||||||||||
| if (modelConfig.reasoningEffort) { | ||||||||||||||||||||||||||||||||||||||||||
| ;(requestParams as any).reasoning_effort = modelConfig.reasoningEffort | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| if (modelConfig.verbosity) { | ||||||||||||||||||||||||||||||||||||||||||
| ;(requestParams as any).verbosity = modelConfig.verbosity | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| if (modelConfig.reasoningEffort) { | ||||||||||||||||||||||||||||||||||||||||||
| ;(requestParams as any).reasoning_effort = modelConfig.reasoningEffort | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+566
to
+569
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Broadened reasoning_effort is good, but apply it to non-streaming path too You apply reasoning_effort in the streaming path (handleChatCompletion), but the non-streaming openAICompletion path doesn’t set it. That creates inconsistent behavior for the same model depending on call path. Within openAICompletion, retrieve modelConfig and apply reasoning_effort similarly: const requestParams: OpenAI.Chat.ChatCompletionCreateParams = {
messages: this.formatMessages(messages),
model: modelId,
stream: false,
temperature: temperature,
...(modelId.startsWith('o1') ||
modelId.startsWith('o3') ||
modelId.startsWith('o4') ||
modelId.startsWith('gpt-5')
? { max_completion_tokens: maxTokens }
: { max_tokens: maxTokens })
}
+ // Align reasoning across non-streaming path
+ const modelConfig = this.configPresenter.getModelConfig(modelId, this.provider.id)
+ if (modelConfig?.reasoningEffort) {
+ ;(requestParams as any).reasoning_effort = modelConfig.reasoningEffort
+ }Do you want me to open a small follow-up PR to add this and tests? 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents💡 Verification agent 🧩 Analysis chainConfirm Chat Completions API expects 'reasoning_effort' vs 'reasoning: {effort}' SDKs differ between Chat Completions and Responses. You’re using reasoning_effort here (Chat), and reasoning: { effort } in the Responses provider. That may be correct, but worth double-checking against the SDK version you ship. 🌐 Web query: 💡 Result: Short answer
Minimal Node example (Responses API)
Official doc pages (references)
If you want, I can:
Citations:
Double-check reasoning parameter usage for new models // legacy Chat Completions provider (o1-family)
if (modelConfig.reasoningEffort) {
;(requestParams as any).reasoning_effort = modelConfig.reasoningEffort
}
// for o3/o4/GPT-5 (Responses API)
const resp = await client.responses.create({
model: modelConfig.name,
input: requestParams.messages,
reasoning: { effort: modelConfig.reasoningEffort }
})• Verify which models this presenter supports and ensure that for any o3/o4/GPT-5 variants you switch to the Responses API. References: 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||
| // verbosity 仅支持 GPT-5 系列模型 | ||||||||||||||||||||||||||||||||||||||||||
| if (modelId.startsWith('gpt-5') && modelConfig.verbosity) { | ||||||||||||||||||||||||||||||||||||||||||
| ;(requestParams as any).verbosity = modelConfig.verbosity | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // 移除推理模型的温度参数 | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Confirm model ID mapping and downstream usage for o1-pro
The o1-pro block changes include maxTokens and match: ['o1-pro'] with reasoningEffort: 'high'. Verify any UI or provider-side heuristics relying on 'o1-mini' are not implicitly tied to this new 'o1-pro' high-effort profile.
Run this to check for hard-coded 'o1-mini' expectations in the repo:
🏁 Script executed:
Length of output: 104
🏁 Script executed:
Length of output: 5571
🏁 Script executed:
Length of output: 92
🏁 Script executed:
Length of output: 128
Missing provider-side mapping for
o1-proWhile both
openAIResponsesProvider.tsandopenAICompatibleProvider.tslist'o1-pro'in theirOPENAI_REASONING_MODELSarrays, the UI’sproviderModelSettings.tshas no entry forid: 'o1-pro'. Without this block, selecting “OpenAI o1 Pro” in the UI will fall back or error.Please add a corresponding settings object in
src/main/presenter/configPresenter/providerModelSettings.ts, for example:— Ensure the fields align with the defaults in
modelDefaultSettings.ts(lines 890–898).🤖 Prompt for AI Agents