fix(api): clear memories endpoint no longer deletes the bank profile (#837)
DELETE /v1/default/banks/{id}/memories and the MCP clear_memories tool
were calling delete_bank() without distinguishing from the actual delete-bank
endpoint. When no fact_type filter was provided, the bank row itself was
deleted along with its memories.
Add a delete_bank_profile parameter to delete_bank() (default True) and
pass False from all clear-memories callers so the bank profile, disposition,
and background are preserved.
This commit is contained in:
parent
087545cc1b
commit
26a64cc00e
5 changed files with 97 additions and 9 deletions
|
|
@ -5098,7 +5098,7 @@ def _register_routes(app: FastAPI):
|
|||
):
|
||||
"""Clear memories for a memory bank, optionally filtered by type."""
|
||||
try:
|
||||
await app.state.memory.delete_bank(bank_id, fact_type=type, request_context=request_context)
|
||||
await app.state.memory.delete_bank(bank_id, fact_type=type, delete_bank_profile=False, request_context=request_context)
|
||||
|
||||
return DeleteResponse(success=True)
|
||||
except OperationValidationError as e:
|
||||
|
|
|
|||
|
|
@ -240,6 +240,7 @@ class MemoryEngineInterface(ABC):
|
|||
bank_id: str,
|
||||
*,
|
||||
fact_type: str | None = None,
|
||||
delete_bank_profile: bool = True,
|
||||
request_context: "RequestContext",
|
||||
) -> dict[str, int]:
|
||||
"""
|
||||
|
|
@ -248,6 +249,8 @@ class MemoryEngineInterface(ABC):
|
|||
Args:
|
||||
bank_id: The memory bank ID.
|
||||
fact_type: If specified, only delete memories of this type.
|
||||
delete_bank_profile: If True, also delete the bank profile row itself.
|
||||
If False, only delete memories/entities/documents but preserve the bank.
|
||||
request_context: Request context for authentication.
|
||||
|
||||
Returns:
|
||||
|
|
|
|||
|
|
@ -3838,6 +3838,7 @@ class MemoryEngine(MemoryEngineInterface):
|
|||
bank_id: str,
|
||||
fact_type: str | None = None,
|
||||
*,
|
||||
delete_bank_profile: bool = True,
|
||||
request_context: "RequestContext",
|
||||
) -> dict[str, int]:
|
||||
"""
|
||||
|
|
@ -3924,20 +3925,21 @@ class MemoryEngine(MemoryEngineInterface):
|
|||
# Delete entities (cascades to unit_entities, entity_cooccurrences, memory_links with entity_id)
|
||||
await conn.execute(f"DELETE FROM {fq_table('entities')} WHERE bank_id = $1", bank_id)
|
||||
|
||||
# Delete the bank profile and retrieve internal_id for HNSW index cleanup
|
||||
internal_id = await conn.fetchval(
|
||||
f"DELETE FROM {fq_table('banks')} WHERE bank_id = $1 RETURNING internal_id", bank_id
|
||||
)
|
||||
if internal_id:
|
||||
bank_internal_id = str(internal_id)
|
||||
|
||||
result = {
|
||||
"memory_units_deleted": units_count,
|
||||
"entities_deleted": entities_count,
|
||||
"documents_deleted": documents_count,
|
||||
"bank_deleted": True,
|
||||
}
|
||||
|
||||
if delete_bank_profile:
|
||||
# Delete the bank profile and retrieve internal_id for HNSW index cleanup
|
||||
internal_id = await conn.fetchval(
|
||||
f"DELETE FROM {fq_table('banks')} WHERE bank_id = $1 RETURNING internal_id", bank_id
|
||||
)
|
||||
if internal_id:
|
||||
bank_internal_id = str(internal_id)
|
||||
result["bank_deleted"] = True
|
||||
|
||||
except Exception as e:
|
||||
raise Exception(f"Failed to delete agent data: {str(e)}")
|
||||
|
||||
|
|
|
|||
|
|
@ -2873,6 +2873,7 @@ def _register_clear_memories(mcp: FastMCP, memory: MemoryEngine, config: MCPTool
|
|||
result = await memory.delete_bank(
|
||||
target_bank,
|
||||
fact_type=type,
|
||||
delete_bank_profile=False,
|
||||
request_context=_get_request_context(config),
|
||||
)
|
||||
return json.dumps({"status": "cleared", "bank_id": target_bank, **result}, default=str)
|
||||
|
|
@ -2905,6 +2906,7 @@ def _register_clear_memories(mcp: FastMCP, memory: MemoryEngine, config: MCPTool
|
|||
result = await memory.delete_bank(
|
||||
target_bank,
|
||||
fact_type=type,
|
||||
delete_bank_profile=False,
|
||||
request_context=_get_request_context(config),
|
||||
)
|
||||
return {"status": "cleared", "bank_id": target_bank, **result}
|
||||
|
|
|
|||
|
|
@ -584,6 +584,87 @@ async def test_delete_bank(api_client):
|
|||
await api_client.delete(f"/v1/default/banks/{test_bank_id}")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delete_bank_nonexistent(api_client):
|
||||
"""Test deleting a bank that doesn't exist returns success with zero counts."""
|
||||
fake_bank_id = f"nonexistent_bank_{datetime.now().timestamp()}"
|
||||
|
||||
response = await api_client.delete(f"/v1/default/banks/{fake_bank_id}")
|
||||
assert response.status_code == 200
|
||||
result = response.json()
|
||||
assert result["success"] is True
|
||||
assert result["deleted_count"] == 0
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_clear_memories_preserves_bank(api_client):
|
||||
"""Test that clearing memories preserves the bank profile.
|
||||
|
||||
Workflow:
|
||||
1. Create a bank with memories
|
||||
2. Clear all memories via DELETE /memories
|
||||
3. Verify the bank still exists with its profile intact
|
||||
4. Verify all memories are gone
|
||||
"""
|
||||
test_bank_id = f"clear_memories_test_{datetime.now().timestamp()}"
|
||||
|
||||
try:
|
||||
# 1. Create bank with memories
|
||||
response = await api_client.post(
|
||||
f"/v1/default/banks/{test_bank_id}/memories",
|
||||
json={
|
||||
"items": [
|
||||
{"content": "Alice is a software engineer.", "context": "team info"},
|
||||
{"content": "Bob works on infrastructure.", "context": "team info"},
|
||||
]
|
||||
},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
|
||||
# Verify bank exists and has data
|
||||
response = await api_client.get(f"/v1/default/banks/{test_bank_id}/stats")
|
||||
assert response.status_code == 200
|
||||
assert response.json()["total_nodes"] > 0
|
||||
|
||||
response = await api_client.get("/v1/default/banks")
|
||||
assert response.status_code == 200
|
||||
bank_ids = [b["bank_id"] for b in response.json()["banks"]]
|
||||
assert test_bank_id in bank_ids
|
||||
|
||||
# 2. Clear all memories
|
||||
response = await api_client.delete(f"/v1/default/banks/{test_bank_id}/memories")
|
||||
assert response.status_code == 200
|
||||
assert response.json()["success"] is True
|
||||
|
||||
# 3. Bank should still exist in the list
|
||||
response = await api_client.get("/v1/default/banks")
|
||||
assert response.status_code == 200
|
||||
bank_ids = [b["bank_id"] for b in response.json()["banks"]]
|
||||
assert test_bank_id in bank_ids, "Bank should still exist after clearing memories"
|
||||
|
||||
# Profile should still be accessible
|
||||
response = await api_client.get(f"/v1/default/banks/{test_bank_id}/profile")
|
||||
assert response.status_code == 200
|
||||
|
||||
# 4. Memories should be gone
|
||||
response = await api_client.get(f"/v1/default/banks/{test_bank_id}/stats")
|
||||
assert response.status_code == 200
|
||||
assert response.json()["total_nodes"] == 0
|
||||
|
||||
finally:
|
||||
await api_client.delete(f"/v1/default/banks/{test_bank_id}")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_clear_memories_nonexistent_bank(api_client):
|
||||
"""Test clearing memories for a bank that doesn't exist returns success."""
|
||||
fake_bank_id = f"nonexistent_clear_{datetime.now().timestamp()}"
|
||||
|
||||
response = await api_client.delete(f"/v1/default/banks/{fake_bank_id}/memories")
|
||||
assert response.status_code == 200
|
||||
assert response.json()["success"] is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_async_retain(api_client):
|
||||
"""Test asynchronous retain functionality.
|
||||
|
|
|
|||
Loading…
Reference in a new issue