-
Notifications
You must be signed in to change notification settings - Fork 2.3k
fix: support simple on/off reasoning toggle for grok-4-fast on OpenRouter #8711
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
base: main
Are you sure you want to change the base?
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 |
---|---|---|
|
@@ -30,14 +30,34 @@ export const getOpenRouterReasoning = ({ | |
reasoningBudget, | ||
reasoningEffort, | ||
settings, | ||
}: GetModelReasoningOptions): OpenRouterReasoningParams | undefined => | ||
shouldUseReasoningBudget({ model, settings }) | ||
? { max_tokens: reasoningBudget } | ||
: shouldUseReasoningEffort({ model, settings }) | ||
? reasoningEffort | ||
? { effort: reasoningEffort } | ||
: undefined | ||
: undefined | ||
}: GetModelReasoningOptions): OpenRouterReasoningParams | undefined => { | ||
// Check if model should use reasoning budget | ||
if (shouldUseReasoningBudget({ model, settings })) { | ||
return { max_tokens: reasoningBudget } | ||
} | ||
|
||
// Check if model should use reasoning effort | ||
if (shouldUseReasoningEffort({ model, settings })) { | ||
return reasoningEffort ? { effort: reasoningEffort } : undefined | ||
} | ||
|
||
// Check if model supports reasoning but not effort or budget (e.g., grok-4-fast on OpenRouter) | ||
// These models use a simple on/off toggle via the exclude parameter | ||
if ( | ||
model.supportedParameters?.includes("reasoning") && | ||
!model.supportsReasoningEffort && | ||
!model.supportsReasoningBudget | ||
) { | ||
// If reasoning is not explicitly disabled, enable it | ||
// We use exclude: false to enable reasoning, and exclude: true to disable | ||
// Check both settings.reasoningEffort and the passed reasoningEffort parameter | ||
const effectiveEffort = reasoningEffort || settings.reasoningEffort | ||
const reasoningEnabled = effectiveEffort !== "minimal" | ||
return reasoningEnabled ? { exclude: false } : { exclude: true } | ||
} | ||
Comment on lines
+44
to
+57
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. The simple toggle logic doesn't respect the |
||
|
||
return undefined | ||
} | ||
|
||
export const getAnthropicReasoning = ({ | ||
model, | ||
|
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.
When both
reasoningEffort
andsettings.reasoningEffort
are undefined, this code enables reasoning by default ({ exclude: false }
). This is inconsistent with how other reasoning models work, where undefined effort typically results in no reasoning parameters being sent. For grok-4-fast, when no effort preference is set, it should returnundefined
instead of defaulting to enabled. This ensures the API's default behavior is respected.