Skip to content

fix(agent): collect after large tool results (#70684) - #80974

Open
Christopher-Schulze wants to merge 3 commits into
NousResearch:mainfrom
Christopher-Schulze:fix/70684-cli-gc-collect
Open

fix(agent): collect after large tool results (#70684)#80974
Christopher-Schulze wants to merge 3 commits into
NousResearch:mainfrom
Christopher-Schulze:fix/70684-cli-gc-collect

Conversation

@Christopher-Schulze

@Christopher-Schulze Christopher-Schulze commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes the Hermes CLI process memory leak described in #70684 by triggering a targeted gc.collect() after large tool results are appended. The tool-result collector is gated on a 1 MB UTF-8 JSON threshold so small results do not pay for a full collection every turn. Context compression already runs the platform-aware trim_memory(reason="post-compression") hook; this update keeps that existing single collection path intact instead of adding a duplicate.

Related Issue

Fixes #70684

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Changes Made

  • agent/tool_executor.py:
    • import gc and add _GC_COLLECT_THRESHOLD_BYTES = 1_000_000.
    • add _maybe_collect_gc_after_tool_result(content, threshold_bytes=...) which serializes content to UTF-8 JSON and calls gc.collect() when the size is >= the threshold.
    • call the helper after every messages.append(...) that adds a tool result in execute_tool_calls_concurrent, execute_tool_calls_sequential, and the shared _append_cancelled_tool_results helper (which also covers the invalid-argument and cancelled/skip paths).
  • tests/agent/test_compression_gc_collect.py: regression test that compression uses the existing post-compression trim path exactly once, so Linux/glibc does not double-collect.
  • tests/run_agent/test_tool_executor_gc_collect.py: regression tests that gc.collect() fires for a >1 MB tool result and not for a small one.

How to Test

  1. Run the regression tests on the current main (427584b76) and confirm the large-tool-result test fails:
    • scripts/run_tests.sh tests/agent/test_compression_gc_collect.py tests/run_agent/test_tool_executor_gc_collect.py
  2. Run the same tests on this branch and confirm they pass (5/5), including the single-collection compression invariant.
  3. Run the project checker: scripts/check.sh --project hermes-agent --worktree worktrees/hermes-agent/70684.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix
  • I've added tests for my changes
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — N/A (stdlib gc only)

@Christopher-Schulze Christopher-Schulze changed the title fix(agent): run gc.collect after large tool results and context compression fix(agent): run gc.collect after large tool results and context compression (#70684) Aug 7, 2026
@alt-glitch alt-glitch added type/perf Performance improvement or optimization P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint area/memory Memory subsystem: store, providers, sync, background reviews area/compression Context compression and continuation sessions sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state needs-repro Bug needs reproduction steps labels Aug 7, 2026
@spfcraze

spfcraze commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Summary:
The compression-path gc.collect() runs a second full collection on glibc/Linux, where origin/main compress() already triggers trim_memory(reason="post-compression") — the net-new coverage for #70684 is the tool-result threshold collector.

Problems:

  • origin/main compress() already calls trim_memory(reason="post-compression") after a successful compaction; hermes_cli/mem_trim.py's trim_memory runs its own gc.collect() before malloc_trim — the added bare gc.collect() before return compressed fires again on the same path.
  • The PR's regression test mocks hermes_cli.mem_trim.trim_memory to hold its call_count == 1 assertion, so the suite cannot see the real compression-path collection it duplicates.
  • agent/tool_executor.py on main has no gc.collect() — the threshold collector is the new part.

Solution:
Restate the PR's scope around the tool-result collector: the compression-path collection is already the single merged trim call on main. Fixes #70684 will auto-close the issue on merge; the compression factor it names is already addressed on main.


Checked against aefa3f5 — the tip of fix/70684-cli-gc-collect when this was written — and 48e2dcd, main at the same moment.

@Christopher-Schulze

Christopher-Schulze commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

Maintenance update: corrected the title to reflect the final scope: collection after large tool results only. Removed the duplicate explicit collection from ContextCompressor.compress(). Compression now relies on the existing platform-aware trim_memory post-compression path exactly once; the large-tool-result threshold collector is unchanged. Rebased onto current main; canonical checker and 5/5 regression tests pass on head cec454a.

@Christopher-Schulze Christopher-Schulze changed the title fix(agent): run gc.collect after large tool results and context compression (#70684) fix(agent): collect after large tool results (#70684) Aug 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/compression Context compression and continuation sessions area/memory Memory subsystem: store, providers, sync, background reviews comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint needs-repro Bug needs reproduction steps P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/perf Performance improvement or optimization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(agent): Hermes CLI process leaks memory (no gc.collect() after tool results or compression)

3 participants