Skip to content

feat: add tool result offload compactor - #12487

Draft
sjrl wants to merge 2 commits into
mainfrom
feat/tool-result-offload-compactor
Draft

feat: add tool result offload compactor#12487
sjrl wants to merge 2 commits into
mainfrom
feat/tool-result-offload-compactor

Conversation

@sjrl

@sjrl sjrl commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Related Issues

Proposed Changes:

Added the experimental ToolResultOffloadCompactor for use with CompactionHook. It keeps fresh tool output directly in the Agent's context, then writes eligible older text results to a ToolResultStore only when context compaction is triggered. The compacted conversation retains a reference and configurable preview, allowing the Agent to retrieve the full stored output later.

from haystack.hooks.compaction import CompactionHook, ToolResultOffloadCompactor
from haystack.hooks.tool_result_offloading import FileSystemToolResultStore
compaction_hook = CompactionHook(
    compactor=ToolResultOffloadCompactor(
        store=FileSystemToolResultStore(root="tool_results"),
        min_keep_steps=1,
        min_tokens=200,
    ),
    context_window=400_000,
    compact_at=0.7,
    compact_to=0.4,
)

How did you test it?

New tests

Notes for the reviewer

Checklist

  • I have read the contributors guidelines and the code of conduct.
  • I have updated the related issue with new insights and changes.
  • I have added unit tests and updated the docstrings.
  • I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test: and added ! in case the PR includes breaking changes.
  • I have documented my code.
  • I have added a release note file, following the contributors guidelines.
  • I have run pre-commit hooks and fixed any issue.

@vercel

vercel Bot commented Aug 27, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
haystack-docs Ready Ready Preview Aug 27, 2026 11:41am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  haystack/hooks/compaction
  __init__.py
  tool_result_offloading.py
  haystack/hooks/tool_result_offloading
  hooks.py 124
  haystack/hooks/tool_result_offloading/types
  protocol.py
Project Total  

This report was generated by python-coverage-comment-action

@linhongyu510 linhongyu510 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic:tests type:documentation Improvements on the docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants