fix(compression): mirror the full trigger recomputation in the suggestion guard
Review follow-up on #67431 (hermes-sweeper): - The viability check compared the floored percentage against the raw context window, but the built-in trigger recomputation also applies the output-token reservation, the 64K floor, and the degenerate-window guard (_compute_threshold_tokens). Mirror that math exactly, so e.g. a 200K window with max_tokens=120K recomputes to max(0.75*80K, 64K)=64K and the suggestion is correctly KEPT for an 80K aux model instead of being suppressed by the raw-window percentage. - Gate the built-in policy behind isinstance(ContextCompressor): external context engines own compaction policy (#44439), so plugin engines keep the plain suggestion untouched. - The non-viable explanation now names the recomputed trigger instead of hardcoding the 75%/512K wording, so it stays accurate when the reservation (not the percentage floor) is what makes the value unreachable. Tests: reservation-viability regression and plugin-engine passthrough, per the review; the floored-branch assertion updated to the recomputed number. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
dbc71fb6e4
commit
19a59f7d7b
|
|
@ -504,19 +504,28 @@ def check_compression_model_feasibility(agent: Any) -> None:
|
|||
new_threshold / main_ctx
|
||||
)
|
||||
safe_pct = int((aux_context / main_ctx) * 100) if main_ctx else 50
|
||||
# The "lower the threshold" suggestion must survive the
|
||||
# compressor's raise-only small-context floor (#67422): for main
|
||||
# windows under 512K, _effective_threshold_percent() raises any
|
||||
# configured value below 75% back up, so recommending e.g. 0.40
|
||||
# would be silently ignored and this warning would reappear every
|
||||
# session. Only offer the option when the floored value still
|
||||
# fits the auxiliary model's context.
|
||||
# The "lower the threshold" suggestion must survive the built-in
|
||||
# trigger recomputation (#67422): _effective_threshold_percent()
|
||||
# raises sub-75% values back up for main windows under 512K, and
|
||||
# _compute_threshold_tokens() further applies the output-token
|
||||
# reservation, the 64K floor, and the degenerate-window guard.
|
||||
# Recommending a value those would override is silently ignored
|
||||
# and this warning would reappear every session — so mirror the
|
||||
# compressor's own math and only offer the option when the
|
||||
# recomputed trigger actually fits the auxiliary model's context.
|
||||
# External engines own compaction policy (#44439); the built-in
|
||||
# floor doesn't apply to them, so keep the plain suggestion.
|
||||
from agent.context_compressor import ContextCompressor as _CC
|
||||
|
||||
recomputed_threshold = None
|
||||
if main_ctx and isinstance(agent.context_compressor, _CC):
|
||||
recomputed_threshold = _CC._compute_threshold_tokens(
|
||||
main_ctx,
|
||||
_CC._effective_threshold_percent(main_ctx, safe_pct / 100),
|
||||
getattr(agent.context_compressor, "max_tokens", None),
|
||||
)
|
||||
threshold_suggestion_viable = (
|
||||
not main_ctx
|
||||
or _CC._effective_threshold_percent(main_ctx, safe_pct / 100) * main_ctx
|
||||
<= aux_context
|
||||
recomputed_threshold is None or recomputed_threshold <= aux_context
|
||||
)
|
||||
# Build human-readable "model (provider)" labels for both
|
||||
# the main model and the compression model so users can
|
||||
|
|
@ -571,9 +580,11 @@ def check_compression_model_feasibility(agent: Any) -> None:
|
|||
f" compression:\n"
|
||||
f" model: <model-with-{old_threshold:,}+-context>\n"
|
||||
f" (Lowering compression.threshold cannot help here — "
|
||||
f"{_main_label}'s {main_ctx:,}-token window is under "
|
||||
f"512K, where Hermes floors the compression trigger at "
|
||||
f"75% and raises any lower configured value back up.)"
|
||||
f"with {_main_label}'s {main_ctx:,}-token window, "
|
||||
f"Hermes's small-context floor and output reservation "
|
||||
f"would recompute the trigger to "
|
||||
f"{recomputed_threshold:,} tokens, still above the "
|
||||
f"compression model's {aux_context:,}.)"
|
||||
)
|
||||
agent._compression_warning = msg
|
||||
agent._emit_status(msg)
|
||||
|
|
|
|||
|
|
@ -98,9 +98,9 @@ def test_auto_corrects_threshold_when_aux_context_below_threshold(mock_get_clien
|
|||
# 200K main is under the 512K small-context limit and 80K/200K = 40% sits
|
||||
# below the 75% floor — a `threshold:` suggestion would be raised back to
|
||||
# 75% and ignored (#67422), so the message must not offer one and must
|
||||
# explain the floor instead.
|
||||
# explain the recomputed trigger instead (0.75 * 200K = 150K).
|
||||
assert "threshold:" not in messages[0]
|
||||
assert "75%" in messages[0]
|
||||
assert "150,000" in messages[0]
|
||||
# Warning stored for gateway replay
|
||||
assert agent._compression_warning is not None
|
||||
# Threshold on the live compressor was actually lowered to aux_context.
|
||||
|
|
@ -564,3 +564,52 @@ def test_threshold_suggestion_kept_for_large_context_main(mock_get_client, mock_
|
|||
|
||||
assert len(messages) == 1
|
||||
assert "threshold: 0.30" in messages[0]
|
||||
|
||||
|
||||
@patch("agent.model_metadata.get_model_context_length", return_value=80_000)
|
||||
@patch("agent.auxiliary_client.get_text_auxiliary_client")
|
||||
def test_threshold_suggestion_kept_when_reservation_shrinks_trigger(mock_get_client, mock_ctx_len):
|
||||
"""Output-token reservation can make a floored suggestion viable again:
|
||||
with max_tokens=120K on a 200K window, the recomputed trigger is
|
||||
max(0.75 * 80K, 64K) = 64K, which fits the 80K aux — so the suggestion
|
||||
must NOT be suppressed by the raw-window percentage check (sweeper
|
||||
regression for #67422)."""
|
||||
agent = _make_agent(main_context=200_000, threshold_percent=0.50)
|
||||
agent.context_compressor.max_tokens = 120_000
|
||||
mock_client = MagicMock()
|
||||
mock_client.base_url = "https://openrouter.ai/api/v1"
|
||||
mock_client.api_key = "sk-aux"
|
||||
mock_get_client.return_value = (mock_client, "google/gemini-3-flash-preview")
|
||||
|
||||
messages = []
|
||||
agent._emit_status = lambda msg: messages.append(msg)
|
||||
|
||||
agent._check_compression_model_feasibility()
|
||||
|
||||
assert len(messages) == 1
|
||||
assert "threshold: 0.40" in messages[0]
|
||||
|
||||
|
||||
@patch("agent.model_metadata.get_model_context_length", return_value=80_000)
|
||||
@patch("agent.auxiliary_client.get_text_auxiliary_client")
|
||||
def test_plugin_engine_keeps_plain_suggestion(mock_get_client, mock_ctx_len):
|
||||
"""External context engines own compaction policy (#44439) — the built-in
|
||||
small-context floor must not suppress the threshold suggestion when the
|
||||
active compressor is not the built-in ContextCompressor."""
|
||||
agent = _make_agent(main_context=200_000, threshold_percent=0.50)
|
||||
plugin_engine = MagicMock() # not spec'd — fails isinstance(ContextCompressor)
|
||||
plugin_engine.context_length = 200_000
|
||||
plugin_engine.threshold_tokens = 100_000
|
||||
agent.context_compressor = plugin_engine
|
||||
mock_client = MagicMock()
|
||||
mock_client.base_url = "https://openrouter.ai/api/v1"
|
||||
mock_client.api_key = "sk-aux"
|
||||
mock_get_client.return_value = (mock_client, "google/gemini-3-flash-preview")
|
||||
|
||||
messages = []
|
||||
agent._emit_status = lambda msg: messages.append(msg)
|
||||
|
||||
agent._check_compression_model_feasibility()
|
||||
|
||||
assert len(messages) == 1
|
||||
assert "threshold: 0.40" in messages[0]
|
||||
|
|
|
|||
Loading…
Reference in New Issue