chore: sync lightspeed release-1.10 to workspace

[Jdubrick]

Backport of #3911 to release 1.10

Backport Chain

• Original / Backport PR build(deps): bump micromatch from 4.0.7 to 4.0.8 #1: [release-1.10] chore(lightspeed): update monaco-editor to ^0.56.0 #3911 (merged)
• This PR: Part 2/2 - Sync to workspace

Purpose

This PR:

1. Brings backported changes from lightspeed/release-1.10 into workspace/lightspeed
2. Triggers the Version Packages workflow
3. After merge, Version Packages PR will update version numbers and CHANGELOG

Important

⚠️ Do not edit this PR manually

This PR should be merged as-is after CI passes.
Any manual changes may interfere with Version Packages generation.

---

Created via backport-create skill (PR #1 already merged as #3911).

Workflow: This PR was created from upstream:lightspeed/release-1.10 (not a fork) to ensure Version Packages workflow triggers correctly.

Made with Cursor

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (workspace/lightspeed@5a0ebed). Learn more about missing BASE report.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@                   Coverage Diff                   @@
##             workspace/lightspeed    #3950   +/-   ##
=======================================================
  Coverage                        ?   60.96%           
=======================================================
  Files                           ?     2098           
  Lines                           ?    65167           
  Branches                        ?    16945           
=======================================================
  Hits                            ?    39726           
  Misses                          ?    25219           
  Partials                        ?      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.13% <ø> (?)
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 5a0ebed...05cfc6a. 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]

PR Summary by Qodo

Backport Lightspeed notebook upload gating + conversation cleanup; bump monaco-editor

🐞 Bug fix ⚙️ Configuration changes 🕐 40+ Minutes

AI Description

• Prevent adding new notebook documents while uploads or document refresh are in progress.
• Delete notebook conversations via the conversations API and clean up vector store files on session
 deletion.
• Bump monaco-editor to ^0.56.0 and add a new UI translation key for the upload-in-progress state.

Diagram

graph TD
  UI["Notebook UI"] --> Gate["Upload gating"] --> I18N["Translations/API report"]
  UI --> Delete["Delete session"] --> SessSvc["SessionService"] --> VSO["VectorStoresOperator"] --> Core{{"Lightspeed core API"}}
  VSO --> VS["Vector stores/files"]
  VSO --> Conv["Conversations"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Enforce upload gating server-side

• ➕ Prevents race conditions even if UI state is stale or multiple clients are used
• ➕ Centralizes the rule near the vector-store operation that needs protection
• ➖ Requires API changes and potentially more complex error handling in clients
• ➖ Riskier for a release-line backport; harder to keep compatibility

2. Background cleanup job for session deletion

• ➕ Makes deletion fast and more resilient (retries, partial failures handled asynchronously)
• ➕ Reduces chance of user-facing delete failures due to transient core API issues
• ➖ Adds operational complexity (queue/job runner) and new failure modes
• ➖ Not aligned with a minimal backport/sync PR

Recommendation: For a release-line backport, the PR’s approach (UI-level gating plus backend cleanup via a dedicated conversations API wrapper) is the best tradeoff: minimal surface-area change, improved correctness, and better encapsulation than direct baseURL calls. Consider server-side gating or async cleanup only if race conditions persist in production or deletion latency/reliability becomes a recurring issue.

Files changed (14) +182 / -95

Enhancement (2) +3 / -1

constant.tsAdjust notebooks system prompt wording +1/-1

Adjust notebooks system prompt wording

• Updates the notebooks system prompt to remove the 'Senior' qualifier. Keeps behavior the same while changing assistant persona wording.

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

ref.tsAdd uploadsInProgress translation string +2/-0

Add uploadsInProgress translation string

• Adds notebook.view.documents.uploadsInProgress to the translation reference map. Enables consistent user-facing messaging when uploads are still being processed.

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

Bug fix (7) +107 / -24

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

Add conversations.delete API wrapper

• Adds a Conversations API section to VectorStoresOperator with a delete method targeting /v2/conversations/{id}. Centralizes HTTP construction, logging, and error handling for conversation deletion.

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

notebooksRouters.tsRequire tool invocation for notebook streaming requests +1/-0

Require tool invocation for notebook streaming requests

• Sets tool_choice: 'required' for the file_search tool in the streaming request payload. This helps ensure the model performs document retrieval rather than answering without using the vector store.

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

sessionService.tsFix session deletion to clean conversations and vector-store files correctly +33/-10

Fix session deletion to clean conversations and vector-store files correctly

• Extends deleteSession to read session metadata and delete the associated conversation via the conversations API when present. Refactors file cleanup to delete vector-store files through vectorStores.files.delete instead of deleting raw Files API entries, then deletes the vector store.

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

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

Plumb documents fetching state into NotebookView

• Captures isFetching from useNotebookDocuments and passes it down to NotebookView. Enables downstream UI to treat document refresh as an in-progress upload state for gating.

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

AddDocumentModal.tsxDisable Add while uploads are in progress and show info message +9/-1

Disable Add while uploads are in progress and show info message

• Adds an optional hasUploadsInProgress prop to block the Add action while processing is ongoing. Displays an informational alert using the new translation key when uploads are in progress.

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

DocumentSidebar.tsxDisable Add button during active uploads and adjust tooltip messaging +11/-4

Disable Add button during active uploads and adjust tooltip messaging

• Adds hasUploadsInProgress and includes it in the add-button disablement condition. Updates tooltip content to differentiate between max-documents reached vs uploads currently in progress.

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

NotebookView.tsxCompute hasUploadsInProgress and gate add-document entry points +14/-6

Compute hasUploadsInProgress and gate add-document entry points

• Introduces isDocumentsFetching prop and computes hasUploadsInProgress from pending uploads or document fetching. Uses this state to disable add actions and to provide more accurate tooltip messaging; passes it to DocumentSidebar and AddDocumentModal.

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

Documentation (1) +1 / -0

report-alpha.api.mdUpdate translation API report with new uploadsInProgress key +1/-0

Update translation API report with new uploadsInProgress key

• Adds the notebook.view.documents.uploadsInProgress key to the generated translation type report. Keeps API surface documentation in sync with the new translation entry.

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

Other (4) +71 / -70

khaki-tomatoes-help.mdAdd changeset for notebook upload gating + conversation deletion backport +10/-0

Add changeset for notebook upload gating + conversation deletion backport

• Introduces a patch changeset documenting the backported fixes. Calls out upload gating to avoid vector store race conditions and proper conversation deletion behavior.

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

wicked-plants-smash.mdAdd changeset for monaco-editor dependency bump +5/-0

Add changeset for monaco-editor dependency bump

• Adds a patch changeset describing the monaco-editor upgrade. Ensures the Version Packages workflow will capture the dependency update.

workspaces/lightspeed/.changeset/wicked-plants-smash.md

package.jsonBump monaco-editor to ^0.56.0 +1/-1

Bump monaco-editor to ^0.56.0

• Updates the lightspeed plugin dependency on monaco-editor from ^0.55.0 to ^0.56.0. This drives corresponding lockfile changes.

workspaces/lightspeed/plugins/lightspeed/package.json

yarn.lockRefresh lockfile for monaco-editor upgrade and transitive updates +55/-69

Refresh lockfile for monaco-editor upgrade and transitive updates

• Updates yarn.lock to reflect monaco-editor ^0.56.0 and associated transitive changes (e.g., dompurify, axios, follow-redirects, ws, vm2). Ensures reproducible installs for the workspace.

workspaces/lightspeed/yarn.lock

[rhdh-qodo-merge]

Code Review by Qodo

🐞 Bugs (4) 📘 Rule violations (2) 🔗 Cross-repo conflicts (1) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 14 rules

✅ Cross-repo context

  Not relevant to this PR: redhat-developer/rhdh

  Not relevant to this PR: redhat-developer/rhdh-chart

  Not relevant to this PR: redhat-developer/rhdh-operator

  Not relevant to this PR: redhat-developer/rhdh-local

1. Upload gating gap 🐞 Bug ≡ Correctness ⭐ New

Description

NotebookView derives hasUploadsInProgress from pendingUploads/isDocumentsFetching, but uploads are
first tracked via uploadingFileNames before pendingUploads is populated. This leaves a window where
the sidebar/modal “Add” controls can remain enabled while an upload is already in-flight, allowing
concurrent uploads and reintroducing the race condition this PR aims to prevent.

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;

Relevance

⭐⭐⭐ High

Close to prior upload-gating changes; likely accept strengthening disablement to close
concurrent-upload race window.

PR-#3140
PR-#3290

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

NotebookView records upload start immediately via uploadingFileNames, but the new gating only
considers pendingUploads/isDocumentsFetching; DocumentSidebar’s Add button disablement is driven by
this gated boolean, so concurrent uploads remain possible during the in-flight gap.

workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/NotebookView.tsx[435-456]
workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/NotebookView.tsx[551-556]
workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/DocumentSidebar.tsx[158-166]

Agent prompt

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

### Issue description
`NotebookView` disables new uploads based on `hasUploadsInProgress`, but `hasUploadsInProgress` is computed as `pendingUploads.length > 0 || isDocumentsFetching`. Since `uploadingFileNames` is updated immediately in `handleFilesUploading()` while `pendingUploads` is only updated later (after `mutateAsync()` returns a `documentId`), there is an interval where an upload has already started yet `hasUploadsInProgress` remains falsy.

### Issue Context
This PR’s stated goal is to prevent additional document uploads while another document is being processed, to avoid vector-store race conditions. The current gating doesn’t cover the initial “upload request in-flight” phase.

### Fix Focus Areas
- workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/NotebookView.tsx[435-456]
- workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/NotebookView.tsx[551-556]

### Suggested fix
Update the boolean to include `uploadingFileNames.length > 0` (or introduce a single authoritative `uploadsInProgress` state that is set synchronously when uploads begin), e.g.:

```ts
const hasUploadsInProgress =
 uploadingFileNames.length > 0 || pendingUploads.length > 0 || isDocumentsFetching;
```

This ensures the add/upload UI is gated from the moment uploads begin, not only after the backend responds with IDs.

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

---

2. Orphaned uploaded files 🐞 Bug ☼ Reliability

Description

SessionService.deleteSession no longer deletes the underlying Files API objects created during
document uploads, so deleting a session can leave uploaded files orphaned and accumulate storage
over time. The new logic only deletes vector-store file associations and then deletes the vector
store.

Code

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

+    // 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}`,
+            );
          }
        }),
      );

Relevance

⭐⭐ Medium

Potentially real leak, but behavior depends on core semantics; fix may require broader
design/verification beyond this PR.

PR-#2861

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The backend upload path creates Files API objects, but the updated session deletion path only
removes vector-store file associations. The test fixture demonstrates uploaded files are stored
independently from vector-store membership and are not removed by vector store deletion, proving an
explicit Files API delete is required to avoid orphans.

workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/sessions/sessionService.ts[186-238]
workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/documents/documentService.ts[100-187]
workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts[411-488]
workspaces/lightspeed/plugins/lightspeed-backend/fixtures/lightspeedCoreHandlers.ts[70-245]

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()` currently removes vector-store file entries via `vectorStores.files.delete(...)` but never calls `files.delete(...)` for the underlying uploaded files.

### Issue Context
- Upload flow creates a Files API resource (`client.files.create`) and then attaches it to a vector store via `vectorStores.files.create({ file_id })`.
- The MSW fixture models these as independent resources (separate in-memory maps), and vector-store deletion does not remove uploaded files.

### Fix Focus Areas
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/sessions/sessionService.ts[186-238]
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts[411-488]
- workspaces/lightspeed/plugins/lightspeed-backend/__fixtures__/lightspeedCoreHandlers.ts[70-245]

### Suggested fix
1. While iterating vector store files, also delete the underlying file resource via `await this.client.files.delete(file.id)` (or the correct identifier if the API returns a different field).
2. Keep robust error handling (log and continue per-file), but ensure both deletions are attempted.
3. Update the MSW fixture to add a `DELETE /v1/files/:id` handler (so tests catch regressions and reflect the intended lifecycle).

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

---

3. DocumentSidebar uses MUI Typography 📘 Rule violation ⌂ Architecture ⭐ New

Description

The updated DocumentSidebar uses Material-UI (@material-ui/core) Typography components in the
notebook/chat UI instead of PatternFly equivalents. This violates the requirement to disallow MUI
usage in chat-related components and increases UI inconsistency/maintenance burden.

Code

workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/DocumentSidebar.tsx[R197-200]

+            <Typography component="div">
              <Button
                variant="link"
                className={classes.addButton}

Relevance

⭐⭐⭐ High

Team previously accepted removing MUI/PF mixing in chat UI; MUI Typography in notebook sidebar
likely fixed.

PR-#3366

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 2537 forbids importing/using MUI in chat-related components. DocumentSidebar
imports Typography from @material-ui/core and the modified block uses `<Typography
component="div">...</Typography>` in the notebook/chat UI.

Rule 2537: Disallow MUI components in chat UI; require PatternFly equivalents
workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/DocumentSidebar.tsx[19-19]
workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/DocumentSidebar.tsx[197-206]

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` (notebook/chat UI) uses Material-UI (`@material-ui/core`) `Typography`, but chat UI must use PatternFly equivalents.

## Issue Context
The PR adds/changes code in `DocumentSidebar` that wraps a disabled PatternFly `Button` with MUI `Typography` (`component="div"`). This is non-compliant with the "Disallow MUI components in chat UI" requirement.

## Fix Focus Areas
- workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/DocumentSidebar.tsx[197-206]

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

---

4. Missing locale translations 🐞 Bug ⚙ Maintainability ⭐ New

Description

The new translation key notebook.view.documents.uploadsInProgress is added to the reference messages
and used in the UI, but it is absent from the non-English locale dictionaries (de/es/fr/it/ja).
Users in those locales will not see a proper localized string for the new message (fallback behavior
depends on the i18n setup).

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.',

Relevance

⭐⭐⭐ High

Precedent: new translation keys must be added to all supported locale files, not only ref.ts.

PR-#3347

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The key is present in the reference messages, and the locale files contain adjacent
notebook.view.documents.* keys (maxReached/uploading) but do not include uploadsInProgress in that
section.

workspaces/lightspeed/plugins/lightspeed/src/translations/ref.ts[63-82]
workspaces/lightspeed/plugins/lightspeed/src/translations/de.ts[208-214]
workspaces/lightspeed/plugins/lightspeed/src/translations/es.ts[206-212]
PR-#3347

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 message key `notebook.view.documents.uploadsInProgress` was added to `translations/ref.ts` and is now used by the notebook UI, but the supported locale files (`de.ts`, `es.ts`, `fr.ts`, `it.ts`, `ja.ts`) do not define this key.

### Issue Context
This repo already maintains per-locale dictionaries for notebook strings; leaving out newly introduced keys causes partial localization.

### Fix Focus Areas
- 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[205-213]
- workspaces/lightspeed/plugins/lightspeed/src/translations/fr.ts[206-214]
- workspaces/lightspeed/plugins/lightspeed/src/translations/it.ts[205-213]
- workspaces/lightspeed/plugins/lightspeed/src/translations/ja.ts[202-210]

### Suggested fix
Add `notebook.view.documents.uploadsInProgress` to each of the locale dictionaries with an appropriate translation (or temporarily use the English string until translated).

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

---

5. AddDocumentModal uses MUI components 📘 Rule violation ⌂ Architecture

Description

Chat notebook upload UI imports and uses MUI components (e.g., @mui/material/Alert,
@mui/material/Button) instead of PatternFly equivalents. This violates the requirement to avoid
MUI in chat-related components, increasing UI inconsistency and maintenance burden.

Code

workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/AddDocumentModal.tsx[R266-270]

+        {hasUploadsInProgress && (
+          <Alert severity="info" className={classes.errorAlert}>
+            {t('notebook.view.documents.uploadsInProgress')}
+          </Alert>
+        )}

Relevance

⭐⭐⭐ High

Strong precedent to remove/avoid MUI in chat UI and migrate to PatternFly components.

PR-#3366

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 2537 disallows MUI usage in chat UI. AddDocumentModal.tsx imports multiple MUI
modules and the changed code path renders a MUI Alert and uses MUI Button props, demonstrating
MUI is being used within the chat/notebooks UI.

Rule 2537: Disallow MUI components in chat UI; require PatternFly equivalents
workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/AddDocumentModal.tsx[20-30]
workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/AddDocumentModal.tsx[266-332]

Agent prompt

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

## Issue description
`AddDocumentModal` (part of the chat/notebooks UI) imports and uses MUI components, but the compliance requirement mandates PatternFly equivalents for chat UI components.

## Issue Context
This PR introduces/updates MUI usage in the modal (e.g., `Alert` and `Button` props like `severity`, `variant="contained"`, `color="primary"`). The rule requires PatternFly components imported from `@patternfly/react-core` (and related PatternFly packages) instead of `@mui/*` or `@material-ui/*`.

## Fix Focus Areas
- workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/AddDocumentModal.tsx[20-35]
- workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/AddDocumentModal.tsx[252-337]

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

---

View more (1)

6. Conversation delete error handling 🐞 Bug ⚙ Maintainability

Description

VectorStoresOperator.conversations.delete bypasses the shared handleHttpError helper and throws a
generic Error with only the HTTP status, losing consistent Backstage error mapping and response-body
details used elsewhere in the operator. This makes failures harder to debug and inconsistent with
other VectorStoresOperator methods.

Code

workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts[R504-520]

+      const response = await fetch(
+        `${this.baseURL}/v2/conversations/${encodeURIComponent(conversationId)}?user_id=${encodeURIComponent(userId)}`,
+        {
+          method: 'DELETE',
+          headers: {
+            'Content-Type': 'application/json',
+          },
+        },
+      );
+
+      if (!response.ok) {
+        this.logger.warn(
+          `Failed to delete conversation ${conversationId}: HTTP ${response.status}`,
+        );
+        throw new Error(
+          `Failed to delete conversation: HTTP ${response.status}`,
+        );

Relevance

⭐⭐⭐ High

Consistent error mapping/detail is a low-risk, local refactor; team often aligns notebooks code to
shared helpers.

PR-#2861

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The operator defines a common handleHttpError for consistent parsing/mapping and uses it across
other endpoints, but the newly added conversations.delete method does not use it and throws a
generic Error, creating inconsistent error semantics.

workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts[58-87]
workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts[141-163]
workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts[491-524]

Agent prompt

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

### Issue description
`VectorStoresOperator.conversations.delete()` implements ad-hoc error handling and throws `new Error(...)` on non-2xx responses, instead of using the existing `handleHttpError(...)` that parses the body and maps HTTP codes to Backstage error types.

### Issue Context
Other operator methods (vector stores + files) consistently call `handleHttpError`, which:
- attempts `response.json()` and falls back to `response.text()`
- logs structured error details
- maps common status codes to `NotFoundError`, `ConflictError`, etc.

### Fix Focus Areas
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts[58-87]
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts[136-163]
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts[491-524]

### Suggested fix
Replace the `if (!response.ok) { ... throw new Error(...) }` block with `await handleHttpError(response, this.logger, 'delete conversation')` so errors are parsed/mapped consistently (keeping return type `Promise<void>` on success). Optionally treat 404 as a no-op if deletion should be idempotent.

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

---

7. Core API version drift 🔗 Cross-repo conflict ☼ Reliability

Description

The Lightspeed backend now (a) always includes tool_choice: 'required' in /v1/responses requests
and (b) calls DELETE /v2/conversations/{id} for session cleanup, which may require a minimum
lightspeed-core version. rhdh-local documents a default lightspeed-core image of 0.5.1 while
rhdh-operator deploys 0.5.3, so compatibility should be verified and/or guarded to avoid
environment-specific runtime failures when notebooks are enabled.

Code

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

+        tool_choice: 'required',

Relevance

⭐⭐ Medium

Compatibility/version-guarding is important but may be seen as out-of-scope or too speculative for
this sync PR.

PR-#3359
PR-#2861

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PR introduces new Lightspeed Core coupling: tool_choice: 'required' is added to
/v1/responses requests and the backend now calls DELETE /v2/conversations/{conversation_id}. In
related repos, rhdh-local documentation indicates the default lightspeed-core image tag is 0.5.1,
while rhdh-operator deploys 0.5.3, implying version skew that can surface as runtime incompatibility
if older Core images don’t support these newer request/endpoint semantics.

workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/notebooksRouters.ts[467-502]
workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts[491-506]
External repo: redhat-developer/rhdh-local, docs/lightspeed/maintaining-lightspeed.md [72-78]
External repo: redhat-developer/rhdh-operator, config/profile/rhdh/default-config/flavours/lightspeed/deployment.yaml [24-28]

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 Lightspeed backend is now sending/using newer Lightspeed Core request/endpoint semantics (`tool_choice` and `/v2/conversations`), but other repos pin different lightspeed-core image tags. If older Core images don’t support these, notebook queries or cleanup behavior can diverge between `rhdh-local` and `rhdh-operator`.

## Issue Context
- `rhdh-local` documents core `0.5.1` as the default.
- `rhdh-operator` deploys core `0.5.3`.
- This repo’s backend now assumes `tool_choice` support and the `/v2/conversations` delete route.

## Fix Focus Areas
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/notebooksRouters.ts[481-492]
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts[491-506]

### Suggested approach
- Make `tool_choice` opt-in via plugin config (or omit it by default), and/or add a compatibility mode toggle for older Core versions.
- Consider a fallback strategy for conversation deletion (e.g., try `/v2/...` and if 404, optionally try the older route if applicable), or clearly document the minimum required lightspeed-core version for notebooks.
- Coordinate with `rhdh-local`/`rhdh-operator` to align on a minimum supported lightspeed-core tag if notebooks is expected to work out-of-the-box.

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

---

Previous review results

Review updated until commit 4983182 ⚖️ Balanced

Results up to commit 4983182 ⚖️ Balanced

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

1. Orphaned uploaded files 🐞 Bug ☼ Reliability

Description

SessionService.deleteSession no longer deletes the underlying Files API objects created during
document uploads, so deleting a session can leave uploaded files orphaned and accumulate storage
over time. The new logic only deletes vector-store file associations and then deletes the vector
store.

Code

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

+    // 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}`,
+            );
          }
        }),
      );

Relevance

⭐⭐ Medium

Potentially real leak, but behavior depends on core semantics; fix may require broader
design/verification beyond this PR.

PR-#2861

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The backend upload path creates Files API objects, but the updated session deletion path only
removes vector-store file associations. The test fixture demonstrates uploaded files are stored
independently from vector-store membership and are not removed by vector store deletion, proving an
explicit Files API delete is required to avoid orphans.

workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/sessions/sessionService.ts[186-238]
workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/documents/documentService.ts[100-187]
workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts[411-488]
workspaces/lightspeed/plugins/lightspeed-backend/fixtures/lightspeedCoreHandlers.ts[70-245]

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()` currently removes vector-store file entries via `vectorStores.files.delete(...)` but never calls `files.delete(...)` for the underlying uploaded files.

### Issue Context
- Upload flow creates a Files API resource (`client.files.create`) and then attaches it to a vector store via `vectorStores.files.create({ file_id })`.
- The MSW fixture models these as independent resources (separate in-memory maps), and vector-store deletion does not remove uploaded files.

### Fix Focus Areas
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/sessions/sessionService.ts[186-238]
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts[411-488]
- workspaces/lightspeed/plugins/lightspeed-backend/__fixtures__/lightspeedCoreHandlers.ts[70-245]

### Suggested fix
1. While iterating vector store files, also delete the underlying file resource via `await this.client.files.delete(file.id)` (or the correct identifier if the API returns a different field).
2. Keep robust error handling (log and continue per-file), but ensure both deletions are attempted.
3. Update the MSW fixture to add a `DELETE /v1/files/:id` handler (so tests catch regressions and reflect the intended lifecycle).

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

---

2. AddDocumentModal uses MUI components 📘 Rule violation ⌂ Architecture

Description

Chat notebook upload UI imports and uses MUI components (e.g., @mui/material/Alert,
@mui/material/Button) instead of PatternFly equivalents. This violates the requirement to avoid
MUI in chat-related components, increasing UI inconsistency and maintenance burden.

Code

workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/AddDocumentModal.tsx[R266-270]

+        {hasUploadsInProgress && (
+          <Alert severity="info" className={classes.errorAlert}>
+            {t('notebook.view.documents.uploadsInProgress')}
+          </Alert>
+        )}

Relevance

⭐⭐⭐ High

Strong precedent to remove/avoid MUI in chat UI and migrate to PatternFly components.

PR-#3366

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 2537 disallows MUI usage in chat UI. AddDocumentModal.tsx imports multiple MUI
modules and the changed code path renders a MUI Alert and uses MUI Button props, demonstrating
MUI is being used within the chat/notebooks UI.

Rule 2537: Disallow MUI components in chat UI; require PatternFly equivalents
workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/AddDocumentModal.tsx[20-30]
workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/AddDocumentModal.tsx[266-332]

Agent prompt

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

## Issue description
`AddDocumentModal` (part of the chat/notebooks UI) imports and uses MUI components, but the compliance requirement mandates PatternFly equivalents for chat UI components.

## Issue Context
This PR introduces/updates MUI usage in the modal (e.g., `Alert` and `Button` props like `severity`, `variant="contained"`, `color="primary"`). The rule requires PatternFly components imported from `@patternfly/react-core` (and related PatternFly packages) instead of `@mui/*` or `@material-ui/*`.

## Fix Focus Areas
- workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/AddDocumentModal.tsx[20-35]
- workspaces/lightspeed/plugins/lightspeed/src/components/notebooks/AddDocumentModal.tsx[252-337]

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

---

3. Conversation delete error handling 🐞 Bug ⚙ Maintainability

Description

VectorStoresOperator.conversations.delete bypasses the shared handleHttpError helper and throws a
generic Error with only the HTTP status, losing consistent Backstage error mapping and response-body
details used elsewhere in the operator. This makes failures harder to debug and inconsistent with
other VectorStoresOperator methods.

Code

workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts[R504-520]

+      const response = await fetch(
+        `${this.baseURL}/v2/conversations/${encodeURIComponent(conversationId)}?user_id=${encodeURIComponent(userId)}`,
+        {
+          method: 'DELETE',
+          headers: {
+            'Content-Type': 'application/json',
+          },
+        },
+      );
+
+      if (!response.ok) {
+        this.logger.warn(
+          `Failed to delete conversation ${conversationId}: HTTP ${response.status}`,
+        );
+        throw new Error(
+          `Failed to delete conversation: HTTP ${response.status}`,
+        );

Relevance

⭐⭐⭐ High

Consistent error mapping/detail is a low-risk, local refactor; team often aligns notebooks code to
shared helpers.

PR-#2861

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The operator defines a common handleHttpError for consistent parsing/mapping and uses it across
other endpoints, but the newly added conversations.delete method does not use it and throws a
generic Error, creating inconsistent error semantics.

workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts[58-87]
workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts[141-163]
workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts[491-524]

Agent prompt

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

### Issue description
`VectorStoresOperator.conversations.delete()` implements ad-hoc error handling and throws `new Error(...)` on non-2xx responses, instead of using the existing `handleHttpError(...)` that parses the body and maps HTTP codes to Backstage error types.

### Issue Context
Other operator methods (vector stores + files) consistently call `handleHttpError`, which:
- attempts `response.json()` and falls back to `response.text()`
- logs structured error details
- maps common status codes to `NotFoundError`, `ConflictError`, etc.

### Fix Focus Areas
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts[58-87]
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts[136-163]
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts[491-524]

### Suggested fix
Replace the `if (!response.ok) { ... throw new Error(...) }` block with `await handleHttpError(response, this.logger, 'delete conversation')` so errors are parsed/mapped consistently (keeping return type `Promise<void>` on success). Optionally treat 404 as a no-op if deletion should be idempotent.

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

---

4. Core API version drift 🔗 Cross-repo conflict ☼ Reliability

Description

The Lightspeed backend now (a) always includes tool_choice: 'required' in /v1/responses requests
and (b) calls DELETE /v2/conversations/{id} for session cleanup, which may require a minimum
lightspeed-core version. rhdh-local documents a default lightspeed-core image of 0.5.1 while
rhdh-operator deploys 0.5.3, so compatibility should be verified and/or guarded to avoid
environment-specific runtime failures when notebooks are enabled.

Code

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

+        tool_choice: 'required',

Relevance

⭐⭐ Medium

Compatibility/version-guarding is important but may be seen as out-of-scope or too speculative for
this sync PR.

PR-#3359
PR-#2861

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PR introduces new Lightspeed Core coupling: tool_choice: 'required' is added to
/v1/responses requests and the backend now calls DELETE /v2/conversations/{conversation_id}. In
related repos, rhdh-local documentation indicates the default lightspeed-core image tag is 0.5.1,
while rhdh-operator deploys 0.5.3, implying version skew that can surface as runtime incompatibility
if older Core images don’t support these newer request/endpoint semantics.

workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/notebooksRouters.ts[467-502]
workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts[491-506]
External repo: redhat-developer/rhdh-local, docs/lightspeed/maintaining-lightspeed.md [72-78]
External repo: redhat-developer/rhdh-operator, config/profile/rhdh/default-config/flavours/lightspeed/deployment.yaml [24-28]

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 Lightspeed backend is now sending/using newer Lightspeed Core request/endpoint semantics (`tool_choice` and `/v2/conversations`), but other repos pin different lightspeed-core image tags. If older Core images don’t support these, notebook queries or cleanup behavior can diverge between `rhdh-local` and `rhdh-operator`.

## Issue Context
- `rhdh-local` documents core `0.5.1` as the default.
- `rhdh-operator` deploys core `0.5.3`.
- This repo’s backend now assumes `tool_choice` support and the `/v2/conversations` delete route.

## Fix Focus Areas
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/notebooksRouters.ts[481-492]
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts[491-506]

### Suggested approach
- Make `tool_choice` opt-in via plugin config (or omit it by default), and/or add a compatibility mode toggle for older Core versions.
- Consider a fallback strategy for conversation deletion (e.g., try `/v2/...` and if 404, optionally try the older route if applicable), or clearly document the minimum required lightspeed-core version for notebooks.
- Coordinate with `rhdh-local`/`rhdh-operator` to align on a minimum supported lightspeed-core tag if notebooks is expected to work out-of-the-box.

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

---

[rhdh-qodo-merge]

Code review by qodo was updated up to the latest commit 4983182

[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