feat(compression): proactive tool-result pruning for large-window models - #62644
feat(compression): proactive tool-result pruning for large-window models#62644Kolektori wants to merge 1 commit into
Conversation
Competing implementation with #62389 for feature #513 (two-phase context management). Both add an opt-in, default-off proactive tool-result prune decoupled from the summarization trigger: this one triggers on |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the no-LLM prune path; the current main premise is valid: agent/context_compressor.py:2853-2856 only prunes inside full compress(), while agent/conversation_loop.py:4769-4778 enters that path only after should_compress().
Problems
agent/conversation_loop.py:4802unconditionally callsprune_tool_results_only()whenever normal compression does not fire. The pluggableContextEnginecontract only requiresupdate_from_response,should_compress, andcompress(agent/context_engine.py:70-106); the repository's conformingStubEnginelacks this method (tests/agent/test_context_engine.py:15-48). An active external engine will therefore raiseAttributeErrorafter a tool call.- The public settings need the established top-level
compression:configuration surface. Defaults/docs currently enumerate that block inhermes_cli/config.py:1419-1432andwebsite/docs/user-guide/configuration.md:738-745, but this PR changes neither.
Suggested changes
- Gate this behavior to the built-in compressor, or add a backwards-compatible optional context-engine hook plus an external-engine regression test.
- Document and default the two settings under
compression:; correct the PR example fromagent.compression.
Automated hermes-sweeper review.
| # is configured and _real_tokens is above it. See | ||
| # ContextCompressor.prune_tool_results_only. | ||
| _pruned_msgs, _pruned_n = _compressor.prune_tool_results_only( | ||
| messages, current_tokens=_real_tokens |
There was a problem hiding this comment.
agent.context_compressor may be a configured external ContextEngine, whose required interface does not define this method (agent/context_engine.py:70-106). Gate this to the built-in compressor or add a safe optional interface/no-op, otherwise any external engine reaches an AttributeError on this post-tool path.
The phase-1 tool-result prune only runs inside compress(), which fires near 50% of the context window, so it never triggers on large-window models; old tool outputs then ride in history and are re-sent every turn. Add prune_tool_results_only(): the same no-LLM prune on a separate, low proactive_prune_tokens trigger, run as an elif to the compression branch. Opt-in (default 0), protects the recent tail by message count. Add the method to the ContextEngine base as a no-op default so pluggable engines inherit it safely (the post-tool-call path never AttributeErrors on a non-built-in engine); the built-in compressor supplies the real prune. Register both keys under the top-level compression config with defaults and document them.
51a0aff to
037ca72
Compare
|
Thanks for the review. Both are fixed in 037ca72. For the external-engine case: On the config surface: both keys now live in the top-level Tests: |
…oactive prune Follow-ups on top of the cherry-picked #62644 mechanism, porting it to current main and closing the salvage-review requirements: - proactive_prune_min_reclaim_tokens (default 4096): a prune only COMMITS when it reclaims a meaningful token batch, measured on the pruned output. A committed prune rewrites already-sent history and invalidates the provider prompt-cache prefix; this hysteresis gate keeps those breaks episodic/amortized (like a compression boundary) instead of firing every tool iteration. 0 disables the gate. (Design point credited to the #62389 review cycle's prune_minimum_tokens.) - Standard no-op caller contract: every skip path returns the INPUT list object; the loop commits only on 'result is not messages' + non-zero count. - Loop call is getattr+callable guarded (plugin engines predating the hook, SimpleNamespace test doubles) and exception-swallowed at debug level. - Config parse follows the compression.max_attempts hardened semantics: booleans rejected, fractional floats rejected, integral floats/numeric strings accepted; negative trigger = disabled. - cli-config.yaml.example documented (all three keys) and gateway _CACHE_BUSTING_CONFIG_KEYS extended so hot-reload rebuilds the agent. - Tests: min-reclaim gate both directions, input-object no-op contract, no-orphan tool_call_id pairing in BOTH directions (#69830 pin rule), default-off zero-behavior-change pin, config parse seam, and behavioral loop-wiring tests (consulted/commit/no-op/absent-method/raising).
…oactive prune Follow-ups on top of the cherry-picked #62644 mechanism, porting it to current main and closing the salvage-review requirements: - proactive_prune_min_reclaim_tokens (default 4096): a prune only COMMITS when it reclaims a meaningful token batch, measured on the pruned output. A committed prune rewrites already-sent history and invalidates the provider prompt-cache prefix; this hysteresis gate keeps those breaks episodic/amortized (like a compression boundary) instead of firing every tool iteration. 0 disables the gate. (Design point credited to the #62389 review cycle's prune_minimum_tokens.) - Standard no-op caller contract: every skip path returns the INPUT list object; the loop commits only on 'result is not messages' + non-zero count. - Loop call is getattr+callable guarded (plugin engines predating the hook, SimpleNamespace test doubles) and exception-swallowed at debug level. - Config parse follows the compression.max_attempts hardened semantics: booleans rejected, fractional floats rejected, integral floats/numeric strings accepted; negative trigger = disabled. - cli-config.yaml.example documented (all three keys) and gateway _CACHE_BUSTING_CONFIG_KEYS extended so hot-reload rebuilds the agent. - Tests: min-reclaim gate both directions, input-object no-op contract, no-orphan tool_call_id pairing in BOTH directions (#69830 pin rule), default-off zero-behavior-change pin, config parse seam, and behavioral loop-wiring tests (consulted/commit/no-op/absent-method/raising).
…oactive prune Follow-ups on top of the cherry-picked #62644 mechanism, porting it to current main and closing the salvage-review requirements: - proactive_prune_min_reclaim_tokens (default 4096): a prune only COMMITS when it reclaims a meaningful token batch, measured on the pruned output. A committed prune rewrites already-sent history and invalidates the provider prompt-cache prefix; this hysteresis gate keeps those breaks episodic/amortized (like a compression boundary) instead of firing every tool iteration. 0 disables the gate. (Design point credited to the #62389 review cycle's prune_minimum_tokens.) - Standard no-op caller contract: every skip path returns the INPUT list object; the loop commits only on 'result is not messages' + non-zero count. - Loop call is getattr+callable guarded (plugin engines predating the hook, SimpleNamespace test doubles) and exception-swallowed at debug level. - Config parse follows the compression.max_attempts hardened semantics: booleans rejected, fractional floats rejected, integral floats/numeric strings accepted; negative trigger = disabled. - cli-config.yaml.example documented (all three keys) and gateway _CACHE_BUSTING_CONFIG_KEYS extended so hot-reload rebuilds the agent. - Tests: min-reclaim gate both directions, input-object no-op contract, no-orphan tool_call_id pairing in BOTH directions (#69830 pin rule), default-off zero-behavior-change pin, config parse seam, and behavioral loop-wiring tests (consulted/commit/no-op/absent-method/raising).
|
Merged via salvage PR #70254 with your commit cherry-picked and authorship preserved — thanks @Kolektori, including your 037ca72 fixes for both review blockers! Your |
…oactive prune Follow-ups on top of the cherry-picked NousResearch#62644 mechanism, porting it to current main and closing the salvage-review requirements: - proactive_prune_min_reclaim_tokens (default 4096): a prune only COMMITS when it reclaims a meaningful token batch, measured on the pruned output. A committed prune rewrites already-sent history and invalidates the provider prompt-cache prefix; this hysteresis gate keeps those breaks episodic/amortized (like a compression boundary) instead of firing every tool iteration. 0 disables the gate. (Design point credited to the NousResearch#62389 review cycle's prune_minimum_tokens.) - Standard no-op caller contract: every skip path returns the INPUT list object; the loop commits only on 'result is not messages' + non-zero count. - Loop call is getattr+callable guarded (plugin engines predating the hook, SimpleNamespace test doubles) and exception-swallowed at debug level. - Config parse follows the compression.max_attempts hardened semantics: booleans rejected, fractional floats rejected, integral floats/numeric strings accepted; negative trigger = disabled. - cli-config.yaml.example documented (all three keys) and gateway _CACHE_BUSTING_CONFIG_KEYS extended so hot-reload rebuilds the agent. - Tests: min-reclaim gate both directions, input-object no-op contract, no-orphan tool_call_id pairing in BOTH directions (NousResearch#69830 pin rule), default-off zero-behavior-change pin, config parse seam, and behavioral loop-wiring tests (consulted/commit/no-op/absent-method/raising).
Summary
An opt-in pass that trims old tool-result payloads out of the re-sent history on a low token trigger, separate from the full-compression trigger. It makes no LLM call and is off by default, so nothing changes unless you turn it on.
Problem
Every turn re-sends the whole message list, so a single large tool output (a
terminaldump, aread_file, aweb_extract) keeps getting re-billed on every turn after it lands. Profiling real sessions, re-sent history was over 70% of input tokens, almost all of it old tool output that never gets trimmed.The compressor already has a cheap, deterministic pass for exactly this:
_prune_old_tool_resultsdedups identical results, summarizes old ones, and truncates oversized tool-call args. The catch is that it only runs as phase 1 ofcompress(), which fires at roughly 50% of the context window. On a 1M-token window that's ~500K, and real sessions rarely come close, so the prune sits there and never runs.Change
Run that same phase-1 prune on its own low trigger,
proactive_prune_tokens, decoupled from compression:ContextCompressor.prune_tool_results_only()runs the prune without the LLM summary phase.elifon the compression branch, so the two never both fire in a turn. Whenshould_compress()is False (the usual case on a large window), the cheap prune gets its shot.protect_last_n), not by a token budget. A token-budget tail on a 1M window would cover the whole session and prune nothing.ContextEnginebase class as a no-op default, so a pluggable context engine that doesn't implement it inherits the no-op instead of raisingAttributeErroron the post-tool-call path. The built-in compressor supplies the real implementation.Configuration
Opt-in, under the top-level
compression:block (same place asthreshold,protect_last_n, etc.):Both keys are registered in the config defaults and documented in the configuration guide.
compression.*keys hot-reload on a running gateway, so tuning the trigger takes effect on the next message.Safety and behavior
protect_last_nmessages are never touched by the summarize or truncate passes. Dedup keeps the newest full copy and only back-references byte-identical older ones, so nothing unique is lost.proactive_prune_min_result_charsfloor is clamped to 200 so a generated summary can't itself be re-summarized into garbage.Performance
On real large sessions this reclaimed 18–30% of snapshot input tokens with no quality change I could see, and
should_compress()stayed False throughout, so the prune did the work rather than compaction.Testing
New
tests/agent/test_proactive_tool_result_pruning.py(9 tests) plus a regression test intests/agent/test_context_engine.pythat a minimal engine implementing only the required interface inherits the base no-op without raising.Backward compatibility
Off by default (
proactive_prune_tokens: 0)._prune_old_tool_resultsgets a newmin_prune_charsargument defaulting to the old hard-coded 200, so the existing compression path is byte-for-byte identical. The newContextEngine.prune_tool_results_onlyis a no-op default, so existing engines keep their current behavior.