Skip to content

fix: resolve data loss atomicity bug in optimization (fixes #7051)#7312

Open
Charanvardhan wants to merge 2 commits intoagno-agi:mainfrom
Charanvardhan:fix-memory-atomicity-7051
Open

fix: resolve data loss atomicity bug in optimization (fixes #7051)#7312
Charanvardhan wants to merge 2 commits intoagno-agi:mainfrom
Charanvardhan:fix-memory-atomicity-7051

Conversation

@Charanvardhan
Copy link
Copy Markdown

Summary

This PR addresses the critical data loss behavior identified in #7051 where both optimize_memories and aoptimize_memories utilize a non-atomic Delete -> Insert flow. Previously, if the database connection dropped or crashed after the deletion stage but before the inserts completed, all of a user's original memories were permanently lost.

Following the approach outlined in the issue, this PR reorders operations to an "insert-before-delete" approach:

  1. Ensure all new optimized memories have unique UUIDs.
  2. Upsert the new optimized memories to the database first.
  3. Identify the original memory IDs and explicitly delete only those old memories after the upserts naturally succeed.

This eliminates the data wipe risk across both sync and async database implementations without negatively impacting base compatibility or requiring cross-database native transactions.

fixes #7051

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Improvement
  • Model update
  • Other:

Checklist

  • Code complies with style guidelines
  • Ran format/validation scripts (./scripts/format.sh and ./scripts/validate.sh)
  • Self-review completed
  • Documentation updated (comments, docstrings)
  • Examples and guides: Relevant cookbook examples have been included or updated (if applicable)
  • Tested in clean environment
  • Tests added/updated (if applicable)

Duplicate and AI-Generated PR Check

  • I have searched existing open pull requests and confirmed that no other PR already addresses this issue
  • If a similar PR exists, I have explained below why this PR is a better approach
  • Check if this PR was entirely AI-generated (by Copilot, Claude Code, Cursor, etc.)

Additional Notes

The unit tests inside libs/agno/tests/unit/memory/test_memory_optimization.py were adjusted since the legacy mock logic asserted against manager.clear_user_memories(), which is deliberately bypassed by this internal logic now directly invoking targeted db.delete_user_memories(memory_ids=...). All new assertions comprehensively pass.

@Charanvardhan Charanvardhan marked this pull request as ready for review April 3, 2026 04:48
@Charanvardhan Charanvardhan requested a review from a team as a code owner April 3, 2026 04:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(aoptimize_memories): delete + insert is not atomic, can lose user data

1 participant