fix(agent): honour fallback entry context_length on failover activation - #90402
Open
gaogage wants to merge 1 commit into
Open
fix(agent): honour fallback entry context_length on failover activation#90402gaogage wants to merge 1 commit into
gaogage wants to merge 1 commit into
Conversation
try_activate_fallback clears the per-config context_length override so the fallback model resolves its real context window instead of inheriting the stale value from the previous model (NousResearch#22387). But the clear is unconditional: when the fallback chain entry itself carries an explicit context_length, that user-configured value is dropped too, and get_model_context_length() falls through to the hardcoded 256K default. Custom/self-hosted models that are not in any known catalog are the common case for explicit per-fallback context_length, so the failover silently mis-reports their window. Keep the NousResearch#22387 clearing behaviour, but first capture the fallback entry's own context_length and restore it after the clear, so resolution honours the per-fallback config when it exists and falls back to catalog resolution when it does not.
Collaborator
Duplicate of #70256: both patches preserve an explicit fallback-entry |
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.
What does this PR do?
try_activate_fallbackclears the per-configcontext_lengthoverride before switching models, so the fallback model resolves its real context window instead of inheriting the stale value from the previous model (#22387).However, the clear is unconditional. When the fallback chain entry itself carries an explicit
context_length(a user-configured per-fallback value), that value is dropped as well, andget_model_context_length()falls through to the hardcoded 256K default. Explicit per-fallbackcontext_lengthis most commonly set for custom/self-hosted models that are not in any known catalog — exactly the models that cannot fall back to catalog resolution — so after a failover their context window is silently mis-reported as 256K.This PR keeps the #22387 clearing behaviour but first captures the fallback entry's own
context_lengthand restores it after the clear:context_length→ honoured (user config wins)Related Issue
Follow-up to #22387
Type of Change
Changes Made
agent/chat_completion_helpers.py(try_activate_fallback): capturefb.get("context_length")before clearingagent._config_context_length, restore it afterwards when present and parseableHow to Test
context_length(e.g. a custom endpoint model with a 128K window) whose primary model uses a differentcontext_lengthcontext_length; entries without one behave exactly as beforeChecklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass — not run locally (live-install environment); happy to iterate if CI flags anythingtry_activate_fallbackif maintainers point at the preferred seamDocumentation & Housekeeping