chore: delete document, document ui

[MaheshtheDev]

No description provided.

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Updated (UTC)
✅ Deployment successful! View logs	supermemory-app	9fd4919	Jan 16 2026, 09:49 PM

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Updated (UTC)
🔵 In progress View logs	supermemory-mcp	82d68b7	Jan 16 2026, 07:15 PM

[MaheshtheDev]

• chore: delete document, document ui #673 👈 (View in Graphite)
• main

---

How to use the Graphite Merge Queue

Add 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.

[claude]

PR Review: Document Delete & Onboarding Refactor

Reviewed PR #673 - Overall the changes look good with some solid refactoring.

Positive Changes

1. Good Architectural Refactoring - Breaking down the monolithic onboarding page into separate layout-based routing
2. Proper Type Safety - New DeleteButtonProps interface with proper UseMutationResult typing
3. Animation Variants Extraction - Moving variants to lib/variants.ts promotes reusability
4. Optimistic Updates - Delete mutation properly handles optimistic updates for both paginated and non-paginated queries
5. Accessibility - Delete button includes proper sr-only labels and focus states

Issues to Address

1. Security: External Script (apps/web/app/layout.tsx:36-43)

• Protocol-relative URL will use HTTP if accessed over HTTP
• No integrity check (SRI) for external script
• Use explicit https:// and add integrity attribute

2. Type Safety Issue (apps/web/hooks/use-document-mutations.ts:367)

• Using any defeats TypeScript purpose
• Define proper union type for QueryData

3. customId Validation (apps/web/components/new/document-modal/index.tsx:61-64)

• customId fallback may fail if it's a client-generated temporary ID
• Add validation to prevent deleting documents with temporary IDs

4. Missing Error Boundary

• New nested layout structure throws errors if contexts aren't available
• Add error boundary in onboarding layout

Summary

Must Fix: Security issue with external script, type safety with any usage, customId validation

Nice to Have: Error boundaries, extract optimistic update logic

Overall solid refactoring work. Just address the security and type safety issues before merging.

[claude]

PR Review: Document Delete & Onboarding UI Refactor

Summary

This PR adds document deletion functionality to the document modal and refactors the onboarding flow into a cleaner nested routing structure. Overall solid work with good separation of concerns.

---

🔴 Critical Issues

1. Security: Missing Authorization Check

Location: apps/web/hooks/use-document-mutations.ts:356-366

The delete mutation doesn't verify ownership before deleting. This could allow users to delete documents they don't own by guessing/knowing document IDs.

Fix: Backend should enforce authorization, but frontend should also validate ownership before calling the API.

2. Race Condition in Delete Flow

Location: apps/web/components/new/document-modal/index.tsx:60-85

The isTemporaryId check uses string prefix matching, but optimistic IDs are created with Date.now() which could theoretically collide if two documents are created in the same millisecond.

Recommendation: Use a more robust ID generation strategy (e.g., crypto.randomUUID() or a counter).

---

⚠️ High Priority Issues

3. Inconsistent Error Handling

Location: apps/web/hooks/use-document-mutations.ts:355-443

The deleteMutation has optimistic updates that filter by both doc.id and doc.customId (line 390, 413), but the isTemporaryId function only checks against a single ID field. This inconsistency could lead to failed deletions not being reverted properly.

Fix: Standardize on one ID field or document why both are needed.

4. Memory Leak in Modal

Location: apps/web/app/new/onboarding/welcome/layout.tsx:74-93

Auto-navigation timers (lines 77-82, 84-87) are set up in useEffect but there's a potential memory leak if the component unmounts before the timer fires during navigation. While there is a cleanup function, rapid navigation could cause issues.

Recommendation: Consider using router.prefetch() and add abort controllers for navigation promises.

---

💡 Code Quality Issues

5. Development Dependency in Production

Location: apps/web/app/layout.tsx:36-41

React Scan is loaded from unpkg CDN, which:

• Adds external dependency at runtime
• Could fail if unpkg is down
• Slows initial page load in dev

Recommendation: Install as dev dependency and use proper bundling.

6. Type Safety Regression

Location: apps/web/hooks/use-document-mutations.ts:411-414

Type casting to unknown then to specific type (line 412) bypasses TypeScript's safety:

const docObj = doc as { id?: string; customId?: string | null }

Fix: Define proper types for document objects at the top of the file.

7. Missing Loading States

Location: apps/web/app/new/onboarding/setup/page.tsx and apps/web/app/new/onboarding/welcome/page.tsx

The pages use useSetupContext() and useWelcomeContext() which throw errors if used outside the correct layout (lines 32-35, 42-45). If these contexts are loading or not yet available, users will see error boundaries instead of loading states.

Recommendation: Add loading states to the layouts and propagate them through context.

---

✅ What Went Well

1. Clean Architecture: The nested layout pattern with separate contexts for welcome/setup flows is a great separation of concerns.

2. Optimistic Updates: The delete mutation properly implements optimistic UI updates with rollback on error - good UX pattern.

3. Accessibility: The delete button has proper ARIA labels and keyboard navigation support.

4. Animation Consistency: Extracting motion variants to lib/variants.ts improves maintainability.

5. Proper localStorage Cleanup: The resetOnboarding function properly cleans up localStorage keys.

---

📝 Minor Suggestions

• Line 183 (apps/web/components/new/document-modal/index.tsx): Empty line should probably be removed (appears to be accidental)
• Consider adding E2E tests for the delete flow since it involves optimistic updates, API calls, and UI state changes
• The StepNotFound components are good error boundaries - consider reusing this pattern elsewhere

---

Test Coverage

✅ Not applicable - Web app has no test setup per repository standards.

---

Recommendation

Request changes - Address the authorization issue (#1) and race condition (#2) before merging. The other issues can be addressed in follow-up PRs but these two are critical for production.

[graphite-app]

Merge activity

• Jan 16, 9:43 PM UTC: Dhravya added this pull request to the Graphite merge queue.
• Jan 16, 9:45 PM UTC: Merged by the Graphite merge queue.