Agentic Setup: Rework ai-init-opt-in logic#34739
Conversation
There was a problem hiding this comment.
Pull request overview
Reworks how the “agentic setup” flow records and consumes AI-related flags, aiming to (1) decouple final scoring from ai-init-opt-in and (2) treat missing init opt-in as true by default.
Changes:
- Persist
ai-init-opt-inwith an explicitanswerboolean and includeanswerin the corresponding telemetry event. - Add an
aiSetupRunflag to the checklist store state and use it to gate idle-time analytics/ghost-stories scheduling. - Update manager Storybook stories and add unit tests around the new
aiSetupRunstate.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| code/lib/create-storybook/src/initiate.ts | Always writes ai-init-opt-in cache entry (now includes answer) and emits telemetry with answer. |
| code/core/src/shared/utils/ai-checklist-flags.ts | Extends cached flag shape (answer?) and changes hasAiInitOptIn semantics (missing flag => opt-in). |
| code/core/src/shared/checklist-store/index.ts | Adds aiSetupRun?: boolean to checklist store state. |
| code/core/src/manager/settings/GuidePage.stories.tsx | Updates mocked store state to include aiSetupRun. |
| code/core/src/manager/components/sidebar/ChecklistWidget.stories.tsx | Updates mocked store state to include aiSetupRun. |
| code/core/src/core-server/utils/checklist.ts | Switches initialization/gating logic to aiSetupRun for scheduling analytics/ghost stories. |
| code/core/src/core-server/utils/checklist.test.ts | Adds tests intended to cover aiSetupRun behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📝 WalkthroughWalkthroughThis PR adds a persisted aiSetupRun boolean read from the filesystem cache, persists ai-init-opt-in with an explicit answer boolean, initializes aiSetupRun during checklist initialization, and gates idle analytics/ghost-story emissions on aiSetupRun; tests and story fixtures are updated accordingly. ChangesAI Setup Run Tracking and Telemetry
🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
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/utils/checklist.ts (1)
106-147:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSync
aiSetupRunwhen late cache writes are detected.
markAiSetupDone()re-readshasAiSetupRun(configDir!), so it can flipitems.aiSetuptodoneafter startup. Butstore.aiSetupRunis only populated by the one-off read above, so ifai-setup-ranappears later in the session, the timer still exits at Lines 173-175 and ghost stories/final scoring never fire.Please either update
aiSetupRunhere whenever the cache check passes, or re-readhasAiSetupRun(configDir!)in the timer instead of consulting stale store state.Also applies to: 173-175
🤖 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 `@code/core/src/core-server/utils/checklist.ts` around lines 106 - 147, The timer can read a stale store.aiSetupRun because markAiSetupDone only checks hasAiSetupRun but doesn't update store.aiSetupRun; modify markAiSetupDone (or isAiSetupCompleted) so that when hasAiSetupRun(configDir!) returns true you also update the checklist store to set aiSetupRun: true (via store.setState) before returning true, and keep the existing aiSetup status update (aiSetup -> 'done'); alternatively ensure the timer re-reads hasAiSetupRun(configDir!) instead of relying on store.getState().aiSetupRun — pick one approach and apply it consistently to avoid the stale-cache path.
🤖 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 `@code/core/src/core-server/utils/checklist.test.ts`:
- Around line 244-255: The test never sets setupRan so it doesn't exercise the
hasAiSetupRun() path; update the test to set setupRan true when preparing flags
(e.g., call setAiFlags with setupRan: true in addition to optedIn: true) before
invoking initializeChecklist so initializeChecklist (and hasAiSetupRun) sees
setupRan and the expectation on mockStore.getState().aiSetupRun can pass.
In `@code/core/src/core-server/utils/checklist.ts`:
- Around line 81-91: The checklist hydration currently only queries
hasAiSetupRun and sets aiSetupRun, which removes the explicit init opt-out and
breaks idle gating; call hasAiInitOptIn(configDir) alongside hasAiSetupRun and
set store state aiOptIn based on its result (e.g., store.setState(state => ({
...state, aiOptIn: <result> }))), and ensure the idle/short-circuit path in
initializeChecklist checks aiOptIn and immediately short-circuits when it
resolves to false so an explicit opt-out remains distinguishable from opt-in.
In `@code/lib/create-storybook/src/initiate.ts`:
- Around line 157-174: The code currently persists answer: hasAiFeature and
sends telemetry even when the AI prompt was never shown
(selectedFeatures.has(Feature.AI) can be false for both “declined” and “not
shown”); change this to a tri-state flow: derive an explicit aiInitOptIn value
(true when the user explicitly accepted, false only when they explicitly
rejected, and undefined/null when the prompt was not shown or not applicable),
then only call cache.set('ai-init-opt-in', {timestamp, configDir:
resolve(configDir), answer: aiInitOptIn}) and telemetry('ai-init-opt-in',
{answer: aiInitOptIn}) when aiInitOptIn is !== undefined (or otherwise persist a
null deliberate sentinel if you prefer), ensuring you use the existing symbols
(hasAiFeature / selectedFeatures.has(Feature.AI), cache.set, telemetry,
configDir) to locate and update the logic.
---
Outside diff comments:
In `@code/core/src/core-server/utils/checklist.ts`:
- Around line 106-147: The timer can read a stale store.aiSetupRun because
markAiSetupDone only checks hasAiSetupRun but doesn't update store.aiSetupRun;
modify markAiSetupDone (or isAiSetupCompleted) so that when
hasAiSetupRun(configDir!) returns true you also update the checklist store to
set aiSetupRun: true (via store.setState) before returning true, and keep the
existing aiSetup status update (aiSetup -> 'done'); alternatively ensure the
timer re-reads hasAiSetupRun(configDir!) instead of relying on
store.getState().aiSetupRun — pick one approach and apply it consistently to
avoid the stale-cache path.
🪄 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: 3ec4c579-7817-4c00-b050-cc4e3ca4bdc8
📒 Files selected for processing (7)
code/core/src/core-server/utils/checklist.test.tscode/core/src/core-server/utils/checklist.tscode/core/src/manager/components/sidebar/ChecklistWidget.stories.tsxcode/core/src/manager/settings/GuidePage.stories.tsxcode/core/src/shared/checklist-store/index.tscode/core/src/shared/utils/ai-checklist-flags.tscode/lib/create-storybook/src/initiate.ts
Package BenchmarksCommit: No significant changes detected, all good. 👏 |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@code/core/src/core-server/utils/checklist.ts`:
- Around line 85-91: The current one-time call to hasAiSetupRun(...) that sets
aiSetupRun once can miss updates written after startup; change it so the idle
flow re-checks and refreshes aiSetupRun rather than relying solely on initial
hydration: move or duplicate the hasAiSetupRun call into the idle-check path
(the idle handler that currently returns early when aiSetupRun is falsy) so it
invokes hasAiSetupRun() again and calls store.setState(state => ({...state,
aiSetupRun: true})) when it returns true, and also ensure markAiSetupDone()
immediately sets store.setState(... aiSetupRun: true) so the state is updated as
soon as setup completes.
🪄 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: cfeba7eb-6636-4846-bcc9-166ddc04db0f
📒 Files selected for processing (6)
code/core/src/core-server/utils/checklist.test.tscode/core/src/core-server/utils/checklist.tscode/core/src/shared/checklist-store/index.tscode/core/src/shared/utils/ai-checklist-flags.test.tscode/core/src/shared/utils/ai-checklist-flags.tscode/lib/create-storybook/src/initiate.ts
✅ Files skipped from review due to trivial changes (1)
- code/core/src/shared/checklist-store/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- code/lib/create-storybook/src/initiate.ts
- code/core/src/core-server/utils/checklist.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
code/core/src/shared/utils/ai-checklist-flags.test.ts (1)
54-62: ⚡ Quick winAdd an explicit legacy-cache regression for missing
answeron matchingconfigDir.This block currently validates only
answer: true. Please also assert the migration-safe case whereansweris absent butconfigDirmatches, and the function still returnstrue.Proposed test addition
it('returns true when the cached configDir matches the resolved input', async () => { mockCacheStore.set('ai-init-opt-in', { timestamp: Date.now(), configDir: resolve('/repo/apps/web/.storybook'), answer: true, }); const { hasAiInitOptIn } = await import('./ai-checklist-flags.ts'); expect(await hasAiInitOptIn('/repo/apps/web/.storybook')).toBe(true); }); + + it('returns true for legacy matching entries without an explicit answer', async () => { + mockCacheStore.set('ai-init-opt-in', { + timestamp: Date.now(), + configDir: resolve('/repo/apps/web/.storybook'), + }); + const { hasAiInitOptIn } = await import('./ai-checklist-flags.ts'); + expect(await hasAiInitOptIn('/repo/apps/web/.storybook')).toBe(true); + });🤖 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 `@code/core/src/shared/utils/ai-checklist-flags.test.ts` around lines 54 - 62, Add a regression test that ensures hasAiInitOptIn returns true when the cache entry for key 'ai-init-opt-in' has a matching configDir but is missing the answer property: create a mockCacheStore.set('ai-init-opt-in', { timestamp: Date.now(), configDir: resolve('/repo/apps/web/.storybook') }) (omit answer), then import hasAiInitOptIn from './ai-checklist-flags.ts' and assert expect(await hasAiInitOptIn('/repo/apps/web/.storybook')).toBe(true); this verifies the migration-safe behavior in the hasAiInitOptIn logic.
🤖 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.
Nitpick comments:
In `@code/core/src/shared/utils/ai-checklist-flags.test.ts`:
- Around line 54-62: Add a regression test that ensures hasAiInitOptIn returns
true when the cache entry for key 'ai-init-opt-in' has a matching configDir but
is missing the answer property: create a mockCacheStore.set('ai-init-opt-in', {
timestamp: Date.now(), configDir: resolve('/repo/apps/web/.storybook') }) (omit
answer), then import hasAiInitOptIn from './ai-checklist-flags.ts' and assert
expect(await hasAiInitOptIn('/repo/apps/web/.storybook')).toBe(true); this
verifies the migration-safe behavior in the hasAiInitOptIn logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 85100e2f-bb07-4a7d-b957-3deaf93628e6
📒 Files selected for processing (6)
code/core/src/core-server/utils/checklist.test.tscode/core/src/core-server/utils/checklist.tscode/core/src/shared/checklist-store/index.tscode/core/src/shared/utils/ai-checklist-flags.test.tscode/core/src/shared/utils/ai-checklist-flags.tscode/lib/create-storybook/src/initiate.ts
✅ Files skipped from review due to trivial changes (1)
- code/core/src/shared/checklist-store/index.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- code/core/src/core-server/utils/checklist.ts
- code/lib/create-storybook/src/initiate.ts
- code/core/src/shared/utils/ai-checklist-flags.ts
|
Failed to publish canary version of this pull request, triggered by @Sidnioulz. See the failed workflow run at: https://github.com/storybookjs/storybook/actions/runs/25658240290 |
|
Failed to publish canary version of this pull request, triggered by @Sidnioulz. See the failed workflow run at: https://github.com/storybookjs/storybook/actions/runs/25674530305 |
What I did
true)Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
sb ai setupnonethelessDocumentation
MIGRATION.MD
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-34739-sha-c810c6c8. Try it out in a new sandbox by runningnpx storybook@0.0.0-pr-34739-sha-c810c6c8 sandboxor in an existing project withnpx storybook@0.0.0-pr-34739-sha-c810c6c8 upgrade.More information
0.0.0-pr-34739-sha-c810c6c8sidnioulz/ai-init-opt-in-safetyc810c6c81778235817)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=34739Summary by CodeRabbit
New Features
Bug Fixes
Tests