Agentic Setup: Add observability#34510
Merged
Sidnioulz merged 35 commits intoproject/sb-agentic-setupfrom Apr 14, 2026
Merged
Conversation
Co-designing telemetry enhancements for tracking human vs agent-driven Storybook instances and feature adoption metrics. - docs/superpowers/specs/2026-04-07-telemetry-enhancements-design.md: comprehensive design covering 3 sequential streams (metadata enrichment, sb ai prepare events, ghost-stories broadening). Section 1 approved, sections 2-8 drafted awaiting user review tomorrow. - scripts/spike-extract-features.ts: throwaway timing spike that proves full inline feature extraction is feasible (~640ms for 505 story files, ~38ms marginal cost over existing parse work). Will be deleted at end of Stream A.
…lify to once-ever gate
|
View your CI Pipeline Execution ↗ for commit f51a483
☁️ Nx Cloud last updated this comment at |
Sidnioulz
commented
Apr 9, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Adds observability for agent-driven workflows and related telemetry plumbing across CLI + core, including sb ai prepare start telemetry, evidence checkpoints, agent-in-CI metadata retention, and a redesigned ghost-stories trigger/gate. This supports WS1 “agentic telemetry” by enabling correlation across agent sessions and increasing ghost-stories sampling coverage without user interaction.
Changes:
- Add
sb ai preparetrait capture, optional YAML frontmatter output, and cache a baseline “ai-setup pending” record for later evidence collection. - Add an
ai-setup-evidencetelemetry event type and awithTelemetry()hook to emit evidence events for agent runs (plus preserveuserSincein CI when an agent is detected). - Redesign ghost-stories triggering (10-minute delayed manager trigger) and simplify the server-side “once ever” gate; add local telemetry tooling scripts.
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/spike-extract-features.ts | Adds a feature-extraction timing spike script (planning/tooling). |
| scripts/mock-telemetry-receiver.ts | Adds a local HTTP server to receive/log telemetry events for debugging/testing. |
| docs/superpowers/specs/2026-04-08-ws1-agentic-telemetry-plan-pm.md | PM-facing plan describing WS1 telemetry changes. |
| docs/superpowers/specs/2026-04-08-ws1-agentic-telemetry-plan-engineer.md | Engineering implementation plan for WS1 changes. |
| docs/superpowers/specs/2026-04-07-telemetry-enhancements-design.md | Design doc capturing telemetry decisions and WS1/WS2 split. |
| code/lib/cli-storybook/src/bin/run.ts | Adds --frontmatter flag to sb ai prepare. |
| code/lib/cli-storybook/src/ai/types.ts | Adds AiPrepareTraits and extends options with frontmatter. |
| code/lib/cli-storybook/src/ai/setup-requirements.ts | Adds prompt-adjacent “requirements” helpers (preview snapshot/hash, AI story detection). |
| code/lib/cli-storybook/src/ai/prompt.ts | Threads a traits accumulator through prompt generation output. |
| code/lib/cli-storybook/src/ai/index.ts | Sends ai-prepare telemetry, writes pending-record baseline, supports frontmatter output. |
| code/core/src/telemetry/types.ts | Adds ai-setup-evidence to the EventType union. |
| code/core/src/telemetry/storybook-metadata.ts | Preserves userSince in CI when an agent is detected. |
| code/core/src/telemetry/storybook-metadata.test.ts | Adds coverage for “agent in CI keeps userSince”. |
| code/core/src/telemetry/index.ts | Re-exports getAiSetupPending and SESSION_TIMEOUT for evidence collection. |
| code/core/src/telemetry/event-cache.ts | Adds getAiSetupPending() cache helper for pending setup record. |
| code/core/src/telemetry/detect-agent.test.ts | Hardens tests by clearing/restoring ambient agent env vars. |
| code/core/src/manager/components/sidebar/useGhostStoriesTrigger.ts | Adds 10-minute delayed trigger after PREVIEW_INITIALIZED. |
| code/core/src/manager/components/sidebar/Sidebar.tsx | Calls useGhostStoriesTrigger() from the Sidebar. |
| code/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsx | Removes modal-based ghost-stories trigger. |
| code/core/src/core-server/withTelemetry.ts | Adds agent-gated evidence collection hook and preview hash comparison. |
| code/core/src/core-server/withTelemetry.test.ts | Adds detect-agent mocking support for new behavior. |
| code/core/src/core-server/server-channel/ghost-stories-channel.ts | Simplifies ghost-stories gate to a once-ever lastEvents['ghost-stories'] check. |
| code/core/src/core-server/server-channel/ghost-stories-channel.test.ts | Updates tests to match the simplified gate (removes sessionId checks). |
| code/core/src/common/satellite-addons.ts | Adds @storybook/addon-mcp to satellite addons list. |
Comments suppressed due to low confidence (1)
code/lib/cli-storybook/src/ai/index.ts:95
- The renderer/builder gate is using
&&, which allows unsupported combinations (e.g. React+Webpack or Vue+Vite) to pass as long as one of the two matches. Since the message says the feature is only available for React renderer with Vite builder, the condition should reject if either differs (use||).
if (
projectInfo.rendererPackage !== '@storybook/react' &&
projectInfo.builderPackage !== '@storybook/builder-vite'
) {
logger.log(
'AI-assisted setup is currently only available for projects using the React renderer with Vite builder. Detected renderer: ' +
projectInfo.rendererPackage +
', builder: ' +
projectInfo.builderPackage
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
294
to
+301
| if (enableTelemetry) { | ||
| telemetry('boot', { eventType }, { stripMetadata: true }); | ||
| } | ||
|
|
||
| if (enableTelemetry) { | ||
| // Fire-and-forget: don't await, don't block the command | ||
| collectAiSetupEvidence(eventType, options).catch(() => {}); | ||
| } |
There was a problem hiding this comment.
New evidence collection behavior is introduced here (fire-and-forget call into collectAiSetupEvidence), but withTelemetry.test.ts doesn't cover any of the new gating/payload logic. Please add unit tests that verify: (1) no event when no agent, (2) no event when no pending record, (3) no event when pending is expired, and (4) event payload fields (previewChanged/doctorRanSinceSetup) when all gates pass.
Member
Author
There was a problem hiding this comment.
Correct, must be fixed.
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds AI-setup telemetry (CLI emits Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (sb ai prepare)
participant FS as Filesystem (preview file)
participant Cache as Cache (ai-setup-pending)
participant Server as Server (withTelemetry / dev)
participant Telemetry as Telemetry Endpoint
CLI->>FS: snapshotPreviewFile(configDir) -> compute previewHash
CLI->>Cache: store ai-setup-pending {sessionId,timestamp,configDir,previewFile,previewHash,traits}
CLI->>Telemetry: emit "ai-prepare"
Server->>Cache: getAiSetupPending()
Server->>FS: snapshotPreviewFile(configDir) -> compute current previewHash
Server->>Server: compare baseline vs current (hasPreviewChanged)
Server->>Telemetry: emit "ai-prepare-evidence" {immediate:true, msSinceAiPrepare, previewChanged, traits, aiAuthoredStories?}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
scripts/mock-telemetry-receiver.ts (1)
30-35: Add a request-size guard for/event-logbodies.
bodyis currently unbounded; a large payload can grow memory unexpectedly. A small cap (e.g., 1 MB) makes the debug server much safer.Suggested hardening patch
const PORT = Number(process.env.PORT || 6007); const LOG_DIR = resolve(process.env.LOG_DIR || '.cache/telemetry-debug'); +const MAX_BODY_BYTES = 1024 * 1024; const events: Array<{ receivedAt: string; [key: string]: unknown }> = []; @@ if (req.method === 'POST' && req.url === '/event-log') { let body = ''; + let bodySize = 0; + let tooLarge = false; req.on('data', (chunk: Buffer) => { + bodySize += chunk.length; + if (bodySize > MAX_BODY_BYTES) { + tooLarge = true; + res.statusCode = 413; + res.end('payload too large'); + req.destroy(); + return; + } body += chunk; }); req.on('end', async () => { + if (tooLarge) { + return; + } try { const data = JSON.parse(body);Also applies to: 48-51
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/mock-telemetry-receiver.ts` around lines 30 - 35, The request body accumulator (variable body) for the /event-log handler is unbounded and needs a size guard; add a max size constant (e.g. MAX_BODY_BYTES = 1 * 1024 * 1024) and in the req.on('data', ...) callback check accumulated length, and if it exceeds the limit immediately stop reading (req.destroy() or req.pause()), send a 413 Payload Too Large response and return; apply the same check to the other identical data handler instance (the second req.on('data'...) at lines referenced) so both places enforce the cap and avoid unbounded memory growth.code/core/src/telemetry/storybook-metadata.test.ts (1)
36-38: Consider usingspy: truefor consistency with other mocks.The
detectAgentmock doesn't use thespy: trueoption that other mocks in this file use (e.g., lines 35, 39-47). While the current approach works, using the consistent pattern would align with the project's mocking guidelines.♻️ Suggested refactor
-vi.mock('./detect-agent.ts', () => ({ - detectAgent: vi.fn().mockReturnValue(undefined), -})); +vi.mock(import('./detect-agent.ts'), { spy: true });Then set the default behavior in
beforeEach:vi.mocked(detectAgent).mockReturnValue(undefined);As per coding guidelines: "Use
vi.mock()with thespy: trueoption for all package and file mocks in Vitest tests"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/telemetry/storybook-metadata.test.ts` around lines 36 - 38, Replace the direct vi.mock call for detectAgent with the project's spy pattern: call vi.mock('./detect-agent.ts', { spy: true, ... }) (keeping the module path) so the mock uses a spy like the others, then move the default return into the test setup by calling vi.mocked(detectAgent).mockReturnValue(undefined) inside beforeEach; target the detectAgent import and the vi.mock/vi.mocked usage when making this change.code/lib/cli-storybook/src/ai/index.ts (1)
135-151: Inconsistent YAML quoting in frontmatter.Some string values are quoted (
framework,renderer,builder) while others are not (storybook,language). YAML interpreters handle this, but consistent quoting improves readability and avoids edge cases if values contain special characters.♻️ Suggested fix for consistent quoting
function buildFrontmatter(projectInfo: ProjectInfo, traits: AiPrepareTraits): string { const lines = [ '---', - `storybook: ${projectInfo.storybookVersion || 'unknown'}`, + `storybook: '${projectInfo.storybookVersion || 'unknown'}'`, `framework: '${projectInfo.framework || 'unknown'}'`, `renderer: '${projectInfo.rendererPackage || 'unknown'}'`, `builder: '${projectInfo.builderPackage || 'unknown'}'`, - `language: ${projectInfo.language}`, - `hasCsfFactoryPreview: ${projectInfo.hasCsfFactoryPreview}`, + `language: '${projectInfo.language}'`, + `hasCsfFactoryPreview: ${String(projectInfo.hasCsfFactoryPreview)}`, 'traits:', ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/lib/cli-storybook/src/ai/index.ts` around lines 135 - 151, The frontmatter built by buildFrontmatter currently mixes quoted and unquoted values (e.g., framework vs storybook/language); update buildFrontmatter so all string-valued fields are consistently quoted: wrap projectInfo.storybookVersion, projectInfo.language and any trait values that are strings in quotes while leaving non-strings (booleans/numbers) unquoted. Locate buildFrontmatter and adjust the lines array generation and the traits loop to detect typeof value === 'string' and add quotes (or use a helper to quote safely) for those entries.code/core/src/core-server/withTelemetry.ts (1)
174-197: Duplicated preview change detection logic.
checkPreviewChangedduplicates logic fromsetup-requirements.ts:hasPreviewChangedbut usesfindConfigFiledirectly instead of callingsnapshotPreviewFile. The comment explains this is to avoid cross-package imports, which is reasonable.Consider whether future changes to preview detection logic might need coordinated updates in both locations. The current duplication is acceptable given the avoidance of circular dependencies.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/core-server/withTelemetry.ts` around lines 174 - 197, checkPreviewChanged duplicates preview-detection logic found in setup-requirements.ts (hasPreviewChanged) but uses findConfigFile directly instead of snapshotPreviewFile; either refactor checkPreviewChanged to delegate to the shared snapshotPreviewFile/hasPreviewChanged utility to avoid drift, or add a clear TODO/comment above checkPreviewChanged noting the duplication and listing the canonical function names (hasPreviewChanged and snapshotPreviewFile) so future changes are synchronized; update the function/inline comment accordingly to reference these symbols.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/core/src/core-server/withTelemetry.test.ts`:
- Around line 12-14: Replace the inline-factory mock with a spy-style mock and
move behavior into beforeEach: call vi.mock('../telemetry/detect-agent.ts', {
spy: true }) (leaving detectAgent exported from that module uninitialized here)
and then in the test suite's beforeEach set the behavior using the mocked/spy
reference to detectAgent (e.g.,
vi.mocked(detectAgent).mockReturnValue(undefined) or mockImplementation) so all
behavior is defined in beforeEach rather than in the vi.mock factory; reference
the detectAgent export used in withTelemetry.test.ts and update beforeEach
accordingly.
In `@code/core/src/telemetry/event-cache.ts`:
- Around line 85-93: The AiSetupPendingRecord interface's traits field is
inconsistent with setup-requirements.ts (here: traits: Record<string,string] vs
setup-requirements.ts using AiPrepareTraits), so either import and use
AiPrepareTraits from types.ts in event-cache.ts to make the interface match
exactly (update the AiSetupPendingRecord definition accordingly) or, if the
looser shape is intentional for cache reads, update the comment above
AiSetupPendingRecord to explicitly state it intentionally uses a relaxed type
and why; reference AiSetupPendingRecord, AiPrepareTraits, and
setup-requirements.ts in your change so reviewers can verify the chosen
approach.
In `@docs/superpowers/specs/2026-04-07-telemetry-enhancements-design.md`:
- Line 7: The anchor link "[§ Resume Tomorrow](`#resume-tomorrow`)" has a case
mismatch with the heading "## Resume tomorrow"; update either the heading to "##
Resume Tomorrow" or the link target/text to match the heading's casing so the
anchor slug generated for "## Resume tomorrow" and the link "#resume-tomorrow"
are consistent across parsers; locate the link and the heading in the document
and make their text/casing identical.
- Line 566: The table row "| Tests as in §4.2 |" is malformed because it
contains a single pipe-delimited cell and breaks the table; replace it with
either a properly formatted table row with matching columns or convert it to
plain paragraph text (e.g., remove the surrounding pipes) so it is not treated
as a table; locate the literal string "| Tests as in §4.2 |" in the document and
update it accordingly to match the surrounding table column count or move it
outside the table block.
In `@scripts/spike-extract-features.ts`:
- Around line 29-38: The glob walker currently discovers files then filters
them, causing traversal into ignored directories; update findStoryFiles to pass
an ignore option into the glob call so the walker never descends into
IGNORE_SEGMENTS (e.g., set glob(..., { cwd, ignore: IGNORE_SEGMENTS.map(s =>
`${s}/**`) }) or equivalent), keep the existing ext check using
STORY_EXTENSIONS, and remove or keep the post-filtering by parts — the key
change is adding the ignore option on the glob invocation so node_modules, dist,
build, storybook-static, .cache are excluded up-front.
- Around line 100-118: In objectLiteralKeys(), add handling for quoted string
keys in both branches (where you currently check t.isIdentifier for
ObjectProperty and ObjectMethod) by also checking t.isStringLiteral and pushing
(key as { value: string }).value into the keys array; keep the existing
Identifier handling (pushing .name) and ensure the function still returns the
keys array at the end. Use the existing t.isStringLiteral() pattern consistent
with the codebase and update the branches in objectLiteralKeys to cover both
Identifier and StringLiteral key types.
---
Nitpick comments:
In `@code/core/src/core-server/withTelemetry.ts`:
- Around line 174-197: checkPreviewChanged duplicates preview-detection logic
found in setup-requirements.ts (hasPreviewChanged) but uses findConfigFile
directly instead of snapshotPreviewFile; either refactor checkPreviewChanged to
delegate to the shared snapshotPreviewFile/hasPreviewChanged utility to avoid
drift, or add a clear TODO/comment above checkPreviewChanged noting the
duplication and listing the canonical function names (hasPreviewChanged and
snapshotPreviewFile) so future changes are synchronized; update the
function/inline comment accordingly to reference these symbols.
In `@code/core/src/telemetry/storybook-metadata.test.ts`:
- Around line 36-38: Replace the direct vi.mock call for detectAgent with the
project's spy pattern: call vi.mock('./detect-agent.ts', { spy: true, ... })
(keeping the module path) so the mock uses a spy like the others, then move the
default return into the test setup by calling
vi.mocked(detectAgent).mockReturnValue(undefined) inside beforeEach; target the
detectAgent import and the vi.mock/vi.mocked usage when making this change.
In `@code/lib/cli-storybook/src/ai/index.ts`:
- Around line 135-151: The frontmatter built by buildFrontmatter currently mixes
quoted and unquoted values (e.g., framework vs storybook/language); update
buildFrontmatter so all string-valued fields are consistently quoted: wrap
projectInfo.storybookVersion, projectInfo.language and any trait values that are
strings in quotes while leaving non-strings (booleans/numbers) unquoted. Locate
buildFrontmatter and adjust the lines array generation and the traits loop to
detect typeof value === 'string' and add quotes (or use a helper to quote
safely) for those entries.
In `@scripts/mock-telemetry-receiver.ts`:
- Around line 30-35: The request body accumulator (variable body) for the
/event-log handler is unbounded and needs a size guard; add a max size constant
(e.g. MAX_BODY_BYTES = 1 * 1024 * 1024) and in the req.on('data', ...) callback
check accumulated length, and if it exceeds the limit immediately stop reading
(req.destroy() or req.pause()), send a 413 Payload Too Large response and
return; apply the same check to the other identical data handler instance (the
second req.on('data'...) at lines referenced) so both places enforce the cap and
avoid unbounded memory growth.
🪄 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: c39a8f4f-6042-44ee-8834-bcc55dc310b9
📒 Files selected for processing (24)
code/core/src/common/satellite-addons.tscode/core/src/core-server/server-channel/ghost-stories-channel.test.tscode/core/src/core-server/server-channel/ghost-stories-channel.tscode/core/src/core-server/withTelemetry.test.tscode/core/src/core-server/withTelemetry.tscode/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsxcode/core/src/manager/components/sidebar/Sidebar.tsxcode/core/src/manager/components/sidebar/useGhostStoriesTrigger.tscode/core/src/telemetry/detect-agent.test.tscode/core/src/telemetry/event-cache.tscode/core/src/telemetry/index.tscode/core/src/telemetry/storybook-metadata.test.tscode/core/src/telemetry/storybook-metadata.tscode/core/src/telemetry/types.tscode/lib/cli-storybook/src/ai/index.tscode/lib/cli-storybook/src/ai/prompt.tscode/lib/cli-storybook/src/ai/setup-requirements.tscode/lib/cli-storybook/src/ai/types.tscode/lib/cli-storybook/src/bin/run.tsdocs/superpowers/specs/2026-04-07-telemetry-enhancements-design.mddocs/superpowers/specs/2026-04-08-ws1-agentic-telemetry-plan-engineer.mddocs/superpowers/specs/2026-04-08-ws1-agentic-telemetry-plan-pm.mdscripts/mock-telemetry-receiver.tsscripts/spike-extract-features.ts
💤 Files with no reviewable changes (1)
- code/core/src/manager/components/sidebar/CreateNewStoryFileModal.tsx
Sidnioulz
commented
Apr 9, 2026
Sidnioulz
commented
Apr 9, 2026
Sidnioulz
commented
Apr 9, 2026
Sidnioulz
commented
Apr 9, 2026
Sidnioulz
commented
Apr 9, 2026
Sidnioulz
commented
Apr 9, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/core/src/core-server/withTelemetry.ts`:
- Around line 215-225: The pending ai-setup record returned by
getAiSetupPending() is only checked for age, so add validation that the record
belongs to the current run: after fetching pending and before computing
msSinceAiPrepare, verify pending.sessionId and pending.configDir match the
current run's session identifiers (e.g., the local sessionId and configDir
values available in this module) and return early if they differ; keep the
existing timestamp/SESSION_TIMEOUT check (use pending.timestamp as before).
Ensure you reference getAiSetupPending(), pending.timestamp, SESSION_TIMEOUT,
pending.sessionId and pending.configDir when making the comparison.
- Around line 241-244: The telemetry check for doctorRanSinceSetup only reads
lastEvents.doctor but ignores the current in-flight boot event; change the logic
that computes doctorBoot/doctorRanSinceSetup to treat the current pending boot
as evidence by comparing lastEvents.doctor with the current pending event (use
pending.timestamp and pending.type or whatever flag denotes the current "doctor"
boot) and take the later timestamp—e.g., set doctorBoot = the more recent of
lastEvents.doctor and pending (if pending indicates a doctor boot) and then
compute doctorRanSinceSetup = Boolean(doctorBoot && doctorBoot.timestamp >
pending.timestamp) so the just-fired doctor boot counts as evidence.
🪄 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: 42185871-24fe-4681-8d20-b0e8b17f0191
📒 Files selected for processing (1)
code/core/src/core-server/withTelemetry.ts
…dev/build commands
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 50 | 50 | 0 |
| Self size | 20.53 MB | 20.54 MB | 🚨 +14 KB 🚨 |
| Dependency size | 16.56 MB | 16.56 MB | 0 B |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 184 | 184 | 0 |
| Self size | 818 KB | 819 KB | 🚨 +1 KB 🚨 |
| Dependency size | 68.18 MB | 68.19 MB | 🚨 +14 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 177 | 177 | 0 |
| Self size | 32 KB | 32 KB | 0 B |
| Dependency size | 66.69 MB | 66.71 MB | 🚨 +14 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 51 | 51 | 0 |
| Self size | 1.05 MB | 1.05 MB | 0 B |
| Dependency size | 37.09 MB | 37.10 MB | 🚨 +14 KB 🚨 |
| Bundle Size Analyzer | node | node |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/core/src/core-server/server-channel/ghost-stories-channel.ts (1)
61-70:⚠️ Potential issue | 🟠 MajorScope the one-shot ghost-stories guard to the current run.
getLastEvents()reads a globallastEventscache keyed only by event type, so checking only for the existence oflastEvents['ghost-stories']makes this a cross-run/global one-shot. After the first recorded ghost-stories event, later Storybook instances can skip immediately even though they never ran it. Compare against the current init/session instead of treating any cached event as a match.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/core-server/server-channel/ghost-stories-channel.ts` around lines 61 - 70, The current guard treats any prior 'ghost-stories' entry in the global lastEvents cache as a permanent one-shot; change it to scope to the current init/session by comparing the stored ghost-stories event against the current init value. Specifically, inside the block that calls getLastEvents() use lastEvents?.init (lastInit) as the current run identifier and replace the unconditional truthy check of lastEvents['ghost-stories'] with a comparison that ensures lastEvents['ghost-stories'] corresponds to lastInit (e.g., only skip when lastEvents['ghost-stories'] === lastInit or has the same run/session id), leaving other behavior (return when !lastEvents || !lastInit) unchanged.
♻️ Duplicate comments (1)
code/core/src/telemetry/ai-prepare-evidence.ts (1)
110-143:⚠️ Potential issue | 🟠 MajorDon't let an undefined
configDirreuse a stale pending record.If callers pass
undefinedhere, Gate 4 is skipped and we still hashpending.configDir+ emitai-prepare-evidencefor whatever repo last wroteai-setup-pending. That can attach stale evidence to the wrong project. Bail out unless the current run can prove it is in the same config dir, and send telemetry with the matched dir.Suggested fix
- if (configDir && pending.configDir !== configDir) { + if (!configDir || pending.configDir !== configDir) { return; } @@ { immediate: true, - configDir, + configDir: pending.configDir, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/telemetry/ai-prepare-evidence.ts` around lines 110 - 143, The current Gate 4 allows a missing configDir to skip the cross-project guard and reuse a stale pending record; change the guard to bail unless there is a current configDir that matches the pending record (replace if (configDir && pending.configDir !== configDir) return; with a check like if (!configDir || pending.configDir !== configDir) return;), use the current configDir (not pending.configDir) when calling checkPreviewChanged, and include the matched config dir in the telemetry payload (e.g. add matchedConfigDir: configDir) when emitting 'ai-prepare-evidence' so evidence is tied to the verified project.
🧹 Nitpick comments (1)
code/core/src/core-server/server-channel/ghost-stories-channel.ts (1)
22-31: Don't treat every test-provider failure as “idle.”This
catchturns anyfullTestProviderStore.getFullState()failure intotrue, so a broken store looks the same as “no tests are running” and we can launch another Vitest run on top of active work. Keep the uninitialized-store fallback if needed, but fail closed for unexpected exceptions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/core-server/server-channel/ghost-stories-channel.ts` around lines 22 - 31, The catch currently treats any exception from fullTestProviderStore.getFullState() as "idle", which hides real failures; update the error handling in the block that calls fullTestProviderStore.getFullState() and computes isRunning so only the known "store not initialized" condition is treated as idle (return true); for any other unexpected exception either rethrow or return false (fail closed) and log the error. Specifically, modify the try/catch around fullTestProviderStore.getFullState() / isRunning so the catch inspects the thrown error (e.g., by error type or message) and only returns true for the explicit uninitialized-store case, otherwise propagate or handle as a real failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/core/src/core-server/withTelemetry.ts`:
- Around line 189-193: Currently collectAiPrepareEvidence is invoked before the
command's run(), causing short-lived mutating commands to record pre-command
state; move the collectAiPrepareEvidence(options.cliOptions.configDir ||
options.presetOptions?.configDir) call to execute after command.run() for
non-dev and non-build commands (i.e., when the command is short-lived), leaving
the existing dev/build telemetry flow (doTelemetry) unchanged; update the
conditional around enableTelemetry to call collectAiPrepareEvidence post-run
(but skip for dev/build/long-running paths) so evidence reflects post-command
filesystem changes.
---
Outside diff comments:
In `@code/core/src/core-server/server-channel/ghost-stories-channel.ts`:
- Around line 61-70: The current guard treats any prior 'ghost-stories' entry in
the global lastEvents cache as a permanent one-shot; change it to scope to the
current init/session by comparing the stored ghost-stories event against the
current init value. Specifically, inside the block that calls getLastEvents()
use lastEvents?.init (lastInit) as the current run identifier and replace the
unconditional truthy check of lastEvents['ghost-stories'] with a comparison that
ensures lastEvents['ghost-stories'] corresponds to lastInit (e.g., only skip
when lastEvents['ghost-stories'] === lastInit or has the same run/session id),
leaving other behavior (return when !lastEvents || !lastInit) unchanged.
---
Duplicate comments:
In `@code/core/src/telemetry/ai-prepare-evidence.ts`:
- Around line 110-143: The current Gate 4 allows a missing configDir to skip the
cross-project guard and reuse a stale pending record; change the guard to bail
unless there is a current configDir that matches the pending record (replace if
(configDir && pending.configDir !== configDir) return; with a check like if
(!configDir || pending.configDir !== configDir) return;), use the current
configDir (not pending.configDir) when calling checkPreviewChanged, and include
the matched config dir in the telemetry payload (e.g. add matchedConfigDir:
configDir) when emitting 'ai-prepare-evidence' so evidence is tied to the
verified project.
---
Nitpick comments:
In `@code/core/src/core-server/server-channel/ghost-stories-channel.ts`:
- Around line 22-31: The catch currently treats any exception from
fullTestProviderStore.getFullState() as "idle", which hides real failures;
update the error handling in the block that calls
fullTestProviderStore.getFullState() and computes isRunning so only the known
"store not initialized" condition is treated as idle (return true); for any
other unexpected exception either rethrow or return false (fail closed) and log
the error. Specifically, modify the try/catch around
fullTestProviderStore.getFullState() / isRunning so the catch inspects the
thrown error (e.g., by error type or message) and only returns true for the
explicit uninitialized-store case, otherwise propagate or handle as a real
failure.
🪄 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: c1fa84a1-50e4-4232-a2ae-d3309d462eec
📒 Files selected for processing (12)
code/core/src/core-server/presets/common-preset.tscode/core/src/core-server/server-channel/ghost-stories-channel.tscode/core/src/core-server/utils/doTelemetry.tscode/core/src/core-server/withTelemetry.test.tscode/core/src/core-server/withTelemetry.tscode/core/src/telemetry/ai-prepare-evidence.test.tscode/core/src/telemetry/ai-prepare-evidence.tscode/core/src/telemetry/event-cache.tscode/core/src/telemetry/index.tscode/core/src/telemetry/storybook-metadata.test.tscode/core/src/telemetry/types.tscode/lib/cli-storybook/src/ai/setup-requirements.ts
✅ Files skipped from review due to trivial changes (3)
- code/core/src/core-server/withTelemetry.test.ts
- code/core/src/telemetry/ai-prepare-evidence.test.ts
- code/core/src/telemetry/event-cache.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- code/core/src/telemetry/index.ts
- code/core/src/telemetry/types.ts
- code/core/src/telemetry/storybook-metadata.test.ts
Sidnioulz
commented
Apr 10, 2026
| '@storybook/addon-coverage', | ||
| '@storybook/addon-webpack5-compiler-babel', | ||
| '@storybook/addon-webpack5-compiler-swc', | ||
| '@storybook/addon-mcp', |
Member
Author
There was a problem hiding this comment.
This was caught up during my review of the existing telemetry, we're not putting this addon in our own satellites bucket.
Sidnioulz
commented
Apr 11, 2026
Sidnioulz
commented
Apr 13, 2026
yannbf
approved these changes
Apr 13, 2026
Member
yannbf
left a comment
yannbf
left a comment
There was a problem hiding this comment.
My tests were pretty promising, and from a quick glance what I'd comment on already has comments and you are already planning on fixing them. I will take a deeper pass but so far LGTM
Sidnioulz
commented
Apr 14, 2026
Sidnioulz
commented
Apr 14, 2026
This was referenced Apr 15, 2026
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What I did
sb aicommandsb ai setupWhat I didn't do
The following is left to do, in this or a follow-up PR:
devrun and in the Vitest runs of the self-healing loop, if needed depending on what @kasperpeulen does via the LLM and what he does viaafterEachhooksai-generatedtagsChecklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
node mock-telemetry-receiver.tsto start the telemetry debug serverexport STORYBOOK_TELEMETRY_URL=http://localhost:6007/event-logto point to the debug serverRun
sb ai prepareon a freshly installed Storybook instanceIn a project without Storybook, run:
export STORYBOOK_TELEMETRY_URL=http://localhost:6007/event-log npx storybook@0.0.0-pr-34510-sha-c891c832 initGo through the install, then fire up an agent and prompt it:
You should see ai-prepare, vitest run, boot:doctor and ai-prepare-evidence in the log. You should see that we capture changes to the preview file.
Next run Storybook, and press "+" to get the ghost stories data.
You should see a new ai-prepare-evidence event fired counting the number of AI written stories.
Caution
Currently this event is not always firing. Must debug further. This is probably because
sb ai preparestill creates stories with anAI Generatedtitle but the telemetry looks for anai-generatedtag. I expect @yannbf might have already changed this on another PR so will sync with him.Next wait 10 minutes until the AI prepare test run triggers.
You should see an ai-prepare-story-scoring event.
dev-after-ai-prepare.log
Init fresh project, open as human and trigger legacy ghost story flow
In a project without Storybook, run:
export STORYBOOK_TELEMETRY_URL=http://localhost:6007/event-log npx storybook@0.0.0-pr-34510-sha-c891c832 initGo through the install, then in the UI, click the "+" button in the sidebar. Notice the ghost-stories event in the debugger.
basic-init.log
Documentation
ø
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This pull request has been released as version
0.0.0-pr-34510-sha-c891c832. Try it out in a new sandbox by runningnpx storybook@0.0.0-pr-34510-sha-c891c832 sandboxor in an existing project withnpx storybook@0.0.0-pr-34510-sha-c891c832 upgrade.More information
0.0.0-pr-34510-sha-c891c832sidnioulz/agentic-telemetry-ws1c891c8321775916873)To request a new release of this pull request, mention the
@storybookjs/coreteam.core team members can create a new canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=34510Summary by CodeRabbit
Telemetry
UX / Behavior
Chores