-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: Conversation merge feature with undo support #3566
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a conversation merge feature, complete with backend logic, API endpoints, and frontend UI/state management. The implementation is comprehensive, covering merge preview, execution, and rollback capabilities. My review focuses on improving the robustness and data consistency of the new feature. I've identified several critical issues related to non-atomic database operations in the backend that could lead to data inconsistency, and lack of robust error handling in the frontend that could cause crashes or poor user experience. I've also included a high-severity comment on Python import practices to improve code maintainability. I've also highlighted how some of these issues relate to established repository rules regarding robust deserialization and consistent UI actions.
- Add null safety for DateTime parsing in Dart API responses - Add firestore import for Query.DESCENDING usage - Make merge/rollback operations atomic with Firestore batch writes - Add error handling and return type to executeMerge - Move imports to top of merge_utils.py per Python best practices
|
demo doesnt show actual merging |
Quick note on testing: Right now I can’t log in with my own Omi account when I launch the dev/debug app. If there’s a step I’m missing, I’m happy to fix that and re-record the video. Otherwise, if you can share a dev/testing config, I can run it end-to-end on real data. I sent a similar note on Discord, but adding it here for visibility. |
Summary
Implements the conversation merge feature requested in #1505, allowing users to combine multiple related conversations into a single unified conversation.
Demo Video
Merge.Conversations.mp4
Changes
Backend (Python)
backend/routers/merge_endpoints.py- REST API endpoints for merge operationsbackend/utils/conversations/merge_utils.py- Core merge logic and validationbackend/database/merge_history.py- Firestore operations for merge trackingbackend/models/merge_history.py- Data models for merge recordsbackend/models/conversation.py- Added merge-related fields to conversation modelFrontend (Flutter/Dart)
app/lib/backend/http/api/merge.dart- API client for merge endpointsapp/lib/providers/conversation_provider.dart- State management for merge operationsapp/lib/pages/conversations/conversations_page.dart- Multi-select UI and merge toolbarapp/lib/pages/conversations/widgets/conversation_list_item.dart- Selection checkboxes and merged badgeTest Plan