fix(openrouter): normalize reasoning controls - #2837
Conversation
PR Summary by QodoNormalize OpenRouter reasoning controls
AI Description
Diagram
High-Level Assessment
Files changed (6)
|
Code Review by Qodo
1. Gemini docstring uses narrative
|
44b6fc5 to
54cdcdb
Compare
|
Code review by qodo was updated up to the latest commit 54cdcdb |
54cdcdb to
2c9e974
Compare
|
Code review by qodo was updated up to the latest commit 2c9e974 |
Route registered models' global reasoning effort through OpenRouter's unified reasoning object instead of LiteLLM 1.98.0's model-gated top-level parameter. Validate provider-specific overrides, preserve explicit disablement, fall back to the global value when they are invalid, normalize max to xhigh, and keep effort and token budgets mutually exclusive. Update the configuration guide and add regression coverage for LiteLLM validation, OpenRouter variants, precedence, model-specific limits, and global-state isolation. References: - https://openrouter.ai/docs/guides/best-practices/reasoning-tokens
2c9e974 to
d6c7f1b
Compare
|
Code review by qodo was updated up to the latest commit d6c7f1b |
IsmaelMartinez
left a comment
There was a problem hiding this comment.
LGTM, tested and works.
Worth saying this is a real breakage rather than a nicety: on stock config openrouter/google/gemini-2.5-pro fails outright with UnsupportedParamsError. Nothing outside openrouter/ changes.
One fix inline: setting only reasoning_max_tokens warns on every request. Qodo's six open findings are comment-phrasing and line-length rules I would not hold you to, though changing_a_model.md:535 is worth a glance since it went from 304 characters to 1098.
| if not effective_reasoning_effort: | ||
| if reasoning_max_tokens > 0 and openrouter_reasoning_effort: | ||
| get_logger().warning( | ||
| f"Ignoring config.reasoning_effort='{openrouter_reasoning_effort}' because " | ||
| "openrouter.reasoning_max_tokens takes precedence." | ||
| ) | ||
| elif reasoning_max_tokens <= 0: | ||
| effective_reasoning_effort = openrouter_reasoning_effort or "" |
There was a problem hiding this comment.
openrouter_reasoning_effort carries config.reasoning_effort, which ships as medium, so anyone setting only openrouter.reasoning_max_tokens gets this warning on every request without having configured anything contradictory.
none is the genuine contradiction, and the only case with a test asserting a warning. Demoting the rest keeps the signal; your 62 tests still pass.
| if not effective_reasoning_effort: | |
| if reasoning_max_tokens > 0 and openrouter_reasoning_effort: | |
| get_logger().warning( | |
| f"Ignoring config.reasoning_effort='{openrouter_reasoning_effort}' because " | |
| "openrouter.reasoning_max_tokens takes precedence." | |
| ) | |
| elif reasoning_max_tokens <= 0: | |
| effective_reasoning_effort = openrouter_reasoning_effort or "" | |
| if not effective_reasoning_effort: | |
| if reasoning_max_tokens > 0 and openrouter_reasoning_effort: | |
| message = ( | |
| f"Ignoring config.reasoning_effort='{openrouter_reasoning_effort}' because " | |
| "openrouter.reasoning_max_tokens takes precedence." | |
| ) | |
| if openrouter_reasoning_effort == "none": | |
| get_logger().warning(message) | |
| else: | |
| get_logger().info(message) | |
| elif reasoning_max_tokens <= 0: | |
| effective_reasoning_effort = openrouter_reasoning_effort or "" |
Summary
Behavior changes
Testing
References