feat: add tool result offload compactor - #12487
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
linhongyu510
left a comment
There was a problem hiding this comment.
There is a cross-run key collision in the new compactor path. ToolResultOffloadCompactor always writes to its constructor-level self.store, while _offload() calls _result_store_key(..., step=index, index=index). For a tool call without an id, two Agent runs with the same conversation shape therefore produce the same key (for example, the result at message 2 becomes 2_search_call2.txt). FileSystemToolResultStore.write() uses Path.write_text, so run B overwrites run As content while run As message still points at that same reference. Reading the older conversation then returns the newer runs tool output. Even with IDs, uniqueness is provider-dependent rather than guaranteed across runs.
The existing ToolResultOffloadHook already documents this boundary and supports a per-run store through RESULT_STORE_CONTEXT_KEY; the compactor has neither that override nor a run/session namespace, while the new public example encourages a single static FileSystemToolResultStore on a reusable Agent. The tests all use a fresh tmp_path and assign cN IDs, so they cannot expose the overwrite.
Please make the compactor store/key run-scoped (or otherwise collision-resistant) and add a regression that calls compact() twice through the same compactor/store with id-less calls and different content. The first references content must remain unchanged and the two references must differ. In multi-user deployments, this is both data corruption and potential cross-session disclosure; random/global uniqueness alone prevents overwrite, but a run-scoped store or namespace is still preferable when retrieval tools can read the store.
Related Issues
Proposed Changes:
Added the experimental
ToolResultOffloadCompactorfor use withCompactionHook. It keeps fresh tool output directly in the Agent's context, then writes eligible older text results to aToolResultStoreonly when context compaction is triggered. The compacted conversation retains a reference and configurable preview, allowing the Agent to retrieve the full stored output later.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.