fix: clear stale _config_context_length when switching providers#24079
Closed
TorvaldUtne wants to merge 1 commit into
Closed
fix: clear stale _config_context_length when switching providers#24079TorvaldUtne wants to merge 1 commit into
TorvaldUtne wants to merge 1 commit into
Conversation
When switching from a cloud provider (context_length: 1000000) to a local provider (context_length: 8192) via /model, the top-level model.context_length stored as _config_context_length at startup persisted and shadowed the new provider's context_length. This caused the context window to show 1M instead of the local provider's actual limit. The fix re-resolves _config_context_length from the live config when switching providers: it checks whether the new provider matches the default provider (and only then uses the top-level context_length), otherwise clears it so the new provider's per-model/per-provider context_length takes effect. Regression tests cover: provider switch clears stale value, no explicit ctx falls back cleanly, same-provider switch preserves config value.
1 task
Collaborator
Contributor
|
Automated hermes-sweeper review: this stale context-length switching bug is already fixed on current main. Evidence:
Thanks for the report and regression coverage. This PR's exact selective re-resolve approach was not the one merged, but the stale |
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
3 participants
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.
fix: clear stale
_config_context_lengthwhen switching providersProblem
When switching from a cloud provider (
context_length: 1000000) to a local provider (context_length: 8192) via/model, the top-levelmodel.context_lengthstored as_config_context_lengthat startup persists and shadows the new provider's context_length. This causes the context window to show 1M instead of the local provider's actual limit.Root Cause
_config_context_lengthis resolved once during__init__frommodel.context_lengthand stored asself._config_context_length. Whenswitch_model()callsget_model_context_length(), it passes this stale value asconfig_context_length, which takes precedence over the new provider's per-model/per-providercontext_length.Fix
In
switch_model(), re-resolve_config_context_lengthfrom the live config before updating the context compressor:providerfield → keep the top-levelcontext_lengthcontext_lengthtakes effect viaget_custom_provider_context_length()Testing
Added 3 regression tests to
tests/run_agent/test_switch_model_context.py:test_switch_to_local_provider_clears_stale_config_context— switching from cloud (1M) to local (8K) clears the stale valuetest_switch_to_local_no_explicit_ctx— no explicit ctx falls back cleanlytest_same_provider_keeps_config_context— switching models within the same provider preserves the config valueRelated
references/model-context-length-bug.md)