refactor(compression): fold simplify-pass findings into feasibility skip
- Reuse telemetry['middle_window_tokens'] for the skip's middle estimate (is-None fallback to a fresh estimate) so log and telemetry agree - Declare prellm_skip_count in the base telemetry schema (fixed shape) - Defer _derive_auto_focus_topic into the non-skip branch (user-turn scan was wasted work on every skip) - Document the skip in compress()'s Algorithm list and force: arg doc - Drop dead call_llm patches from 7 tests (unreachable with _generate_summary mocked)
This commit is contained in:
kshitijk4poor
@@ -1393,6 +1393,7 @@ class ContextCompressor(ContextEngine):
|
||||
"protected_head_tokens": None,
|
||||
"protected_tail_tokens": None,
|
||||
"middle_window_tokens": None,
|
||||
"prellm_skip_count": 0,
|
||||
"aux_prompt_tokens": None,
|
||||
"aux_output_reservation": None,
|
||||
"aux_provider": "",
|
||||
@@ -5892,7 +5893,11 @@ This compaction should PRIORITISE preserving all information related to the focu
|
||||
1. Prune old tool results (cheap pre-pass, no LLM call)
|
||||
2. Protect head messages (system prompt + first exchange)
|
||||
3. Find tail boundary by token budget (~20K tokens of recent context)
|
||||
4. Summarize middle turns with structured LLM prompt
|
||||
4. Summarize middle turns with structured LLM prompt (skipped
|
||||
pre-LLM when the middle is below
|
||||
``_FEASIBILITY_SKIP_MIDDLE_FRACTION`` of the threshold after a
|
||||
prior real-usage ineffectiveness strike — the deterministic
|
||||
fallback drop recovers the negligible savings instead)
|
||||
5. On re-compression, iteratively update the previous summary
|
||||
|
||||
Blank platform-echo user rows trailing the latest actionable user
|
||||
@@ -5912,7 +5917,9 @@ This compaction should PRIORITISE preserving all information related to the focu
|
||||
everything else. Inspired by Claude Code's ``/compact``.
|
||||
force: If True, clear any active summary-failure cooldown before
|
||||
running so a manual ``/compress`` can retry immediately after
|
||||
an auto-compression abort. Auto-compress callers pass False.
|
||||
an auto-compression abort, and bypass the pre-LLM feasibility
|
||||
skip so an explicit user request always exercises the full
|
||||
summary path. Auto-compress callers pass False.
|
||||
memory_context: Optional provider-supplied context to preserve in
|
||||
the summary prompt. Whitespace-only values are ignored.
|
||||
"""
|
||||
@@ -6203,7 +6210,6 @@ This compaction should PRIORITISE preserving all information related to the focu
|
||||
)
|
||||
|
||||
# Phase 3: Generate structured summary
|
||||
summary_focus_topic = focus_topic or self._derive_auto_focus_topic(messages)
|
||||
|
||||
# Pre-LLM feasibility check: if the middle section is too small to
|
||||
# yield meaningful token savings, skip the expensive LLM summarization
|
||||
@@ -6226,7 +6232,14 @@ This compaction should PRIORITISE preserving all information related to the focu
|
||||
# handling paths are always exercised on explicit user request.
|
||||
feasibility_skip = False
|
||||
if not force and self._ineffective_compression_count >= 1:
|
||||
middle_tokens = estimate_messages_tokens_rough(turns_to_summarize)
|
||||
# _record_compression_regions already estimated this exact window
|
||||
# into the telemetry dict above; reuse it so the log line and
|
||||
# telemetry can never disagree. The regions helper no-ops when the
|
||||
# telemetry attr isn't a dict, so fall back to a fresh estimate
|
||||
# when the key is absent/None (0 is a legitimate value).
|
||||
middle_tokens = telemetry.get("middle_window_tokens")
|
||||
if middle_tokens is None:
|
||||
middle_tokens = estimate_messages_tokens_rough(turns_to_summarize)
|
||||
if middle_tokens < int(
|
||||
self.threshold_tokens * _FEASIBILITY_SKIP_MIDDLE_FRACTION
|
||||
):
|
||||
@@ -6248,6 +6261,9 @@ This compaction should PRIORITISE preserving all information related to the focu
|
||||
if feasibility_skip:
|
||||
summary = None # No LLM call; Phase 4 inserts the deterministic fallback
|
||||
else:
|
||||
# Deriving the auto focus topic scans recent user turns — only pay
|
||||
# for it when a summary will actually be generated.
|
||||
summary_focus_topic = focus_topic or self._derive_auto_focus_topic(messages)
|
||||
summary = self._generate_summary(
|
||||
turns_to_summarize,
|
||||
focus_topic=summary_focus_topic,
|
||||
|
||||
@@ -2727,9 +2727,7 @@ class TestPreLlmFeasibilityCheck:
|
||||
compressor._ineffective_compression_count = 1 # one prior real strike
|
||||
msgs = self._make_messages()
|
||||
|
||||
with patch("agent.context_compressor.call_llm") as mock_llm, \
|
||||
patch.object(compressor, "_generate_summary") as mock_gen:
|
||||
mock_llm.return_value = (None, None)
|
||||
with patch.object(compressor, "_generate_summary") as mock_gen:
|
||||
# Middle section is tiny → feasibility skip fires
|
||||
compressor.compress(msgs, force=False)
|
||||
|
||||
@@ -2747,9 +2745,7 @@ class TestPreLlmFeasibilityCheck:
|
||||
compressor._last_summary_auth_failure = True # stale from prior failure
|
||||
msgs = self._make_messages()
|
||||
|
||||
with patch("agent.context_compressor.call_llm") as mock_llm, \
|
||||
patch.object(compressor, "_generate_summary") as mock_gen:
|
||||
mock_llm.return_value = (None, None)
|
||||
with patch.object(compressor, "_generate_summary") as mock_gen:
|
||||
result = compressor.compress(msgs, force=False)
|
||||
|
||||
# Should NOT abort — should produce compressed output with fallback
|
||||
@@ -2763,9 +2759,7 @@ class TestPreLlmFeasibilityCheck:
|
||||
compressor._last_summary_network_failure = True # stale
|
||||
msgs = self._make_messages()
|
||||
|
||||
with patch("agent.context_compressor.call_llm") as mock_llm, \
|
||||
patch.object(compressor, "_generate_summary") as mock_gen:
|
||||
mock_llm.return_value = (None, None)
|
||||
with patch.object(compressor, "_generate_summary") as mock_gen:
|
||||
result = compressor.compress(msgs, force=False)
|
||||
|
||||
assert result is not None
|
||||
@@ -2777,9 +2771,7 @@ class TestPreLlmFeasibilityCheck:
|
||||
compressor._ineffective_compression_count = 1
|
||||
msgs = self._make_messages()
|
||||
|
||||
with patch("agent.context_compressor.call_llm") as mock_llm, \
|
||||
patch.object(compressor, "_generate_summary", return_value="LLM summary") as mock_gen:
|
||||
mock_llm.return_value = (None, None)
|
||||
with patch.object(compressor, "_generate_summary", return_value="LLM summary") as mock_gen:
|
||||
compressor.compress(msgs, force=True)
|
||||
|
||||
# _generate_summary MUST have been called despite tiny middle section
|
||||
@@ -2791,9 +2783,7 @@ class TestPreLlmFeasibilityCheck:
|
||||
compressor._ineffective_compression_count = 0
|
||||
msgs = self._make_messages()
|
||||
|
||||
with patch("agent.context_compressor.call_llm") as mock_llm, \
|
||||
patch.object(compressor, "_generate_summary", return_value="LLM summary") as mock_gen:
|
||||
mock_llm.return_value = (None, None)
|
||||
with patch.object(compressor, "_generate_summary", return_value="LLM summary") as mock_gen:
|
||||
compressor.compress(msgs, force=False)
|
||||
|
||||
mock_gen.assert_called_once()
|
||||
@@ -2836,9 +2826,7 @@ class TestPreLlmFeasibilityCheck:
|
||||
msgs.append({"role": "user", "content": f"recent request {i}"})
|
||||
msgs.append({"role": "assistant", "content": "big result " + "x" * 20000})
|
||||
|
||||
with patch("agent.context_compressor.call_llm") as mock_llm, \
|
||||
patch.object(compressor, "_generate_summary") as mock_gen:
|
||||
mock_llm.return_value = (None, None)
|
||||
with patch.object(compressor, "_generate_summary") as mock_gen:
|
||||
result = compressor.compress(msgs, force=False)
|
||||
|
||||
mock_gen.assert_not_called()
|
||||
@@ -2859,9 +2847,7 @@ class TestPreLlmFeasibilityCheck:
|
||||
msgs = self._make_messages()
|
||||
|
||||
for _ in range(2):
|
||||
with patch("agent.context_compressor.call_llm") as mock_llm, \
|
||||
patch.object(compressor, "_generate_summary") as mock_gen:
|
||||
mock_llm.return_value = (None, None)
|
||||
with patch.object(compressor, "_generate_summary") as mock_gen:
|
||||
compressor.compress(list(msgs), force=False)
|
||||
mock_gen.assert_not_called()
|
||||
# Mirror the boundary wrapper's bookkeeping
|
||||
|
||||
Reference in New Issue
Block a user