Skip to content

Add compaction-time micro-compaction policy (AgentSpec.microcompact, default off) - #279

Open
raymondginger2018-sudo wants to merge 1 commit into
HKUDS:mainfrom
raymondginger2018-sudo:pr/microcompact
Open

raymondginger2018-sudo wants to merge 1 commit into
HKUDS:mainfrom
raymondginger2018-sudo:pr/microcompact

Conversation

@raymondginger2018-sudo

Copy link
Copy Markdown
Contributor

Summary

Adds a cheap first rung to the compaction ladder: before the summarizer is ever considered, a model-free pass replaces stale tool-result contents with a placeholder. Borrowed from ZCode's applyMicrocompactPolicy, adapted to this runtime's message model.

Armed only via AgentSpec.microcompact — the default None keeps existing runs byte-for-byte unchanged.

Changes

  • New core/agent_runtime/microcompact.py: keep-recency policy with dual triggers (token pressure at 0.9x context budget OR idle timeout), pair integrity (the tool_call_id is preserved while content becomes "[Old tool result content cleared]"), media blocks pass through untouched, copy-on-write over the message list, and a min-savings convergence floor so an ineffective clear never loops.
  • core/agent_runtime/runner.py: wire the pass into compact_history when a policy is armed; after a clear, recompute the prompt estimate and return early once pressure is resolved.
  • New tests/test_microcompact.py (12 tests: triggers, recency window, pair integrity, media pass-through, copy-on-write, convergence).

Tests

  • tests/test_microcompact.py — 12 passed
  • CI-exact lint: pre-commit run --all-files (ruff 0.15.21 check + format) — passed
  • Local preflight verdict recorded against this revision (baseline reds unchanged, no new failures)

Notes

  • Independent PR — no dependency on other open PRs; no follow-up wiring required for the default path.
  • The policy is inert unless a caller opts in, so this can land safely ahead of any config-surface work.

@Zongwei9888

Copy link
Copy Markdown
Collaborator

Thanks @raymondginger2018-sudo for a well-structured PR. The off-by-default guard is careful: with it off, the runtime behaves exactly as before (the compaction and runner tests are unchanged, and the full suite passes). We can't merge it as is, though:

  1. Nothing in the product sets AgentRunSpec.microcompact, so users can't turn it on.
  2. When it is on, apply_microcompact re-checks pressure with its own chars/4 estimate over message text (microcompact.py:194-207). That misses tool-call arguments, tool schemas and the provider's real token count, so in our runs it didn't fire for Chinese or write-heavy sessions and the summarizer ran anyway. The runner has already decided there's pressure, so it should pass force=True (runner.py:1855).
  3. The "keep recent" window is a count, and _maybe_compact runs before the model reads the latest tool batch. With more than five parallel calls, results are cleared before the model sees them.
  4. Results already saved to disk lose their Full output saved to: path, so the model can't find them again.
  5. The idle trigger has no clock behind it: the runner never passes idle_seconds.

A version we'd accept: fixes 2-4 (never clear the latest tool batch, keep the saved-output path line); drops or properly wires the idle trigger; adds a user-facing switch; and adds integration tests through AgentRunner._maybe_compact (one showing the summarizer is skipped and tool-call pairs stay intact, one showing history is unchanged when it's off). Please also revert the import reordering in runner.py, and replace the ZCode references (and the module paths that don't exist here, tool_result_pruner, prefix_churn) with a plain description.

Add a cheap first rung to the compaction ladder: before the summarizer is
considered, old tool results (all but the most recent N and the entire latest
tool batch) are replaced by a placeholder. The runner owns the decision — the
pressure gate has already fired on the provider's real token count, so the pass
makes no second estimate.

- Keep the whole latest tool batch (the one the model has not read yet), not
  just a count-based recency window, so more than N parallel calls can no longer
  be cleared before the model reads them.
- Preserve a spilled result's "Full output saved to:" pointer so the model can
  still find the output on disk.
- Drop the idle trigger and the internal pressure re-check; the runner decides.
- Expose a user-facing switch (agents.defaults.microcompact), off by default so
  existing runs are unchanged.

Tests cover the clear policy directly plus integration through
AgentRunner._maybe_compact: one showing the summarizer is skipped and tool-call
pairs stay intact, one showing the history is unchanged when it is off.
@raymondginger2018-sudo

Copy link
Copy Markdown
Contributor Author

Thanks @Zongwei9888 — all of this is addressed in the force-pushed revision (now rebased onto the latest main, squashed into one commit). Point-by-point against your review:

  1. Nothing in the product sets AgentRunSpec.microcompact, so users can't turn it on.

Fixed — there's now a user-facing switch. agents.defaults.microcompact (in AgentDefaults, default False) is read in build_agent_session, passed into AgentSession, and wired to the main run's AgentRunSpec.microcompact = MicrocompactPolicy() if on else None. With it off (the default) nothing changes; turning it on arms the pass under the runner's existing pressure gate.

  1. When it is on, apply_microcompact re-checks pressure with its own chars/4 estimate ... it should pass force=True.

Rather than passing force=True to bypass the second estimate, I removed the second estimate entirely. apply_microcompact no longer takes force/context_budget_tokens/idle_seconds — it has no gate at all. The runner's pressure gate has already decided there's pressure using the provider's real token count (which sees tool-call arguments and tool schemas), so the pass just clears. There is no chars/4 estimate left to disagree with the gate, which is why the Chinese / write-heavy sessions that previously fell through to the summarizer now clear as intended.

  1. The "keep recent" window is a count ... With more than five parallel calls, results are cleared before the model sees them.

Fixed — the pass now protects the entire latest tool batch, not just a count. It scans for the most recent assistant message with tool_calls and protects every result whose tool_call_id is in that batch, in addition to the recency window. A batch of any size (6, 10, …) survives whole. Regression test: test_giant_parallel_batch_is_fully_protected.

  1. Results already saved to disk lose their Full output saved to: path.

Fixed — when a stale result is cleared, the pass preserves the [tool output persisted] and Full output saved to: … lines on top of the placeholder, so the model can still find the output on disk. Regression test: test_saved_output_path_line_is_preserved.

  1. The idle trigger has no clock behind it: the runner never passes idle_seconds.

Fixed by dropping it. The idle trigger (and idle_timeout_s on the policy) is gone; the pass is purely pressure-gated via the runner, which is the only place a real clock/token count lives.

On the acceptance list:

  • Never clear the latest tool batch — done (point 3), keep the saved-output path line — done (point 4).
  • Drop or wire the idle trigger — dropped (point 5).
  • User-facing switch — done (point 1).
  • Integration tests through AgentRunner._maybe_compact — added two:
    • test_microcompact_on_skips_summarizer_and_keeps_tool_pairs: an over-budget history with huge stale results and a 6-call latest batch → the summarizer is never called (provider.calls == 0), the stale results become placeholders, the whole latest batch survives verbatim, and every tool result still pairs with the assistant that issued it.
    • test_microcompact_off_leaves_history_unchanged: the same over-budget history with the switch off → the history comes back unchanged (same object, no placeholder), because microcompact never fires before the PreCompact hook can stop the summarizer.
  • Revert the import reordering in runner.py — done; the imports are back in upstream order, with a single microcompact import added where it belongs.
  • Replace the ZCode references (and the non-existent tool_result_pruner, prefix_churn module paths) with a plain description — done; the module and runner comments now describe the behaviour plainly with no external-project or non-existent-module references.

Local verification: ruff format (idempotent) + ruff check clean; tests/test_microcompact.py 13/13; test_session_compaction.py + test_tool_result_pruner.py 16/16; the config/session/runner wiring suites pass (the one failure in that group, test_environment_context_is_resolved_and_xml_safe, is a pre-existing Windows path-separator baseline red — it fails identically on a pristine checkout and touches a file this PR doesn't modify).

Happy to adjust anything further.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants