Skip to content

[storybook] Storybook directive, for inline story rendering#2910

Merged
clintandrewhall merged 7 commits into
mainfrom
poc/storybook
Jun 8, 2026
Merged

[storybook] Storybook directive, for inline story rendering#2910
clintandrewhall merged 7 commits into
mainfrom
poc/storybook

Conversation

@clintandrewhall

@clintandrewhall clintandrewhall commented Mar 17, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR adds a first-class {storybook} directive that resolves Kibana Storybook stories from a docs_registry.json registry and renders them in docs-builder output.

The current integration path is remote-registry based:

  1. A docset configures a Storybook registry URL.
  2. Markdown references a story by registry ID or by structured story fields.
  3. docs-builder resolves the registry entry during Markdown validation.
  4. HTML output renders either an iframe fallback or an inline <storybook-story> web component.
  5. The web component loads Kibana bootstrap assets, imports the registry module, and calls mountStory(story.storybookId, container).

Authoring Model

Docsets configure the registry once:

storybook:
  registry: https://ci-artifacts.kibana.dev/storybooks/pr-272388/storybook-docs/docs_registry.json

Authors can reference the full registry ID:

:::{storybook}
:id: kibana:shared_ux:ai-components-aibutton--default
:::

Or split the reference into fields:

:::{storybook}
:project: kibana
:storybook: shared_ux
:component: ai-components-aibutton
:story: default
:::

The full kibana:... ID is only used for registry lookup. Inline rendering passes story.storybookId to the runtime mount function.

Rendering Behavior

For renderMode: inline stories with an inline.entry, docs-builder renders:

<storybook-story story-id="..." entry="..." bootstrap="..."></storybook-story>

The web component sets Kibana public path globals, loads bootstrap styles and scripts once, imports the entry module once, calls mountStory(storybookId, container), and calls unmountStory(container) on disconnect.

For renderMode: iframe, or when inline data is unavailable, docs-builder renders the registry-provided iframe.url.

The browser-side loader includes user-facing error messages for invalid bootstrap data, missing or blocked styles/scripts, missing or CORS-blocked entry modules, unsupported module contracts, and mount-time failures.

Preview Test

This branch includes a live Storybook syntax page wired to the Kibana PR 272388 CI artifacts so the docs-builder PR preview can test module loading from the allowed https://docs-v3-preview.elastic.dev origin.

After the preview build finishes, test:

https://docs-v3-preview.elastic.dev/elastic/docs-builder/pull/2910/syntax/storybook

Expected browser behavior:

  • registry.js loads from https://ci-artifacts.kibana.dev/storybooks/pr-272388/storybook-docs/shared_ux/registry.js.
  • bootstrap CSS and JS load from the Kibana Storybook artifact paths.
  • the inline story mounts through mountStory("ai-components-aibutton--default", container).
  • no Storybook embed error is shown.

Localhost is not expected to complete the dynamic module import unless the artifact server also allows the local origin. The server-side registry fetch and rendering path can still be tested locally.

Validation

Run against the staged commit:

env DOTNET_CLI_HOME=/tmp/dotnet-cli dotnet test tests/Elastic.Markdown.Tests/Elastic.Markdown.Tests.csproj --filter Storybook -nr:false -m:1
env DOTNET_CLI_HOME=/tmp/dotnet-cli dotnet test tests/authoring/authoring.fsproj --filter Storybook -nr:false -m:1
env DOTNET_CLI_HOME=/tmp/dotnet-cli dotnet build -nr:false -m:1

All passed. The build still reports existing NuGet advisory, Browserslist, and frontend deprecation warnings.

Note

This PR was written with extensive assistance from GPT-5.

@github-actions

github-actions Bot commented Mar 17, 2026

Copy link
Copy Markdown

🔍 Preview links for changed docs

@coderabbitai

coderabbitai Bot commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a new {storybook} directive that resolves stories from a configured registry and renders them inline via a storybook-story web component or as an iframe. Changes include registry JSON models, directive parsing and validation, Razor view and view-model, client web component to load/bootstrap/mount story modules, site CSS and dynamic import, LLM renderer output, documentation, and tests.

Sequence Diagram(s)

sequenceDiagram
  participant Author as Author/Markdown
  participant Parser as DirectiveBlockParser
  participant Block as StorybookBlock
  participant Registry as docs_registry.json
  participant Renderer as DirectiveHtmlRenderer
  participant View as StorybookView.cshtml
  participant Element as storybook-story (web component)
  participant Entry as Story entry module

  Author->>Parser: writes `{storybook}` block
  Parser->>Block: create & finalize
  Block->>Registry: ReadRegistry (docset configured URL)
  Block->>Block: FindStory & materialize StoryUrl/InlineEntry
  Renderer->>View: RenderRazorSlice with StorybookViewModel
  View->>Element: render <storybook-story> or <iframe>
  Element->>Entry: import(entry URL) & load bootstrap
  Entry->>Element: mountStory(storyId, container)
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 2.44% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description comprehensively explains the Storybook directive feature, authoring model, rendering behavior, testing approach, and validation steps—all directly related to the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title '[storybook] Storybook directive, for inline story rendering' accurately describes the main change: adding a new Storybook directive that enables inline story rendering.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch poc/storybook

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@src/Elastic.Documentation.Site/Assets/web-components/StorybookStory/StorybookStoryComponent.tsx`:
- Around line 50-70: attributeChangedCallback/mount can start overlapping mounts
when attributes change rapidly; add a guard to ensure only the latest mount
proceeds. Implement a per-instance version counter (e.g., this._mountVersion) or
an AbortController token that you increment/create at the start of mount(),
capture in local scope, and check/abort before/after awaiting loadBundle and
before calling this.bundle.mountStory; ensure any previous pending mount is
considered stale (skip rendering into this.container) if its version/token
doesn't match the current one. Update attributeChangedCallback to call mount()
as before but rely on the new guard inside mount() to prevent concurrent
renders.

In `@src/Elastic.Markdown/Myst/Directives/Storybook/StorybookBlock.cs`:
- Around line 136-140: In StorybookBlock (the method that validates the :root:
Storybook URI), trim trailing slashes from rootUri.AbsolutePath (or trim rawRoot
before creating/normalizing the Uri) before checking for "/iframe.html" so
values like ".../iframe.html/" are caught; update the same check/trim logic used
around the other iframe guard (the block referenced at lines 172-176) so both
places perform the trim/normalization prior to the EndsWith/Equals checks and
then proceed with the existing normalization and error assignment.

In `@src/Elastic.Markdown/Myst/Renderers/LlmMarkdown/LlmBlockRenderers.cs`:
- Around line 794-797: The attributes written for the <storybook> element are
not escaped: sanitize/HTML-encode the attribute values before writing them
(escape block.IframeTitle and block.StoryUrl output used in the src attribute,
and any other attribute values like block.Height if it's not strictly numeric),
e.g. call an existing utility or add a small helper (use
LlmRenderingHelpers.MakeAbsoluteUrl for URL resolution then pass that result
through an HTML-attribute-encoding method) and replace direct interpolations in
the renderer.Writer.Write calls so that quotes, ampersands and angle brackets
are encoded and cannot break or inject markup.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e3c4ca7c-c50c-4f00-9611-8945ddab6588

📥 Commits

Reviewing files that changed from the base of the PR and between 1797e4a and ceffd19.

📒 Files selected for processing (26)
  • docs/_docset.yml
  • docs/syntax/directives.md
  • docs/syntax/index.md
  • docs/syntax/storybook.md
  • src/Elastic.Documentation.Configuration/Builder/ConfigurationFile.cs
  • src/Elastic.Documentation.Configuration/Toc/DocumentationSetFile.cs
  • src/Elastic.Documentation.Site/Assets/main.ts
  • src/Elastic.Documentation.Site/Assets/markdown/storybook.css
  • src/Elastic.Documentation.Site/Assets/styles.css
  • src/Elastic.Documentation.Site/Assets/web-components/StorybookStory/StorybookStoryComponent.tsx
  • src/Elastic.Markdown/Myst/Directives/DirectiveBlockParser.cs
  • src/Elastic.Markdown/Myst/Directives/DirectiveHtmlRenderer.cs
  • src/Elastic.Markdown/Myst/Directives/Storybook/StorybookBlock.cs
  • src/Elastic.Markdown/Myst/Directives/Storybook/StorybookView.cshtml
  • src/Elastic.Markdown/Myst/Directives/Storybook/StorybookViewModel.cs
  • src/Elastic.Markdown/Myst/Renderers/LlmMarkdown/LlmBlockRenderers.cs
  • src/Elastic.Markdown/Myst/Renderers/PlainText/PlainTextBlockRenderers.cs
  • storybook-kibana-local-testing.md
  • storybook-kibana-mdx-transform.md
  • tests/Elastic.Markdown.Tests/Directives/DirectiveBaseTests.cs
  • tests/Elastic.Markdown.Tests/Directives/StorybookTests.cs
  • tests/Elastic.Markdown.Tests/MockFileSystemExtensions.cs
  • tests/authoring/Blocks/Storybook.fs
  • tests/authoring/Framework/Setup.fs
  • tests/authoring/LlmMarkdown/LlmMarkdownOutput.fs
  • tests/authoring/authoring.fsproj

Comment thread src/Elastic.Markdown/Myst/Directives/Storybook/StorybookBlock.cs Outdated
Comment thread src/Elastic.Markdown/Myst/Renderers/LlmMarkdown/LlmBlockRenderers.cs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (1)
src/Elastic.Markdown/Myst/Renderers/LlmMarkdown/LlmBlockRenderers.cs (1)

787-789: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Escape <storybook> attribute values before writing them.

src and title are written raw into quoted attributes; special characters can break the serialized tag and inject malformed markup into LLM output.

Proposed fix
+using System.Net;
...
 	private static void WriteStorybookBlock(LlmMarkdownRenderer renderer, StorybookBlock block)
 	{
 		if (string.IsNullOrEmpty(block.StoryUrl))
 			return;
 
 		renderer.EnsureBlockSpacing();
 		renderer.Writer.Write("<storybook");
-		renderer.Writer.Write($" src=\"{LlmRenderingHelpers.MakeAbsoluteUrl(renderer, block.StoryUrl)}\"");
-		renderer.Writer.Write($" height=\"{block.Height}\"");
-		renderer.Writer.Write($" title=\"{block.IframeTitle}\"");
+		var src = WebUtility.HtmlEncode(LlmRenderingHelpers.MakeAbsoluteUrl(renderer, block.StoryUrl) ?? string.Empty);
+		var title = WebUtility.HtmlEncode(block.IframeTitle ?? string.Empty);
+		renderer.Writer.Write($" src=\"{src}\"");
+		renderer.Writer.Write($" height=\"{block.Height}\"");
+		renderer.Writer.Write($" title=\"{title}\"");
 		renderer.Writer.WriteLine(">");
 		if (block.Count > 0)
 			WriteChildrenWithIndentation(renderer, block, "  ");
 		renderer.Writer.WriteLine("</storybook>");
 		renderer.EnsureLine();
 	}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Elastic.Markdown/Myst/Renderers/LlmMarkdown/LlmBlockRenderers.cs` around
lines 787 - 789, The src and title attribute values are written raw and can
break the serialized tag; update the writes in LlmBlockRenderers (the calls that
use LlmRenderingHelpers.MakeAbsoluteUrl(renderer, block.StoryUrl) and
block.IframeTitle) to HTML-escape/attribute-encode those values before writing
them to renderer.Writer.Write so quotes and other special chars are escaped;
ensure both src and title use the same escaping helper (or a new utility) and
preserve the existing quotes around the attribute values.
🧹 Nitpick comments (1)
src/Elastic.Markdown/Myst/Directives/Storybook/StorybookBlock.cs (1)

135-171: 🏗️ Heavy lift

Registry is re-fetched/re-read for every directive block.

TryLoadRegistryReadRegistry runs inside FinalizeAndValidate, which executes per {storybook} block. A page (or build) with N directives triggers N downloads/reads of the same registry JSON, plus a fresh HttpClient each time. Consider caching the deserialized registry per StorybookRegistry URL for the build.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Elastic.Markdown/Myst/Directives/Storybook/StorybookBlock.cs` around
lines 135 - 171, TryLoadRegistry/ReadRegistry are called per {storybook} block
causing repeated reads/downloads and new HttpClient instances; cache the
registry by keying off Build.Configuration.StorybookRegistry (the rawRegistry
URL/path) and return the cached deserialized StorybookRegistry from
TryLoadRegistry instead of re-reading. Implement a concurrent cache (e.g., a
static ConcurrentDictionary<string, StorybookRegistry> or a cache held on the
Build/context) that stores the deserialized StorybookRegistry (or the raw JSON)
the first time ReadRegistry/TryDeserializeRegistry runs, reuse the cached value
on subsequent calls, and change ReadRegistry to reuse a single shared HttpClient
for HTTP requests rather than instantiating a new HttpClient per call.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@src/Elastic.Documentation.Site/Assets/web-components/StorybookStory/StorybookStoryComponent.tsx`:
- Around line 160-179: The cache key for loadBootstrap currently uses
bootstrap.publicPath (variable cacheKey) which causes collisions when different
bootstrap objects share the same publicPath; change cacheKey to uniquely
represent the asset set by including scripts and styles content (e.g., compute
cacheKey = JSON.stringify({ publicPath: bootstrap.publicPath, scripts:
(bootstrap.scripts ?? []).slice().join('|'), styles: (bootstrap.styles ??
[]).slice().join('|') }) or otherwise serialize/sort the scripts/styles arrays)
so bootstrapCache entries are keyed by the actual assets; update references to
cacheKey in loadBootstrap and keep existing async asset-loading logic with
loadScript/loadStylesheet unchanged.

In `@src/Elastic.Markdown/Myst/Directives/Storybook/StorybookBlock.cs`:
- Around line 158-171: ReadRegistry currently blocks on
http.GetStringAsync(...).GetAwaiter().GetResult() which can stall builds; change
ReadRegistry to an async Task<string> ReadRegistryAsync(string rawRegistry,
CancellationToken ct) and replace the sync call with await
http.GetStringAsync(uri, ct) (or use HttpClient.SendAsync with
HttpCompletionOption.ResponseContentRead and ct) and set an explicit timeout
(either http.Timeout or wrap the call in a linked CancellationTokenSource with a
configured timeout). Propagate the async signature and CancellationToken up
through TryLoadRegistry and FinalizeAndValidate so registry loading is
end-to-end async and cancellable; also avoid short-lived HttpClient if possible
(reuse a shared HttpClient) or at least ensure handler disposal semantics remain
correct.

In `@src/Elastic.Markdown/Myst/Directives/Storybook/StorybookView.cshtml`:
- Around line 13-17: The iframe in StorybookView.cshtml is rendered without a
sandbox; add a least-privilege sandbox attribute to the <iframe> to restrict the
framed Storybook content (update the markup that uses Model.StoryUrl and
Model.IframeTitle). Modify the iframe to include a sandbox attribute such as
sandbox="allow-scripts allow-same-origin" (start with only these permissions and
only add additional flags like allow-popups or allow-forms if Storybook
functionality breaks), and consider keeping loading="lazy" and style as-is.

---

Duplicate comments:
In `@src/Elastic.Markdown/Myst/Renderers/LlmMarkdown/LlmBlockRenderers.cs`:
- Around line 787-789: The src and title attribute values are written raw and
can break the serialized tag; update the writes in LlmBlockRenderers (the calls
that use LlmRenderingHelpers.MakeAbsoluteUrl(renderer, block.StoryUrl) and
block.IframeTitle) to HTML-escape/attribute-encode those values before writing
them to renderer.Writer.Write so quotes and other special chars are escaped;
ensure both src and title use the same escaping helper (or a new utility) and
preserve the existing quotes around the attribute values.

---

Nitpick comments:
In `@src/Elastic.Markdown/Myst/Directives/Storybook/StorybookBlock.cs`:
- Around line 135-171: TryLoadRegistry/ReadRegistry are called per {storybook}
block causing repeated reads/downloads and new HttpClient instances; cache the
registry by keying off Build.Configuration.StorybookRegistry (the rawRegistry
URL/path) and return the cached deserialized StorybookRegistry from
TryLoadRegistry instead of re-reading. Implement a concurrent cache (e.g., a
static ConcurrentDictionary<string, StorybookRegistry> or a cache held on the
Build/context) that stores the deserialized StorybookRegistry (or the raw JSON)
the first time ReadRegistry/TryDeserializeRegistry runs, reuse the cached value
on subsequent calls, and change ReadRegistry to reuse a single shared HttpClient
for HTTP requests rather than instantiating a new HttpClient per call.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 9ef84f38-7c33-4e35-b46e-9431486ab6d4

📥 Commits

Reviewing files that changed from the base of the PR and between ceffd19 and 3773d85.

📒 Files selected for processing (23)
  • docs/_docset.yml
  • docs/syntax/directives.md
  • docs/syntax/index.md
  • docs/syntax/storybook.md
  • src/Elastic.Documentation.Configuration/Builder/ConfigurationFile.cs
  • src/Elastic.Documentation.Configuration/Serialization/YamlStaticContext.cs
  • src/Elastic.Documentation.Configuration/Toc/DocumentationSetFile.cs
  • src/Elastic.Documentation.Site/Assets/main.ts
  • src/Elastic.Documentation.Site/Assets/markdown/storybook.css
  • src/Elastic.Documentation.Site/Assets/styles.css
  • src/Elastic.Documentation.Site/Assets/web-components/StorybookStory/StorybookStoryComponent.tsx
  • src/Elastic.Markdown/Myst/Directives/DirectiveBlockParser.cs
  • src/Elastic.Markdown/Myst/Directives/DirectiveHtmlRenderer.cs
  • src/Elastic.Markdown/Myst/Directives/Storybook/StorybookBlock.cs
  • src/Elastic.Markdown/Myst/Directives/Storybook/StorybookRegistry.cs
  • src/Elastic.Markdown/Myst/Directives/Storybook/StorybookView.cshtml
  • src/Elastic.Markdown/Myst/Directives/Storybook/StorybookViewModel.cs
  • src/Elastic.Markdown/Myst/Renderers/LlmMarkdown/LlmBlockRenderers.cs
  • tests/Elastic.Markdown.Tests/Directives/DirectiveBaseTests.cs
  • tests/Elastic.Markdown.Tests/Directives/StorybookTests.cs
  • tests/Elastic.Markdown.Tests/MockFileSystemExtensions.cs
  • tests/authoring/Blocks/Storybook.fs
  • tests/authoring/authoring.fsproj
✅ Files skipped from review due to trivial changes (5)
  • docs/syntax/directives.md
  • src/Elastic.Documentation.Site/Assets/styles.css
  • tests/authoring/authoring.fsproj
  • docs/syntax/index.md
  • docs/syntax/storybook.md
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/Elastic.Markdown/Myst/Directives/DirectiveBlockParser.cs
  • src/Elastic.Documentation.Site/Assets/main.ts
  • src/Elastic.Markdown/Myst/Directives/DirectiveHtmlRenderer.cs
  • tests/Elastic.Markdown.Tests/MockFileSystemExtensions.cs

Comment thread src/Elastic.Markdown/Myst/Directives/Storybook/StorybookBlock.cs
Comment thread src/Elastic.Markdown/Myst/Directives/Storybook/StorybookView.cshtml
- Escape title/src attributes in the LLM <storybook> output (LlmBlockRenderers).
- Key the bootstrap asset cache on publicPath + scripts + styles to avoid collisions.
- Bound the registry fetch with an explicit timeout and cancellation token.
- Add a least-privilege sandbox to the fallback storybook iframe.
- Use [with(...)] collection-expression args and AwesomeAssertions after the main rebase.
- storybook.md: the PR 272388 registry now exposes the bare docs ID
  `ai-components-aibutton--default` under both `kibana:presentation` and
  `kibana:shared_ux`, so the bare-`:id:` live example resolves ambiguously
  and fails the doc build. Replace the live block with a note; the
  fully-qualified examples still render.
- StorybookStoryComponent.tsx: add `export {}` so the file is a module
  (validates the `declare global` Window augmentation and the dynamic
  import from main.ts), and apply prettier formatting.
- storybook.css: apply prettier formatting.
- StorybookTests.cs: fix import ordering and add the `/*lang=json,strict*/`
  marker on the JSON fixture to satisfy `dotnet format`.
Replace the per-directive HttpClient/handler with a process-wide
`static readonly HttpClient` so registry fetches pool connections
instead of churning sockets. Uses `SocketsHttpHandler` with a
5-minute `PooledConnectionLifetime` to bound DNS staleness under the
long-lived serve/watch process. Per-request timeout and cancellation
are unchanged.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tests/Elastic.Markdown.Tests/Directives/StorybookTests.cs (1)

140-141: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Assert diagnostic severity on error-path tests.

These assertions only check message text, so they can still pass if the diagnostic regresses to Warning. Please also assert Severity.Error for these cases.

Suggested tightening
- Collector.Diagnostics.Should().Contain(d => d.Message.Contains("does not contain id 'kibana:content_management:ai-components-aibutton--default'"));
+ Collector.Diagnostics.Should().Contain(d =>
+   d.Severity == Severity.Error &&
+   d.Message.Contains("does not contain id 'kibana:content_management:ai-components-aibutton--default'"));

- Collector.Diagnostics.Should().Contain(d => d.Message.Contains("requires docset.yml storybook.registry"));
+ Collector.Diagnostics.Should().Contain(d =>
+   d.Severity == Severity.Error &&
+   d.Message.Contains("requires docset.yml storybook.registry"));

- Collector.Diagnostics.Should().Contain(d => d.Message.Contains("requires :id: or :project:"));
+ Collector.Diagnostics.Should().Contain(d =>
+   d.Severity == Severity.Error &&
+   d.Message.Contains("requires :id: or :project:"));

Also applies to: 217-218, 229-230

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/Elastic.Markdown.Tests/Directives/StorybookTests.cs` around lines 140 -
141, The test DoesNotFallbackToAnotherStorybook currently only asserts that
Collector.Diagnostics contains a message substring; update it to also assert the
diagnostic Severity is Error by matching a diagnostic whose Message contains
"does not contain id
'kibana:content_management:ai-components-aibutton--default'" and whose Severity
== DiagnosticSeverity.Error (i.e., change the assertion on Collector.Diagnostics
to an expression that checks both d.Message.Contains(...) and d.Severity ==
DiagnosticSeverity.Error). Apply the same tightening to the other similar
assertions (the other tests that assert only message text) so each diagnostic
check verifies both the message content and Severity.Error.
🧹 Nitpick comments (1)
tests/Elastic.Markdown.Tests/Directives/StorybookTests.cs (1)

90-90: ⚡ Quick win

Rename test methods to Method_Scenario_Expected.

Current names don’t follow the repository’s test naming rule, which makes test intent less consistent across the suite.
As per coding guidelines: "Test method naming convention: Method_Scenario_Expected."

Also applies to: 102-102, 125-125, 140-140, 153-153, 166-166, 184-184, 198-198, 217-217, 229-229, 242-242

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/Elastic.Markdown.Tests/Directives/StorybookTests.cs` at line 90, Rename
the test methods in this file to follow the "Method_Scenario_Expected"
convention; for example change ResolvesStory to a descriptive name like
Resolve_Story_ReturnsExpectedResult (and similarly rename the other test methods
referenced in the comment), update the method identifier (e.g., ResolvesStory)
to the new name, and adjust any references or attributes (test framework
decorators) if present so the tests compile and run with the new identifiers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@tests/Elastic.Markdown.Tests/Directives/StorybookTests.cs`:
- Around line 140-141: The test DoesNotFallbackToAnotherStorybook currently only
asserts that Collector.Diagnostics contains a message substring; update it to
also assert the diagnostic Severity is Error by matching a diagnostic whose
Message contains "does not contain id
'kibana:content_management:ai-components-aibutton--default'" and whose Severity
== DiagnosticSeverity.Error (i.e., change the assertion on Collector.Diagnostics
to an expression that checks both d.Message.Contains(...) and d.Severity ==
DiagnosticSeverity.Error). Apply the same tightening to the other similar
assertions (the other tests that assert only message text) so each diagnostic
check verifies both the message content and Severity.Error.

---

Nitpick comments:
In `@tests/Elastic.Markdown.Tests/Directives/StorybookTests.cs`:
- Line 90: Rename the test methods in this file to follow the
"Method_Scenario_Expected" convention; for example change ResolvesStory to a
descriptive name like Resolve_Story_ReturnsExpectedResult (and similarly rename
the other test methods referenced in the comment), update the method identifier
(e.g., ResolvesStory) to the new name, and adjust any references or attributes
(test framework decorators) if present so the tests compile and run with the new
identifiers.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 7fbe42f9-ce0d-4f71-bc1b-606213899928

📥 Commits

Reviewing files that changed from the base of the PR and between 045aa38 and 30cf479.

📒 Files selected for processing (5)
  • docs/syntax/storybook.md
  • src/Elastic.Documentation.Site/Assets/markdown/storybook.css
  • src/Elastic.Documentation.Site/Assets/web-components/StorybookStory/StorybookStoryComponent.tsx
  • src/Elastic.Markdown/Myst/Directives/Storybook/StorybookBlock.cs
  • tests/Elastic.Markdown.Tests/Directives/StorybookTests.cs
✅ Files skipped from review due to trivial changes (1)
  • docs/syntax/storybook.md
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/Elastic.Documentation.Site/Assets/markdown/storybook.css
  • src/Elastic.Documentation.Site/Assets/web-components/StorybookStory/StorybookStoryComponent.tsx
  • src/Elastic.Markdown/Myst/Directives/Storybook/StorybookBlock.cs

Comment on lines +871 to +888
private static void WriteStorybookBlock(LlmMarkdownRenderer renderer, StorybookBlock block)
{
if (string.IsNullOrEmpty(block.StoryUrl))
return;

renderer.EnsureBlockSpacing();
var src = WebUtility.HtmlEncode(LlmRenderingHelpers.MakeAbsoluteUrl(renderer, block.StoryUrl));
renderer.Writer.Write("<storybook");
renderer.Writer.Write($" src=\"{src}\"");
renderer.Writer.Write($" height=\"{block.Height}\"");
renderer.Writer.Write($" title=\"{WebUtility.HtmlEncode(block.IframeTitle)}\"");
renderer.Writer.WriteLine(">");
if (block.Count > 0)
WriteChildrenWithIndentation(renderer, block, " ");
renderer.Writer.WriteLine("</storybook>");
renderer.EnsureLine();
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot can you think of output that is more appropriate for an LLM to read?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced the visual/runtime attributes (src, height) with attributes that give an LLM meaningful context about the component:

<storybook title="AI Button / Default story" project="kibana" storybook="shared_ux" story-id="ai-components-aibutton--default">
  ...
</storybook>
  • title — human-readable name of the story
  • project — top-level project (e.g. kibana), helps scope where the component lives
  • storybook — bundle/library name (e.g. shared_ux), helps an LLM point developers to the right package
  • story-id — the Storybook story identifier (e.g. ai-components-aibutton--default), which developers can use directly to find and reference the component

The XML-like markup is kept consistent with other LLM block elements like <agent-skill>.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot this output is not entirely helpful. The LLM needs to know the original URL so it can follow it and read the code

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added src with the story URL so an LLM can follow it to read the component code. The output now looks like:

<storybook title="AI Button / Default story" project="kibana" storybook="shared_ux" story-id="ai-components-aibutton--default" src="http://127.0.0.1:6007/storybook/shared_ux/iframe.html?id=ai-components-aibutton--default&amp;viewMode=story">
  ...
</storybook>

Copilot AI requested a review from reakaleek June 3, 2026 17:21
Copilot AI temporarily deployed to integration-tests June 3, 2026 17:22 Inactive
<storybook-story> injects its Kibana bootstrap <script>/<link> into
<head> and tracks them in module-level load caches. htmx's head
reconciliation strips every non-<style> head element on navigation,
which (1) breaks an in-flight first load — the stylesheet onload never
fires, so loadBootstrap's style await hangs and the script loop never
runs, leaving an empty container with no further requests — and
(2) leaves the caches pointing at elements that no longer exist.

Preserve elements tagged data-storybook-script/data-storybook-style in
the htmx:removingHeadElement handler, mirroring the existing <style>
preservation for the global nav.
Copilot AI temporarily deployed to integration-tests June 3, 2026 17:36 Inactive
@clintandrewhall

Copy link
Copy Markdown
Contributor Author

@reakaleek I'm going to hold off merging until elastic/kibana#272388 merges, which is just waiting on an actual document with an actual example story. The other option is to strike the actual embeds from docs/syntax/storybook.md.

If we get slowed down, I'll merge without the embeds until Kibana docs have solid examples. TTL EOW.

@clintandrewhall

Copy link
Copy Markdown
Contributor Author

Merging, as elastic/kibana#272927 is blocked, but ready to go.

@clintandrewhall clintandrewhall changed the title [storybook] PoC of Storybook directive, for inline stories [storybook] Storybook directive, for inline story rendering Jun 8, 2026
@clintandrewhall clintandrewhall merged commit 182938f into main Jun 8, 2026
25 checks passed
@clintandrewhall clintandrewhall deleted the poc/storybook branch June 8, 2026 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants