Conversation
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
supermemory-app | 022fab2 | Mar 10 2026, 05:36 AM |
How to use the Graphite Merge QueueAdd the label Main to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
supermemory-app | 71c7400 | Commit Preview URL | Mar 10 2026, 07:52 AM |
| { documentIds: ids }, | ||
| { | ||
| onSuccess: () => { | ||
| setSelectedDocumentIds(new Set()) | ||
| setIsSelectionMode(false) | ||
| if (selectedDocument && ids.includes(selectedDocument.id ?? "")) { | ||
| setDocId(null) | ||
| } | ||
| }, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Bug: Performing a bulk delete clears any unsaved text in the quick note draft input due to an onSuccess callback firing unintentionally.
Severity: MEDIUM
Suggested Fix
Decouple the onClose logic from the bulk delete mutation's onSuccess callback. The onSuccess for bulkDeleteMutation should only handle logic related to the deletion itself, such as clearing selection state. The resetDraft() function should not be called as a side effect of a successful bulk deletion.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: apps/web/app/(app)/page.tsx#L186-L197
Potential issue: When a user performs a bulk delete operation, two separate `onSuccess`
callbacks are triggered. One is defined at the `useMutation` level, which calls an
`onClose` handler that in turn calls `resetDraft()`. This occurs even if the user has
unsaved text in the quick note input field. The `resetDraft()` function then clears this
unsaved text without warning, leading to data loss for the user. This bug is triggered
specifically when a user has an unsaved quick note draft and successfully completes a
bulk delete action.
Did we get this right? 👍 / 👎 to inform future reviews.
Prasanna721
left a comment
There was a problem hiding this comment.
can you fix the build issues?
|
can select & delete button can be at the right of dismiss? |
what do you mean @Prasanna721 i didn't get it |
|
Claude finished @MaheshtheDev's task in 58s —— View job Review Complete
Overview: Adds bulk delete functionality for documents in the nova app with selection mode UI. Previously flagged issues - now resolved:
Issues found: None - this looks good to ship. Score: 9/10 Good implementation with selection mode UI, optimistic updates, proper error handling with rollback, and a confirmation dialog. All previously identified bugs have been addressed. The Cloudflare deployment failure mentioned in comments appears to be infrastructure-related rather than a code issue in this PR. |
There was a problem hiding this comment.
Overview: Adds bulk delete functionality for documents in the nova app with selection mode UI.
Issues found:
- Bug (confirmed from Sentry):
bulkDeleteMutation.onSuccesscallsonClose?.()which resets the quick note draft unintentionally. Users with unsaved text in the quick note input will lose their draft when performing bulk delete. See inline comment for the fix.
Regarding build issues: The TypeScript errors in the type-check output are pre-existing in @supermemory/tools (test files) and unrelated to this PR's changes. The Cloudflare deployment failure appears to be infrastructure-related rather than a code issue in this PR.
Score: 8/10
Good implementation overall - the selection mode UI, optimistic updates, and confirmation dialog are well done. Just needs the one bug fix before merging.
| ) | ||
| queryClient.invalidateQueries({ queryKey: ["documents-with-memories"] }) | ||
| onClose?.() | ||
| }, |
There was a problem hiding this comment.
Bug confirmed: The Sentry AI report is correct. Calling onClose?.() here will unintentionally clear the quick note draft when bulk delete succeeds.
The onClose callback is designed for note/link/file mutations where the user saves content and the modal closes (resetting the draft makes sense there). But for bulk delete, this side effect is unexpected - users deleting documents shouldn't lose their unsaved quick note text.
Fix: Remove onClose?.() from this onSuccess callback. The inline onSuccess in handleBulkDelete already handles clearing selection state appropriately.
| }, | |
| onSuccess: (_data, variables) => { | |
| analytics.documentsBulkDeleted({ count: variables.documentIds.length }) | |
| toast.success( | |
| `${variables.documentIds.length} document${variables.documentIds.length === 1 ? "" : "s"} deleted`, | |
| ) | |
| queryClient.invalidateQueries({ queryKey: ["documents-with-memories"] }) | |
| }, |

Uh oh!
There was an error while loading. Please reload this page.