-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: respect 1M token context window for auto-condensing #9833
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Update Task.ts to use actual context window from api.getModel().info - This ensures the 1M token extension is properly respected when enabled - Fixes auto-condensing triggering at base model limits instead of extended limits Fixes #9831
Reviewed the latest commit. The reviewer @mrubens is correct - this PR does not actually fix the 1M token context window issue. Critical Issue: The revert commit (ca758de) removes previous changes and returns the code to a functionally identical state. Both versions call Root Cause Analysis: Since
Recommendation: This PR should not be merged as it does not address issue #9831. A deeper investigation is needed to find where the 1M token context window extension is failing to be applied or respected. Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
|
@roomote did anything actually change in this PR? Think about this more deeply. |
You're correct - the PR changes were a no-op. The code I've reverted the changes. The underlying issue #9831 needs further investigation to identify the actual root cause. |
The previous changes were functionally equivalent to the original code. The api.getModel().info already returns the correct 1M context window when awsBedrock1MContext is enabled (see bedrock.ts lines 1074-1081). The issue #9831 requires further investigation to find the actual root cause.
|
@hannesrudolph let's close this and have remote try again? |
This PR attempts to address Issue #9831. Feedback and guidance are welcome!
Problem
When auto context condensing is enabled with the 1M token option for Claude Sonnet on Amazon Bedrock, the context was being condensed at the base model's context window (e.g., 100,000 tokens) instead of the extended 1M token limit.
Root Cause
The
Task.tsfile was usingmodelInfo.contextWindowwhich represented the base model's context window and didn't account for the 1M token extension that's applied whenawsBedrock1MContextis enabled.Solution
Updated
Task.tsto get the actual context window fromapi.getModel().infowhich includes the 1M token extension when enabled. This ensures that:attemptApiRequestmethod uses the correct context window for managing contexthandleContextWindowExceededErrormethod uses the correct context window for error recoveryChanges
src/core/task/Task.tsto useapi.getModel()to get the actual model configuration including any context window extensionsTesting
Fixes #9831