plugin(lightspeed-notebooks): de-duplicate documents and fix document deletion

[JslYoon]

Hey, I just made a Pull Request!

• https://redhat.atlassian.net/jira/software/c/projects/RHDHBUGS/boards/5367?assignee=712020%3A9695a675-537a-42ce-9b87-0495545b71be&selectedIssue=RHDHBUGS-3200
• https://redhat.atlassian.net/browse/RHDHBUGS-3199

Cherry pick issues to fix two commits indicated above.
Commit e49fc2c fixes the first issue that was breaking the delete documents.

✔️ 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)

[rhdh-qodo-merge]

PR Summary by Qodo

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

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 Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.97%. Comparing base (230b90c) to head (f36806b).
✅ 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    65128   -12     
  Branches                  16929    16924    -5     
=====================================================
- Hits                      39719    39715    -4     
+ Misses                    25199    25191    -8     
  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.40% <ø> (+0.07%)	⬆️	Carriedforward from 230b90c
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...f36806b. 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]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Missing SSE end event ✓ Resolved 🐞 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

---

2. Sidebar add not disabled ✓ Resolved 🐞 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

---

3. Private baseURL coupling ✓ Resolved 🐞 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

---

[Jdubrick]

Is this the cherry pick of https://github.com/redhat-developer/rhdh-plugins/pull/3140/commits? It looks like there is a commit that wasn't in the original fix? e49fc2c

Can we update the description so we know what change is being brought over, associated bug with the fix version, etc?

[Jdubrick]

I think this is going to need to be a patch, I don't think minor is appropriate for a backport fix. There is docs here and an example here for naming as well for the changeset and the PR title

[Jdubrick]

Can we update the description so its very clear why the backport, commit its from, etc? Something like https://github.com/backstage/community-plugins/pull/7777/changes#diff-8f790b830a5a1ede0c535edcbfe803bfb27de75537befc4461e2cfb9e1421302R7?

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud