Agentic Setup: Rename 'ai prepare' to 'ai setup' across the codebase#34543
Conversation
|
View your CI Pipeline Execution ↗ for commit ec0fa15
☁️ Nx Cloud last updated this comment at |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughSystematic rename of AI "prepare" identifiers to "ai setup" across the codebase: checklist keys, event names, telemetry functions/types, CLI command and options, prompts, and related tests. Functionality and control flow remain unchanged. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
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/telemetry/ai-setup-utils.test.ts (1)
25-30:⚠️ Potential issue | 🟠 MajorUse
spy: truemock style for this event-cache mock and move behavior setup tobeforeEach.This mock uses a factory pattern with inline behavior outside
beforeEach, violating repository guidelines requiringspy: trueand behavior setup inbeforeEachblocks.♻️ Suggested refactor
-vi.mock('./event-cache.ts', async (importOriginal) => { - const actual = await importOriginal<typeof import('./event-cache.ts')>(); - return { - ...actual, - getAiSetupPending: vi.fn(() => undefined), - }; -}); +vi.mock('./event-cache.ts', { spy: true });beforeEach(() => { vi.resetAllMocks(); vi.mocked(telemetry).mockResolvedValue(undefined); + vi.mocked(getAiSetupPending).mockResolvedValue(undefined); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/telemetry/ai-setup-utils.test.ts` around lines 25 - 30, The inline factory mock for event-cache (vi.mock('./event-cache.ts', async (importOriginal) => { ... getAiSetupPending: vi.fn(() => undefined) })) should be converted to a spy-style mock and the behavior set in beforeEach: change the vi.mock call to use spy: true so the real module is retained and methods can be spied on (reference getAiSetupPending) and remove the inline fn override; then in the test suite's beforeEach call vi.spyOn or replace getAiSetupPending with a mock implementation (e.g., vi.fn()) to return the desired value and reset/restore it in afterEach. Ensure you reference getAiSetupPending when creating the spy in beforeEach so behavior is configured per-test.
🧹 Nitpick comments (2)
code/core/src/manager/components/sidebar/useDelayedAnalyticsTrigger.test.ts (1)
69-83: Make this test set the AI-setup session precondition explicitly.This case currently relies on ambient global state for the
ai-setupgate. SeedSTORYBOOK_LAST_EVENTSandSTORYBOOK_SESSION_IDin-test so it validates the real behavior deterministically.✅ Suggested hardening
it('emits GHOST_STORIES_REQUEST and AI_SETUP_ANALYTICS_REQUEST after delay', () => { const api = createMockApi(); mockUseStorybookApi.mockReturnValue(api as any); + (globalThis as any).STORYBOOK_SESSION_ID = 'session-1'; + (globalThis as any).STORYBOOK_LAST_EVENTS = { + 'ai-setup': { body: { sessionId: 'session-1' } }, + }; renderHook(() => useDelayedAnalyticsTrigger()); api._trigger(PREVIEW_INITIALIZED); expect(api.emit).not.toHaveBeenCalled(); vi.advanceTimersByTime(4 * 60 * 1000); expect(api.emit).toHaveBeenCalledWith(GHOST_STORIES_REQUEST); expect(api.emit).toHaveBeenCalledWith(AI_SETUP_ANALYTICS_REQUEST); expect(api.emit).toHaveBeenCalledTimes(2); + delete (globalThis as any).STORYBOOK_LAST_EVENTS; + delete (globalThis as any).STORYBOOK_SESSION_ID; });As per coding guidelines, “Test real behavior, not just syntax patterns, and use coverage when useful”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/manager/components/sidebar/useDelayedAnalyticsTrigger.test.ts` around lines 69 - 83, The test for useDelayedAnalyticsTrigger relies on ambient ai-setup gate state; explicitly seed the session precondition by setting STORYBOOK_LAST_EVENTS and STORYBOOK_SESSION_ID before rendering the hook so the analytics branch is exercised deterministically. In the test for useDelayedAnalyticsTrigger, call the same storage/mocking mechanism used elsewhere to set STORYBOOK_LAST_EVENTS (include the recent events indicating ai-setup is pending) and STORYBOOK_SESSION_ID (a non-empty session id) prior to renderHook(), then trigger PREVIEW_INITIALIZED and advance timers and assert that api.emit was called with GHOST_STORIES_REQUEST and AI_SETUP_ANALYTICS_REQUEST as before.code/lib/create-storybook/src/commands/FinalizationCommand.ts (1)
92-92: Deduplicate the AI setup prompt literal to avoid message drift.The same sentence is hardcoded twice; extracting one constant here will keep future edits consistent.
♻️ Proposed refactor
import * as find from 'empathic/find'; import { dedent } from 'ts-dedent'; +const AI_SETUP_PROMPT = 'Run `npx storybook ai setup` and follow its instructions precisely.'; + export type FinalizationCommandOptions = { @@ CLI_COLORS.storybook( dedent`Storybook is installed but is not entirely set up yet. - To finish setting up, now run \`npx storybook ai setup\` and follow its instructions precisely.` + To finish setting up, now ${AI_SETUP_PROMPT}` ) ); @@ - ${CLI_COLORS.storybook(`Run \`npx storybook ai setup\` and follow its instructions precisely.`)} + ${CLI_COLORS.storybook(AI_SETUP_PROMPT)} `); } }Also applies to: 114-114
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/lib/create-storybook/src/commands/FinalizationCommand.ts` at line 92, The AI setup instruction string is duplicated in FinalizationCommand.ts; extract the literal into a single constant (e.g., AI_SETUP_PROMPT or SETUP_AI_MESSAGE) near the top of the module and replace both hardcoded occurrences with that constant so future edits stay in sync; update the references in the FinalizationCommand message-building code (where the string appears) and any other uses (notably the second occurrence around the end of the file) to use the new constant.
🤖 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/cli/globalSettings.ts`:
- Line 32: The stored key was renamed to aiSetup but legacy files use
checklist.items.aiPrepare, so add a backward-compatible migration in the loader
in globalSettings.ts that runs before schema parsing: check the loaded raw
settings for checklist.items?.aiPrepare and, if aiSetup is undefined, set
aiSetup = checklist.items.aiPrepare (then delete the legacy
checklist.items.aiPrepare key); ensure this mapping only occurs when aiSetup is
not already present so existing explicit aiSetup values are preserved.
In `@code/core/src/core-server/utils/checklist.ts`:
- Around line 67-74: The code only checks for a cached 'ai-setup' event via
getEventCacheEntry('ai-setup'), which misses users who previously ran the old
'ai-prepare' command; update the logic in the checklist utility to consider both
events (e.g., call getEventCacheEntry for 'ai-setup' OR 'ai-prepare' or check
both results) and if either exists and store.getState().items.aiSetup?.status
=== 'open', call store.setState to set items.aiSetup.status to 'done'
(preserving the existing merge pattern used in the current setState update).
In `@code/core/src/shared/checklist-store/checklistData.tsx`:
- Around line 166-176: The nudge payload ID emitted in the onClick handler is
still using the old 'prepare' identifier, so update the emitted payload to use
the checklist item's new id 'aiSetup'; locate the action.onClick callback that
calls api.emit(AI_PROMPT_NUDGE, { id: 'prepare', origin:
'onboarding-checklist-side' }) and change the id value to 'aiSetup' (keep
AI_PROMPT_NUDGE and origin unchanged) so analytics/handling align with the
renamed aiSetup flow.
In `@code/core/src/telemetry/ai-setup-utils.ts`:
- Around line 13-16: Update the misleading comment block that starts "Determines
whether a story index entry was authored by the `sb ai setup` flow" to reflect
that the implementation currently checks the "ai-generated" tag (not a title
prefix); mention that when migrating to a title-prefix approach the check should
be swapped, and reference the current tag name "ai-generated" so future edits
know which symbol to change (the tag check in this module).
---
Outside diff comments:
In `@code/core/src/telemetry/ai-setup-utils.test.ts`:
- Around line 25-30: The inline factory mock for event-cache
(vi.mock('./event-cache.ts', async (importOriginal) => { ... getAiSetupPending:
vi.fn(() => undefined) })) should be converted to a spy-style mock and the
behavior set in beforeEach: change the vi.mock call to use spy: true so the real
module is retained and methods can be spied on (reference getAiSetupPending) and
remove the inline fn override; then in the test suite's beforeEach call vi.spyOn
or replace getAiSetupPending with a mock implementation (e.g., vi.fn()) to
return the desired value and reset/restore it in afterEach. Ensure you reference
getAiSetupPending when creating the spy in beforeEach so behavior is configured
per-test.
---
Nitpick comments:
In `@code/core/src/manager/components/sidebar/useDelayedAnalyticsTrigger.test.ts`:
- Around line 69-83: The test for useDelayedAnalyticsTrigger relies on ambient
ai-setup gate state; explicitly seed the session precondition by setting
STORYBOOK_LAST_EVENTS and STORYBOOK_SESSION_ID before rendering the hook so the
analytics branch is exercised deterministically. In the test for
useDelayedAnalyticsTrigger, call the same storage/mocking mechanism used
elsewhere to set STORYBOOK_LAST_EVENTS (include the recent events indicating
ai-setup is pending) and STORYBOOK_SESSION_ID (a non-empty session id) prior to
renderHook(), then trigger PREVIEW_INITIALIZED and advance timers and assert
that api.emit was called with GHOST_STORIES_REQUEST and
AI_SETUP_ANALYTICS_REQUEST as before.
In `@code/lib/create-storybook/src/commands/FinalizationCommand.ts`:
- Line 92: The AI setup instruction string is duplicated in
FinalizationCommand.ts; extract the literal into a single constant (e.g.,
AI_SETUP_PROMPT or SETUP_AI_MESSAGE) near the top of the module and replace both
hardcoded occurrences with that constant so future edits stay in sync; update
the references in the FinalizationCommand message-building code (where the
string appears) and any other uses (notably the second occurrence around the end
of the file) to use the new constant.
🪄 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: e3340302-a96d-4d85-b695-791cc5e170d4
📒 Files selected for processing (38)
code/core/src/cli/globalSettings.tscode/core/src/core-events/index.tscode/core/src/core-server/presets/common-preset.tscode/core/src/core-server/server-channel/ai-setup-channel.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.tscode/core/src/core-server/utils/doTelemetry.tscode/core/src/core-server/withTelemetry.test.tscode/core/src/core-server/withTelemetry.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/globals/exports.tscode/core/src/manager/settings/Checklist/AiSetupBlock.stories.tsxcode/core/src/manager/settings/Checklist/AiSetupBlock.tsxcode/core/src/manager/settings/Checklist/Checklist.stories.tsxcode/core/src/manager/settings/GuidePage.stories.tsxcode/core/src/manager/settings/GuidePage.tsxcode/core/src/shared/checklist-store/checklistData.state.tscode/core/src/shared/checklist-store/checklistData.tsxcode/core/src/shared/constants/ai-prompts.tscode/core/src/telemetry/ai-setup-utils.test.tscode/core/src/telemetry/ai-setup-utils.tscode/core/src/telemetry/event-cache.test.tscode/core/src/telemetry/event-cache.tscode/core/src/telemetry/index.tscode/core/src/telemetry/types.tscode/lib/cli-storybook/src/ai/index.tscode/lib/cli-storybook/src/ai/types.tscode/lib/cli-storybook/src/bin/run.tscode/lib/create-storybook/src/commands/FinalizationCommand.test.tscode/lib/create-storybook/src/commands/FinalizationCommand.tscode/lib/create-storybook/src/commands/UserPreferencesCommand.test.tscode/lib/create-storybook/src/commands/UserPreferencesCommand.tscode/lib/create-storybook/src/initiate.tscode/lib/create-storybook/src/services/FeatureCompatibilityService.test.tscode/lib/create-storybook/src/services/FeatureCompatibilityService.ts
| id: 'aiSetup', | ||
| label: 'Set up with AI', | ||
| icon: WandIcon, | ||
| criteria: 'ai prepare command has been run', | ||
| criteria: 'ai setup command has been run', | ||
| showOnGuidePage: false, | ||
| action: { | ||
| label: 'Copy prompt', | ||
| copyContent: AI_PREPARE_PROMPT, | ||
| copyContent: AI_SETUP_PROMPT, | ||
| onClick: ({ api }) => { | ||
| api.emit(AI_PROMPT_NUDGE, { id: 'prepare', origin: 'onboarding-checklist-side' }); | ||
| }, |
There was a problem hiding this comment.
Update the nudge payload ID to match the renamed setup flow.
The checklist item is now aiSetup, but the emitted nudge still uses id: 'prepare'. That leaves this path on the old identifier and can break setup-specific handling/analytics.
🔧 Proposed fix
action: {
label: 'Copy prompt',
copyContent: AI_SETUP_PROMPT,
onClick: ({ api }) => {
- api.emit(AI_PROMPT_NUDGE, { id: 'prepare', origin: 'onboarding-checklist-side' });
+ api.emit(AI_PROMPT_NUDGE, { id: 'setup', origin: 'onboarding-checklist-side' });
},
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| id: 'aiSetup', | |
| label: 'Set up with AI', | |
| icon: WandIcon, | |
| criteria: 'ai prepare command has been run', | |
| criteria: 'ai setup command has been run', | |
| showOnGuidePage: false, | |
| action: { | |
| label: 'Copy prompt', | |
| copyContent: AI_PREPARE_PROMPT, | |
| copyContent: AI_SETUP_PROMPT, | |
| onClick: ({ api }) => { | |
| api.emit(AI_PROMPT_NUDGE, { id: 'prepare', origin: 'onboarding-checklist-side' }); | |
| }, | |
| id: 'aiSetup', | |
| label: 'Set up with AI', | |
| icon: WandIcon, | |
| criteria: 'ai setup command has been run', | |
| showOnGuidePage: false, | |
| action: { | |
| label: 'Copy prompt', | |
| copyContent: AI_SETUP_PROMPT, | |
| onClick: ({ api }) => { | |
| api.emit(AI_PROMPT_NUDGE, { id: 'setup', origin: 'onboarding-checklist-side' }); | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/core/src/shared/checklist-store/checklistData.tsx` around lines 166 -
176, The nudge payload ID emitted in the onClick handler is still using the old
'prepare' identifier, so update the emitted payload to use the checklist item's
new id 'aiSetup'; locate the action.onClick callback that calls
api.emit(AI_PROMPT_NUDGE, { id: 'prepare', origin: 'onboarding-checklist-side'
}) and change the id value to 'aiSetup' (keep AI_PROMPT_NUDGE and origin
unchanged) so analytics/handling align with the renamed aiSetup flow.
| * Determines whether a story index entry was authored by the `sb ai setup` flow. | ||
| * Currently checks title prefix. When we migrate to a tag-based approach, | ||
| * swap this to check for the tag instead — this is the single swap point. | ||
| */ |
There was a problem hiding this comment.
Comment contradicts current implementation.
The comment says this checks title prefix, but Line 18 checks the ai-generated tag. Please update the comment to avoid misleading future edits.
✏️ Suggested doc fix
/**
* Determines whether a story index entry was authored by the `sb ai setup` flow.
- * Currently checks title prefix. When we migrate to a tag-based approach,
- * swap this to check for the tag instead — this is the single swap point.
+ * Currently checks for the `ai-generated` tag.
*/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * Determines whether a story index entry was authored by the `sb ai setup` flow. | |
| * Currently checks title prefix. When we migrate to a tag-based approach, | |
| * swap this to check for the tag instead — this is the single swap point. | |
| */ | |
| /** | |
| * Determines whether a story index entry was authored by the `sb ai setup` flow. | |
| * Currently checks for the `ai-generated` tag. | |
| */ |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/core/src/telemetry/ai-setup-utils.ts` around lines 13 - 16, Update the
misleading comment block that starts "Determines whether a story index entry was
authored by the `sb ai setup` flow" to reflect that the implementation currently
checks the "ai-generated" tag (not a title prefix); mention that when migrating
to a title-prefix approach the check should be swapped, and reference the
current tag name "ai-generated" so future edits know which symbol to change (the
tag check in this module).
Manual testing
N/A 🐸
Summary by CodeRabbit