Skip to content

(fix): notebooks check to lowercase - #2824

Merged
Jdubrick merged 2 commits into
redhat-developer:mainfrom
Jdubrick:fix-notebooks-ref
Apr 20, 2026
Merged

(fix): notebooks check to lowercase#2824
Jdubrick merged 2 commits into
redhat-developer:mainfrom
Jdubrick:fix-notebooks-ref

Conversation

@Jdubrick

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)

Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>
@rhdh-qodo-merge

rhdh-qodo-merge Bot commented Apr 17, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1)   📘 Rule violations (0)   📎 Requirement gaps (0)
🐞\ ☼ Reliability (1)

Grey Divider


Action required

1. Old config key ignored 🐞
Description
The Notebooks feature flag is now read only from lightspeed.notebooks.enabled, so configs using
the previous casing lightspeed.Notebooks.enabled will silently evaluate to false and the
Notebooks router/endpoints will not be registered.
Code

workspaces/lightspeed/plugins/lightspeed-backend/src/plugin.ts[66]

+          config.getOptionalBoolean('lightspeed.notebooks.enabled') ?? false;
Relevance

⭐⭐⭐ High

Previously used lightspeed.Notebooks.enabled; prior review rejected lowercasing, so this change
likely breaks existing configs.

PR-#2499
PR-#2742

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
lightspeedPlugin gates router registration on a single config lookup of
lightspeed.notebooks.enabled, with a default of false, so any config that still sets
lightspeed.Notebooks.enabled will not be picked up. The rest of the notebooks implementation
consistently uses the lowercase lightspeed.notebooks.* namespace, indicating this casing is the
intended canonical key, making a fallback/deprecation path important for upgrades.

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

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

### Issue description
`aiNotebooksEnabled` is now derived solely from `lightspeed.notebooks.enabled`. Any deployments still using the older `lightspeed.Notebooks.enabled` key will have AI Notebooks disabled with no warning.

### Issue Context
This is an upgrade-compatibility break because YAML/config keys are case-sensitive and the code defaults to `false` when the new key is missing.

### Fix Focus Areas
- workspaces/lightspeed/plugins/lightspeed-backend/src/plugin.ts[65-67]

### Suggested implementation
- Read the new key first.
- If unset, fall back to the old key (`lightspeed.Notebooks.enabled`).
- If the old key is used, log a deprecation warning telling operators to migrate to `lightspeed.notebooks.enabled`.
- (Optional) Add a small regression test around plugin init/config gating if there is an existing test harness for plugin registration.

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


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@rhdh-gh-app

rhdh-gh-app Bot commented Apr 17, 2026

Copy link
Copy Markdown

Changed Packages

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

@rhdh-qodo-merge

Copy link
Copy Markdown

Review Summary by Qodo

Fix notebooks configuration key to lowercase

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Fix configuration key lookup for notebooks feature
• Changed 'lightspeed.Notebooks.enabled' to lowercase 'lightspeed.notebooks.enabled'
• Ensures proper configuration retrieval for AI notebooks functionality
Diagram
flowchart LR
  A["Config Key<br/>lightspeed.Notebooks.enabled"] -- "changed to lowercase" --> B["Config Key<br/>lightspeed.notebooks.enabled"]
  B -- "enables proper lookup" --> C["AI Notebooks Feature"]
Loading

Grey Divider

File Changes

1. workspaces/lightspeed/plugins/lightspeed-backend/src/plugin.ts 🐞 Bug fix +1/-1

Fix notebooks config key case sensitivity

• Fixed configuration key case sensitivity issue
• Changed 'lightspeed.Notebooks.enabled' to 'lightspeed.notebooks.enabled'
• Ensures configuration lookup matches expected lowercase convention

workspaces/lightspeed/plugins/lightspeed-backend/src/plugin.ts


Grey Divider

Qodo Logo

Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>
@sonarqubecloud

Copy link
Copy Markdown

@Jdubrick
Jdubrick merged commit 4379c1a into redhat-developer:main Apr 20, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants