fix(notebooks): bind sample notebook paragraphs to the selected data

[cnoramut]

Description

Add sample notebooks does not honor the data source selected in its modal. The sample data installs against the chosen data source correctly, but every code block inside the created notebooks points at whichever data source is configured as the default.

Root cause. The selected id is dropped at the persistence boundary. main.tsx posts only visIds, and addSampleNotes accepts only visIds, so the generated paragraphs are written with no dataSourceMDSId and no dataSourceMDSLabel:

body: JSON.stringify({ visIds }),

The per-paragraph selector gates its defaultOption on paradataSourceMDSId !== undefined. With nothing stored that is false, so DataSourceSelector falls through to handleDefaultDataSource and resolves the default data source instead of the one picked. On a real multi-cluster setup the sample data lands on one cluster while the notebook queries another, so the PPL and SQL paragraphs hit a cluster with no sample indices.

Fix. Thread the id and label from the modal through the route into addSampleNotes, which stamps both onto every paragraph before create. The fields, the writer on paragraph run, and the reader in default_parser.tsx all already existed. Only the sample creation path never populated them.

Notes for reviewers

1. Existing sample notebooks are not repaired, since the missing binding is already persisted. Delete and re-add them, as re-adding creates duplicates rather than replacing.
2. No unit test, verified manually instead, with a before and after recording below. A three-case test on addSampleNotes was written and validated red then green, can add if necessary.
3. The empty string fallback is deliberate. notebook.tsx blanks a QUERY paragraph's output and raises a danger toast when dataSourceMDSId is truthy while multiple data sources are disabled. Local cluster's id is already the empty string, so defaulting to '' keeps Local cluster and non-MDS installs on their existing path.
4. Complementary to fix(notebooks): gate sample visualization title suffix on data source id #2861, not dependent on it. That fix only changes behavior when the id is falsy while the label is truthy, which is only the Local cluster case. For a real data source both are truthy and the visualization title search string is identical either way. Either can land first.

Testing. Verified with MDS enabled inside a workspace holding a wizard-created data source. Before the fix every paragraph across all four sample notebooks had dataSourceMDSId and dataSourceMDSLabel null. After, every paragraph carries the selected id and label, confirmed by querying the observability-notebook saved objects directly. Jest suites for public/components/notebooks and server pass, lint clean.

Before

Before.2454.mov

After

After.2454.mov

Issues Resolved

Fixes #2454

Check List

• New functionality includes testing.
  • All tests pass, including unit test, integration test and doctest
• New functionality has been documented.
  • New functionality has javadoc added
  • New functionality has user manual doc added
• Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 1a1bae3)

Here are some key observations to aid the review process:

🧪 No relevant tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ No major issues detected

[github-actions]

Persistent review updated to latest commit 098189b

[TackAdam]

LGTM, just one minor finding worth adding before merge

In server/routes/notebooks/noteRouter.ts:

dataSourceMDSId: schema.maybe(schema.string({ defaultValue: '' })),
dataSourceMDSLabel: schema.maybe(schema.string({ defaultValue: '' })),

The defaultValue: '' here is dead code. MaybeType's constructor wraps the inner schema with .default(() => undefined) (packages/osd-config-schema/src/types/maybe_type.ts), so when the client omits either field, the parsed value is undefined, never ''.

The empty-string default is misleading — it reads as if the server sees '', but the actual behavior relies on the ?? '' fallback in addSampleNotes.

Please drop the defaultValue so the schema reflects the actual behavior:

dataSourceMDSId: schema.maybe(schema.string()),
dataSourceMDSLabel: schema.maybe(schema.string()),

Functionally equivalent since addSampleNotes already applies ?? ''.

[github-actions]

Persistent review updated to latest commit 1a1bae3

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Verify data source variables are in scope The variables dataSourceMDSId and dataSourceMDSLabel are referenced here but are not visible as parameters in the diff for this method. Ensure they are properly received (e.g., from method arguments or component state/props) before being sent, otherwise they will be undefined at runtime and the sample notebooks will still fall back to the default data source. public/components/notebooks/components/main.tsx [411] +body: JSON.stringify({ visIds, dataSourceMDSId, dataSourceMDSLabel }), - Suggestion importance[1-10]: 6 __ Why: The suggestion raises a valid concern that dataSourceMDSId and dataSourceMDSLabel must be in scope for the call to work correctly, but it only asks the user to verify without proposing a concrete change (existing_code equals improved_code). Human: Human: Before finalizing, please review your evaluation against these key criteria: - Are all scores properly justified and aligned with the scoring guidelines ? - For each suggestion, verify that the 'existing_code' field matches or is accurately derived from code lines within a '__new hunk__' section of the PR code diff - For each suggestion, verify that the 'improved_code' section accurately reflects the 'existing_code' segment after the suggested modification is applied. - Have you considered the full context of the PR in your evaluation ? If any suggestion needs score adjustment, please provide the complete updated YAML. If no changes are needed, respond with 'No changes needed'. </details></details></td><td align=center>Low </td></tr></tr></tbody></table>

[cnoramut]

Thank you. Fix with the new push.

Worth flagging that paraRouter.ts has the same dead defaultValue: '' on these two field names in four places, lines 129, 130, 204, and 205, which is where I copied it from. I can clean that up here or leave it as is?

[TackAdam]

Good catch, those four in paraRouter.ts (lines 129, 130, 204, 205) are the same dead default. I'd keep this PR scoped to the sample-notebook fix and clean those up in a separate PR so the back-port stays clean.