mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-15 14:22:43 +00:00
fix(compaction): judge the anti-thrash verdict on real usage, not in should_compress
Third correction, and the load-bearing one. The previous commit put the
"did compaction clear the threshold?" verdict inside should_compress(). But
conversation_loop calls should_compress() TWICE per turn with two different
measures (turn_context.py / conversation_loop.py:1033 and :4789):
* pre-API : request_pressure_tokens -- a rough estimate that can dip BELOW
the threshold
* post-API: real prompt tokens -- which stay above it
So the rough reading reset the strike every turn and the loop never stopped.
Reproduced: 8 compactions in 8 turns under the real two-call pattern, even
with the previous fix applied. (My earlier repro only called should_compress()
once per turn, which is why it looked contained.)
Move the verdict to update_from_response(), the one place that sees the
provider's real prompt_tokens for the just-compacted conversation, guarded by
the existing awaiting_real_usage_after_compression flag so it fires exactly
once per compaction. Real-vs-real: it cannot be fooled by a rough sub-threshold
reading, and (from the previous commit's lesson) never subtracts an estimate
from a real count. should_compress() goes back to a plain threshold test plus
the pre-existing cooldown and anti-thrash guards.
New test test_rough_preflight_reading_does_not_reopen_the_loop drives the real
two-call-per-turn pattern and fails on the prior should_compress()-based commit.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
d172445629
commit
7f9485707d
2 changed files with 112 additions and 53 deletions
|
|
@ -1185,6 +1185,42 @@ class ContextCompressor(ContextEngine):
|
|||
self.last_rough_tokens_when_real_prompt_fit = self.last_compression_rough_tokens
|
||||
else:
|
||||
self.last_rough_tokens_when_real_prompt_fit = 0
|
||||
|
||||
# Anti-thrashing verdict, judged HERE because this is the only place
|
||||
# that sees the provider's real prompt count for the just-compacted
|
||||
# conversation. Effectiveness is "did the prompt get under the
|
||||
# threshold?", not "did the message list shrink?": compaction can
|
||||
# only shrink messages, while the system prompt and tool schemas are
|
||||
# an incompressible floor (with 50+ tools, 20-30K tokens — see
|
||||
# #14695). When that floor alone meets the threshold, every pass
|
||||
# shrinks messages by a healthy margin yet leaves the prompt over the
|
||||
# line, so the next turn compacts again, forever.
|
||||
#
|
||||
# It must NOT live in should_compress(): that runs twice per turn
|
||||
# with two different measures (a rough preflight estimate and the
|
||||
# real post-response count, #36718), and the rough one can dip below
|
||||
# the threshold and reset the strike every turn, re-opening the loop.
|
||||
# Keying on real usage compares like with like and fires exactly once
|
||||
# per compaction.
|
||||
if self._verify_compaction_cleared_threshold:
|
||||
if self.last_prompt_tokens >= self.threshold_tokens:
|
||||
self._ineffective_compression_count += 1
|
||||
if not self.quiet_mode:
|
||||
logger.warning(
|
||||
"Compaction did not clear the threshold: %d real "
|
||||
"tokens still >= %d. The incompressible prompt "
|
||||
"(system prompt + tool schemas) may already exceed "
|
||||
"it, in which case shrinking messages cannot help. "
|
||||
"ineffective_compression_count=%d",
|
||||
self.last_prompt_tokens, self.threshold_tokens,
|
||||
self._ineffective_compression_count,
|
||||
)
|
||||
else:
|
||||
self._ineffective_compression_count = 0
|
||||
# Consume the pending-verification flag once real usage arrives, whether
|
||||
# or not prompt_tokens was reported, so a usage-less response can't leave
|
||||
# it armed for a later, unrelated reading.
|
||||
self._verify_compaction_cleared_threshold = False
|
||||
self.awaiting_real_usage_after_compression = False
|
||||
|
||||
def should_defer_preflight_to_real_usage(self, rough_tokens: int) -> bool:
|
||||
|
|
@ -1237,41 +1273,8 @@ class ContextCompressor(ContextEngine):
|
|||
where each pass removes only 1-2 messages.
|
||||
"""
|
||||
tokens = prompt_tokens if prompt_tokens is not None else self.last_prompt_tokens
|
||||
|
||||
# Verify the previous compaction against the SAME measure this method
|
||||
# gates on. Effectiveness is "did the prompt get under the threshold?",
|
||||
# not "did the message list shrink?": compaction can only shrink
|
||||
# messages, while the system prompt and tool schemas are an
|
||||
# incompressible floor (with 50+ tools, 20-30K tokens — see #14695).
|
||||
# When that floor alone meets the threshold, every pass can shrink
|
||||
# messages by a healthy margin and still leave the prompt over the line,
|
||||
# so the next turn compacts again, forever.
|
||||
#
|
||||
# Both sides of this comparison are the caller's own token count, so it
|
||||
# is immune to the skew between a rough estimate and the provider's real
|
||||
# tokenizer — an analytic "floor = real_prompt - rough_estimate" is not,
|
||||
# and would misread that skew as an incompressible floor.
|
||||
if tokens < self.threshold_tokens:
|
||||
# The prompt fits. Whatever happened before, we are not thrashing.
|
||||
self._ineffective_compression_count = 0
|
||||
self._verify_compaction_cleared_threshold = False
|
||||
return False
|
||||
|
||||
# Past this point the prompt is over the threshold. If a compaction just
|
||||
# ran, it failed to clear it — one strike. Two strikes and we stop and
|
||||
# tell the user, instead of compacting on every turn forever.
|
||||
if self._verify_compaction_cleared_threshold:
|
||||
self._verify_compaction_cleared_threshold = False
|
||||
self._ineffective_compression_count += 1
|
||||
if not self.quiet_mode:
|
||||
logger.warning(
|
||||
"Compaction did not clear the threshold: %d tokens still "
|
||||
">= %d. The incompressible prompt (system prompt + tool "
|
||||
"schemas) may already exceed it, in which case shrinking "
|
||||
"messages cannot help. ineffective_compression_count=%d",
|
||||
tokens, self.threshold_tokens,
|
||||
self._ineffective_compression_count,
|
||||
)
|
||||
# Do not trigger compression while the summary LLM is in cooldown.
|
||||
# On a 429/transient failure _generate_summary() sets a cooldown and
|
||||
# returns None; compress() then inserts a static fallback marker and
|
||||
|
|
|
|||
|
|
@ -15,14 +15,20 @@ Two defects made it dead code:
|
|||
|
||||
Together these made a mis-sized context window present as a hung CLI rather than a
|
||||
warning. Effectiveness is now scored against the goal -- did the prompt get under the
|
||||
threshold? -- and the check is made in ``should_compress()`` against the caller's own
|
||||
token count on both sides.
|
||||
threshold? -- judged in ``update_from_response()``, the one place that sees the
|
||||
provider's real prompt count for the just-compacted conversation.
|
||||
|
||||
That last detail matters. An analytic ``floor = current_tokens - rough_estimate(messages)``
|
||||
looks equivalent and is not: it silently absorbs the skew between the rough estimator and
|
||||
the provider's real tokenizer, reports it as incompressible overhead, and disables
|
||||
compaction on a perfectly healthy session. ``test_no_false_positive_under_tokenizer_skew``
|
||||
pins that.
|
||||
Two subtleties this pins:
|
||||
|
||||
* The verdict must NOT live in ``should_compress()``. That runs twice per turn with
|
||||
two different measures (a rough preflight estimate and the real post-response count,
|
||||
#36718); the rough one can dip below the threshold and reset the strike every turn,
|
||||
re-opening the loop. ``test_rough_preflight_reading_does_not_reopen_the_loop``.
|
||||
|
||||
* It must not be computed analytically as ``floor = current_tokens - rough_estimate``.
|
||||
That subtracts an estimate from a real count, absorbs the tokenizer skew into "floor",
|
||||
and disables compaction on a healthy session.
|
||||
``test_no_false_positive_under_tokenizer_skew``.
|
||||
"""
|
||||
import pytest
|
||||
|
||||
|
|
@ -52,6 +58,22 @@ def _messages(n: int, size: int = 1500) -> list:
|
|||
return msgs
|
||||
|
||||
|
||||
def _turn(cc, msgs, real_prompt_tokens):
|
||||
"""One agent turn as conversation_loop drives it.
|
||||
|
||||
should_compress() gates on the real prompt count; if it fires, compress()
|
||||
runs and the provider then reports real usage for the shorter conversation.
|
||||
The anti-thrashing verdict is judged in update_from_response() against that
|
||||
real count -- not inside should_compress(), which is called twice per turn
|
||||
with two different measures. Returns (msgs, did_compact).
|
||||
"""
|
||||
if not cc.should_compress(real_prompt_tokens):
|
||||
return msgs, False
|
||||
msgs = cc.compress(msgs, current_tokens=real_prompt_tokens)
|
||||
cc.update_from_response({"prompt_tokens": real_prompt_tokens})
|
||||
return msgs, True
|
||||
|
||||
|
||||
class TestSavingsBasis:
|
||||
def test_savings_does_not_depend_on_current_tokens(self):
|
||||
"""Savings is a property of the messages, not of the caller's token count.
|
||||
|
|
@ -88,22 +110,48 @@ class TestFutilityGuard:
|
|||
cc = _compressor(threshold_tokens=24_576)
|
||||
msgs = _messages(14)
|
||||
# Full prompt is far above threshold and dominated by system + tools,
|
||||
# so it never drops no matter how much the message list shrinks.
|
||||
# so the real count never drops no matter how the message list shrinks.
|
||||
real_prompt_tokens = 33_564
|
||||
|
||||
fired = 0
|
||||
for _ in range(6):
|
||||
if cc.should_compress(real_prompt_tokens):
|
||||
msgs, did = _turn(cc, msgs, real_prompt_tokens)
|
||||
if did:
|
||||
fired += 1
|
||||
msgs = cc.compress(msgs, current_tokens=real_prompt_tokens)
|
||||
msgs.append({"role": "user", "content": "next " + "w" * 4000})
|
||||
|
||||
assert cc._ineffective_compression_count >= 2
|
||||
assert not cc.should_compress(real_prompt_tokens), (
|
||||
"compaction that cannot clear the threshold must stop and warn"
|
||||
"compaction that cannot clear the threshold must stop"
|
||||
)
|
||||
assert fired <= 3, f"expected the loop to break early, compacted {fired}x"
|
||||
|
||||
def test_rough_preflight_reading_does_not_reopen_the_loop(self):
|
||||
"""should_compress() runs twice per turn with two different measures.
|
||||
|
||||
The pre-API gate uses a rough estimate that can dip below the threshold;
|
||||
the post-response gate uses the real count that does not. If the verdict
|
||||
lived in should_compress(), the rough reading would reset the strike
|
||||
every turn and the loop would never stop. Judging it in
|
||||
update_from_response() (real-vs-real) closes that hole.
|
||||
"""
|
||||
cc = _compressor(threshold_tokens=24_576)
|
||||
msgs = _messages(13)
|
||||
rough, real = 20_000, 33_564 # rough dips under; real never does
|
||||
|
||||
fired = 0
|
||||
for _ in range(8):
|
||||
cc.should_compress(rough) # pre-API gate (rough)
|
||||
msgs, did = _turn(cc, msgs, real) # post-response gate (real) + usage
|
||||
if did:
|
||||
fired += 1
|
||||
msgs.append({"role": "user", "content": "more " + "w" * 3000})
|
||||
|
||||
assert fired <= 2, (
|
||||
f"a sub-threshold rough reading must not re-open the loop; "
|
||||
f"compacted {fired}x"
|
||||
)
|
||||
|
||||
def test_effective_compaction_still_resets_the_counter(self):
|
||||
"""A compaction that gets the prompt under the threshold is not thrashing."""
|
||||
cc = _compressor(threshold_tokens=750_000)
|
||||
|
|
@ -131,26 +179,34 @@ class TestFutilityGuard:
|
|||
real_prompt = floor + int(skew * estimate_messages_tokens_rough(msgs))
|
||||
assert cc.should_compress(real_prompt)
|
||||
msgs = cc.compress(msgs, current_tokens=real_prompt)
|
||||
real_prompt = floor + int(skew * estimate_messages_tokens_rough(msgs))
|
||||
real_after = floor + int(skew * estimate_messages_tokens_rough(msgs))
|
||||
cc.update_from_response({"prompt_tokens": real_after}) # provider's real count
|
||||
|
||||
assert real_prompt < cc.threshold_tokens, "compaction really did clear it"
|
||||
assert not cc.should_compress(real_prompt)
|
||||
assert real_after < cc.threshold_tokens, "compaction really did clear it"
|
||||
assert cc._ineffective_compression_count == 0, (
|
||||
"tokenizer skew must not be mistaken for an incompressible floor"
|
||||
)
|
||||
|
||||
def test_counter_resets_once_the_prompt_fits_again(self):
|
||||
"""One failed pass must not permanently disable compaction."""
|
||||
def test_a_failed_pass_records_exactly_one_strike(self):
|
||||
"""A compaction that leaves the real prompt over the threshold: one strike.
|
||||
|
||||
The verdict is judged once, when the provider reports real usage — not on
|
||||
every should_compress() reading.
|
||||
"""
|
||||
cc = _compressor(threshold_tokens=24_576)
|
||||
msgs = _messages(14)
|
||||
|
||||
cc.should_compress(33_564)
|
||||
assert cc.should_compress(33_564)
|
||||
cc.compress(msgs, current_tokens=33_564)
|
||||
cc.should_compress(33_564) # still over -> 1 strike
|
||||
assert cc._ineffective_compression_count == 0, "no verdict before real usage"
|
||||
|
||||
cc.update_from_response({"prompt_tokens": 33_564}) # still over
|
||||
assert cc._ineffective_compression_count == 1
|
||||
|
||||
cc.should_compress(1_000) # now under -> forgiven
|
||||
assert cc._ineffective_compression_count == 0
|
||||
# A later reading, rough or real, must not add phantom strikes.
|
||||
cc.should_compress(33_564)
|
||||
cc.should_compress(20_000)
|
||||
assert cc._ineffective_compression_count == 1
|
||||
|
||||
|
||||
class TestMinimumMessagesBranch:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue