-
Notifications
You must be signed in to change notification settings - Fork 2.1k
[Feature]: Select/deselect all sources in a Notebook #223 #231
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| # Bulk Source Action Feature | ||
|
|
||
| ## Overview | ||
| This feature adds bulk action capabilities for including/excluding sources in a chat session for notebooks. When dealing with a high number of sources, this feature saves time by avoiding unnecessary scrolls multiple times per session. | ||
|
|
||
| ## Implementation Details | ||
|
|
||
| ### Backend Changes | ||
|
|
||
| 1. **API Endpoint**: Added a new endpoint `/api/notebooks/{notebook_id}/sources/bulk` that supports bulk operations (add/remove) for sources in a notebook. | ||
|
|
||
| 2. **Request Model**: | ||
| - `BulkSourceOperationRequest` with fields: | ||
| - `source_ids`: List of source IDs to operate on | ||
| - `operation`: Either "add" or "remove" | ||
|
|
||
| 3. **Response Model**: | ||
| - `BulkSourceOperationResponse` with: | ||
| - `message`: Summary of the operation | ||
| - `results`: Detailed results for each source operation | ||
|
|
||
| ### Frontend Changes | ||
|
|
||
| 1. **API Client**: Updated the notebooks API client to include the new `bulkSourceOperation` method. | ||
|
|
||
| 2. **Hooks**: Added `useBulkSourceOperation` hook in `use-sources.ts` to handle bulk operations with proper error handling and UI feedback. | ||
|
|
||
| 3. **UI Components**: | ||
| - Created `BulkSourceActionDialog` component for selecting multiple sources | ||
| - Updated `SourcesColumn` component to include bulk action options in the dropdown menu | ||
|
|
||
| 4. **User Experience**: | ||
| - Added "Bulk Add Sources" and "Bulk Remove Sources" options to the sources dropdown | ||
| - Implemented select-all functionality in the bulk action dialog | ||
| - Added visual feedback for selected sources count | ||
| - Integrated with existing toast notifications for operation results | ||
|
|
||
| ## How to Use | ||
|
|
||
| 1. Navigate to a notebook page | ||
| 2. Click the "Add Source" dropdown button in the Sources column | ||
| 3. Select either "Bulk Add Sources" or "Bulk Remove Sources" | ||
| 4. In the dialog that appears: | ||
| - Use the checkboxes to select which sources to include/exclude | ||
| - Use the "Select All" checkbox to quickly select/deselect all sources | ||
| - Click the action button to perform the bulk operation | ||
| 5. The sources will be added/removed from the notebook in a single operation | ||
|
|
||
| ## Benefits | ||
|
|
||
| - **Time Savings**: Eliminates the need to individually add/remove many sources | ||
| - **Efficiency**: Reduces repetitive scrolling and clicking when managing large numbers of sources | ||
| - **User Experience**: Provides a more streamlined workflow for source management | ||
| - **Error Handling**: Gracefully handles partial failures with detailed feedback | ||
|
|
||
| ## Technical Notes | ||
|
|
||
| - The implementation follows the existing code patterns and conventions | ||
| - All operations are atomic - if one source fails, others continue to process | ||
| - Proper error handling and user feedback is implemented | ||
| - The feature integrates seamlessly with existing source management workflows |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| from typing import List, Optional | ||
| from typing import List, Literal, Optional | ||
|
|
||
| from fastapi import APIRouter, HTTPException, Query | ||
| from pydantic import BaseModel | ||
| from loguru import logger | ||
|
|
||
| from api.models import NotebookCreate, NotebookResponse, NotebookUpdate | ||
|
|
@@ -180,6 +181,104 @@ async def update_notebook(notebook_id: str, notebook_update: NotebookUpdate): | |
| ) | ||
|
|
||
|
|
||
| class BulkSourceOperationRequest(BaseModel): | ||
| source_ids: List[str] | ||
| operation: Literal["add", "remove"] | ||
|
|
||
|
|
||
| @router.post("/notebooks/{notebook_id}/sources/bulk") | ||
| async def bulk_source_operation( | ||
| notebook_id: str, request: BulkSourceOperationRequest | ||
| ): | ||
| """Bulk add or remove sources from a notebook.""" | ||
| try: | ||
| # Check if notebook exists | ||
| notebook = await Notebook.get(notebook_id) | ||
| if not notebook: | ||
| raise HTTPException(status_code=404, detail="Notebook not found") | ||
|
|
||
| results = [] | ||
| for source_id in request.source_ids: | ||
| try: | ||
| if request.operation == "add": | ||
| # Check if source exists | ||
| source = await Source.get(source_id) | ||
| if not source: | ||
| results.append({ | ||
| "source_id": source_id, | ||
| "success": False, | ||
| "error": "Source not found" | ||
| }) | ||
| continue | ||
|
|
||
| # Check if reference already exists (idempotency) | ||
| existing_ref = await repo_query( | ||
| "SELECT * FROM reference WHERE out = $source_id AND in = $notebook_id", | ||
| { | ||
| "notebook_id": ensure_record_id(notebook_id), | ||
| "source_id": ensure_record_id(source_id), | ||
| }, | ||
| ) | ||
|
|
||
| # If reference doesn't exist, create it | ||
| if not existing_ref: | ||
| await repo_query( | ||
| "RELATE $source_id->reference->$notebook_id", | ||
| { | ||
| "notebook_id": ensure_record_id(notebook_id), | ||
| "source_id": ensure_record_id(source_id), | ||
| }, | ||
| ) | ||
|
|
||
| results.append({ | ||
| "source_id": source_id, | ||
| "success": True, | ||
| "message": "Source added to notebook successfully" | ||
| }) | ||
|
|
||
| elif request.operation == "remove": | ||
| # Delete the reference record linking source to notebook | ||
| await repo_query( | ||
| "DELETE FROM reference WHERE out = $notebook_id AND in = $source_id", | ||
| { | ||
| "notebook_id": ensure_record_id(notebook_id), | ||
| "source_id": ensure_record_id(source_id), | ||
| }, | ||
| ) | ||
|
|
||
| results.append({ | ||
| "source_id": source_id, | ||
| "success": True, | ||
| "message": "Source removed from notebook successfully" | ||
| }) | ||
|
|
||
| except Exception as e: | ||
| logger.error( | ||
| f"Error processing source {source_id} for notebook {notebook_id}: {str(e)}" | ||
| ) | ||
| results.append({ | ||
| "source_id": source_id, | ||
| "success": False, | ||
| "error": str(e) | ||
| }) | ||
|
Comment on lines
+201
to
+263
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 HIGH - Missing transaction handling with rollback Category: performance Description: Suggestion: Confidence: 87% |
||
|
|
||
| success_count = sum(1 for r in results if r["success"]) | ||
| return { | ||
| "message": f"Bulk operation completed. {success_count}/{len(results)} operations successful.", | ||
| "results": results | ||
| } | ||
|
|
||
| except HTTPException: | ||
| raise | ||
| except Exception as e: | ||
| logger.error( | ||
| f"Error performing bulk source operation for notebook {notebook_id}: {str(e)}" | ||
| ) | ||
| raise HTTPException( | ||
| status_code=500, detail=f"Error performing bulk source operation: {str(e)}" | ||
| ) | ||
|
|
||
|
|
||
| @router.post("/notebooks/{notebook_id}/sources/{source_id}") | ||
| async def add_source_to_notebook(notebook_id: str, source_id: str): | ||
| """Add an existing source to a notebook (create the reference).""" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,4 +23,4 @@ services: | |
| volumes: | ||
| - ./notebook_data:/app/data | ||
| restart: always | ||
|
|
||
|
|
||
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.
🔴 CRITICAL - N+1 database queries in bulk operation loop
Category: performance
Description:
Loop executes Source.get() and multiple repo_query() calls for each source_id, causing severe performance degradation at scale
Suggestion:
Batch fetch all sources with a single query: sources = await Source.get_many(request.source_ids), then process in-memory. For reference operations, consider bulk RELATE query or batched operations.
Confidence: 92%
Rule:
perf_n_plus_one_queries