feat: add DELETE endpoint for individual memory units - #796
Closed
chriscoey wants to merge 3 commits into
Closed
Conversation
Expose the existing `MemoryEngine.delete_memory_unit()` as an HTTP
DELETE route at `/v1/default/banks/{bank_id}/memories/{memory_id}`.
The engine method (line 3729) already handles CASCADE deletion of links
and entity associations, observation invalidation, and re-consolidation
triggering. This commit only adds the HTTP route and response model.
Closes vectorize-io#791
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tests the new DELETE /v1/default/banks/{bank_id}/memories/{memory_id}
endpoint: successful deletion (200), memory removed from list, and
404 on non-existent or already-deleted memory.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ry_unit The engine's delete_memory_unit() had two gaps: 1. No bank_id filter on SQL queries — a caller could delete a memory from any bank by knowing its UUID, violating bank isolation. 2. No validate_bank_write() call — bypassed write authorization that all other destructive methods enforce. Changes: - Add bank_id parameter to interface and implementation - Add AND bank_id = $2 to SELECT and DELETE queries - Add validate_bank_write() call matching delete_document pattern - Update all callers: HTTP handler, MCP tools, existing tests - Rename response field unit_id → memory_id for consistency - Add model_config with json_schema_extra example - Add UUID validation in handler (400 instead of 500) - Add cross-bank isolation test and invalid UUID test - Use explicit response model construction Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
nicoloboschi
requested changes
Mar 31, 2026
nicoloboschi
left a comment
nicoloboschi
left a comment
Collaborator
There was a problem hiding this comment.
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!
Author
|
Makes sense, you're right. I see the document delete endpoint already handles cascading cleanup correctly. The use case I had in mind was individual memory units becoming stale over time while other units from the same document are still valid — document-level deletion felt too coarse for that. But I see now that the document is the right unit of management here, and punching holes in it at the unit level breaks that integrity. Happy to close this. |
1 task
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #791.
Summary
Add
DELETE /v1/default/banks/{bank_id}/memories/{memory_id}— exposes the existingMemoryEngine.delete_memory_unit()method as an HTTP route.The engine method already handled CASCADE deletion, observation invalidation, and re-consolidation. This PR adds the HTTP route and fixes two gaps in the engine method:
delete_memory_unit()previously accepted onlyunit_idwithout bank scoping — a caller could delete a memory from any bank by knowing its UUID. Now filters withAND bank_id = $2and passesbank_idfrom the URL path.validate_bank_write()call, matching every other destructive method in the engine.Also: response model with OpenAPI example, UUID validation (400 not 500), cross-bank isolation test, updated all callers (MCP tools, existing engine tests).
Note: Checked-in OpenAPI spec and generated clients will need regeneration via
./scripts/generate-openapi.shafter merge.Test plan
test_observation_invalidation.py) — updated to passbank_id