-
Notifications
You must be signed in to change notification settings - Fork 5
feat: use batch KV delete for clearing history #322
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
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
WalkthroughThis PR updates the Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant clearHistory as clearHistory()
participant delAll as OpenSecret.delAll()
participant fallback as Manual Deletion<br/>(per-key)
User->>clearHistory: Trigger history clear
clearHistory->>delAll: Attempt bulk delete
rect rgb(200, 220, 255)
Note over delAll,fallback: New bulk delete attempt
alt Bulk Delete Success
delAll-->>clearHistory: Success
clearHistory-->>User: Done
else Bulk Delete Fails
delAll-->>clearHistory: Error
rect rgb(255, 220, 200)
Note over clearHistory,fallback: Fallback triggered
clearHistory->>fallback: Delete history_list
fallback->>fallback: Delete each chat_ key
fallback-->>clearHistory: Complete
end
clearHistory-->>User: Done
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Deploying maple with
|
| Latest commit: |
042c438
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e9c22e8b.maple-ca8.pages.dev |
| Branch Preview URL: | https://kv-delete-all.maple-ca8.pages.dev |
Greptile OverviewGreptile SummaryOptimized the
Confidence Score: 5/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant AccountMenu
participant LocalStateContext
participant OpenSecret SDK
participant KV Storage
User->>AccountMenu: Click "Delete History"
AccountMenu->>LocalStateContext: clearHistory()
alt Batch delete available (SDK 1.5.2+)
LocalStateContext->>OpenSecret SDK: delAll()
OpenSecret SDK->>KV Storage: Batch delete all keys
KV Storage-->>OpenSecret SDK: Success
OpenSecret SDK-->>LocalStateContext: Success
else Batch delete fails or unavailable
LocalStateContext->>OpenSecret SDK: list()
OpenSecret SDK->>KV Storage: List all keys
KV Storage-->>OpenSecret SDK: Return keys
OpenSecret SDK-->>LocalStateContext: Return keys
LocalStateContext->>OpenSecret SDK: del("history_list")
LocalStateContext->>OpenSecret SDK: del("chat_*") for each chat
OpenSecret SDK->>KV Storage: Delete individual keys
KV Storage-->>OpenSecret SDK: Success
OpenSecret SDK-->>LocalStateContext: Success
end
LocalStateContext-->>AccountMenu: Complete
AccountMenu->>User: History cleared
|
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
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: 1
📜 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/state/LocalStateContext.tsx(2 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/state/LocalStateContext.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript typing and avoid
anywhen possible
Files:
frontend/src/state/LocalStateContext.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: OpenSecretCloud/Maple PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-19T21:31:44.925Z
Learning: Applies to src/**/*.{tsx} : Use React context for global state management when needed
📚 Learning: 2025-07-19T21:31:44.925Z
Learnt from: CR
Repo: OpenSecretCloud/Maple PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-19T21:31:44.925Z
Learning: Applies to src/**/*.{tsx} : Use React context for global state management when needed
Applied to files:
frontend/src/state/LocalStateContext.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). (5)
- GitHub Check: build-android
- GitHub Check: build-ios
- GitHub Check: build-macos (universal-apple-darwin)
- GitHub Check: build-linux
- GitHub Check: Cloudflare Pages
🔇 Additional comments (2)
frontend/src/state/LocalStateContext.tsx (1)
287-299: Excellent implementation of bulk delete with graceful fallback.The try-catch pattern with fallback to manual deletion is well-designed:
- Prioritizes the optimized
delAll()bulk operation- Gracefully degrades to the previous implementation on any failure
- Handles backward compatibility, partial failures, and missing method scenarios
- Uses
Promise.allfor efficient parallel deletion in the fallback pathfrontend/package.json (1)
20-20: Verified: @opensecret/react 1.5.2 is current and has no known security advisories.Version 1.5.2 is confirmed as the latest available in npm registry, and no security vulnerabilities were detected.
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.