-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix(sliding-window): prevent premature context condensing #6918
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -12,6 +12,13 @@ import { ANTHROPIC_DEFAULT_MAX_TOKENS } from "@roo-code/types" | |||||
| */ | ||||||
| export const TOKEN_BUFFER_PERCENTAGE = 0.1 | ||||||
|
|
||||||
| /** | ||||||
| * Safety buffer applied when approaching the hard context limit. | ||||||
| * Keep very small so we don't trigger premature condensing on large windows. | ||||||
| */ | ||||||
| const SAFETY_BUFFER_PERCENTAGE = 0.01 // 1% of total context window | ||||||
| const MIN_SAFETY_BUFFER_TOKENS = 2048 | ||||||
|
|
||||||
| /** | ||||||
| * Counts tokens for user content using the provider's token counting implementation. | ||||||
| * | ||||||
|
|
@@ -118,10 +125,12 @@ export async function truncateConversationIfNeeded({ | |||||
| // Calculate total effective tokens (totalTokens never includes the last message) | ||||||
| const prevContextTokens = totalTokens + lastMessageTokens | ||||||
|
|
||||||
| // Calculate available tokens for conversation history | ||||||
| // Truncate if we're within TOKEN_BUFFER_PERCENTAGE of the context window | ||||||
| // Calculate effective capacity available for conversation history after reserving output tokens | ||||||
| const allowedTokens = contextWindow * (1 - TOKEN_BUFFER_PERCENTAGE) - reservedTokens | ||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider extracting this calculation into a named function like
Suggested change
|
||||||
|
|
||||||
| // Note: allowedTokens already includes a dynamic buffer and reserved output tokens | ||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment mentions keeping 'a single source of truth' but doesn't clarify what that source is or how it differs from the previous approach. Could we make this more explicit? Perhaps something like: 'allowedTokens is now the single threshold for both percentage and absolute checks'? |
||||||
| // Keep a single source of truth for truncation thresholds to align with tests and behavior. | ||||||
|
|
||||||
| // Determine the effective threshold to use | ||||||
| let effectiveThreshold = autoCondenseContextPercent | ||||||
| const profileThreshold = profileThresholds[currentProfileId] | ||||||
|
|
@@ -143,6 +152,7 @@ export async function truncateConversationIfNeeded({ | |||||
| // If no specific threshold is found for the profile, fall back to global setting | ||||||
|
|
||||||
| if (autoCondenseContext) { | ||||||
| // Base percent on effective capacity so visualization and behavior are aligned | ||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does 'visualization' refer to here? Is this the UI progress bar, debug output, or something else? A bit more specificity would help future maintainers understand the alignment you're achieving. |
||||||
| const contextPercent = (100 * prevContextTokens) / contextWindow | ||||||
| if (contextPercent >= effectiveThreshold || prevContextTokens > allowedTokens) { | ||||||
| // Attempt to intelligently condense the context | ||||||
|
|
||||||
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.
Are these constants intentional? They're defined but never used in the implementation. Were they meant to be incorporated into the threshold calculation, or are they placeholders for future enhancements?