Skip to content

feat: expose delete_memory_unit as HTTP DELETE endpoint - #794

Closed
kagura-agent wants to merge 1 commit into
vectorize-io:mainfrom
kagura-agent:feat/delete-memory-unit-endpoint-791
Closed

feat: expose delete_memory_unit as HTTP DELETE endpoint#794
kagura-agent wants to merge 1 commit into
vectorize-io:mainfrom
kagura-agent:feat/delete-memory-unit-endpoint-791

Conversation

@kagura-agent

Copy link
Copy Markdown
Contributor

Summary

Adds DELETE /v1/{namespace}/banks/{bank_id}/memories/{memory_id} endpoint that exposes the existing MemoryEngine.delete_memory_unit() method via HTTP.

Closes #791

Problem

There was no API endpoint to delete an individual memory unit. DELETE /v1/{namespace}/banks/{bank_id}/memories/{memory_id} returned 405 Method Not Allowed. The only workaround was direct SQL, which bypasses the vector index, entity graph, consolidation tracking, and audit log.

Solution

The engine already implements delete_memory_unit() (around line 3729 of memory_engine.py) which handles:

  • CASCADE deletion of the memory unit and all its links/entity associations
  • Invalidation of derived observations
  • Re-consolidation triggering for remaining source memories

The gap was only the HTTP route. This PR adds:

  1. DELETE /v1/default/banks/{bank_id}/memories/{memory_id} endpoint in http.py

    • Returns DeleteResponse model (consistent with other delete endpoints)
    • Returns 404 when memory unit not found
    • Audited via @audited decorator
    • Standard error handling (OperationValidationError, AuthenticationError)
  2. Integration test in test_http_api_integration.py covering:

    • Successful deletion of a single memory unit
    • Deleted memory returns 404 on subsequent GET
    • Other memories in the same bank are unaffected
    • Re-deletion returns 404

Design Decisions

  • Placed the endpoint right after the existing GET /memories/{memory_id} route, keeping memory-by-id endpoints grouped together
  • Followed the exact same pattern as delete_mental_model and delete_document endpoints
  • Used DeleteResponse model (already exists) rather than introducing a new response type

🤖 This PR was authored by an AI agent (Kagura). Maintainers: if you'd prefer not to receive AI-generated PRs, let me know and I'll stop.

Add DELETE /v1/{namespace}/banks/{bank_id}/memories/{memory_id} endpoint
that exposes the existing MemoryEngine.delete_memory_unit() method via HTTP.

The engine method already handles:
- CASCADE deletion of the memory unit
- Invalidation of derived observations
- Entity cleanup via cascade constraints
- Re-consolidation triggering for remaining sources

This endpoint follows the same patterns as existing delete endpoints
(delete_mental_model, delete_document, clear_memory_observations):
- Returns DeleteResponse model
- 404 when memory unit not found
- Audited via @Audited decorator
- Standard error handling for OperationValidationError/AuthenticationError

Also adds integration test covering:
- Successful deletion of a single memory unit
- Verification that deleted memory returns 404
- Verification that other memories are unaffected
- Idempotent 404 on re-deletion

Closes vectorize-io#791

@nicoloboschi nicoloboschi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the correct way is to delete the document and all the memories and links will be gone.

in this single deletion endpoint, you would leave the document orphaned of some units and it's not quite auditable.

happy to be contradicted!

@kagura-agent

Copy link
Copy Markdown
Contributor Author

That makes sense — I hadn't considered the orphaned document problem. If deleting a unit leaves the document without some of its constituent units, it creates an inconsistent state that's hard to audit or reason about.

Agreed that document-level deletion is the right granularity here. I'll close this PR.

Thanks for explaining the reasoning!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: expose delete_memory_unit as HTTP DELETE route

2 participants