[Feature]: Select/deselect all sources in a Notebook #223 by MasterAffan · Pull Request #231 · lfnovo/open-notebook

MasterAffan · 2025-10-29T17:45:56Z

Description
Summary

Adds bulk actions to include or exclude multiple sources in a notebook’s chat session in one step.
Reduces repeated scrolling and per-source toggling when working with large source lists.

Why this helps

For notebooks with many sources, this saves time per session by batching actions and minimizing UI friction.

Implementation details

Backend: Adds a POST-based bulk operation that processes each source individually, isolates errors, and returns per-item status along with a success count.
Frontend: Introduces a bulk action dialog with multi-select checkboxes, “Select All”, real-time selection count, and loading states. Uses mutation hooks to invalidate relevant queries and show toasts on success/failure.
Keys: Uses unique React keys across bulk dialogs (e.g., include/exclude) by prefixing with the operation type to avoid collisions.

API (generic shape)

Request:
- action: "include" | "exclude"
- sourceIds: string[]
Response:
- successCount: number
- results: Array<{ sourceId: string; status: "success" | "error"; message?: string }>

UI/UX

New bulk action entry point from the Sources column in the notebooks view.
Confirm dialog with clear selection feedback and disabled state during processing.
Toast feedback indicating overall success with any per-item failures surfaced in the result list.

How to test

Open a notebook with many sources.
Open the Sources column → trigger bulk action (include or exclude).
Use “Select All” and/or pick specific sources.
Confirm the action and observe:
- Loading state during mutation
- Toast feedback after completion
- Notebook chat context reflects included/excluded sources
Verify partial failure handling: simulate a failing source and check per-item statuses in the response.

Affected areas (indicative)

Backend: api/routers/notebooks.py (bulk action route), sources/context services
Frontend:
- frontend/src/components/notebooks/BulkSourceActionDialog.tsx
- frontend/src/app/(dashboard)/notebooks/components/SourcesColumn.tsx
- related hooks and API utilities

Notes

Follows bulk operation pattern with error isolation and detailed results.
Invalidates notebook/source queries post-mutation for fresh data.
No breaking changes to existing single-source actions.

Checklist

Backend bulk route validates input and returns per-item results
Frontend dialog supports multi-select and “Select All”
Mutation hooks invalidate relevant queries
Unique React keys used across dialogs
Tests/Manual checks cover success and partial failure cases
Docs/Help texts updated in UI where applicable

Demo Video

2025-10-27.12-08-56.mp4

Linked issues

Closes: [Feature]: Select/deselect all sources in a Notebook #223

MasterAffan · 2025-10-30T07:35:52Z

@lfnovo pls have a look

MasterAffan · 2025-10-30T07:37:43Z

can you please add hacktoberfest label in the pull req

diffray-bot · 2025-12-10T02:49:57Z

+  useEffect(() => {
+    if (!open) return
+
+    const loadSources = async () => {
+      setIsLoading(true)
+      try {
+        if (operation === 'add') {
+          // For add: fetch all sources and filter out ones already in notebook
+          const allSources = await sourcesApi.list({
+            limit: 100,
+            offset: 0,
+            sort_by: 'created',
+            sort_order: 'desc',
+          })
+          const sourcesToAdd = allSources.filter(s => !currentSourceIds.has(s.id))
+          setAvailableSources(sourcesToAdd)
+        } else {
+          // For remove: use sources already in the notebook
+          setAvailableSources(currentNotebookSources || [])
+        }
+      } catch (error) {
+        console.error('Error loading sources:', error)
+        setAvailableSources([])
+      } finally {
+        setIsLoading(false)
+      }
+    }
+
+    loadSources()
+  }, [open, operation, currentSourceIds, currentNotebookSources])


🟠 HIGH - Race condition in async useEffect without cancellation

Category: bug

Description:
Async fetch operation in useEffect lacks cancellation. If dialog reopens or dependencies change rapidly, stale responses can overwrite current state.

Suggestion:
Add a cleanup function with a cancelled flag or AbortController to prevent stale responses from updating state after unmount or dependency change.

Confidence: 88%
Rule: react_async_race_condition

diffray-bot · 2025-12-10T02:49:57Z

+        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)
+                })


🔴 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

diffray-bot · 2025-12-10T02:49:57Z

+        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)
+                })


🟠 HIGH - Missing transaction handling with rollback

Category: performance

Description:
Bulk operation with multiple database statements lacks transaction wrapping, risking partial failures and data inconsistency

Suggestion:
Wrap the loop in a database transaction with try/except to commit on success and rollback on failure. Use transaction context manager or begin/commit/rollback pattern.

Confidence: 87%
Rule: py_add_transaction_handling_with_rollback

diffray-bot · 2025-12-10T02:49:58Z

diffray code review

Free public review - Want AI code reviews on your PRs? Check out diffray.ai

Summary

Validated 18 issues: 4 kept (1 critical security, 1 high race condition, 2 high performance), 14 filtered (6 low-value logging suggestions, 2 magic number nitpicks, 5 architectural preferences, 1 incorrect optimization claim)

Issues Found: 4

See 3 individual line comment(s) for details.

Full issue list (click to expand)

🟠 HIGH - Race condition in async useEffect without cancellation

File: frontend/src/components/notebooks/BulkSourceActionDialog.tsx``:49-78

Category: bug

Description: Async fetch operation in useEffect lacks cancellation. If dialog reopens or dependencies change rapidly, stale responses can overwrite current state.

Suggestion: Add a cleanup function with a cancelled flag or AbortController to prevent stale responses from updating state after unmount or dependency change.

Confidence: 88%

Rule: react_async_race_condition

🔴 CRITICAL - SQL injection in ORDER BY clause

File: api/routers/notebooks.py``:23-28

Category: security

Description: User-controlled order_by parameter directly interpolated into SQL query using f-string without validation

Suggestion: Use a whitelist to validate order_by parameter. Only allow specific column names and directions (e.g., 'updated desc', 'created asc', 'name desc'). Example: ALLOWED_ORDER = {'updated desc', 'updated asc', 'created desc', 'created asc', 'name desc', 'name asc'}; if order_by not in ALLOWED_ORDER: raise HTTPException(400, 'Invalid order_by')

Confidence: 98%

Rule: sec_sql_string_concat

🔴 CRITICAL - N+1 database queries in bulk operation loop

File: api/routers/notebooks.py``:201-263

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

🟠 HIGH - Missing transaction handling with rollback

File: api/routers/notebooks.py``:201-263

Category: performance

Description: Bulk operation with multiple database statements lacks transaction wrapping, risking partial failures and data inconsistency

Suggestion: Wrap the loop in a database transaction with try/except to commit on success and rollback on failure. Use transaction context manager or begin/commit/rollback pattern.

Confidence: 87%

Rule: py_add_transaction_handling_with_rollback

_{Powered by diffray - AI Code Review Agent}

added bulk add and bulk remove of resources

bd8c956

coderabbitai Bot mentioned this pull request Oct 30, 2025

Feature/batch upload and gdrive integration henryperkins/open-notebook#1

Merged

diffray-bot reviewed Dec 10, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature]: Select/deselect all sources in a Notebook #223#231

[Feature]: Select/deselect all sources in a Notebook #223#231
MasterAffan wants to merge 1 commit intolfnovo:mainfrom
MasterAffan:main

MasterAffan commented Oct 29, 2025 •

edited

Loading

Uh oh!

MasterAffan commented Oct 30, 2025

Uh oh!

MasterAffan commented Oct 30, 2025

Uh oh!

diffray-bot Dec 10, 2025

Uh oh!

diffray-bot Dec 10, 2025

Uh oh!

diffray-bot Dec 10, 2025

Uh oh!

diffray-bot commented Dec 10, 2025

🟠 HIGH - Race condition in async useEffect without cancellation

🔴 CRITICAL - SQL injection in ORDER BY clause

🔴 CRITICAL - N+1 database queries in bulk operation loop

🟠 HIGH - Missing transaction handling with rollback

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

MasterAffan commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MasterAffan commented Oct 30, 2025

Uh oh!

MasterAffan commented Oct 30, 2025

Uh oh!

diffray-bot Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

diffray-bot Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

diffray-bot Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

diffray-bot commented Dec 10, 2025

diffray code review

Summary

Issues Found: 4

🟠 HIGH - Race condition in async useEffect without cancellation

🔴 CRITICAL - SQL injection in ORDER BY clause

🔴 CRITICAL - N+1 database queries in bulk operation loop

🟠 HIGH - Missing transaction handling with rollback

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

MasterAffan commented Oct 29, 2025 •

edited

Loading