UI: Conditionally display AI copy prompt#34555
Merged
Merged
Conversation
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:
📝 WalkthroughWalkthroughAnalytics and ghost-story emission logic is migrated from manager-side delayed hook to server-side checklist utility with idle-timer coordination. Server now tracks AI opt-in status, debounces analytics via idle window after story-index invalidation, and controls ghost-story eligibility without session-id gating. Init-time telemetry records AI feature selection. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Server as Server<br/>(checklist init)
participant Cache as Event Cache
participant TestProvider as Universal Test<br/>Provider
participant Analytics as Analytics/<br/>Ghost Stories
rect rgba(100, 150, 200, 0.5)
Note over Server,Cache: Startup Phase
Server->>Cache: Check ai-setup event
alt ai-setup exists
Server->>Server: Schedule idle timer<br/>(4 min)
end
Server->>Cache: Check ai-init-opt-in event
Server->>Server: Mark checklist loaded
end
rect rgba(150, 100, 200, 0.5)
Note over Server,TestProvider: Idle Window Coordination
Server->>TestProvider: Register onStateChange listener
loop On state change or STORY_INDEX_INVALIDATED
Server->>Server: Reset idle timer
end
end
rect rgba(100, 200, 100, 0.5)
Note over Server,Analytics: Emission on Idle
Server->>Server: Idle window elapsed
alt aiOptIn && ai-setup exists && not emitted
Server->>Analytics: Emit GHOST_STORIES_REQUEST
Server->>Analytics: Emit AI_SETUP_ANALYTICS_REQUEST
Server->>Server: Mark analyticsEmitted = true
Server->>TestProvider: Unregister listener
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 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: 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 `@code/core/src/core-server/utils/checklist.ts`:
- Around line 64-68: The aiSetup update only flips status when
merged.items.aiSetup?.status === 'open', which leaves statuses like 'skipped' or
'accepted' unchanged; change the condition to overwrite any non-'done' state
when aiSetupEvent exists (e.g., if (aiSetupEvent && merged.items.aiSetup?.status
!== 'done') or simply if (aiSetupEvent) ) so that merged.items.aiSetup is
assigned { ...merged.items.aiSetup, status: 'done' } whenever the ai-setup event
ran; update the block referencing aiSetupEvent and merged.items.aiSetup
accordingly.
- Around line 30-60: The Promise.all includes getEventCacheEntry('ai-setup') so
a failure in that optional cache read aborts initializeChecklist(); fix by
isolating that call: remove getEventCacheEntry('ai-setup') from the Promise.all
and instead await it separately with its own try/catch (or use .catch(() =>
undefined)) so aiSetupEvent becomes a best-effort value without preventing the
main await for globalSettings/cache from completing; update references to
aiSetupEvent accordingly in initializeChecklist().
In `@code/core/src/manager/components/sidebar/useDelayedAnalyticsTrigger.test.ts`:
- Around line 43-55: The test setup in beforeEach relies on both sides of the
session check being undefined, which is brittle; set an explicit session id
variable and assign it to global.STORYBOOK_SESSION_ID and to the seeded ai-setup
event (global.STORYBOOK_LAST_EVENTS['ai-setup'].body.sessionId) so the
same-session fixture is deterministic; update beforeEach to create a fixed
sessionId (e.g., const sessionId = 'test-session') and use that value in both
global.STORYBOOK_SESSION_ID and the ai-setup body, and ensure afterEach clears
global.STORYBOOK_SESSION_ID when deleting STORYBOOK_LAST_EVENTS.
🪄 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: f82fb85c-2f8e-4f33-bebf-ef05e4b6204f
📒 Files selected for processing (6)
code/core/src/core-server/utils/checklist.tscode/core/src/manager/components/sidebar/useDelayedAnalyticsTrigger.test.tscode/core/src/manager/components/sidebar/useDelayedAnalyticsTrigger.tscode/core/src/shared/checklist-store/checklistData.tsxcode/core/src/telemetry/types.tscode/lib/create-storybook/src/initiate.ts
|
View your CI Pipeline Execution ↗ for commit f2b55f5
☁️ Nx Cloud last updated this comment at |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
code/core/src/core-server/utils/checklist.test.ts (1)
97-99: Keep the event-cache mock behavior inbeforeEach.This case is useful, but it adds another inline mock implementation inside a test. A shared
mockImplementationinbeforeEachwith per-test inputs keeps the file aligned with the repo’s Vitest pattern.♻️ Suggested pattern
+ let eventCacheEntry: CacheEntry | undefined; + let eventCacheError: Error | undefined; + beforeEach(async () => { + eventCacheEntry = undefined; + eventCacheError = undefined; + const { get: getEventCacheEntry } = await import('../../telemetry/event-cache.ts'); + vi.mocked(getEventCacheEntry).mockImplementation(async () => { + if (eventCacheError) { + throw eventCacheError; + } + return eventCacheEntry; + }); + mockStore = MockUniversalStore.create<StoreState, StoreEvent>( UNIVERSAL_CHECKLIST_STORE_OPTIONS, vi ); }); @@ - const { get: getEventCacheEntry } = await import('../../telemetry/event-cache.ts'); - vi.mocked(getEventCacheEntry).mockRejectedValue(new Error('cache read failed')); + eventCacheError = new Error('cache read failed');As per coding guidelines "Implement mock behaviors in
beforeEachblocks in Vitest tests" and "Avoid inline mock implementations within test cases in Vitest tests".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/core-server/utils/checklist.test.ts` around lines 97 - 99, Move the inline mock for the event cache out of the individual test and into the suite-level beforeEach: set a default mock behavior for the imported get function (getEventCacheEntry) using vi.mocked(getEventCacheEntry).mockImplementation(...) or .mockResolvedValue(...) in beforeEach, and then override per-test with mockRejectedValueOnce / mockResolvedValueOnce inside the specific "still initializes when reading ai-setup from the event cache fails" test if needed; update references to getEventCacheEntry in that test to rely on the beforeEach default and only apply one-off .mockRejectedValueOnce for this failure case.
🤖 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/lib/create-storybook/src/services/TelemetryService.test.ts`:
- Around line 165-179: Move all inline mockReturnValue/mock implementations for
getProcessAncestry out of individual it(...) blocks into beforeEach hooks in
TelemetryService.test.ts; specifically, in tests that call
TelemetryService.trackInitWithContext (and other tests referencing
getProcessAncestry), set vi.mocked(getProcessAncestry).mockReturnValue(...)
inside a beforeEach (or additional contextual beforeEach blocks) so each test's
mock behavior is prepared before the test runs, and remove the inline
vi.mocked(getProcessAncestry).mockReturnValue calls from the it(...) cases
(e.g., the test using TelemetryService and Feature.AI).
---
Nitpick comments:
In `@code/core/src/core-server/utils/checklist.test.ts`:
- Around line 97-99: Move the inline mock for the event cache out of the
individual test and into the suite-level beforeEach: set a default mock behavior
for the imported get function (getEventCacheEntry) using
vi.mocked(getEventCacheEntry).mockImplementation(...) or .mockResolvedValue(...)
in beforeEach, and then override per-test with mockRejectedValueOnce /
mockResolvedValueOnce inside the specific "still initializes when reading
ai-setup from the event cache fails" test if needed; update references to
getEventCacheEntry in that test to rely on the beforeEach default and only apply
one-off .mockRejectedValueOnce for this failure case.
🪄 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: 1b3fee04-b1f2-421c-b827-a863d379f5a0
📒 Files selected for processing (11)
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/components/sidebar/useDelayedAnalyticsTrigger.test.tscode/core/src/manager/components/sidebar/useDelayedAnalyticsTrigger.tscode/core/src/manager/settings/GuidePage.stories.tsxcode/core/src/shared/checklist-store/checklistData.tsxcode/core/src/telemetry/types.tscode/lib/create-storybook/src/initiate.tscode/lib/create-storybook/src/services/TelemetryService.test.tscode/lib/create-storybook/src/services/TelemetryService.ts
✅ Files skipped from review due to trivial changes (1)
- code/lib/create-storybook/src/initiate.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- code/core/src/telemetry/types.ts
- code/core/src/manager/components/sidebar/useDelayedAnalyticsTrigger.ts
- code/core/src/core-server/utils/checklist.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
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.test.ts (1)
101-106:⚠️ Potential issue | 🟡 MinorMock
getEventCacheEntry()by event type in these scenarios.
initializeChecklist()now performs two separate cache lookups ('ai-init-opt-in'and'ai-setup'), but these tests stub a single resolved value for every call. That means the “ai-setup exists” cases also implicitly make opt-in exist, so they aren’t actually isolating the conditions named in the test titles.🧪 Suggested pattern
- vi.mocked(getEventCacheEntry).mockResolvedValue({ - timestamp: Date.now(), - body: { eventType: 'ai-setup' } as TelemetryEvent, - } satisfies CacheEntry); + const aiSetupEntry = { + timestamp: Date.now(), + body: { eventType: 'ai-setup' } as TelemetryEvent, + } satisfies CacheEntry; + + vi.mocked(getEventCacheEntry).mockImplementation(async (eventType) => + eventType === 'ai-setup' ? aiSetupEntry : undefined + );As per coding guidelines "Implement mock behaviors in
beforeEachblocks in Vitest tests" and "Avoid inline mock implementations within test cases in Vitest tests".Also applies to: 172-175, 191-194, 212-215, 255-258, 281-284
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/core-server/utils/checklist.test.ts` around lines 101 - 106, Tests currently stub getEventCacheEntry with a single resolved value for every call, which falsely makes both 'ai-init-opt-in' and 'ai-setup' appear present; update the tests to mock getEventCacheEntry by inspecting the incoming event type so each call returns the proper CacheEntry (or null) for 'ai-init-opt-in' vs 'ai-setup' to match the scenario being tested. Move this mock setup into a shared beforeEach and use vi.mocked(getEventCacheEntry).mockImplementation(async (eventType) => { return eventType === 'ai-setup' ? { timestamp: ..., body: { eventType: 'ai-setup' } } : null }) (and analogous variants for other test cases) so initializeChecklist sees only the intended cache entries. Ensure references to initializeChecklist and getEventCacheEntry from event-cache.ts are used to locate the code to change.
🤖 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/utils/checklist.ts`:
- Around line 86-99: The markAiSetupDone function currently returns false when
the ai-setup event exists but store.getState().items.aiSetup?.status === 'done';
change the logic in markAiSetupDone (which calls getEventCacheEntry and reads
store.getState().items.aiSetup?.status) to first check for the aiSetupEvent and,
if it exists, return true regardless of the current store status; only call
store.setState to set status: 'done' when the status is not already 'done'. This
ensures markAiSetupDone returns success when the event is present while avoiding
redundant state writes.
In `@code/core/src/manager/components/sidebar/useChecklist.ts`:
- Around line 149-151: The memo that builds allItems is missing the AI opt-in
flag so checkAvailable (which reads storeState.aiOptIn) can be stale; update the
dependency list for the useMemo that produces allItems to include
checklistState.aiOptIn (or storeState.aiOptIn) so the isAvailable calculation
(which calls checkAvailable) re-runs when AI opt-in changes, ensuring AI
checklist items appear immediately when the server flips aiOptIn.
---
Outside diff comments:
In `@code/core/src/core-server/utils/checklist.test.ts`:
- Around line 101-106: Tests currently stub getEventCacheEntry with a single
resolved value for every call, which falsely makes both 'ai-init-opt-in' and
'ai-setup' appear present; update the tests to mock getEventCacheEntry by
inspecting the incoming event type so each call returns the proper CacheEntry
(or null) for 'ai-init-opt-in' vs 'ai-setup' to match the scenario being tested.
Move this mock setup into a shared beforeEach and use
vi.mocked(getEventCacheEntry).mockImplementation(async (eventType) => { return
eventType === 'ai-setup' ? { timestamp: ..., body: { eventType: 'ai-setup' } } :
null }) (and analogous variants for other test cases) so initializeChecklist
sees only the intended cache entries. Ensure references to initializeChecklist
and getEventCacheEntry from event-cache.ts are used to locate the code to
change.
🪄 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: c6ec0296-6993-45a8-8ab5-710e2946983c
📒 Files selected for processing (14)
code/core/src/builder-manager/index.tscode/core/src/core-server/build-dev.onboarding.test.tscode/core/src/core-server/presets/common-preset.tscode/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/components/sidebar/Sidebar.tsxcode/core/src/manager/components/sidebar/useChecklist.tscode/core/src/manager/components/sidebar/useDelayedAnalyticsTrigger.test.tscode/core/src/manager/components/sidebar/useDelayedAnalyticsTrigger.tscode/core/src/manager/settings/GuidePage.stories.tsxcode/core/src/shared/checklist-store/checklistData.tsxcode/core/src/shared/checklist-store/index.tscode/lib/create-storybook/src/initiate.ts
💤 Files with no reviewable changes (4)
- code/core/src/manager/components/sidebar/Sidebar.tsx
- code/core/src/builder-manager/index.ts
- code/core/src/manager/components/sidebar/useDelayedAnalyticsTrigger.ts
- code/core/src/manager/components/sidebar/useDelayedAnalyticsTrigger.test.ts
✅ Files skipped from review due to trivial changes (3)
- code/core/src/core-server/build-dev.onboarding.test.ts
- code/core/src/shared/checklist-store/index.ts
- code/lib/create-storybook/src/initiate.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- code/core/src/manager/settings/GuidePage.stories.tsx
- code/core/src/shared/checklist-store/checklistData.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
code/core/src/core-server/utils/checklist.ts (1)
87-100:⚠️ Potential issue | 🟠 MajorReturn success when
ai-setupalready exists, even if the store is alreadydone.
markAiSetupDone()currently returnsfalsewhen the cachedai-setupevent exists butstore.getState().items.aiSetup?.statusis already'done'. That breaks the startup path at Lines 143-149: after a restart, the checklist can load from persisted project state withaiSetup: done, but the idle scheduler never starts, soGHOST_STORIES_REQUEST/AI_SETUP_ANALYTICS_REQUESTare dropped unless another invalidation or test-provider change happens.🛠️ Suggested fix
const markAiSetupDone = async () => { try { const aiSetupEvent = await getEventCacheEntry('ai-setup'); - if (!aiSetupEvent || store.getState().items.aiSetup?.status === 'done') { + if (!aiSetupEvent) { return false; } - store.setState((state) => ({ - ...state, - items: { - ...state.items, - aiSetup: { ...state.items.aiSetup, status: 'done' }, - }, - })); + if (store.getState().items.aiSetup?.status !== 'done') { + store.setState((state) => ({ + ...state, + items: { + ...state.items, + aiSetup: { ...state.items.aiSetup, status: 'done' }, + }, + })); + } return true; } catch { return false; } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/core-server/utils/checklist.ts` around lines 87 - 100, markAiSetupDone currently returns false when the cached 'ai-setup' event exists but store.getState().items.aiSetup?.status === 'done', which prevents downstream startup actions; update markAiSetupDone to return true when aiSetupEvent is present even if the store already reports status 'done' (only return false when aiSetupEvent is missing), and keep the existing state update logic (store.setState and return true) when the store isn't done; locate and change this logic in the markAiSetupDone function that calls getEventCacheEntry('ai-setup') and references store.getState().items.aiSetup?.status.
🧹 Nitpick comments (1)
code/core/src/core-server/utils/checklist.test.ts (1)
44-57: Move the newuniversalTestProviderStoremock behavior out of the top-level factory.This adds a custom
onStateChangeimplementation directly insidevi.mock(...), so the listener behavior is defined at module-load time instead of per test. Please keep the mock declaration at the top, but set the implementation inbeforeEachviavi.mocked(...)so it follows the repo's Vitest mocking rules.As per coding guidelines, "Use
vi.mock()with thespy: trueoption for all package and file mocks in Vitest tests" and "Implement mock behaviors inbeforeEachblocks in Vitest tests".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/core-server/utils/checklist.test.ts` around lines 44 - 57, The current mock defines a custom onStateChange implementation at module-load inside vi.mock which makes listener behavior global; move that behavior into beforeEach by keeping the vi.mock declaration top-level but replacing the inline implementation with a spy (spy: true) and then in beforeEach call vi.mocked(universalTestProviderStore).mockImplementation or mockReturnValue for onStateChange to push/remove listeners into testProviderStateChangeListeners; update references to universalTestProviderStore and the onStateChange mock via vi.mocked(...) so each test gets a fresh listener setup and teardown.
🤖 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/server-channel/ai-setup-channel.ts`:
- Around line 37-38: Remove the raw console.log call in ai-setup-channel.ts and
route the message through the Storybook server logger: replace the
console.log('AI ANALYTICS!', { lastAISetup, lastSetupStoryScoringRun }) call
with logger.debug(...) using the Storybook node logger (import from
'storybook/internal/node-logger' if not already imported) and include the same
contextual object (lastAISetup, lastSetupStoryScoringRun) in the debug call for
tracing.
In `@code/core/src/core-server/server-channel/ghost-stories-channel.test.ts`:
- Around line 361-365: The test documents that session matching was removed but
doesn't assert it: add an explicit assertion that mockTelemetry.getSessionId was
not called to ensure initGhostStoriesChannel() doesn't start consulting session
id again; locate the test in ghost-stories-channel.test.ts near the existing
expectations for mockTelemetry.getLastEvents and
mockTelemetry.getStorybookMetadata and add
expect(mockTelemetry.getSessionId).not.toHaveBeenCalled() alongside those checks
so the CI will fail if getSessionId() is invoked.
In `@code/core/src/core-server/server-channel/ghost-stories-channel.ts`:
- Around line 37-42: Remove the debug console.log in ghost-stories-channel.ts
and replace it with the Storybook server logger: import the node logger (e.g.
import { logger } from 'storybook/internal/node-logger') and call
logger.debug(...) with the same payload (lastInit, lastAISetup,
lastGhostStoriesRun, lastRelevantEvent) instead of console.log so normal server
flows don't spam stdout; ensure the import is added at top and the console.log
line is deleted.
In `@code/core/src/core-server/utils/checklist.ts`:
- Around line 118-141: Remove the two raw console.log calls inside
scheduleIdleCheck (the ones logging 'SCHEDULE IDLE CHECK!' and 'MARKING SETUP AS
DONE!') and replace them with the Storybook server logger from
'storybook/internal/node-logger' (e.g., import nodeLogger and call
nodeLogger.debug or nodeLogger.info). Update the top of the file to import the
logger, then change the console.log usages in the scheduleIdleCheck function
that surrounds analyticsTimer, markAiSetupDone(), and AI_IDLE_DELAY_MS to use
the logger instead so server-side tracing follows project logging guidelines.
---
Duplicate comments:
In `@code/core/src/core-server/utils/checklist.ts`:
- Around line 87-100: markAiSetupDone currently returns false when the cached
'ai-setup' event exists but store.getState().items.aiSetup?.status === 'done',
which prevents downstream startup actions; update markAiSetupDone to return true
when aiSetupEvent is present even if the store already reports status 'done'
(only return false when aiSetupEvent is missing), and keep the existing state
update logic (store.setState and return true) when the store isn't done; locate
and change this logic in the markAiSetupDone function that calls
getEventCacheEntry('ai-setup') and references
store.getState().items.aiSetup?.status.
---
Nitpick comments:
In `@code/core/src/core-server/utils/checklist.test.ts`:
- Around line 44-57: The current mock defines a custom onStateChange
implementation at module-load inside vi.mock which makes listener behavior
global; move that behavior into beforeEach by keeping the vi.mock declaration
top-level but replacing the inline implementation with a spy (spy: true) and
then in beforeEach call vi.mocked(universalTestProviderStore).mockImplementation
or mockReturnValue for onStateChange to push/remove listeners into
testProviderStateChangeListeners; update references to
universalTestProviderStore and the onStateChange mock via vi.mocked(...) so each
test gets a fresh listener setup and teardown.
🪄 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: d661eed2-1a5f-46a9-b1df-ec2b776575b8
📒 Files selected for processing (5)
code/core/src/core-server/server-channel/ai-setup-channel.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/utils/checklist.test.tscode/core/src/core-server/utils/checklist.ts
yannbf
force-pushed
the
yann/conditional-ai-feature
branch
from
017cf67 to
21be9dc
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
code/core/src/core-server/utils/checklist.ts (1)
87-92:⚠️ Potential issue | 🟠 MajorReturn success whenever the
ai-setupevent exists.The boolean returned here drives the restart path at Lines 142-147. Returning
falsewhen the cache entry exists butitems.aiSetupis already'done'skips the idle reschedule after a restart, so ghost-story / analytics emission can still be lost until another invalidation arrives.🛠️ Proposed fix
const markAiSetupDone = async () => { try { const aiSetupEvent = await getEventCacheEntry('ai-setup'); - if (!aiSetupEvent || store.getState().items.aiSetup?.status === 'done') { + if (!aiSetupEvent) { return false; } - store.setState((state) => ({ - ...state, - items: { - ...state.items, - aiSetup: { ...state.items.aiSetup, status: 'done' }, - }, - })); + if (store.getState().items.aiSetup?.status !== 'done') { + store.setState((state) => ({ + ...state, + items: { + ...state.items, + aiSetup: { ...state.items.aiSetup, status: 'done' }, + }, + })); + } return true; } catch { return false; } };Also applies to: 141-147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/core-server/utils/checklist.ts` around lines 87 - 92, The current logic in markAiSetupDone uses getEventCacheEntry('ai-setup') and then returns false if the cache entry exists but store.getState().items.aiSetup?.status === 'done', which prevents signaling success after a restart; change the flow so that if aiSetupEvent is missing you return false, but if aiSetupEvent exists you return true even when items.aiSetup.status === 'done' (i.e., only bail out when the cache entry is absent), and apply the same adjustment to the equivalent block handling the restart/reschedule path referenced around the other ai-setup handling so both places consistently return success when the cache entry exists.
🤖 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/utils/checklist.ts`:
- Around line 118-138: The code only calls markAiSetupDone() inside the delayed
analyticsTimer callback, which means items.aiSetup can remain stale for the full
idle delay; change scheduleIdleCheck (and the analogous block around lines
154-173) to call markAiSetupDone() immediately (fire-and-forget) when
scheduleIdleCheck runs or when the cache entry appears so the store's
items.aiSetup is synced right away, but keep the existing setTimeout debounce
for emitting GHOST_STORIES_REQUEST and AI_SETUP_ANALYTICS_REQUEST using
analyticsTimer/AI_IDLE_DELAY_MS and the analyticsEmitted/channel logic;
reference functions/variables: scheduleIdleCheck, markAiSetupDone,
analyticsTimer, analyticsEmitted, channel.emit(GHOST_STORIES_REQUEST) and
channel.emit(AI_SETUP_ANALYTICS_REQUEST).
---
Duplicate comments:
In `@code/core/src/core-server/utils/checklist.ts`:
- Around line 87-92: The current logic in markAiSetupDone uses
getEventCacheEntry('ai-setup') and then returns false if the cache entry exists
but store.getState().items.aiSetup?.status === 'done', which prevents signaling
success after a restart; change the flow so that if aiSetupEvent is missing you
return false, but if aiSetupEvent exists you return true even when
items.aiSetup.status === 'done' (i.e., only bail out when the cache entry is
absent), and apply the same adjustment to the equivalent block handling the
restart/reschedule path referenced around the other ai-setup handling so both
places consistently return success when the cache entry exists.
🪄 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: 028a81c9-9ec5-48fc-b8a6-2b2e330b3498
📒 Files selected for processing (4)
code/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/utils/checklist.test.tscode/core/src/core-server/utils/checklist.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
- code/core/src/core-server/server-channel/ghost-stories-channel.ts
- code/core/src/core-server/utils/checklist.test.ts
kasperpeulen
approved these changes
Apr 17, 2026
This was referenced May 1, 2026
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
2 participants
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.
Closes #
What I did
Only display the AI features (copy prompt button and ai stories tests) in Storybook UI if:
storybook ai setupcommandChecklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Caution
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
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-34555-sha-822568b5. Try it out in a new sandbox by runningnpx storybook@0.0.0-pr-34555-sha-822568b5 sandboxor in an existing project withnpx storybook@0.0.0-pr-34555-sha-822568b5 upgrade.More information
0.0.0-pr-34555-sha-822568b5yann/conditional-ai-feature822568b51776441911)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=34555Summary by CodeRabbit
New Features
Bug Fixes
Chores