Repository navigation
fix(openai): recompress cached Responses units whose CCR entries expired - #4146
stephan-nb wants to merge 3 commits into
Conversation
PR governanceThis PR does not yet satisfy the required template fields:
Please update the PR body, or move the PR back to draft while it is still in progress. |
github-actions
Bot
added
the
status: needs author action
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| def test_openai_responses_unit_cache_recompresses_when_ccr_entry_expired(): | ||
| # The unit cache has no TTL; the CCR entry behind its marker does. Reusing the | ||
| # cached result after the entry expired would ship a marker no retrieval can | ||
| # redeem, so the unit must be compressed (and its entry stored) again. | ||
| from headroom.cache.compression_store import ( | ||
| CompressionStore, | ||
| clear_request_compression_store, | ||
| set_request_compression_store, | ||
| ) | ||
|
|
||
| store = CompressionStore(max_entries=100, default_ttl=60) | ||
| set_request_compression_store(store) | ||
| try: | ||
| handler, calls = _marker_compressing_handler(store) | ||
| long_text = " ".join(f"word{i}" for i in range(180)) | ||
|
|
||
| first = _compress_shell_output(handler, long_text, "req_expire_one") | ||
| hash_key = first.rsplit("hash=", 1)[1].rstrip("]") | ||
| assert store.exists(hash_key) | ||
|
|
||
| store._backend.delete(hash_key) # what TTL expiry does to the entry | ||
| assert not store.exists(hash_key) | ||
|
|
||
| second = _compress_shell_output(handler, long_text, "req_expire_two") | ||
|
|
||
| assert calls["count"] == 2 | ||
| assert second == first | ||
| assert store.exists(hash_key) | ||
| finally: | ||
| clear_request_compression_store() |
There was a problem hiding this comment.
🔍 After-fix runtime proof is missing
The PR's runtime evidence comes from the unpatched proxy; it explicitly says the patched proxy was not tested end to end. These mocked tests do not supply the after-fix evidence required for Real Behavior Proof.
Was this helpful? React with 👍 or 👎 to provide feedback.
| original = getattr(result, "original", "") | ||
| if isinstance(original, str): | ||
| hashes.difference_update( | ||
| match.group(1).lower() for match in _CCR_HASH_RE.finditer(original) | ||
| ) |
There was a problem hiding this comment.
🔴 Expired generated marker survives cache hits
When a unit generates a marker already present in its input, hashes.difference_update excludes that generated hash. After its CCR entry expires, cached output keeps an unredeemable marker instead of recompressing.
Learn more
A unit can contain a CCR marker while also compressing marker-free text beside it. The marker-preserving compressor routes that text separately, and its newly stored entry can have the same hash as the inherited marker. Removing every hash found in original also removes that newly generated hash. After the entry expires, cache hits skip the liveness check for it and ship an unredeemable generated marker.
Example: A previous output carries marker H for text T. A later tool output contains T followed by that marker. Compressing T again stores H and emits another H; after H expires, this unit stays cached and sends both expired markers instead of restoring the entry.
Recommended fix: Track hashes newly stored during compression rather than inferring their origin from set subtraction. If that is unavailable, preserve liveness checks for hashes shared by the input and generated text, with a test covering the duplicate-hash case.
Was this helpful? React with 👍 or 👎 to provide feedback.
This branch has not been deployed
1 participant
Description
The Responses unit-result cache has no TTL, but the CCR entries behind the markers in its results expire after the CCR TTL and are written only when a unit is compressed. After expiry, a cache hit reuses a result whose marker no retrieval can redeem (
Content not found). The result is silent lossy compression on long-running proxies._get_openai_responses_cached_unitnow checks, on a hit, that every CCR hash in the cachedcompressedtext stillexists()in the compression store. Hashes come from the existing_CCR_HASH_RE, which matches<<ccr:...>>,Retrieve more: hash=andRetrieve original: hash=. If one is missing, the entry is evicted and the lookup returnsNone. The unit then goes through_compress_and_store, which writes the CCR entry again. Results without markers and an unreachable store keep the old behaviour.Fixes #4145.
Type of Change
Changes Made
headroom/proxy/handlers/openai.py:_openai_responses_cached_ccr_markers_live()added;_get_openai_responses_cached_unit()evicts and misses when it returns False.tests/test_openai_responses_compression_units.py: two tests...._recompresses_when_ccr_entry_expired: the entry is deleted after the first request; the second request compresses again (2 router calls), produces the same output and restores the entry...._reuses_result_while_ccr_entry_live: a live entry still gives a cache hit (1 router call).Testing
pytest tests/test_openai_responses_compression_units.py: 46 passed (44 before plus the 2 new).pytest tests/test_openai_codex_routing.py tests/test_openai_responses_compression_units.py: 87 passed...._recompresses_when_ccr_entry_expiredfails (assert 1 == 2); the live-entry test passes either way.ruff checkandruff format --checkclean on both files.maturinfrommainat976aa714(Python 3.14, macOS arm64).Real Behavior Proof
catof three prose files (38.7 KB) through Codex on the same model, more than 1800 s apart, without restarting the proxy. Then callheadroom_retrieveon the shipped marker hash.0a5f6643447032a6ec5a2528and9498590711d246862a70f1cf.headroom_retrievereturnsContent not foundandccr_entrieshas no row. A model with no cache entry recompresses and the rows reappear.Runtime Rollout Safety
Notes for review
cache.get(key) is result), so a concurrent re-store is not dropped.store(..., explicit_hash=...)for producers with their own hash). If any producer emits a marker hash that is not its exact store key, this check would turn its hits into misses. That is a performance cost, not a correctness one.Review Readiness
Checklist
CHANGELOG.md