Skip to content

Lightspeed/release 1.10#3557

Closed
JslYoon wants to merge 2 commits into
workspace/lightspeedfrom
lightspeed/release-1.10
Closed

Lightspeed/release 1.10#3557
JslYoon wants to merge 2 commits into
workspace/lightspeedfrom
lightspeed/release-1.10

Conversation

@JslYoon

@JslYoon JslYoon commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Hey, I just made a Pull Request!

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or Updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)

JslYoon and others added 2 commits June 18, 2026 14:54
* documents verified before adding another one in notebooks

Signed-off-by: Lucas <lyoon@redhat.com>

# Conflicts:
#	workspaces/lightspeed/plugins/lightspeed/src/translations/ref.ts

* adding changeset

Signed-off-by: Lucas <lyoon@redhat.com>

* Update API reports for new translation key

Add notebook.view.documents.uploadsInProgress to lightspeed translation API

Signed-off-by: Lucas <lyoon@redhat.com>

* address comments

Signed-off-by: Lucas <lyoon@redhat.com>

* fix document deletion

Signed-off-by: Lucas <lyoon@redhat.com>

* fix: restore SSE end event, fix upload gating, and refactor conversation deletion

- Restore legacy 'end' event emission in notebooksRouters SSE transform
- Extract citations from file_search tool calls for referenced_documents
- Fix DocumentSidebar to respect hasUploadsInProgress when disabling Add button
- Add conversations API to VectorStoresOperator for proper abstraction
- Update SessionService to use conversations.delete instead of internal baseURL access

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* fix(lightspeed): update changeset from minor to patch

* fix(lightspeed): update changeset description for backport

---------

Signed-off-by: Lucas <lyoon@redhat.com>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
…Es (#3481)

Signed-off-by: Jessica He <jhe@redhat.com>
@codecov

codecov Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 28.20513% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.96%. Comparing base (230b90c) to head (0476a85).
⚠️ Report is 2 commits behind head on workspace/lightspeed.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@                   Coverage Diff                    @@
##           workspace/lightspeed    #3557      +/-   ##
========================================================
- Coverage                 60.97%   60.96%   -0.02%     
========================================================
  Files                      2098     2098              
  Lines                     65140    65167      +27     
  Branches                  16929    16940      +11     
========================================================
+ Hits                      39719    39726       +7     
- Misses                    25199    25219      +20     
  Partials                    222      222              
Flag Coverage Δ *Carryforward flag
adoption-insights 83.58% <ø> (ø) Carriedforward from ca95e26
ai-integrations 70.03% <ø> (ø) Carriedforward from ca95e26
app-defaults 69.60% <ø> (ø) Carriedforward from ca95e26
augment 69.36% <ø> (ø) Carriedforward from ca95e26
bulk-import 72.86% <ø> (ø) Carriedforward from ca95e26
cost-management 16.49% <ø> (ø) Carriedforward from ca95e26
dcm 32.85% <ø> (ø) Carriedforward from ca95e26
extensions 61.79% <ø> (ø) Carriedforward from ca95e26
global-floating-action-button 74.30% <ø> (ø) Carriedforward from ca95e26
global-header 61.68% <ø> (ø) Carriedforward from ca95e26
homepage 50.95% <ø> (ø) Carriedforward from ca95e26
konflux 91.01% <ø> (ø) Carriedforward from ca95e26
lightspeed 68.13% <28.20%> (-0.21%) ⬇️
mcp-integrations 81.59% <ø> (ø) Carriedforward from ca95e26
orchestrator 36.36% <ø> (ø) Carriedforward from ca95e26
quickstart 62.88% <ø> (ø) Carriedforward from ca95e26
sandbox 79.56% <ø> (ø) Carriedforward from ca95e26
scorecard 83.58% <ø> (ø) Carriedforward from ca95e26
theme 64.54% <ø> (ø) Carriedforward from ca95e26
translations 8.49% <ø> (ø) Carriedforward from ca95e26
x2a 78.28% <ø> (ø) Carriedforward from ca95e26

*This pull request uses carry forward flags. Click here to find out more.


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 230b90c...0476a85. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rhdh-qodo-merge

Copy link
Copy Markdown

PR Summary by Qodo

Lightspeed 1.10 backport: gate notebook uploads and fix session deletion cleanup
🐞 Bug fix ✨ Enhancement 🕐 40+ Minutes

Grey Divider

Description

• Prevent adding notebook documents while uploads/doc fetch are still in progress.
• Fix session deletion to also delete its conversation and vector-store file entries.
• Require file_search tool usage and add a user-facing translation for the new gating state.
Diagram

graph TD
  U["Notebook user"] --> UI["Notebook UI"] --> BE["Notebooks backend"] --> OP["VectorStoresOperator"] --> LS["Lightspeed service"]
  BE -->|"SSE query (tool_choice=required)"| LS
  OP -->|"DELETE conversation"| CONV["Conversations API"]
  OP -->|"DELETE vector-store files"| VS[("Vector store")]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Server-side upload gating/queueing
  • ➕ Eliminates race conditions even if multiple clients/tabs upload simultaneously
  • ➕ Keeps UI simpler (backend becomes the source of truth)
  • ➖ Requires API semantics/design (reject vs queue) and more backend changes
  • ➖ May need new status endpoints/events to provide good UX
2. Centralize upload-in-progress state in a shared hook/store
  • ➕ Avoids prop-drilling (LightSpeedChat → NotebookView → Sidebar/Modal)
  • ➕ Reduces risk of inconsistent gating logic across components
  • ➖ Introduces shared state complexity (context/store) for a relatively small feature
  • ➖ Still needs careful handling of server-fetch vs local pending uploads

Recommendation: The PR’s approach (UI gating + explicit backend cleanup via a conversations API wrapper) is a good low-risk backport for the 1.10 line. Consider adding server-side enforcement in a future mainline change to fully prevent concurrent uploads across tabs/clients; for this release branch, the current UI guardrails plus improved deletion cleanup strike a reasonable balance.

Files changed (11) +121 / -25

Enhancement (4) +42 / -4
constant.tsAdjust notebooks system prompt persona wording +1/-1

Adjust notebooks system prompt persona wording

• Updates the notebook system prompt to remove the 'Senior' qualifier while keeping the document-grounding constraints intact.

workspaces/lightspeed/plugins/lightspeed-backend/src/service/constant.ts

VectorStoresOperator.tsAdd conversations.delete API wrapper +36/-0

Add conversations.delete API wrapper

• Extends the operator with a Conversations API abstraction to delete conversations via the Lightspeed service, including logging and error handling.

workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts

LightSpeedChat.tsxPropagate documents-fetching state into NotebookView +3/-3

Propagate documents-fetching state into NotebookView

• Plumbs isFetching from useNotebookDocuments into NotebookView so the notebook can treat document refresh as an in-progress state for gating uploads.

workspaces/lightspeed/plugins/lightspeed/src/components/LightSpeedChat.tsx

ref.tsAdd uploadsInProgress notebook translation string +2/-0

Add uploadsInProgress notebook translation string

• Adds a new translation message used when the UI blocks additional uploads while existing uploads are still processing.

workspaces/lightspeed/plugins/lightspeed/src/translations/ref.ts

Bug fix (5) +68 / -21
notebooksRouters.tsRequire tool usage for notebook queries +1/-0

Require tool usage for notebook queries

• Forces the responses API request to require tool execution (file_search) by setting tool_choice='required', strengthening document-grounding behavior.

workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/notebooksRouters.ts

sessionService.tsDelete conversations and vector-store file entries when deleting a session +33/-10

Delete conversations and vector-store file entries when deleting a session

• Enhances session deletion to remove any associated conversation via the operator abstraction and deletes vector-store file entries before deleting the vector store itself, reducing orphaned resources.

workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/sessions/sessionService.ts

AddDocumentModal.tsxDisable adding files while uploads are in progress and show info alert +9/-1

Disable adding files while uploads are in progress and show info alert

• Adds hasUploadsInProgress support to block the Add action during active processing and displays an informational alert explaining why uploads are gated.

workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/AddDocumentModal.tsx

DocumentSidebar.tsxDisable sidebar Add button while uploads are in progress +11/-4

Disable sidebar Add button while uploads are in progress

• Extends the sidebar to respect hasUploadsInProgress, disabling the Add button and switching tooltip text to the new uploads-in-progress message.

workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/DocumentSidebar.tsx

NotebookView.tsxCompute hasUploadsInProgress and gate Add interactions consistently +14/-6

Compute hasUploadsInProgress and gate Add interactions consistently

• Introduces a unified hasUploadsInProgress condition (pending uploads or document refetching) and wires it to both the sidebar and upload modal, with updated tooltip messaging.

workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/NotebookView.tsx

Documentation (1) +1 / -0
report-alpha.api.mdUpdate translation API report with uploadsInProgress key +1/-0

Update translation API report with uploadsInProgress key

• Adds the new notebook document gating translation key to the generated translation API report.

workspaces/lightspeed/plugins/lightspeed/report-alpha.api.md

Other (1) +10 / -0
khaki-tomatoes-help.mdAdd patch changeset documenting the backported notebook fixes +10/-0

Add patch changeset documenting the backported notebook fixes

• Introduces a patch changeset describing the backport scope for upload gating and conversation deletion cleanup in notebooks.

workspaces/lightspeed/.changeset/khaki-tomatoes-help.md

@rhdh-qodo-merge

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Orphaned uploaded files 🐞 Bug ☼ Reliability
Description
SessionService.deleteSession removes vector-store file entries but does not delete the underlying
Files API objects created during uploads, so deleting a notebook can leave uploaded file content
orphaned. Over time this can retain user data and increase storage usage even though the
notebook/session is gone.
Code

workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/sessions/sessionService.ts[R205-225]

+    // Delete all vector store files (same pattern as documentService.deleteDocument)
    try {
      const filesResponse =
        await this.client.vectorStores.files.list(sessionId);
-      const fileIds = filesResponse.data?.map((f: any) => f.file_id) || [];
+      const files = filesResponse.data || [];
+
+      this.logger.info(
+        `Deleting ${files.length} vector store files for session ${sessionId}`,
+      );

      await Promise.all(
-        fileIds.map(async (fileId: string) => {
+        files.map(async (file: any) => {
          try {
-            await this.client.files.delete(fileId);
-            this.logger.info(`Deleted file ${fileId} from Files API`);
+            await this.client.vectorStores.files.delete(sessionId, file.id);
+            this.logger.info(
+              `Deleted vector store file ${file.id} (${file.attributes?.title || 'untitled'})`,
+            );
          } catch (error) {
-            this.logger.warn(`Failed to delete file ${fileId}: ${error}`);
+            this.logger.warn(
+              `Failed to delete vector store file ${file.id}: ${error}`,
+            );
Evidence
The upload flow creates Files API objects, but the updated session deletion flow only removes vector
store associations; fixtures show these are distinct stores, so removing the association does not
remove the file itself.

workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/sessions/sessionService.ts[186-237]
workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/documents/documentService.ts[100-116]
workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/documents/documentService.ts[267-285]
workspaces/lightspeed/plugins/lightspeed-backend/fixtures/lightspeedCoreHandlers.ts[70-74]
workspaces/lightspeed/plugins/lightspeed-backend/fixtures/lightspeedCoreHandlers.ts[230-243]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`SessionService.deleteSession` deletes vector-store file associations but never deletes the uploaded file objects from the Files API. Since uploads are created via `VectorStoresOperator.files.create`, notebook deletion can leave behind orphaned files.

### Issue Context
- Upload path: `DocumentService.uploadFile` creates a file via `client.files.create()` and then associates it to the vector store using `vectorStores.files.create(..., { file_id })`.
- Delete path (after this PR): session deletion only calls `vectorStores.files.delete(...)`.
- Repo test fixture models the Files API store separately from vector-store associations; deleting a vector-store file only removes the association.

### Fix Focus Areas
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/sessions/sessionService.ts[186-237]
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/documents/documentService.ts[100-116]
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/documents/documentService.ts[267-285]

### Suggested change
1. In `SessionService.deleteSession`, after `vectorStores.files.delete(sessionId, file.id)` succeeds, also call `this.client.files.delete(file.id)` (or whatever identifier corresponds to the Files API object) to remove the uploaded file itself.
2. Consider aligning `DocumentService.deleteDocument` with its docstring (“Delete ... from vector store and Files API”) by also deleting the Files API object there as well, so document-level deletes don’t leak files either.
3. Log/handle failures carefully (e.g., if Files API delete 404s, treat as already-gone).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Refetch blocks document upload 🐞 Bug ≡ Correctness
Description
NotebookView treats isDocumentsFetching (react-query background refetch state) as “uploads in
progress”, disabling the Add Document UI and modal submission during ordinary refetches
(focus/reconnect/invalidation) even when no uploads are happening. This can prevent users from
uploading and shows an incorrect “uploads in progress” message.
Code

workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/NotebookView.tsx[R553-555]

+  const hasUploadsInProgress = pendingUploads.length > 0 || isDocumentsFetching;
+  const isAddDisabled =
+    totalDocumentCount >= NOTEBOOK_MAX_FILES || hasUploadsInProgress;
Evidence
isFetching from useNotebookDocuments is a generic refetch signal (staleTime=0) and is wired into
hasUploadsInProgress, which then disables Add/upload UI in multiple places.

workspaces/lightspeed/plugins/lightspeed/src/components/LightSpeedChat.tsx[693-699]
workspaces/lightspeed/plugins/lightspeed/src/hooks/notebooks/useNotebookDocuments.ts[24-35]
workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/NotebookView.tsx[551-556]
workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/AddDocumentModal.tsx[266-332]
workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/DocumentSidebar.tsx[158-207]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`hasUploadsInProgress` is computed as `pendingUploads.length > 0 || isDocumentsFetching`. `isDocumentsFetching` represents any documents-query refetch, not actual document processing/uploads, so it can disable uploads unexpectedly.

### Issue Context
- `useNotebookDocuments` sets `staleTime: 0`, so background refetches are common.
- `isDocumentsFetching` is passed from `LightSpeedChat` into `NotebookView`, then used to disable add/upload controls and to show the “uploadsInProgress” tooltip/alert.

### Fix Focus Areas
- workspaces/lightspeed/plugins/lightspeed/src/components/LightSpeedChat.tsx[693-699]
- workspaces/lightspeed/plugins/lightspeed/src/hooks/notebooks/useNotebookDocuments.ts[24-35]
- workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/NotebookView.tsx[551-556]
- workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/AddDocumentModal.tsx[266-332]
- workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/DocumentSidebar.tsx[158-207]

### Suggested change
1. Compute `hasUploadsInProgress` purely from real upload/process signals (e.g., `pendingUploads.length > 0`, or a backend-derived “any in_progress” status if available).
2. Keep `isDocumentsFetching` separate (e.g., for showing a small loading indicator), but don’t use it to block adding documents or to display the “uploads in progress” messaging.
3. Ensure the tooltip/message shown matches the real reason the button is disabled (max reached vs actual uploads).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

3. Missing locale message key 🐞 Bug ⚙ Maintainability
Description
The new translation key notebook.view.documents.uploadsInProgress is added to the translation
reference/English messages but is not defined in other locale bundles (de/es/fr/etc.), so
non-English users will not get a localized string for this message. This will result in
fallback-to-English or a missing-key display depending on the translation runtime behavior.
Code

workspaces/lightspeed/plugins/lightspeed/src/translations/ref.ts[R79-80]

+  'notebook.view.documents.uploadsInProgress':
+    'Please wait for current uploads to complete before adding more documents.',
Evidence
The key exists in the reference messages, but at least several locale bundles around the same
notebook-documents section do not include it.

workspaces/lightspeed/plugins/lightspeed/src/translations/ref.ts[63-82]
workspaces/lightspeed/plugins/lightspeed/src/translations/de.ts[206-214]
workspaces/lightspeed/plugins/lightspeed/src/translations/es.ts[206-214]
workspaces/lightspeed/plugins/lightspeed/src/translations/fr.ts[206-214]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
A new i18n key was added to the reference messages, but the locale-specific message files don’t define it.

### Issue Context
The UI now calls `t('notebook.view.documents.uploadsInProgress')`, so missing locale entries will reduce translation quality for non-English users.

### Fix Focus Areas
- workspaces/lightspeed/plugins/lightspeed/src/translations/ref.ts[63-83]
- workspaces/lightspeed/plugins/lightspeed/src/translations/de.ts[206-220]
- workspaces/lightspeed/plugins/lightspeed/src/translations/es.ts[206-218]
- workspaces/lightspeed/plugins/lightspeed/src/translations/fr.ts[206-220]
- workspaces/lightspeed/plugins/lightspeed/src/translations/it.ts[206-220]
- workspaces/lightspeed/plugins/lightspeed/src/translations/ja.ts[203-214]

### Suggested change
Add `notebook.view.documents.uploadsInProgress` to each locale file with an appropriate translation (or temporarily copy the English string until proper translations are available).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@rhdh-qodo-merge rhdh-qodo-merge Bot added enhancement New feature or request bug_fix Other labels Jun 23, 2026
@JslYoon JslYoon closed this Jun 24, 2026
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants