-
Notifications
You must be signed in to change notification settings - Fork 5
feat: implement delete all conversations functionality #319
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
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughBumps Changes
Sequence DiagramsequenceDiagram
participant User
participant Dialog as ConfirmDeleteDialog
participant LocalKV as Local KV Storage
participant OSAPI as OpenSecret API
participant App
User->>Dialog: Confirm delete
Dialog->>LocalKV: clearHistory()
alt KV clear success
LocalKV-->>Dialog: success
else KV clear fails
LocalKV-->>Dialog: error
Note over Dialog: Continue to server-side deletion
end
Dialog->>OSAPI: listConversations(limit:1)
OSAPI-->>Dialog: conversations (possibly empty)
alt conversations exist
Dialog->>OSAPI: deleteConversations(...)
OSAPI-->>Dialog: success / error
else no conversations
Note over Dialog: skip server deletion
end
Note over Dialog: Post-cleanup
Dialog->>App: invalidate queries (chatHistory, conversations, archivedChats)
Dialog->>App: navigate home
App-->>User: navigation / UI refreshed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
🔇 Additional comments (2)
Comment |
Deploying maple with
|
| Latest commit: |
80759e3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5d8644df.maple-ca8.pages.dev |
| Branch Preview URL: | https://feat-delete-all-conversation.maple-ca8.pages.dev |
Greptile OverviewGreptile SummaryThis PR implements a comprehensive delete all conversations feature that handles both local archived chats (stored in KV) and server-side conversations (via API). The implementation updates the Key Changes:
Implementation approach: Confidence Score: 4/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant UI as ConfirmDeleteDialog
participant KV as KV Store (clearHistory)
participant API as OpenSecret API
participant QC as QueryClient
participant Nav as Router
User->>UI: Click "Delete History"
UI->>UI: Show confirmation dialog
User->>UI: Confirm deletion
UI->>UI: handleDeleteHistory()
Note over UI,KV: Step 1: Delete archived chats
UI->>KV: clearHistory()
alt KV deletion successful
KV-->>UI: Success
UI->>UI: Log: "History (KV) cleared"
else KV deletion fails
KV-->>UI: Error
UI->>UI: Log error & continue
end
Note over UI,API: Step 2: Delete server conversations
UI->>API: listConversations({limit: 1})
alt Conversations exist
API-->>UI: {data: [conversations...]}
UI->>API: deleteConversations()
alt API deletion successful
API-->>UI: Success
UI->>UI: Log: "Server conversations deleted"
else API deletion fails
API-->>UI: Error
UI->>UI: Log error
end
else No conversations
API-->>UI: {data: []}
UI->>UI: Skip deletion
end
Note over UI,Nav: Step 3: Always refresh & navigate
UI->>QC: invalidateQueries(["chatHistory"])
UI->>QC: invalidateQueries(["conversations"])
UI->>QC: invalidateQueries(["archivedChats"])
UI->>Nav: navigate({to: "/"})
|
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.
2 files reviewed, 1 comment
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/components/AccountMenu.tsx (1)
55-88: Delete-all flow is sound; consider minor UX hardeningThe new
ConfirmDeleteDialogflow (KV clear → server delete → query invalidation → navigate home) is logically consistent and nicely isolates errors so that a failure in one step doesn’t block the others. UsinguseOpenSecrethere aligns with the existing usage inAccountMenuand keeps the OpenSecret-specific logic localized.Two optional refinements you might consider:
- Prevent repeated clicks on “Delete” during the async operation (e.g., local
isDeletingstate to disable the action button) to avoid redundant delete requests.- If it’s possible to open this dialog while no user is authenticated, an early guard like
if (!os.auth.user) { /* skip server delete */ }could reduce noisy logs, though your current try/catch already keeps this safe.No blocking issues from my side.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
frontend/package.json(1 hunks)frontend/src/components/AccountMenu.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use 2-space indentation, double quotes, and a 100-character line limit for formatting
Use camelCase for variable and function names
Use try/catch with specific error types for error handling
Files:
frontend/src/components/AccountMenu.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript typing and avoid
anywhen possible
Files:
frontend/src/components/AccountMenu.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-android
- GitHub Check: build-linux
🔇 Additional comments (1)
frontend/package.json (1)
20-20: @opensecret/react bump matches new usageThe version bump to
1.5.1is consistent with the newuseOpenSecret-driven conversation deletion flow inAccountMenu.tsx. From a code perspective this looks fine; just ensure your lockfile and CI/test suite pick up any behavioral changes from the library upgrade.
- Update @opensecret/react to v1.5.1 - Implement combined delete logic for both archived and server-side conversations in AccountMenu - Ensure robust error handling during deletion process Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
00f4a9a to
80759e3
Compare
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.
2 files reviewed, no comments
This PR implements a robust delete all conversations functionality. It updates the @opensecret/react SDK to v1.5.1 and modifies the AccountMenu component to clear both local archived chats (KV) and server-side conversations. The implementation handles errors independently for each deletion method to ensure maximum cleanup.
Summary by CodeRabbit
Chores
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.