diff --git a/CHANGELOG.md b/CHANGELOG.md index 85b360cff..bb71d3f99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -102,6 +102,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Bug Fixes +* **cache/ccr:** stop counting a successful eviction as a retrieval in the compression feedback learner, which inverted the learning signal. When an entry is evicted without ever being retrieved, `CompressionStore` emits a synthetic `retrieval_type="eviction_success"` event to mark that the compression was sufficient (the LLM never needed the original). `CompressionFeedback.record_retrieval` had no branch for it, so — because the type is not `"full"` — it was counted as a *search retrieval*, inflating the tool's `retrieval_rate`/`search_rate`. `get_compression_hints` reads a high retrieval rate as "compressing too aggressively" and backs off, so a compression that actually worked pushed the learner toward *less* compression (a standalone repro scores one successful eviction as a 100% retrieval rate). The event is now recognized and left out of the retrieval counters; the compression is still counted by `record_compression` at store time, so a never-retrieved entry correctly yields a low retrieval rate. Genuine retrievals are unaffected. * **proxy/anthropic:** don't launder a non-2xx upstream into HTTP 200 when enterprise security scans the response. On the non-streaming `/v1/messages` path the response-scan branch rebuilt the reply as `httpx.Response(status_code=200)` and returned it without checking the upstream status, so a rate-limit (429), overloaded (529), or other 4xx error whose JSON body was scanned reached the client as an HTTP 200 — the client's retry/backoff never fired and an error looked like success. The branch is now gated on a 200 upstream, matching the sibling CCR/cache/buffered-stream blocks in the same handler; non-2xx responses fall through and keep their real status. * **learn:** classify timeout and connection tool failures correctly instead of as generic runtime errors. In `classify_error` the generic `RUNTIME_ERROR` pattern (`Traceback|Exception:|Error:`) was checked before the dedicated `TIMEOUT` and `CONNECTION_ERROR` patterns. Because every Python exception repr is `XxxError: ...`, a `TimeoutError: ...` or `ConnectionError: ...` matched the catch-all first and was miscategorized as `RUNTIME_ERROR`, leaving those two categories unreachable for the common colon-repr form (they only fired for tokenless phrasings like `deadline exceeded`). The `TIMEOUT` and `CONNECTION_ERROR` patterns are now checked before the generic catch-all; tokenless generic errors still classify as `RUNTIME_ERROR`. * **tokenizers:** resolve HuggingFace tokenizer names by the most-specific prefix. `get_tokenizer_name` scanned `MODEL_TO_TOKENIZER` in dict-insertion order and returned the first key the model merely starts with, so a short family key shadowed a more-specific one — `qwen2-7b-instruct` matched `qwen` before `qwen2`/`qwen2-7b` and resolved to the Qwen1 tokenizer (a different vocabulary, hence wrong token counts); `qwen2.5-*` and `deepseek-v2.x` were mis-resolved the same way. It now picks the longest matching prefix, mirroring the order-dependent-prefix guard the sibling tiktoken `get_encoding_for_model` already documents. diff --git a/headroom/cache/compression_feedback.py b/headroom/cache/compression_feedback.py index e4a320444..c5fdf7de0 100644 --- a/headroom/cache/compression_feedback.py +++ b/headroom/cache/compression_feedback.py @@ -273,6 +273,19 @@ class CompressionFeedback: if not tool_name: return + # An entry evicted without ever being retrieved is a compression + # SUCCESS, not a retrieval: the LLM never needed the original data (see + # CompressionStore._record_eviction_success). It arrives here as + # retrieval_type="eviction_success"; because that is not "full" it used + # to fall into the search_retrievals branch below and inflate + # retrieval_rate/search_rate, which drove get_compression_hints toward + # LESS aggressive compression -- the inverse of the intended signal. The + # compression itself was already counted by record_compression at store + # time, so a never-retrieved entry already yields a low retrieval rate; + # this event must not be counted as a retrieval. + if event.retrieval_type == "eviction_success": + return + with self._lock: self._total_retrievals += 1 diff --git a/tests/test_ccr_feedback.py b/tests/test_ccr_feedback.py index 077a495e4..5cd8ab1e5 100644 --- a/tests/test_ccr_feedback.py +++ b/tests/test_ccr_feedback.py @@ -86,6 +86,50 @@ class TestCompressionFeedback: assert pattern.retrieval_rate == 0.5 assert pattern.full_retrieval_rate == 1.0 # All were full retrievals + def test_eviction_success_is_not_counted_as_retrieval(self): + """An eviction-without-retrieval is a compression success, not a retrieval. + + The event arrives with retrieval_type="eviction_success". Because that + isn't "full" it used to fall into the search_retrievals branch and + inflate retrieval_rate/search_rate, driving get_compression_hints toward + less aggressive compression — the inverse of the intended signal. It must + leave the retrieval counters untouched. + """ + feedback = CompressionFeedback() + feedback.record_compression("test_tool", 100, 10) + + event = RetrievalEvent( + hash="abc123", + query=None, + items_retrieved=0, + total_items=100, + tool_name="test_tool", + timestamp=time.time(), + retrieval_type="eviction_success", + ) + feedback.record_retrieval(event, strategy="smart") + + pattern = feedback.get_all_patterns()["test_tool"] + assert pattern.total_retrievals == 0 + assert pattern.search_retrievals == 0 + assert pattern.retrieval_rate == 0.0 # a successful compression, not a retrieval + + # A genuine retrieval afterward is still counted. + feedback.record_retrieval( + RetrievalEvent( + hash="def456", + query="find errors", + items_retrieved=50, + total_items=100, + tool_name="test_tool", + timestamp=time.time(), + retrieval_type="search", + ) + ) + pattern = feedback.get_all_patterns()["test_tool"] + assert pattern.total_retrievals == 1 + assert pattern.search_retrievals == 1 + def test_hints_default_with_no_data(self): """Default hints returned when no data exists.""" feedback = CompressionFeedback()