mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-18 04:41:56 +00:00
fix: clean stale conversation mappings on response eviction/deletion
ResponseStore.put() and .delete() now remove conversations rows that reference evicted or deleted response IDs, preventing 404 errors when a conversation name is reused after its backing response was purged. Adds regression tests for delete, eviction, and handler-level reuse. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
23ac522d37
commit
814c60092b
2 changed files with 86 additions and 5 deletions
|
|
@ -356,15 +356,34 @@ class ResponseStore:
|
|||
# Evict oldest entries beyond max_size
|
||||
count = self._conn.execute("SELECT COUNT(*) FROM responses").fetchone()[0]
|
||||
if count > self._max_size:
|
||||
self._conn.execute(
|
||||
"DELETE FROM responses WHERE response_id IN "
|
||||
"(SELECT response_id FROM responses ORDER BY accessed_at ASC LIMIT ?)",
|
||||
(count - self._max_size,),
|
||||
)
|
||||
# Collect IDs that will be evicted
|
||||
evict_ids = [
|
||||
row[0]
|
||||
for row in self._conn.execute(
|
||||
"SELECT response_id FROM responses ORDER BY accessed_at ASC LIMIT ?",
|
||||
(count - self._max_size,),
|
||||
).fetchall()
|
||||
]
|
||||
if evict_ids:
|
||||
placeholders = ",".join("?" for _ in evict_ids)
|
||||
# Clear conversation mappings pointing to evicted responses
|
||||
self._conn.execute(
|
||||
f"DELETE FROM conversations WHERE response_id IN ({placeholders})",
|
||||
evict_ids,
|
||||
)
|
||||
# Delete evicted responses
|
||||
self._conn.execute(
|
||||
f"DELETE FROM responses WHERE response_id IN ({placeholders})",
|
||||
evict_ids,
|
||||
)
|
||||
self._conn.commit()
|
||||
|
||||
def delete(self, response_id: str) -> bool:
|
||||
"""Remove a response from the store. Returns True if found and deleted."""
|
||||
# Clear conversation mappings pointing to this response
|
||||
self._conn.execute(
|
||||
"DELETE FROM conversations WHERE response_id = ?", (response_id,)
|
||||
)
|
||||
cursor = self._conn.execute(
|
||||
"DELETE FROM responses WHERE response_id = ?", (response_id,)
|
||||
)
|
||||
|
|
|
|||
|
|
@ -105,6 +105,29 @@ class TestResponseStore:
|
|||
store = ResponseStore(max_size=10)
|
||||
assert store.delete("resp_missing") is False
|
||||
|
||||
def test_delete_clears_conversation_mapping(self):
|
||||
"""Deleting a response also removes conversation mappings that reference it."""
|
||||
store = ResponseStore(max_size=10)
|
||||
store.put("resp_1", {"output": "hello"})
|
||||
store.set_conversation("chat-a", "resp_1")
|
||||
assert store.get_conversation("chat-a") == "resp_1"
|
||||
store.delete("resp_1")
|
||||
assert store.get_conversation("chat-a") is None
|
||||
|
||||
def test_eviction_clears_conversation_mapping(self):
|
||||
"""LRU eviction also removes conversation mappings for evicted responses."""
|
||||
store = ResponseStore(max_size=2)
|
||||
store.put("resp_1", {"output": "one"})
|
||||
store.set_conversation("chat-a", "resp_1")
|
||||
store.put("resp_2", {"output": "two"})
|
||||
store.set_conversation("chat-b", "resp_2")
|
||||
# Adding a 3rd should evict resp_1 and its conversation mapping
|
||||
store.put("resp_3", {"output": "three"})
|
||||
assert store.get("resp_1") is None
|
||||
assert store.get_conversation("chat-a") is None
|
||||
# resp_2 mapping should still be intact
|
||||
assert store.get_conversation("chat-b") == "resp_2"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _IdempotencyCache
|
||||
|
|
@ -2870,6 +2893,45 @@ class TestConversationParameter:
|
|||
# Conversation mapping should NOT be set since store=false
|
||||
assert adapter._response_store.get_conversation("ephemeral-chat") is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_conversation_reuse_after_eviction_no_404(self, adapter):
|
||||
"""After eviction clears a conversation mapping, reusing that name starts fresh (no 404)."""
|
||||
adapter._response_store = ResponseStore(max_size=1)
|
||||
app = _create_app(adapter)
|
||||
async with TestClient(TestServer(app)) as cli:
|
||||
with patch.object(adapter, "_run_agent", new_callable=AsyncMock) as mock_run:
|
||||
mock_run.return_value = (
|
||||
{"final_response": "First", "messages": [], "api_calls": 1},
|
||||
{"input_tokens": 10, "output_tokens": 5, "total_tokens": 15},
|
||||
)
|
||||
# Create conversation -> resp stored
|
||||
resp1 = await cli.post("/v1/responses", json={
|
||||
"input": "hello",
|
||||
"conversation": "my-chat",
|
||||
})
|
||||
assert resp1.status == 200
|
||||
|
||||
# Evict by adding another response
|
||||
mock_run.return_value = (
|
||||
{"final_response": "Other", "messages": [], "api_calls": 1},
|
||||
{"input_tokens": 10, "output_tokens": 5, "total_tokens": 15},
|
||||
)
|
||||
await cli.post("/v1/responses", json={"input": "other"})
|
||||
|
||||
# Conversation mapping should have been cleaned by eviction
|
||||
assert adapter._response_store.get_conversation("my-chat") is None
|
||||
|
||||
# Reuse conversation name — should start fresh, not 404
|
||||
mock_run.return_value = (
|
||||
{"final_response": "Restarted", "messages": [], "api_calls": 1},
|
||||
{"input_tokens": 10, "output_tokens": 5, "total_tokens": 15},
|
||||
)
|
||||
resp3 = await cli.post("/v1/responses", json={
|
||||
"input": "hello again",
|
||||
"conversation": "my-chat",
|
||||
})
|
||||
assert resp3.status == 200
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# X-Hermes-Session-Id header (session continuity)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue