Skip to content

Commit 08fb845

Browse files
fix(ccr): return stored content when headroom_retrieve query matches nothing (#1213) (#1236)
## 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, "<<small>>")` → `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": <text>, "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
1 parent bd55a42 commit 08fb845

2 files changed

Lines changed: 44 additions & 0 deletions

File tree

headroom/ccr/mcp_server.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,23 @@ async def _retrieve_content(
424424
"results": results,
425425
"count": len(results),
426426
}
427+
# The query matched no items above the relevance floor, but the
428+
# entry itself may still be present and unexpired. An empty search
429+
# is not the same as a missing/expired hash, so fall back to the
430+
# full content rather than reporting it as not found.
431+
entry = store.retrieve(hash_key)
432+
if entry:
433+
self._stats.record_retrieval(hash_key)
434+
return {
435+
"hash": hash_key,
436+
"source": "local",
437+
"query": query,
438+
"results": [],
439+
"count": 0,
440+
"original_content": entry.original_content,
441+
"note": "Entry exists but no item matched the query above "
442+
"the relevance threshold; returning the full content.",
443+
}
427444
else:
428445
entry = store.retrieve(hash_key)
429446
if entry:

tests/test_ccr_mcp_server.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,3 +65,30 @@ def test_mcp_retrieves_proxy_stored_content(fresh_store) -> None:
6565

6666
assert result.get("source") == "local"
6767
assert result["original_content"] == original
68+
69+
70+
def test_mcp_retrieve_with_nonmatching_query_returns_full_content(fresh_store) -> None:
71+
"""A query that matches no item above the relevance floor must still return
72+
the stored entry (it exists and is unexpired) rather than the "Content not
73+
found" error, which is reserved for genuine misses."""
74+
pytest.importorskip("mcp", reason="MCP SDK required")
75+
original = "the the the the the the the the the the\n" * 5
76+
hash_key = get_compression_store().store(original, "<<small>>")
77+
# Precondition: the query genuinely matches nothing above the BM25 floor.
78+
assert get_compression_store().search(hash_key, "zzqx_nonmatching_token") == []
79+
80+
server = mcp_server.HeadroomMCPServer(check_proxy=False)
81+
result = asyncio.run(server._retrieve_content(hash_key, query="zzqx_nonmatching_token"))
82+
83+
assert "error" not in result
84+
assert result.get("source") == "local"
85+
assert result["original_content"] == original
86+
assert result["count"] == 0
87+
88+
89+
def test_mcp_retrieve_missing_hash_still_errors(fresh_store) -> None:
90+
"""A genuinely missing hash must still report "Content not found"."""
91+
pytest.importorskip("mcp", reason="MCP SDK required")
92+
server = mcp_server.HeadroomMCPServer(check_proxy=False)
93+
result = asyncio.run(server._retrieve_content("nonexistent_hash", query="anything"))
94+
assert "Content not found" in result.get("error", "")

0 commit comments

Comments
 (0)