fix(compression): floor safe_pct at small-context threshold before recommending - #67444
fix(compression): floor safe_pct at small-context threshold before recommending#67444JonthanaHanh wants to merge 1 commit into
Conversation
…commending The compression feasibility warning calculates safe_pct independently and can recommend a threshold below the 75% small-context floor applied by ContextCompressor._effective_threshold_percent(). Users who configure the recommended value see the same warning repeat next session. Import the floor constants and apply max(safe_pct, floor_pct) so the warning always recommends a threshold the system will actually honor. Closes NousResearch#67422
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the mismatch between the warning and the small-context floor. The premise is real on current main, but this patch does not fully make the guidance actionable.
Problems
agent/conversation_compression.py:312produces 40% for an 80K auxiliary model and 200K main model. Raising that to 75% at this PR's added line 347 still yields a 150K trigger, which exceeds the 80K auxiliary context.agent/context_compressor.py:1221-1223applies the percent floor, whileagent/context_compressor.py:1253-1265computes the actual trigger. The same warning can therefore recur after following the revised recommendation.- The diff adds no regression coverage. Existing generic warning coverage is in
tests/run_agent/test_compression_feasibility.py:68-99; the 75% compressor floor is covered separately attests/agent/test_context_compressor.py:2512-2518.
Suggested changes
- Base whether to offer
compression.thresholdon the built-in compressor's full recomputed trigger, and omit the option when that trigger cannot fit the auxiliary context. - Add floored-unviable and viable-boundary tests; keep non-built-in context engines out of built-in threshold-policy assumptions.
This is an automated hermes-sweeper review.
| from agent.context_compressor import _SMALL_CTX_WINDOW_LIMIT, _SMALL_CTX_THRESHOLD_PERCENT | ||
| if main_ctx and main_ctx < _SMALL_CTX_WINDOW_LIMIT: | ||
| floor_pct = int(_SMALL_CTX_THRESHOLD_PERCENT * 100) | ||
| safe_pct = max(safe_pct, floor_pct) |
There was a problem hiding this comment.
This changes a 40% recommendation to 75%, but it can still be unusable: for an 80K auxiliary model and 200K main model, the 75% trigger is 150K, so the warning repeats. Please base whether to offer this option on the built-in compressor's full recomputed trigger and omit it when that trigger exceeds aux_context.
|
Fixed via #69332 (merged) — salvage of #67431, which mirrors the compressor's full trigger recomputation (effective floor + output-token reservation + degenerate-window guard) rather than clamping alone; when no achievable threshold helps, it now says so instead of recommending a dead number. Your clamp correctly identified the floor interaction and you're credited as an independent fix in the merged PR body. Thanks! |
Summary
The compression feasibility warning calculates
safe_pctindependently and can recommend acompression.thresholdbelow the 75% small-context floor applied byContextCompressor._effective_threshold_percent(). Users who configure the recommended value see the same warning repeat next session because the floor raises it back to 75%.Fixes #67422
Root Cause
In
conversation_compression.py,safe_pctis calculated asint((aux_context / main_ctx) * 100)without considering the small-context floor. For context windows below 512K,ContextCompressorapplies a hard-coded 75% raise-only floor, but the warning doesn't account for this.Fix
Import
_SMALL_CTX_WINDOW_LIMITand_SMALL_CTX_THRESHOLD_PERCENTfromcontext_compressorand applymax(safe_pct, floor_pct)so the warning always recommends a threshold the system will actually honor.Changes
agent/conversation_compression.py: Added small-context floor check aftersafe_pctcalculation