RHIDP-13056: separating notebooks and lightspeed vector_store

[JslYoon]

Hey, I just made a Pull Request!

• 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]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Hardcoded vector_store_ids ☑ 🐞 Bug ≡ Correctness

Description

The /v1/query handler computes and caches a vector store id, but then overwrites
request.body.vector_store_ids with ['asdf'], which will cause lightspeed-core to query a
non-existent/wrong vector store. This also adds an extra list() call on the request path without any
benefit since the computed id is never used.

Code

workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts[R600-609]

+        // get the vector store id for the rhdh-product-docs vector store
+        if (lightspeed_vector_store_id === '') {
+          const vectorStores = await vectorStoresOperator.vectorStores.list();
+          lightspeed_vector_store_id =
+            vectorStores.data.find((v: any) =>
+              v.name.startsWith('rhdh-product-docs'),
+            )?.id || '';
+        }
+        request.body.vector_store_ids = ['asdf'];
+

Relevance

⭐⭐⭐ High

Overwriting computed vector store id with ['asdf'] is a clear placeholder/bug; breaks queries and
wastes list call.

PR-#2742

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The code lists vector stores and assigns lightspeed_vector_store_id, then immediately ignores it by
setting vector_store_ids to a hardcoded placeholder string.

workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts[583-609]
workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/VectorStoresOperator.ts[226-241]

Agent prompt

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

### Issue description
`/v1/query` currently sets `request.body.vector_store_ids = ['asdf']`, which is a placeholder and ignores the fetched `lightspeed_vector_store_id`. This will break downstream file_search/vector store usage.

### Issue Context
The handler already fetches vector stores and attempts to find one whose name starts with `rhdh-product-docs`, but never uses the result.

### Fix Focus Areas
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts[600-609]

### Proposed fix
- Replace the hardcoded `['asdf']` with:
 - `request.body.vector_store_ids = [lightspeed_vector_store_id]` when a non-empty id is found.
 - If no id is found, do not set `vector_store_ids` (or set to an empty array) so lightspeed-core behavior is predictable.
- Wrap the `vectorStoresOperator.vectorStores.list()` call in a try/catch (and consider a timeout) so a temporary upstream failure doesn’t unnecessarily fail all queries.
- Remove the unused `lightspeed_vector_store_id` logic if vector stores should not be injected here.

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

---

2. Request body logged to stdout ☑ 🐞 Bug ⛨ Security

Description

The /v1/query handler logs the full JSON request body using console.log, which can leak user queries
and other sensitive fields into stdout and bypasses Backstage logger controls/redaction. This will
run on every /v1/query request.

Code

workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts[626]

+        console.log('requestBodyasdf', requestBody);

Relevance

⭐⭐⭐ High

Logging full request body via console.log can leak sensitive data and bypass Backstage logger
controls; should be removed.

PR-#2345

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The handler stringifies the full request body and prints it via console.log right before sending the
upstream request.

workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts[618-637]

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 `console.log('requestBodyasdf', requestBody)` is present in the `/v1/query` handler, causing request payloads to be emitted to stdout.

### Issue Context
`requestBody` is `JSON.stringify(request.body)` and may contain sensitive user-provided data.

### Fix Focus Areas
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts[618-637]

### Proposed fix
- Remove the console.log entirely.
- If logging is needed for debugging, replace it with `logger.debug(...)` and log only minimal, non-sensitive fields (or add explicit redaction).

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

---

3. Notebooks ignores servicePort ☑ 🐞 Bug ≡ Correctness

Description

createNotebooksRouter now hardcodes lightspeedBaseUrl to localhost:8080, removing the prior
config-based port override, so notebooks calls will fail whenever lightspeed-core isn’t reachable at
that default. This is inconsistent with the main lightspeed router which still honors
lightspeed.servicePort from config.

Code

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

+  const lightspeedBaseUrl = `http://${DEFAULT_LIGHTSPEED_SERVICE_HOST}:${DEFAULT_LIGHTSPEED_SERVICE_PORT}`;

Relevance

⭐⭐⭐ High

Repo historically made lightspeed.servicePort configurable; hardcoding localhost:8080 in notebooks
router likely a regression.

PR-#1064
PR-#2499

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The notebooks router constructs its upstream base URL only from DEFAULT_* constants (no config
override), while the main router still reads lightspeed.servicePort from config to reach the same
upstream service port.

workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/notebooksRouters.ts[61-89]
workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts[104-114]
workspaces/lightspeed/plugins/lightspeed-backend/src/service/constant.ts[21-27]

Agent prompt

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

### Issue description
AI Notebooks upstream URL is currently hardcoded to `http://localhost:8080`, which removes the previously supported config override for the lightspeed-core port. This can break notebooks functionality in any deployment where lightspeed-core is not on the default port/host.

### Issue Context
The main backend router still uses `config.getOptionalNumber('lightspeed.servicePort') ?? DEFAULT_LIGHTSPEED_SERVICE_PORT`, but notebooksRouter no longer does.

### Fix Focus Areas
- workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/notebooksRouters.ts[65-70]

### Proposed fix
- Reintroduce reading `lightspeed.servicePort` (and ideally a `lightspeed.serviceHost` if supported) from config.
- Fall back to `DEFAULT_LIGHTSPEED_SERVICE_HOST` / `DEFAULT_LIGHTSPEED_SERVICE_PORT` when config values are not present.
- Keep the existing log line so operators can see the resolved upstream URL.

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

---

4. Workspace config enables notebooks ☑ 🐞 Bug ⚙ Maintainability

Description

The workspace app-config.yaml now sets lightspeed.notebooks.enabled: true while the surrounding
comment says the feature is disabled by default, which is confusing and can unintentionally enable
notebooks in this workspace configuration. The embedding_model is also changed from an env var to a
hardcoded value, reducing deploy-time configurability.

Code

workspaces/lightspeed/app-config.yaml[R20-29]

lightspeed:
  notebooks:
-    enabled: false
+    enabled: true
    queryDefaults:
-      model: redhataillama-31-8b-instruct
+      model: redhataillama-31-8b-instruct ## move these to run.yaml, provider id needs to match
      provider_id: vllm
    sessionDefaults:
      provider_id: notebooks
-      embedding_model: ${LLAMA_STACK_EMBEDDING_MODEL}
+      embedding_model: sentence-transformers/all-mpnet-base-v2
      embedding_dimension: 768

Relevance

⭐ Low

Team previously rejected changing notebooks default-enable mismatch in workspace app-config; likely
intentional for dev workspace.

PR-#2499

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The config comment states notebooks are disabled by default, but the value was flipped to enabled
and embedding_model was hardcoded in this workspace configuration.

workspaces/lightspeed/app-config.yaml[19-29]
workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/sessions/sessionService.ts[51-64]

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 workspace config contradicts its own comment about notebooks being disabled by default, and hardcodes the embedding model.

### Issue Context
This file is likely used by developers running the lightspeed workspace, so unexpected enablement/hardcoding can create confusion.

### Fix Focus Areas
- workspaces/lightspeed/app-config.yaml[19-29]

### Proposed fix
- Either revert `lightspeed.notebooks.enabled` to `false` or update the comment to match the intended behavior.
- Consider restoring `embedding_model` to an environment-substituted value (or document why it must be hardcoded for this workspace).

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

---

[rhdh-gh-app]

Important

This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior.

Changed Packages

Package Name	Package Path	Changeset Bump	Current Version
@red-hat-developer-hub/backstage-plugin-lightspeed-backend	workspaces/lightspeed/plugins/lightspeed-backend	minor	v2.2.1

[rhdh-qodo-merge]

Review Summary by Qodo

✨ Enhancement 📦 Other

Walkthroughs

Description

• Refactored lightspeed service configuration to use default constants
• Separated notebooks and lightspeed vector store integration logic
• Added VectorStoresOperator for managing vector stores in main router
• Enabled AI Notebooks feature and updated embedding model configuration
• Removed OpenAPI specification file for notebooks endpoints

Diagram

flowchart LR
  A["Constants Configuration"] -->|DEFAULT_LIGHTSPEED_SERVICE_HOST| B["Notebooks Router"]
  A -->|DEFAULT_LIGHTSPEED_SERVICE_PORT| B
  C["Main Router"] -->|VectorStoresOperator| D["Vector Store Management"]
  D -->|Query Vector Stores| E["Lightspeed Service"]
  F["App Config"] -->|Enable Notebooks| G["Feature Toggle"]
  F -->|Update Embedding Model| H["Configuration"]

Loading

File Changes

1. workspaces/lightspeed/plugins/lightspeed-backend/src/service/constant.ts ⚙️ Configuration changes +1/-1

Refactored service host configuration constants

• Added DEFAULT_LIGHTSPEED_SERVICE_HOST constant set to 'localhost'
• Removed hardcoded LIGHTSPEED_SERVICE_HOST constant with value '0.0.0.0'
• Consolidated service configuration to use default constants

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

---

2. workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/notebooksRouters.ts ✨ Enhancement +2/-5

Simplified notebooks router service configuration

• Updated imports to use DEFAULT_LIGHTSPEED_SERVICE_HOST instead of LIGHTSPEED_SERVICE_HOST
• Simplified lightspeed base URL construction to use default constants directly
• Removed dynamic port configuration from config object

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

---

3. workspaces/lightspeed/plugins/lightspeed-backend/src/service/router.ts ✨ Enhancement +19/-1

Integrated vector store operator into main router

• Added import for VectorStoresOperator class
• Instantiated VectorStoresOperator in main router with service URL and logger
• Added logic to fetch and cache vector store ID for rhdh-product-docs
• Integrated vector store ID into query request body
• Added debug console.log statement for request body logging

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

---

View more (2)

4. workspaces/lightspeed/app-config.yaml ⚙️ Configuration changes +3/-3

Enabled notebooks and updated embedding model

• Changed lightspeed.notebooks.enabled from false to true
• Updated lightspeed.notebooks.sessionDefaults.embedding_model from environment variable to
 hardcoded 'sentence-transformers/all-mpnet-base-v2'
• Added comment noting configuration should be moved to run.yaml

workspaces/lightspeed/app-config.yaml

---

5. workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/openapi.yaml 📝 Documentation +0/-1147

Removed notebooks OpenAPI specification file

• Removed entire OpenAPI 3.0.3 specification file (1147 lines)
• Included comprehensive API documentation for sessions, documents, and query endpoints
• Removed all schema definitions, security schemes, and endpoint descriptions

workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/openapi.yaml

---

[Jdubrick]

@JslYoon can we add a changeset too for a patch bump please

[JslYoon]

/review

[rhdh-qodo-merge]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 RHIDP-13056 - Partially compliant Compliant requirements: Fetch all vector stores and select a vector store by name prefix, caching the selected id for reuse Non-compliant requirements: On Lightspeed startup, fetch all vector stores Find the vector store whose name has the prefix rhdh_docs Only add vector_store_ids when it is not passed to the endpoint Apply the behavior specifically to the streaming_queries endpoint Requires further human verification: Confirm which endpoint is actually used for streaming (/v1/query vs streaming_queries) in the deployed environment and that the fix applies to the intended request path(s) Confirm the exact expected vector store naming convention/prefix (rhdh_docs vs rhdh-product-docs vs rhdh-docs) and that it matches production data Validate runtime behavior with multiple vector stores present (ensuring queries no longer pull from all vector stores)

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Issue The code always overwrites request.body.vector_store_ids (even when the client already provided it) and may set it to [''] when no matching vector store is found. This could unintentionally disable intended vector-store scoping or cause downstream API errors; consider guarding the assignment (only set when missing/empty) and failing fast or skipping mutation if no id is found. const user_id = user.userEntityRef; logger.info(`/v1/query receives call from user: ${user_id}`); await authorizer.authorizeUser( lightspeedChatCreatePermission, credentials, ); // get the vector store id for the rhdh-product-docs vector store if (lightspeed_vector_store_id === '') { const vectorStores = await vectorStoresOperator.vectorStores.list(); lightspeed_vector_store_id = vectorStores.data.find((v: any) => v.name.startsWith('rhdh-product-docs'), )?.id || ''; } request.body.vector_store_ids = [lightspeed_vector_store_id]; Startup Behavior The vector store id is fetched lazily on first /v1/query call rather than at service startup as described in the ticket. This can create first-request latency and makes failures happen during user requests; consider moving prefetch to router initialization or a dedicated startup path, with retries/logging. const vectorStoresOperator = VectorStoresOperator.getInstance( `http://0.0.0.0:${port}`, logger, ); let lightspeed_vector_store_id: string = ''; // Parse admin-configured MCP servers from app-config. Singleton Risk VectorStoresOperator is now a singleton; subsequent getInstance calls ignore potentially different lightspeedCoreUrl or logger values, which can lead to unexpected cross-test/process coupling or misconfiguration if multiple routers/environments instantiate it differently. Consider enforcing consistent parameters, keying by base URL, or documenting/validating the singleton assumptions. export class VectorStoresOperator { private static instance: VectorStoresOperator | null = null; private baseURL: string; private logger: LoggerService; private constructor(lightspeedCoreUrl: string, logger: LoggerService) { this.baseURL = lightspeedCoreUrl; this.logger = logger; } /** * Get the singleton instance of VectorStoresOperator * @param lightspeedCoreUrl - Lightspeed core URL (required on first call) * @param logger - Logger service (required on first call) * @returns The singleton instance */ static getInstance( lightspeedCoreUrl: string, logger: LoggerService, ): VectorStoresOperator { if (!VectorStoresOperator.instance) { VectorStoresOperator.instance = new VectorStoresOperator( lightspeedCoreUrl, logger, ); } return VectorStoresOperator.instance; } /** * Reset the singleton instance (primarily for testing) */ static resetInstance(): void { VectorStoresOperator.instance = null; }

📄 References No matching references available

[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

[Jdubrick]

/lgtm