fix: read max_tokens from custom_providers per-model config#28988
Closed
zccyman wants to merge 1 commit into
Closed
fix: read max_tokens from custom_providers per-model config#28988zccyman wants to merge 1 commit into
zccyman wants to merge 1 commit into
Conversation
Previously, only context_length was read from custom_providers per-model config. max_tokens was silently ignored, always falling back to 4096. Changes: - Extract get_custom_provider_model_field() as generic lookup helper - Add get_custom_provider_max_tokens() symmetric to context_length - Read custom_providers max_tokens in agent_init.py after context_length - Add 10 regression tests for max_tokens lookup Closes NousResearch#28046
Collaborator
Contributor
Author
|
Superseded by PR #28995 which includes this fix plus the Config-Runtime Contract Registry (Phase 1). |
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
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.
Summary
Fixes #28046 —
max_tokensconfigured undercustom_providers[].models.<model>.max_tokenswas silently ignored, always defaulting to 4096.The codebase already had
get_custom_provider_context_length()for reading per-modelcontext_lengthfromcustom_providers, but no equivalent formax_tokens.Changes
Extract
get_custom_provider_model_field()— generic lookup helper that searchescustom_providersentries for any per-model field. Replaces the inline logic inget_custom_provider_context_length().Add
get_custom_provider_max_tokens()— thin wrapper over the generic helper, symmetric toget_custom_provider_context_length().Read
max_tokensinagent_init.py— after the existingcontext_lengthcustom_providers lookup, adds a symmetric block that callsget_custom_provider_max_tokens()whenagent.max_tokens is None.10 regression tests — full coverage for the new lookup: matching, trailing-slash insensitivity, zero/negative rejection, string coercion, coexistence with context_length, first-match-wins, None inputs.
Backward Compatibility
get_custom_provider_context_length()signature unchanged —custom_providersremains a positional arg (3 existing tests pass without modification)agent.max_tokens is None(no override from top-level config or constructor) — pure additive fallbackRoot Cause Pattern
This is the same pattern as #28961 (pre_tool_call missing session_id) and #28984 (Typed Plugin Hook Protocol FR): configuration/state flows through call chains without schema enforcement, so adding a new config field requires manually updating every bridge — and omissions are silent.