feat: add context compaction hook - #12196
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
f150d3a to
bc92e30
Compare
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # The conversation is not yet large enough to compact, so leave it alone. | ||
| return None | ||
| # Calculate an overhead that is not compactable: the system prompt, tool schemas, and chat-template overhead. | ||
| overhead = estimated - self.token_counter.count(messages=messages) |
There was a problem hiding this comment.
If the overhead is negative, should we log a warning to alert the user that the token_counter tends to overestimate?
| :param min_keep_messages: The fewest recent messages to keep even when they exceed the target. Keeping at least | ||
| 1 is recommended, since the caller may be mid-step with a pending tool call whose result is appended right | ||
| after compaction. `0` removes everything after the leading system messages. |
There was a problem hiding this comment.
I have some doubts about min_keep_messages
- I'm not sure that the explanation is correct
- looking at how it works, what about using
min_keep_turnsinstead?
There was a problem hiding this comment.
Thats fair! For something like min_keep_turns what would you say a full turn is? Is one turn a "user" --> "assistant" --> "all tool results" group?
There was a problem hiding this comment.
- Sorry, maybe
min_keep_stepsis better where step = assistant msg + all tool results - I now notice that we risk losing the first user message that probably contains the most relevant info
| Implement `to_dict` so the compactor's settings survive serialization. The default `from_dict` passes them straight | ||
| back to the constructor, which is enough for plain values; override it when `to_dict` emitted something that has to | ||
| be rebuilt first, such as a `Secret` or a nested component. | ||
| """ |
There was a problem hiding this comment.
Somewhere we should mention that a Compactor should not divide tool results from tool calls
|
I left some comments but overall I like the design! |
Related Issues
Proposed Changes:
Added experimental context compaction for the
Agent.ContextCompactionHookruns before LLM calls and shortens the conversation when it reaches a configured fraction of the model's context window.The first built-in strategy,
SlidingWindowCompactor, preserves leading system messages and as much recent history as the target allows. It replaces removed history with a short omission note.The hook uses provider-reported context usage when available and locally estimates the request otherwise, including
tool schemas. Leave headroom above
compact_atfor the next reply and its tool results.SlidingWindowCompactorkeeps tool calls together with their results, so it may retain slightly more history than the requested target. Compaction is lossy: removed messages cannot be recovered or summarized by this strategy. Implement theCompactorprotocol to provide a custom strategy.ContextCompactionHookandSlidingWindowCompactoremit anExperimentalWarningand may change without a deprecation cycle.How did you test it?
New tests
Notes for the reviewer
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:and added!in case the PR includes breaking changes.