Skip to content

fix(memories): make batch vector ops fault-tolerant, sync vectors on edit/delete-all#7102

Merged
kodjima33 merged 2 commits into
BasedHardware:mainfrom
AasheeshLikePanner:fix/memories-vector-sync
Jun 11, 2026
Merged

fix(memories): make batch vector ops fault-tolerant, sync vectors on edit/delete-all#7102
kodjima33 merged 2 commits into
BasedHardware:mainfrom
AasheeshLikePanner:fix/memories-vector-sync

Conversation

@AasheeshLikePanner

Copy link
Copy Markdown
Contributor

Summary

Fixes issue #7037 - Memories: batch vector upsert not fault-tolerant, edit/delete-all skip vector sync

Changes

  • backend/routers/memories.py:
    • Batch create: Separated Firestore save from vector upsert into separate operations, wrapped vector upsert in try/except to prevent 500 errors when vectors fail (memories still saved)
    • Edit memory: Added vector re-upsert after Firestore update so semantic search returns fresh embeddings
    • Delete all: Fetches memory IDs before Firestore delete, then deletes vectors from Pinecone to prevent orphaned vectors

Impact

Addresses all three failure modes:

  1. Batch create vector failure → returns success, logs error (memories already in Firestore)
  2. Edit memory vector failure → returns success, logs error (content still updated)
  3. Delete all vector failure → returns success, logs error (Firestore deleted, vectors may remain but won't crash)

Testing

  • 47 unit tests passed
  • Logic verified: all three endpoints now follow the same fault-tolerant pattern as single create/delete endpoints (lines 70-73, 177-180)

@greptile-apps

greptile-apps Bot commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR makes vector operations fault-tolerant across three memory endpoints: batch create now separates Firestore from Pinecone so a vector failure doesn't roll back saved memories; edit re-upserts the vector after updating Firestore; and delete-all collects memory IDs before wiping Firestore then bulk-deletes vectors in 1,000-ID chunks. The delete_all_memory_vectors helper correctly chunks Pinecone deletes and wraps each chunk independently.

Confidence Score: 4/5

Safe to merge with minor cleanup; no blocking defects in the changed paths.

Only P2 findings remain: a dead unreachable break branch in the ID-collection loop and a redundant outer try/except that will never fire. The core fault-tolerant logic and chunking are correct.

backend/routers/memories.py — the ID-collection loop and the outer try/except around delete_all_memory_vectors.

Important Files Changed

Filename Overview
backend/routers/memories.py Three fault-tolerant patterns added: batch-create separates Firestore from Pinecone, edit re-upserts vector, delete-all collects IDs before wiping Firestore. Minor issues: duplicate unreachable if not memories: break in the ID-collection loop, and the outer try/except around delete_all_memory_vectors is dead code since that function never raises.
backend/database/vector_db.py New delete_all_memory_vectors helper correctly chunks IDs at 1,000 per Pinecone request and wraps each chunk in its own try/except, matching the fault-tolerant design goal.

Reviews (4): Last reviewed commit: "fix: P1 issues in delete_memories" | Re-trigger Greptile

Comment thread backend/routers/memories.py Outdated
Comment thread backend/routers/memories.py Outdated
@mdmohsin7

Copy link
Copy Markdown
Member

@greptile-apps re-review

Comment thread backend/routers/memories.py
Comment thread backend/database/vector_db.py Outdated
Comment thread backend/database/vector_db.py Outdated
@AasheeshLikePanner AasheeshLikePanner force-pushed the fix/memories-vector-sync branch from dd096c3 to 5e631b1 Compare May 2, 2026 14:19
@AasheeshLikePanner

Copy link
Copy Markdown
Contributor Author

@greptile-apps re-review

@kodjima33 kodjima33 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.

backend — approve only (DIRTY, author needs to rebase)

@kodjima33

Copy link
Copy Markdown
Collaborator

Thanks for this 🙏 — heads up that main has moved under this PR and it needs a manual rebase before it can land:

  • edit_memory re-embed and single delete_memory vector cleanup are already on main (the upsert_memory_vector/delete_memory_vector in try/except), so those parts are now redundant.
  • The batch-create path was refactored on main to run the Firestore write + Pinecone upsert through run_blocking(db_executor, _persist) (not asyncio.to_thread), which conflicts structurally with this PR. Rebasing automatically would risk reverting that change, so I don't want to guess here.

The still-valuable piece is the delete-all orphaned-vector cleanup. Could you rebase onto latest main and re-scope to just that (folding fault-tolerance into main's existing run_blocking structure)? Happy to land it once it's clean. 🙏

@AasheeshLikePanner AasheeshLikePanner force-pushed the fix/memories-vector-sync branch 2 times, most recently from ee67a19 to 771b459 Compare June 11, 2026 06:38
@AasheeshLikePanner AasheeshLikePanner force-pushed the fix/memories-vector-sync branch from 771b459 to c56e183 Compare June 11, 2026 06:40
@AasheeshLikePanner

Copy link
Copy Markdown
Contributor Author

hi @kodjima33 , made some changes. can you check it out?

@kodjima33 kodjima33 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.

Backend bug fix: fault-tolerant batch vector delete + purge vectors on delete-all (#7037); conf 4/5

@kodjima33 kodjima33 merged commit 299bb5a into BasedHardware:main Jun 11, 2026
1 check passed
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.

3 participants