-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix: respect autoCondenseContext setting on context window overflow #7957
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
Conversation
- Update Task.handleContextWindowExceededError() to read user settings from state - No longer force autoCondenseContext=true when context window is exceeded - Use user's configured autoCondenseContextPercent instead of FORCED_CONTEXT_REDUCTION_PERCENT - Add comprehensive tests for the new behavior Fixes #7953
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own code is like debugging in a mirror - everything looks backwards but the bugs are still mine.
| private async handleContextWindowExceededError(): Promise<void> { | ||
| const state = await this.providerRef.deref()?.getState() | ||
| const { profileThresholds = {} } = state ?? {} | ||
| const { profileThresholds = {}, autoCondenseContext = true, autoCondenseContextPercent = 100 } = state ?? {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice fix! The implementation correctly addresses the issue by reading autoCondenseContext and autoCondenseContextPercent from state instead of forcing them.
| `[Task#${this.taskId}] Context window exceeded for model ${this.api.getModel().id}. ` + | ||
| `Current tokens: ${contextTokens}, Context window: ${contextWindow}. ` + | ||
| `Forcing truncation to ${FORCED_CONTEXT_REDUCTION_PERCENT}% of current context.`, | ||
| `Auto-condense: ${autoCondenseContext}, Threshold: ${autoCondenseContextPercent}%.`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good update to the log message - it now accurately reflects what's happening instead of claiming to force truncation.
| @@ -0,0 +1,389 @@ | |||
| // npx vitest src/core/task/__tests__/Task.handleContextWindowExceeded.spec.ts | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent test coverage! The tests comprehensively verify the new behavior with different scenarios including disabled auto-condense, enabled auto-condense, undefined state, and edge cases.
Summary
This PR fixes the issue where context condensing was triggered even when the auto-condense setting was disabled. The problem occurred specifically in the overflow handler path when the context window was exceeded.
Problem
As detailed by @hannesrudolph in #7953:
Task.handleContextWindowExceededError()was forcing summarization regardless of user settingsautoCondenseContext: trueand usingFORCED_CONTEXT_REDUCTION_PERCENT(75%)Solution
Task.handleContextWindowExceededError()to respect the user'sautoCondenseContextandautoCondenseContextPercentsettings from stateChanges
src/core/task/Task.ts
handleContextWindowExceededError()to readautoCondenseContextandautoCondenseContextPercentfrom statesrc/core/task/tests/Task.handleContextWindowExceeded.spec.ts
Testing
Impact
This change ensures that users who have disabled auto-condensing will no longer see unexpected summarization when their context window is exceeded. The system will respect their preference and use simple truncation instead.
Fixes #7953
Important
Fixes
Task.handleContextWindowExceededError()to respect user settings for auto-condensing, with tests verifying the behavior.Task.handleContextWindowExceededError()now respectsautoCondenseContextandautoCondenseContextPercentsettings from state.Task.handleContextWindowExceeded.spec.tsto verify behavior with differentautoCondenseContextsettings.Task.tsto reflect actual settings used.This description was created by
for 8fbc6d7. You can customize this summary. It will automatically update as commits are pushed.