feat: New multi chat deletion feature - #2208
Merged
Merged
Conversation
github-actions
Bot
added
frontend
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughBulk chat deletion now spans frontend selection, the ChangesBulk chat deletion
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Navigation
participant useBulkDeleteSessionsMutation
participant bulk_delete_sessions_endpoint
participant ChatService
User->>Navigation: Select conversations
Navigation->>useBulkDeleteSessionsMutation: Submit selected session IDs
useBulkDeleteSessionsMutation->>bulk_delete_sessions_endpoint: DELETE /sessions
bulk_delete_sessions_endpoint->>ChatService: Delete authorized sessions
ChatService-->>bulk_delete_sessions_endpoint: Return deleted and failed IDs
bulk_delete_sessions_endpoint-->>useBulkDeleteSessionsMutation: Return deletion result
useBulkDeleteSessionsMutation-->>Navigation: Invalidate conversation queries
Navigation-->>User: Show result and update conversation list
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
github-actions
Bot
added
enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/app/routes/internal.py (1)
210-215: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDeclare a typed response for
DELETE /sessions.
bulk_delete_sessions_endpointreturnsJSONResponse({"deleted": ..., "failed": ...}), while the route registration omitsresponse_model. Add aBulkDeleteSessionsResponse/alias and register it onapp.add_api_routeso the success payload shape is part of the API contract and OpenAPI docs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/routes/internal.py` around lines 210 - 215, Define or reuse a typed BulkDeleteSessionsResponse model/alias matching the deleted and failed fields returned by bulk_delete_sessions_endpoint, then pass it as response_model in the app.add_api_route registration for DELETE /sessions so the success payload is reflected in the API contract and OpenAPI schema.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/components/navigation.tsx`:
- Line 450: Correct the data-testid on the selection-toggle element in the
navigation component from the misspelled value to "chat-select-toggle" so it
matches the selector expected by Chat page tests and enables selection mode.
In `@src/api/chat.py`:
- Around line 55-56: Update BulkDeleteBody validation for session_ids to reject
duplicate IDs with a 400 response, or deduplicate the IDs before ownership
filtering and delete_sessions processing. Ensure each session is deleted at most
once.
- Around line 241-266: The bulk-delete business logic currently resides in the
route handler. Move ownership filtering and deletion result aggregation from the
handler into ChatService or another service resolved through the dependency path
defined in src/dependencies.py, removing the direct session_ownership_service
import. Keep the handler limited to validating/parsing the request, invoking the
injected service, and mapping its result to the HTTP response.
In `@tests/unit/api/test_bulk_delete_sessions_endpoint.py`:
- Around line 38-41: Remove the unused type: ignore comments from the test setup
and response parsing around the user argument, json.loads(resp.body), and the
corresponding line near the later test case. Leave the test behavior unchanged
and ensure no unnecessary suppression remains.
---
Nitpick comments:
In `@src/app/routes/internal.py`:
- Around line 210-215: Define or reuse a typed BulkDeleteSessionsResponse
model/alias matching the deleted and failed fields returned by
bulk_delete_sessions_endpoint, then pass it as response_model in the
app.add_api_route registration for DELETE /sessions so the success payload is
reflected in the API contract and OpenAPI schema.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1ea05181-d408-49ef-98e4-d509c75ff5b6
📒 Files selected for processing (12)
frontend/app/api/mutations/useBulkDeleteSessionsMutation.tsfrontend/components/bulk-delete-button.tsxfrontend/components/chat-renderer.tsxfrontend/components/navigation.tsxfrontend/hooks/useChatSelection.tsxfrontend/tests/core/bulk_delete_chats.spec.tsfrontend/tests/pages/Chat.tssrc/api/chat.pysrc/app/routes/internal.pysrc/services/chat_service.pytests/unit/api/test_bulk_delete_sessions_endpoint.pytests/unit/test_chat_service_bulk_delete.py
github-actions
Bot
added
enhancement
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/components/navigation.tsx (2)
166-177: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRoute the empty-list fallback through
handleNewConversation().If bulk deletion removes the current conversation and no conversations remain, Lines 175-176 call
startNewConversation()directly. The centralizedhandleNewConversation()path preservesonNewConversation,refreshConversations, and thenewConversationevent. The bulk path skips these updates, so parent state or listeners can remain stale after deleting all chats.Proposed fix
} else { setCurrentConversationId(null); - startNewConversation(); + handleNewConversation(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/components/navigation.tsx` around lines 166 - 177, Update the empty-list branch in the bulk deletion logic to call handleNewConversation() instead of startNewConversation(), while preserving the currentConversationId reset. Keep the remaining-conversation path unchanged so the centralized new-conversation flow updates parent state, refreshes conversations, and emits the expected event.
581-606: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not nest the checkbox inside the conversation button.
<button>is invalid ancestor markup for the nested interactive checkbox descendant. Move the checkbox and conversation title/actions out as sibling controls, or replace the wrapper with a non-interactive container. Test keyboard navigation and screen-reader behavior after the change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/components/navigation.tsx` around lines 581 - 606, The conversation checkbox is nested inside the clickable conversation button, creating invalid interactive markup and inaccessible keyboard behavior. Update the conversation item around the selection checkbox and `loadConversation` handler so the checkbox and conversation title/actions are sibling controls, or use a non-interactive container while preserving their current selection and loading behavior; verify keyboard navigation and screen-reader semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@frontend/components/navigation.tsx`:
- Around line 166-177: Update the empty-list branch in the bulk deletion logic
to call handleNewConversation() instead of startNewConversation(), while
preserving the currentConversationId reset. Keep the remaining-conversation path
unchanged so the centralized new-conversation flow updates parent state,
refreshes conversations, and emits the expected event.
- Around line 581-606: The conversation checkbox is nested inside the clickable
conversation button, creating invalid interactive markup and inaccessible
keyboard behavior. Update the conversation item around the selection checkbox
and `loadConversation` handler so the checkbox and conversation title/actions
are sibling controls, or use a non-interactive container while preserving their
current selection and loading behavior; verify keyboard navigation and
screen-reader semantics.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: cfc1a07f-1122-4b8e-b2b9-272df6c77325
📒 Files selected for processing (2)
frontend/components/navigation.tsxfrontend/hooks/useChatSelection.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/hooks/useChatSelection.tsx
github-actions
Bot
added
enhancement
github-actions
Bot
added
enhancement
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/unit/test_chat_service_bulk_delete.py (1)
69-77: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for continuation after an exception.
The test at Line 75 passes one session ID. It verifies failure conversion, but it does not verify that a later session is still processed. Add a second owned ID and assert both result lists and both deletion calls. This validates the best-effort loop in
src/services/chat_service.py, Lines 940-968.Proposed test extension
- _patch_owner(monkeypatch, {"x": "alice"}) - svc.delete_session = AsyncMock(side_effect=RuntimeError("boom")) + _patch_owner(monkeypatch, {"x": "alice", "y": "alice"}) + svc.delete_session = AsyncMock( + side_effect=[RuntimeError("boom"), {"success": True, "error": None}] + ) - result = await svc.delete_sessions("alice", ["x"]) + result = await svc.delete_sessions("alice", ["x", "y"]) - assert result == {"deleted": [], "failed": ["x"]} + assert result == {"deleted": ["y"], "failed": ["x"]} + assert svc.delete_session.await_count == 2🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_chat_service_bulk_delete.py` around lines 69 - 77, Extend test_bulk_delete_never_raises_on_error with a second owned session ID, configure the mock to fail for the first deletion while allowing the second to complete, and assert the result contains the first ID in failed and the second in deleted. Also verify delete_session was called for both IDs, confirming delete_sessions continues processing after an exception.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/unit/test_chat_service_bulk_delete.py`:
- Around line 69-77: Extend test_bulk_delete_never_raises_on_error with a second
owned session ID, configure the mock to fail for the first deletion while
allowing the second to complete, and assert the result contains the first ID in
failed and the second in deleted. Also verify delete_session was called for both
IDs, confirming delete_sessions continues processing after an exception.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fede0c21-2e6f-4e63-a3e6-096f568a3685
📒 Files selected for processing (6)
frontend/components/navigation.tsxfrontend/hooks/useChatSelection.tsxsrc/api/chat.pysrc/services/chat_service.pytests/unit/api/test_bulk_delete_sessions_endpoint.pytests/unit/test_chat_service_bulk_delete.py
🚧 Files skipped from review as they are similar to previous changes (4)
- src/services/chat_service.py
- frontend/hooks/useChatSelection.tsx
- src/api/chat.py
- frontend/components/navigation.tsx
VisveshJ
approved these changes
Aug 5, 2026
VisveshJ
left a comment
VisveshJ
left a comment
Collaborator
There was a problem hiding this comment.
good feature! was able to delete multiple chats and preserve some that id like to keep
github-actions
Bot
added
lgtm
enhancement
github-actions
Bot
added
enhancement
Vchen7629
force-pushed
the
chat-multi-delete-feature
branch
from
August 6, 2026 00:45
648571a to
ce3dc65
Compare
github-actions
Bot
added
enhancement
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1465 , This pr adds a new multi-chat deletion feature to openrag. Users can select one, multiple, or all their existing chats by clicking the new select button next to the new chat button (+).
Demo
screen-capture.15.webm
Summary by CodeRabbit
New Features
Tests