refactor: replace manual conflict handling with SQLite ON CONFLICT semantics#311
refactor: replace manual conflict handling with SQLite ON CONFLICT semantics#311phernandez wants to merge 3 commits intomainfrom
Conversation
…mantics - Add upsert_entity_optimistic() method using INSERT ... ON CONFLICT DO UPDATE - Add update_entity_optimistic() for file_path-based updates - Add update_entity_by_id_optimistic() for ID-based updates - Replace direct entity_repository.update() calls in sync service - Eliminates race conditions between API and sync operations - Simplifies error handling by removing manual IntegrityError catching Co-authored-by: Paul Hernandez <phernandez@users.noreply.github.com>
|
@claude update this branch with the latest from main and evaluate if the logic is sound. I don't like making new flavors of existing methods. Lets just have one "update", otherwise it's confusing. |
|
Claude finished @phernandez's task —— View job Refactoring complete! ✅I've successfully refactored the code to consolidate the multiple update methods into a single, cleaner approach and pushed the changes to the branch. Changes Made:
Benefits:
Review Summary:The logic is now sound and consistent throughout. The refactored approach provides the same race condition protection as the original PR, but with a much cleaner and more intuitive API. The branch has been updated with these improvements. Branch: claude/issue-205-20250716-2110 |
- Replace upsert_entity() with SQLite ON CONFLICT implementation - Override update() to use atomic ON CONFLICT semantics - Add update_by_file_path() convenience method - Remove redundant *_optimistic method variants - Update sync_service to use consolidated API This simplifies the API while maintaining the same race condition protection through SQLite's native conflict resolution. Co-authored-by: Paul Hernandez <phernandez@users.noreply.github.com>
|
Claude finished @phernandez's task —— View job Code Review Complete ✅Todo List:
Review SummaryThe refactoring to consolidate multiple update methods into a single, cleaner approach using SQLite's ON CONFLICT semantics is well-executed and sound. The logic is correct and the implementation successfully addresses the original goals of eliminating race conditions and simplifying the API. Strengths ✅
Team Checklist Review✅ Code Quality & Standards
✅ Testing & Documentation
✅ Basic Memory Architecture
✅ Security & Performance
Minor Observations
RecommendationThis PR is ready to merge. The refactoring successfully achieves its goals of:
The logic is sound and the implementation follows Basic Memory's coding standards and architectural principles. |
|
Closing in favor of #328 which implements a much simpler solution (71 lines vs 300+) using database-level conflict resolution. |
This PR addresses issue #205 by replacing manual conflict handling with SQLite's native ON CONFLICT semantics.
Changes:
Benefits:
Generated with Claude Code