From 08fb845fe37478af2c2f55c402df77d7a448fc86 Mon Sep 17 00:00:00 2001 From: jichaowang02-lang Date: Sun, 21 Jun 2026 17:15:49 +0100 Subject: [PATCH] fix(ccr): return stored content when headroom_retrieve query matches nothing (#1213) (#1236) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Fixes #1213. `headroom_retrieve` with a `query` returns *"Content not found"* for entries that exist and are unexpired, whenever the query matches no item above the BM25 relevance floor. `HeadroomMCPServer._retrieve_content`'s `query` branch returns only inside `if results:`. An empty `store.search()` result — legitimate when no item clears `score_threshold=0.3` (common for repetitive / low-diversity content, or a query token that matches nothing) — falls through to the generic *"Content not found. It may have expired or the hash may be incorrect."* error, even though `store.retrieve(hash_key)` would return the entry. This conflates *hash missing/expired* with *query matched zero items* and silently discards a valid entry. The `query=None` branch already does the right thing (`store.retrieve`), so the two paths were asymmetric. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - `headroom/ccr/mcp_server.py`: in `_retrieve_content`, when `query` is given but `store.search()` returns empty, fall back to `store.retrieve(hash_key)` and return the full content (`results=[]`, `count=0`, plus an explanatory `note`) instead of falling through. Genuine misses (`retrieve` → `None`) still reach the "Content not found" error. - `tests/test_ccr_mcp_server.py`: regression tests (below). ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check`) - [ ] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text $ pytest tests/test_ccr_mcp_server.py -q 5 passed $ ruff check headroom/ccr/mcp_server.py tests/test_ccr_mcp_server.py All checks passed! $ ruff format --check ... 2 files already formatted ``` ## Real Behavior Proof - Environment: Python 3.13, `HeadroomMCPServer(check_proxy=False)` against the real shared `CompressionStore` (no proxy / network). - Exact command / steps: `store.store(repetitive_text, "<>")` → `hash`; then `_retrieve_content(hash, query="zzqx_nonmatching_token")`. - Observed result: **before** the fix → `{"error": "Content not found. ..."}` while `store.retrieve(hash)` returns the entry; **after** → `{"source": "local", "original_content": , "count": 0, "note": "Entry exists but no item matched ..."}`. A genuinely missing hash still returns the error. - Not tested: end-to-end through a running proxy / live MCP client (verified at the store + `_retrieve_content` level, which is where the bug lives). ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review --- headroom/ccr/mcp_server.py | 17 +++++++++++++++++ tests/test_ccr_mcp_server.py | 27 +++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/headroom/ccr/mcp_server.py b/headroom/ccr/mcp_server.py index 650e375bc..5cdfc37dd 100644 --- a/headroom/ccr/mcp_server.py +++ b/headroom/ccr/mcp_server.py @@ -424,6 +424,23 @@ class HeadroomMCPServer: "results": results, "count": len(results), } + # The query matched no items above the relevance floor, but the + # entry itself may still be present and unexpired. An empty search + # is not the same as a missing/expired hash, so fall back to the + # full content rather than reporting it as not found. + entry = store.retrieve(hash_key) + if entry: + self._stats.record_retrieval(hash_key) + return { + "hash": hash_key, + "source": "local", + "query": query, + "results": [], + "count": 0, + "original_content": entry.original_content, + "note": "Entry exists but no item matched the query above " + "the relevance threshold; returning the full content.", + } else: entry = store.retrieve(hash_key) if entry: diff --git a/tests/test_ccr_mcp_server.py b/tests/test_ccr_mcp_server.py index d217593e2..7ecb983d0 100644 --- a/tests/test_ccr_mcp_server.py +++ b/tests/test_ccr_mcp_server.py @@ -65,3 +65,30 @@ def test_mcp_retrieves_proxy_stored_content(fresh_store) -> None: assert result.get("source") == "local" assert result["original_content"] == original + + +def test_mcp_retrieve_with_nonmatching_query_returns_full_content(fresh_store) -> None: + """A query that matches no item above the relevance floor must still return + the stored entry (it exists and is unexpired) rather than the "Content not + found" error, which is reserved for genuine misses.""" + pytest.importorskip("mcp", reason="MCP SDK required") + original = "the the the the the the the the the the\n" * 5 + hash_key = get_compression_store().store(original, "<>") + # Precondition: the query genuinely matches nothing above the BM25 floor. + assert get_compression_store().search(hash_key, "zzqx_nonmatching_token") == [] + + server = mcp_server.HeadroomMCPServer(check_proxy=False) + result = asyncio.run(server._retrieve_content(hash_key, query="zzqx_nonmatching_token")) + + assert "error" not in result + assert result.get("source") == "local" + assert result["original_content"] == original + assert result["count"] == 0 + + +def test_mcp_retrieve_missing_hash_still_errors(fresh_store) -> None: + """A genuinely missing hash must still report "Content not found".""" + pytest.importorskip("mcp", reason="MCP SDK required") + server = mcp_server.HeadroomMCPServer(check_proxy=False) + result = asyncio.run(server._retrieve_content("nonexistent_hash", query="anything")) + assert "Content not found" in result.get("error", "")