fix: add context window overflow protection#27
fix: add context window overflow protection#27efecnc wants to merge 2 commits intoaltaidevorg:mainfrom
Conversation
Long conversations could exceed the model's context window, causing an immediate 400 error with no recovery path (compaction only ran after successful responses, creating a deadlock). Adds trim_context_to_budget() which estimates token count and removes oldest messages from the front when over budget, preserving: - The system message (index 0) - Tool call/response pairs (removed atomically) A marker message is inserted so the model knows earlier context was trimmed. Default budget is 120k tokens (conservative for most models).
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a context trimming mechanism to ensure chat history stays within a defined token budget. It adds a trim_context_to_budget function that prunes older messages while preserving the system message and keeping tool call/response pairs intact. A review comment suggests optimizing this function to avoid O(N^2) complexity by updating the token estimate incrementally instead of recalculating it in every iteration of the trimming loop.
| let estimate_tokens = |msgs: &[crate::utils::ChatMessage]| -> usize { | ||
| msgs.iter() | ||
| .map(|m| { | ||
| m.content.as_ref().map_or(0, |c| c.text_content().len()) / 4 | ||
| + m.tool_calls | ||
| .as_ref() | ||
| .map_or(0, |tcs| tcs.iter().map(|t| t.function.arguments.len() / 4).sum()) | ||
| }) | ||
| .sum() | ||
| }; |
There was a problem hiding this comment.
The estimate_tokens closure is re-calculated in every iteration of the while loop, leading to O(N^2) complexity. While acceptable for small contexts, it could be optimized by calculating the initial token count and updating it incrementally as messages are removed.
|
✅ Review fixes applied:
Ready for re-review. |
1 participant
Summary
Root Cause
The reasoning loop fetched full context from memory and sent it directly to the provider with no size check. The existing compaction logic only triggered after a successful response, creating a deadlock: context too large → 400 error → no response → no compaction → permanent failure.
Test plan
cargo checkpasses (no new warnings)