refactor: route max_completion_tokens on provider, drop per-model flag (#30) - #56
Merged
Merged
Conversation
#30) Every current and future OpenAI chat model uses max_completion_tokens, so the per-model uses_max_completion_tokens flag duplicated a fact already captured by the provider key. Remove the field, the four gpt-5.x entries that set it, and the model_uses_completion_tokens helper; branch on config.provider == "OpenAI API" in _build_litellm_kwargs for both the max_completion_tokens kwarg and the temperature omission. No behaviour change: the wrapper builds identical kwargs for every current model. Tests updated to key the routing assertions off provider. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #30.
What
Every current and future OpenAI chat model uses
max_completion_tokens(the deprecatedmax_tokensis gone across all OpenAI chat completions), so the per-modeluses_max_completion_tokensflag duplicated a fact already captured by the provider key — and was one more thing to remember on every new OpenAI model.uses_max_completion_tokensfromModelInfoand from the four gpt-5.x entries incore/models.py.model_uses_completion_tokenshelper.core/llm.py:_build_litellm_kwargs, branch onconfig.provider == "OpenAI API"for both themax_completion_tokenskwarg and the temperature omission (the whole OpenAI family is the reasoning family).Behaviour
No change. The wrapper builds identical LiteLLM kwargs for every current model — OpenAI still gets
max_completion_tokens+ no temperature, Anthropic itsmax_tokensdefault, Google its safety settings, Custom its forced OpenAI routing. The Custom path is untouched.Tests
tests/test_models.py: dropped the two flag-invariant tests + helper import.tests/test_llm.py: renamed the three OpenAI cases to assert provider-based routing.🤖 Generated with Claude Code