fix(compression): clamp recommended threshold to small-context floor (#67422) - #67450
fix(compression): clamp recommended threshold to small-context floor (#67422)#67450webtecnica wants to merge 1 commit into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for identifying a real warning mismatch: current main emits a raw aux_context / main_ctx recommendation in agent/conversation_compression.py:312-353, while the built-in compressor applies a 75% floor at agent/context_compressor.py:1209-1223.
Problems
- The new
_threshold_below_floortest still compares against the raw context window. The actual persisted trigger is computed from the effective input budget, includingmax_tokensreservation, the 64K floor, and the degenerate-window rule (agent/context_compressor.py:1226-1265). This can suppress a threshold recommendation that would be viable after recomputation. - The diff also adds lock refresh, session-rotation checks, user-anchor rewriting, and immediate rotated-session persistence, although the stated fix is only warning guidance. It adds no tests despite these session-state changes.
Suggested changes
- Split the warning change from the unrelated compression/session work.
- Compute built-in-compressor viability through the same effective-percent and
_compute_threshold_tokens()contract, and add real-compressor regressions for an output-reservation case plus external-engine passthrough.
Automated hermes-sweeper review.
| _SMALL_CTX_THRESHOLD_PERCENT, | ||
| ) | ||
| _min_floor_pct = int(_SMALL_CTX_THRESHOLD_PERCENT * 100) | ||
| _threshold_below_floor = ( |
There was a problem hiding this comment.
This raw aux_context / main_ctx gate does not match the trigger that a persisted setting will produce: _compute_threshold_tokens() uses the effective input window after max_tokens reservation plus the 64K and degenerate-window rules (agent/context_compressor.py:1226-1265). Recompute that token trigger before deciding that a 75% setting is non-viable.
| return False | ||
|
|
||
|
|
||
| def _refresh_persisted_compression_guards(compressor: Any) -> None: |
There was a problem hiding this comment.
This lock-guard refresh is unrelated to the threshold-warning fix and begins a large session-state change set in this PR. Please move it to a focused compression/session-rotation change so the warning correction can be evaluated independently.
|
The warning-floor bug is fixed via #69332 (merged) — salvage of #67431, which mirrors the full trigger recomputation and was the earliest submission. Your omit-when-unreachable idea was directionally right and is credited in the merged PR body. The unrelated ~300 lines bundled here (rotation guards, user-anchor rewrite, breaker refresh) would need to be re-filed as separate focused PRs to be reviewable — happy to look at each on its own merits. Thanks! |
Closes #67422
Problem
The auxiliary-compression feasibility warning can recommend a
compression.thresholdvalue below the small-context floor (75%). Models with context windows under 512K have their threshold raised by_effective_threshold_percent, so a recommended value like0.73gets raised back to0.75on the next session, causing the warning to repeat indefinitely.Fix
The warning in
check_compression_model_feasibility()now accounts for the same small-context floor used byContextCompressor._effective_threshold_percent():aux_context < main_ctx * 0.75), the warning omits the threshold recommendation entirely and explains that a larger auxiliary compression model is required — since lowering the threshold wouldn't help.Changes
agent/conversation_compression.py: Added small-context floor awareness to the warning recommendation incheck_compression_model_feasibility().