Skip to content

Lightspeed cherry pick clean#3428

Open
JslYoon wants to merge 5 commits into
redhat-developer:workspace/lightspeedfrom
JslYoon:lightspeed-cherry-pick-clean
Open

Lightspeed cherry pick clean#3428
JslYoon wants to merge 5 commits into
redhat-developer:workspace/lightspeedfrom
JslYoon:lightspeed-cherry-pick-clean

Conversation

@JslYoon

@JslYoon JslYoon commented Jun 16, 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 added 5 commits June 16, 2026 18:06
Signed-off-by: Lucas <lyoon@redhat.com>

# Conflicts:
#	workspaces/lightspeed/plugins/lightspeed/src/translations/ref.ts
Signed-off-by: Lucas <lyoon@redhat.com>
Add notebook.view.documents.uploadsInProgress to lightspeed translation API

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

Copy link
Copy Markdown

@rhdh-qodo-merge

Copy link
Copy Markdown

PR Summary by Qodo

Prevent concurrent notebook uploads and improve notebook session cleanup
🐞 Bug fix ✨ Enhancement 📝 Documentation 🕐 40+ Minutes

Grey Divider

Description

• Block adding notebook documents while uploads or document processing are in progress.
• Add an i18n key and update translation API reports for the new message.
• Fix notebook session deletion by cleaning up conversations and vector-store files.
Diagram

graph TD
  UI["Notebook upload UI"] --> Hook(["useNotebookDocuments"]) --> Router["Notebooks router"] --> Core{{"Lightspeed core"}}
  Router --> Session["SessionService"] --> Core
  UI --> I18N["i18n message"]
  subgraph Legend
    direction LR
    _ui["UI"] ~~~ _hook(["Hook"]) ~~~ _ext{{"External"}}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Backend-enforced concurrency (return 409 while processing)
  • ➕ Prevents race conditions and bypass (multi-tab, direct API calls)
  • ➕ Centralizes the rule in a single authoritative layer
  • ➖ Requires a reliable backend notion of “processing in progress” and persistence
  • ➖ More implementation and testing effort in backend services
2. Gate solely from upload mutation state (avoid tying to document refetch)
  • ➕ Less chance of blocking uploads due to transient refetches
  • ➕ Simplifies UI logic by relying on explicit upload state
  • ➖ May miss in-progress work after reload/cross-tab unless state is persisted/synced
  • ➖ Still doesn’t prevent API-level concurrent uploads without backend checks

Recommendation: The PR’s UI gating is a pragmatic UX fix and should reduce user-triggered concurrency issues. Consider adding a backend 409/guard as a follow-up to make the constraint robust against races and non-UI callers.

Files changed (10) +96 / -60

Enhancement (4) +15 / -7
constant.tsAdjust notebooks system prompt wording +1/-1

Adjust notebooks system prompt wording

• Updates the notebook system prompt persona text (Senior Research Analyst → Research Analyst).

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

LightSpeedChat.tsxPass document fetch state into NotebookView +3/-3

Pass document fetch state into NotebookView

• Reads 'isFetching' from 'useNotebookDocuments' and passes it down so the notebook UI can treat document refetching as an uploads-in-progress state.

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

DocumentSidebar.tsxShow uploads-in-progress tooltip messaging +9/-3

Show uploads-in-progress tooltip messaging

• Adds 'hasUploadsInProgress' and switches the disabled Add tooltip text between “max reached” and “uploads in progress”.

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

ref.tsAdd uploads-in-progress translation message +2/-0

Add uploads-in-progress translation message

• Adds 'notebook.view.documents.uploadsInProgress' string used by the notebook UI when uploads/processing are ongoing.

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

Bug fix (4) +75 / -53
notebooksRouters.tsRequire tool usage in queries and trim completed-event legacy output +2/-36

Require tool usage in queries and trim completed-event legacy output

• Adds 'tool_choice: 'required'' to enforce tool execution (file_search) for notebook queries. Removes legacy 'end' event emission on 'response.completed' (leaving logging and a commented-out push).

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

sessionService.tsDelete conversations and vector-store files when deleting sessions +50/-10

Delete conversations and vector-store files when deleting sessions

• Enhances 'deleteSession' to delete the associated conversation when present and explicitly remove vector-store files before deleting the vector store. Improves logging and error handling to reduce orphaned resources.

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

AddDocumentModal.tsxDisable Add action while uploads are in progress +9/-1

Disable Add action while uploads are in progress

• Introduces 'hasUploadsInProgress' prop, shows an informational alert when set, and disables the modal Add button to prevent concurrent uploads.

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

NotebookView.tsxCompute uploads-in-progress state and wire it into upload UI +14/-6

Compute uploads-in-progress state and wire it into upload UI

• Adds 'isDocumentsFetching' prop and derives 'hasUploadsInProgress' from pending uploads and document fetching. Uses this to disable add actions and to pass state to DocumentSidebar and AddDocumentModal.

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

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

Update translation API report for new key

• Adds 'notebook.view.documents.uploadsInProgress' to the generated translation reference report.

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

Other (1) +5 / -0
khaki-tomatoes-help.mdAdd changeset for upload-in-progress behavior +5/-0

Add changeset for upload-in-progress behavior

• Adds a minor changeset noting that documents cannot be uploaded while processing is in progress.

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

@codecov

codecov Bot commented Jun 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 26.47059% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.97%. Comparing base (230b90c) to head (e49fc2c).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@                  Coverage Diff                  @@
##           workspace/lightspeed    #3428   +/-   ##
=====================================================
  Coverage                 60.97%   60.97%           
=====================================================
  Files                      2098     2098           
  Lines                     65140    65148    +8     
  Branches                  16929    16935    +6     
=====================================================
+ Hits                      39719    39725    +6     
- Misses                    25199    25201    +2     
  Partials                    222      222           
Flag Coverage Δ *Carryforward flag
adoption-insights 83.58% <ø> (ø) Carriedforward from 230b90c
ai-integrations 70.03% <ø> (ø) Carriedforward from 230b90c
app-defaults 69.60% <ø> (ø) Carriedforward from 230b90c
augment 69.36% <ø> (ø) Carriedforward from 230b90c
bulk-import 72.86% <ø> (ø) Carriedforward from 230b90c
cost-management 16.49% <ø> (ø) Carriedforward from 230b90c
dcm 32.85% <ø> (ø) Carriedforward from 230b90c
extensions 61.79% <ø> (ø) Carriedforward from 230b90c
global-floating-action-button 74.30% <ø> (ø) Carriedforward from 230b90c
global-header 61.68% <ø> (ø) Carriedforward from 230b90c
homepage 50.95% <ø> (ø) Carriedforward from 230b90c
konflux 91.01% <ø> (ø) Carriedforward from 230b90c
lightspeed 68.34% <26.47%> (+<0.01%) ⬆️
mcp-integrations 81.59% <ø> (ø) Carriedforward from 230b90c
orchestrator 36.36% <ø> (ø) Carriedforward from 230b90c
quickstart 62.88% <ø> (ø) Carriedforward from 230b90c
sandbox 79.56% <ø> (ø) Carriedforward from 230b90c
scorecard 83.58% <ø> (ø) Carriedforward from 230b90c
theme 64.54% <ø> (ø) Carriedforward from 230b90c
translations 8.49% <ø> (ø) Carriedforward from 230b90c
x2a 78.28% <ø> (ø) Carriedforward from 230b90c

*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...e49fc2c. 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

Code Review by Qodo

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

Grey Divider


Action required

1. Missing SSE end event 🐞 Bug ≡ Correctness
Description
createResponsesApiTransform no longer emits the legacy event: 'end' message on
response.completed, so the UI won’t receive referenced_documents and may never finalize a
response when the stream produces no output_text.delta tokens.
Code

workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/notebooksRouters.ts[R228-237]

          } else if (eventType === 'response.completed') {
-            const usage = parsed?.response?.usage;
-
            // Log the full response to see what we're getting
            logger.info(
              `Full response.completed event: ${JSON.stringify(parsed?.response, null, 2)}`,
            );

            // Extract citations/sources from tool calls (file_search results)
-            const toolCalls = parsed?.response?.tool_calls || [];
-            logger.info(
-              `Tool calls received: ${JSON.stringify(toolCalls, null, 2)}`,
-            );
-
-            const referencedDocuments: any[] = [];
-
-            for (const toolCall of toolCalls) {
-              if (toolCall.tool_name === 'file_search') {
-                logger.info(
-                  `Found file_search tool call: ${JSON.stringify(toolCall, null, 2)}`,
-                );
-                const citations = toolCall.content?.citations || [];
-                for (const citation of citations) {
-                  referencedDocuments.push({
-                    document_id: citation.document_id || citation.file_id,
-                    content: citation.text || citation.content,
-                  });
-                }
-              }
-            }

-            logger.info(
-              `Referenced documents: ${JSON.stringify(referencedDocuments)}`,
-            );
-
-            const legacy = {
-              event: 'end',
-              data: {
-                referenced_documents: referencedDocuments,
-                input_tokens: usage?.input_tokens,
-                output_tokens: usage?.output_tokens,
-              },
-            };
-            this.push(`data: ${JSON.stringify(legacy)}\n\n`);
+            // this.push(`data: ${JSON.stringify(legacy)}\n\n`);
          } else {
Evidence
The backend transform no longer pushes an end event on completion, while the frontend explicitly
handles event === 'end' to apply referenced_documents and clear loading for the last message.

workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/notebooksRouters.ts[156-240]
workspaces/lightspeed/plugins/lightspeed/src/hooks/useConversationMessages.ts[712-752]

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

## Issue description
The Responses API SSE transform in `notebooksRouters.ts` stopped pushing the legacy `{ event: 'end', data: ... }` payload when `eventType === 'response.completed'`. The frontend’s streaming handler still relies on this event to attach `referenced_documents` (sources) and to reliably transition the last message out of loading when no text deltas are emitted.

## Issue Context
- The backend currently logs `response.completed` and does not push any completion event.
- The frontend listens for `event === 'end'` to read `data.referenced_documents` and finalize the last message.

## Fix Focus Areas
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/notebooksRouters.ts[156-240]
- workspaces/lightspeed/plugins/lightspeed/src/hooks/useConversationMessages.ts[712-752]

## What to change
1. Restore emitting the legacy `end` event inside the `response.completed` branch.
2. If citations extraction is still desired, reintroduce parsing of citations/tool-calls (or equivalent response fields) and populate `referenced_documents`; otherwise, emit `end` with an empty `referenced_documents: []` at minimum.
3. Ensure an `end` event is emitted even when there are no `response.output_text.delta` tokens (tool-only responses / empty answers), so the client can reliably finalize state.

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



Remediation recommended

2. Sidebar add not disabled 🐞 Bug ≡ Correctness
Description
DocumentSidebar accepts hasUploadsInProgress but still computes isAddDisabled only from
document count, leaving the sidebar Add button enabled during uploads while NotebookView treats
uploads/fetching as disabling conditions.
Code

workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/DocumentSidebar.tsx[R186-199]

        </Typography>
        {isAddDisabled ? (
          <Tooltip
-            content={t('notebook.view.documents.maxReached')}
+            content={
+              hasUploadsInProgress
+                ? t('notebook.view.documents.uploadsInProgress')
+                : t('notebook.view.documents.maxReached')
+            }
            position="top"
          >
-            <span>
+            <Typography component="div">
              <Button
                variant="link"
                className={classes.addButton}
Evidence
NotebookView computes hasUploadsInProgress and intends it to disable adding, but
DocumentSidebar continues to disable only when max files is reached, using hasUploadsInProgress
only for tooltip text inside the disabled branch.

workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/DocumentSidebar.tsx[158-216]
workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/NotebookView.tsx[551-576]

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

## Issue description
`DocumentSidebar` receives `hasUploadsInProgress` but does not use it when determining whether the Add button is disabled. This makes upload gating inconsistent across the notebook UI (collapsed add icon is disabled, but the expanded sidebar add button remains enabled).

## Issue Context
`NotebookView` computes `hasUploadsInProgress` and passes it down, but `DocumentSidebar` still derives `isAddDisabled` from `totalCount` only.

## Fix Focus Areas
- workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/DocumentSidebar.tsx[158-216]
- workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/NotebookView.tsx[551-576]

## What to change
- Update `DocumentSidebar`’s `isAddDisabled` calculation to include `hasUploadsInProgress` (e.g., `totalCount >= NOTEBOOK_MAX_FILES || hasUploadsInProgress`).
- Ensure the tooltip text matches the actual disabled reason (uploads vs max reached).

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



Informational

3. Private baseURL coupling 🐞 Bug ⚙ Maintainability
Description
SessionService.deleteSession reads VectorStoresOperator’s private baseURL via `(this.client as
any).baseURL` to call the conversations API, creating brittle coupling to an internal field that
isn’t part of the operator’s public interface.
Code

workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/sessions/sessionService.ts[R191-205]

+    // Delete associated conversation if it exists
+    if (conversationId) {
+      try {
+        // Access the baseURL from the VectorStoresOperator client
+        const baseURL = (this.client as any).baseURL;
+        const response = await fetch(
+          `${baseURL}/v2/conversations/${encodeURIComponent(conversationId)}?user_id=${encodeURIComponent(userId)}`,
+          {
+            method: 'DELETE',
+            headers: {
+              'Content-Type': 'application/json',
+            },
+          },
+        );
+
Evidence
baseURL is declared private in VectorStoresOperator, yet SessionService accesses it via `as
any`, which is a direct sign it’s not part of the public API contract.

workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts[97-105]
workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/sessions/sessionService.ts[186-205]

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` reaches into `VectorStoresOperator` internals (`(this.client as any).baseURL`) to build a raw URL for deleting conversations. This bypasses the operator abstraction and will be fragile if the operator implementation changes.

## Issue Context
`VectorStoresOperator` already centralizes lightspeed-core calls and holds `baseURL` as a private field.

## Fix Focus Areas
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/sessions/sessionService.ts[186-220]
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts[97-125]

## What to change
- Add a small public method on `VectorStoresOperator` (e.g., `conversations.delete(conversationId, userId)` or a `getBaseUrl()` accessor) and use that from `SessionService`.
- Keep the HTTP error handling consistent with the rest of `VectorStoresOperator` (so failures are logged/handled uniformly).

ⓘ 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 documentation Improvements or additions to documentation enhancement New feature or request Bug fix labels Jun 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug fix documentation Improvements or additions to documentation enhancement New feature or request workspace/lightspeed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant