Skip to content

Add loading state, error handling, and retry mechanism for memory operations#3503

Merged
beastoin merged 1 commit intomainfrom
u73ut_mem_lost
Nov 25, 2025
Merged

Add loading state, error handling, and retry mechanism for memory operations#3503
beastoin merged 1 commit intomainfrom
u73ut_mem_lost

Conversation

@beastoin
Copy link
Copy Markdown
Collaborator

@beastoin beastoin commented Nov 25, 2025

loading state, error handling, and retry mechanism for memory operations

deploy

#3498

@beastoin beastoin merged commit 3fb855a into main Nov 25, 2025
1 check passed
@beastoin beastoin deleted the u73ut_mem_lost branch November 25, 2025 12:15
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a 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 loading state, error handling, and a retry mechanism for memory operations, which significantly improves the user experience. The implementation is well-executed across both desktop and mobile dialogs, and the provider methods have been appropriately updated to return a success status. My review focuses on a couple of issues related to analytics tracking: one is a bug causing premature event firing, and the other is a missing event in one of the widgets. The suggested fixes will ensure accurate and consistent analytics.

Comment thread app/lib/desktop/pages/memories/widgets/desktop_memory_dialog.dart
Comment on lines +172 to +181
try {
success = await widget.provider.editMemory(
widget.memory,
contentController.text,
widget.memory.category,
);
} catch (e) {
success = false;
debugPrint('Error saving memory: $e');
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The _handleSave method is missing an analytics call to track when a memory is successfully edited. This is inconsistent with other similar dialogs in the app (e.g., memory_dialog.dart) and means this important user action will not be tracked. Please add a call to MixpanelManager().memoriesPageEditedMemory() when the edit operation is successful.

Suggested change
try {
success = await widget.provider.editMemory(
widget.memory,
contentController.text,
widget.memory.category,
);
} catch (e) {
success = false;
debugPrint('Error saving memory: $e');
}
try {
success = await widget.provider.editMemory(
widget.memory,
contentController.text,
widget.memory.category,
);
if (success) {
MixpanelManager().memoriesPageEditedMemory();
}
} catch (e) {
success = false;
debugPrint('Error saving memory: $e');
}

Glucksberg pushed a commit to Glucksberg/omi-local that referenced this pull request Apr 28, 2026
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.

1 participant