mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
## 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
This commit is contained in:
parent
bd55a426bc
commit
08fb845fe3
2 changed files with 44 additions and 0 deletions
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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, "<<small>>")
|
||||
# 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", "")
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue