Agentic Setup: Remove redundant ai-setup-evidence event#34698
Merged
Conversation
Contributor
|
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 (10)
💤 Files with no reviewable changes (5)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAI evidence-collection and its pending-cache/snapshot plumbing were removed across telemetry, server, and CLI code. Story authorship detection simplified to an ChangesAI Setup Evidence Removal
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (ai/index.ts)
participant Server as Core Server (withTelemetry / doTelemetry)
participant Telemetry as Telemetry Module
participant Cache as Event Cache
CLI->>Telemetry: telemetry('ai-setup', payload)
CLI-->>CLI: write output / log
Server->>Telemetry: telemetry('boot', payload)
Telemetry->>Cache: getLastEvents / getPrecedingUpgrade
Note right of Cache: ai-setup-pending API removed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
Review rate limit: 4/5 reviews remaining, refill in 12 minutes. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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.ts (1)
3-9:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStale JSDoc comment contradicts the implementation.
The comment states "Currently checks title prefix" but the implementation checks for the
ai-generatedtag. The comment should be updated to reflect the actual tag-based detection.📝 Proposed 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. + * Determines whether a story index entry was authored by the `sb ai setup` flow + * by checking for the `ai-generated` tag. */ export function isStoryCreatedByAISetup(entry: IndexEntry): boolean { return entry.type === 'story' && (entry.tags?.includes('ai-generated') ?? false); }🤖 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 3 - 9, The JSDoc for isStoryCreatedByAISetup is stale (mentions "checks title prefix") while the function actually checks for the 'ai-generated' tag; update the comment to describe the tag-based detection (e.g., "Determines whether a story index entry was authored by the `sb ai setup` flow by checking the 'ai-generated' tag. Swap to tag check here when migrating from title-based detection.") and remove any mention of title-prefix behavior so the doc matches the implementation in isStoryCreatedByAISetup (and keep IndexEntry referenced if needed).
🧹 Nitpick comments (1)
code/core/src/telemetry/ai-setup-utils.test.ts (1)
7-55: ⚡ Quick winRemove unused mock infrastructure.
These mocks and setup are leftover from the removed test suites and are not used by the current
isStoryCreatedByAISetuptests:
findConfigFilemock (lines 8-14)detectAgentmock (lines 16-18)getAiSetupPendingmock (lines 20-26)telemetrymock (lines 28-34) and its import (line 37)readFilemock (lines 39-45)- The entire
beforeEachblock (lines 47-55)The
isStoryCreatedByAISetupfunction has no external dependencies requiring mocks.♻️ Proposed cleanup
import { beforeEach, describe, expect, it, vi } from 'vitest'; import type { IndexEntry, StoryIndex } from 'storybook/internal/types'; import { isStoryCreatedByAISetup } from './ai-setup-utils.ts'; -// Mock modules with spy pattern -vi.mock('storybook/internal/common', async (importOriginal) => { - const actual = await importOriginal<typeof import('storybook/internal/common')>(); - return { - ...actual, - findConfigFile: vi.fn(), - }; -}); - -vi.mock('./detect-agent.ts', () => ({ - detectAgent: vi.fn(() => undefined), -})); - -vi.mock('./event-cache.ts', async (importOriginal) => { - const actual = await importOriginal<typeof import('./event-cache.ts')>(); - return { - ...actual, - getAiSetupPending: vi.fn(() => undefined), - }; -}); - -vi.mock('./index.ts', async (importOriginal) => { - const actual = await importOriginal<typeof import('./index.ts')>(); - return { - ...actual, - telemetry: vi.fn(), - }; -}); - -// Import mocked modules for spy access -import { telemetry } from './index.ts'; - -vi.mock('node:fs/promises', async (importOriginal) => { - const actual = await importOriginal<typeof import('node:fs/promises')>(); - return { - ...actual, - readFile: vi.fn(), - }; -}); - -beforeEach(() => { - vi.resetAllMocks(); - vi.mocked(telemetry).mockImplementation(async (_eventType, payloadOrFactory) => { - if (typeof payloadOrFactory === 'function') { - return payloadOrFactory(); - } - return payloadOrFactory; - }); -}); - describe('isStoryCreatedByAISetup', () => {🤖 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 7 - 55, Remove leftover mock infrastructure that is unused by isStoryCreatedByAISetup: delete the vi.mock of 'storybook/internal/common' that creates findConfigFile, the vi.mock of './detect-agent.ts' that mocks detectAgent, the vi.mock of './event-cache.ts' that mocks getAiSetupPending, the vi.mock of './index.ts' that mocks telemetry and the import of telemetry, the vi.mock of 'node:fs/promises' that mocks readFile, and the entire beforeEach block that resets mocks and re-implements telemetry; ensure only tests exercising isStoryCreatedByAISetup remain and no external mocks or telemetry import/reference are left in the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@code/core/src/telemetry/ai-setup-utils.ts`:
- Around line 3-9: The JSDoc for isStoryCreatedByAISetup is stale (mentions
"checks title prefix") while the function actually checks for the 'ai-generated'
tag; update the comment to describe the tag-based detection (e.g., "Determines
whether a story index entry was authored by the `sb ai setup` flow by checking
the 'ai-generated' tag. Swap to tag check here when migrating from title-based
detection.") and remove any mention of title-prefix behavior so the doc matches
the implementation in isStoryCreatedByAISetup (and keep IndexEntry referenced if
needed).
---
Nitpick comments:
In `@code/core/src/telemetry/ai-setup-utils.test.ts`:
- Around line 7-55: Remove leftover mock infrastructure that is unused by
isStoryCreatedByAISetup: delete the vi.mock of 'storybook/internal/common' that
creates findConfigFile, the vi.mock of './detect-agent.ts' that mocks
detectAgent, the vi.mock of './event-cache.ts' that mocks getAiSetupPending, the
vi.mock of './index.ts' that mocks telemetry and the import of telemetry, the
vi.mock of 'node:fs/promises' that mocks readFile, and the entire beforeEach
block that resets mocks and re-implements telemetry; ensure only tests
exercising isStoryCreatedByAISetup remain and no external mocks or telemetry
import/reference are left in the file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ea917294-913f-42c9-a5ea-9a7a6d92c3df
📒 Files selected for processing (10)
code/core/src/core-server/utils/doTelemetry.tscode/core/src/core-server/withTelemetry.test.tscode/core/src/core-server/withTelemetry.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/lib/cli-storybook/src/ai/index.test.tscode/lib/cli-storybook/src/ai/index.ts
💤 Files with no reviewable changes (5)
- code/lib/cli-storybook/src/ai/index.test.ts
- code/core/src/core-server/withTelemetry.ts
- code/core/src/telemetry/index.ts
- code/core/src/core-server/withTelemetry.test.ts
- code/core/src/telemetry/event-cache.ts
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the legacy “AI setup evidence” telemetry mechanism (ai-setup-evidence) and its supporting pending-record cache/snapshot logic, relying instead on Vitest scoring/reporting events for agentic setup outcomes.
Changes:
- Remove
ai-setup-pendingcache writes and preview snapshotting fromsb ai setup. - Delete the telemetry cache utilities/types used to read/flush pending AI setup records and remove related tests.
- Stop
withTelemetry/doTelemetryfrom attempting to collect and sendai-setup-evidence.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| code/lib/cli-storybook/src/ai/index.ts | Removes pending-record snapshot/cache side effects from aiSetup, leaving only the ai-setup event. |
| code/lib/cli-storybook/src/ai/index.test.ts | Removes tests that were exclusively validating the deleted pending-record behavior. |
| code/core/src/telemetry/index.ts | Stops re-exporting the removed AI setup pending cache API/types. |
| code/core/src/telemetry/event-cache.ts | Deletes AiSetupPendingRecord and the get/flush helpers for ai-setup-pending. |
| code/core/src/telemetry/event-cache.test.ts | Removes the test suite covering the deleted AI setup pending cache helpers. |
| code/core/src/telemetry/ai-setup-utils.ts | Removes evidence-collection and preview hashing utilities, leaving only story tagging helper(s). |
| code/core/src/telemetry/ai-setup-utils.test.ts | Removes tests for deleted utilities; keeps tests for isStoryCreatedByAISetup. |
| code/core/src/core-server/withTelemetry.ts | Removes fire-and-forget collectAiSetupEvidence() call after boot. |
| code/core/src/core-server/withTelemetry.test.ts | Updates expectations to no longer require collectAiSetupEvidence() to run. |
| code/core/src/core-server/utils/doTelemetry.ts | Removes dev telemetry path that attempted AI setup evidence collection when index/stats are present. |
Comments suppressed due to low confidence (1)
code/core/src/telemetry/ai-setup-utils.ts:7
- The JSDoc is now inaccurate: the implementation checks for the
ai-generatedtag, but the comment still says it “currently checks title prefix” and suggests a future migration to tags. Please update the comment to reflect the current tag-based behavior (or adjust the implementation if the title-prefix check is still intended).
/**
* 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.
*/
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
yannbf
approved these changes
May 4, 2026
Sidnioulz
force-pushed
the
sidnioulz/remove-ai-setup-evidence
branch
from
7461b82 to
8ffee83
Compare
Contributor
There was a problem hiding this comment.
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.ts (1)
4-7:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate outdated doc comment to match current behavior.
The comment still says this checks a title prefix, but the implementation is already tag-based. Keeping this stale text risks future regressions.
Suggested 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. + * Uses the `ai-generated` tag on story entries. */🤖 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 4 - 7, Update the outdated doc comment above the function in ai-setup-utils.ts (e.g., isSbAiSetupEntry) to state the current behavior: it checks for the presence of the sb ai setup tag (tag-based) rather than a title prefix; mention the exact tag/key the implementation looks for and keep the note that this is the single swap point if you later migrate to a different detection method.
🧹 Nitpick comments (1)
code/core/src/telemetry/ai-setup-utils.test.ts (1)
7-21: ⚡ Quick winAdd a non-story regression test case.
Given the helper explicitly checks
entry.type === 'story', adding a case for a tagged non-story entry would harden this behavior.Suggested test addition
describe('isStoryCreatedByAISetup', () => { @@ it('returns false for regular stories', () => { expect(isStoryCreatedByAISetup({ type: 'story', title: 'Foo' } as IndexEntry)).toBe(false); }); + + it('returns false for non-story entries even when tagged', () => { + expect( + isStoryCreatedByAISetup({ + type: 'docs', + title: 'Foo', + tags: ['ai-generated'], + } as IndexEntry) + ).toBe(false); + }); });🤖 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 7 - 21, Add a regression test that ensures isStoryCreatedByAISetup only returns true for entries where entry.type === 'story' by adding a case that passes a non-story IndexEntry (e.g., type: 'note' or 'task') with tags including 'ai-generated' and asserting the function returns false; locate the test suite for isStoryCreatedByAISetup in ai-setup-utils.test.ts and add the new it(...) block referencing isStoryCreatedByAISetup and IndexEntry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@code/core/src/telemetry/ai-setup-utils.ts`:
- Around line 4-7: Update the outdated doc comment above the function in
ai-setup-utils.ts (e.g., isSbAiSetupEntry) to state the current behavior: it
checks for the presence of the sb ai setup tag (tag-based) rather than a title
prefix; mention the exact tag/key the implementation looks for and keep the note
that this is the single swap point if you later migrate to a different detection
method.
---
Nitpick comments:
In `@code/core/src/telemetry/ai-setup-utils.test.ts`:
- Around line 7-21: Add a regression test that ensures isStoryCreatedByAISetup
only returns true for entries where entry.type === 'story' by adding a case that
passes a non-story IndexEntry (e.g., type: 'note' or 'task') with tags including
'ai-generated' and asserting the function returns false; locate the test suite
for isStoryCreatedByAISetup in ai-setup-utils.test.ts and add the new it(...)
block referencing isStoryCreatedByAISetup and IndexEntry.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 71bbd07a-745e-4473-9b3e-3b83208dba35
📒 Files selected for processing (10)
code/core/src/core-server/utils/doTelemetry.tscode/core/src/core-server/withTelemetry.test.tscode/core/src/core-server/withTelemetry.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/lib/cli-storybook/src/ai/index.test.tscode/lib/cli-storybook/src/ai/index.ts
💤 Files with no reviewable changes (5)
- code/core/src/telemetry/index.ts
- code/core/src/core-server/withTelemetry.ts
- code/core/src/telemetry/event-cache.ts
- code/lib/cli-storybook/src/ai/index.test.ts
- code/core/src/core-server/withTelemetry.test.ts
✅ Files skipped from review due to trivial changes (1)
- code/core/src/telemetry/event-cache.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- code/core/src/core-server/utils/doTelemetry.ts
Sidnioulz
force-pushed
the
sidnioulz/remove-ai-setup-evidence
branch
from
8ffee83 to
8595235
Compare
Contributor
There was a problem hiding this comment.
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/doTelemetry.ts (1)
37-47:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep
versionStatusoutside theindexAndStatsguard.
versionStatusdoes not depend on story index collection, so a missingindexAndStatspayload should not suppress it. As written, recoverable index-free runs drop that telemetry field entirely.♻️ Proposed fix
- const payload = { - precedingUpgrade: await getPrecedingUpgrade(), - }; + const payload = { + precedingUpgrade: await getPrecedingUpgrade(), + versionStatus: versionUpdates && versionCheck ? versionStatus(versionCheck) : 'disabled', + }; if (indexAndStats) { Object.assign(payload, { - versionStatus: versionUpdates && versionCheck ? versionStatus(versionCheck) : 'disabled', storyIndex: summarizeIndex(indexAndStats.storyIndex), storyStats: indexAndStats.stats, }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/core/src/core-server/utils/doTelemetry.ts` around lines 37 - 47, The telemetry payload currently only adds versionStatus when indexAndStats exists; move the versionStatus assignment out of that indexAndStats guard so it is always included. Concretely, when building payload (the variable named payload that already includes precedingUpgrade from getPrecedingUpgrade()), add versionStatus: versionUpdates && versionCheck ? versionStatus(versionCheck) : 'disabled' at the top-level, then keep the indexAndStats-specific fields (storyIndex via summarizeIndex and storyStats) inside the existing indexAndStats conditional; this ensures versionStatus is present even when indexAndStats is undefined.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@code/core/src/core-server/utils/doTelemetry.ts`:
- Around line 37-47: The telemetry payload currently only adds versionStatus
when indexAndStats exists; move the versionStatus assignment out of that
indexAndStats guard so it is always included. Concretely, when building payload
(the variable named payload that already includes precedingUpgrade from
getPrecedingUpgrade()), add versionStatus: versionUpdates && versionCheck ?
versionStatus(versionCheck) : 'disabled' at the top-level, then keep the
indexAndStats-specific fields (storyIndex via summarizeIndex and storyStats)
inside the existing indexAndStats conditional; this ensures versionStatus is
present even when indexAndStats is undefined.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d084e4d2-17a0-456b-b623-0726578e5384
📒 Files selected for processing (10)
code/core/src/core-server/utils/doTelemetry.tscode/core/src/core-server/withTelemetry.test.tscode/core/src/core-server/withTelemetry.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/lib/cli-storybook/src/ai/index.test.tscode/lib/cli-storybook/src/ai/index.ts
💤 Files with no reviewable changes (5)
- code/core/src/telemetry/event-cache.ts
- code/core/src/core-server/withTelemetry.test.ts
- code/core/src/core-server/withTelemetry.ts
- code/core/src/telemetry/index.ts
- code/lib/cli-storybook/src/ai/index.test.ts
✅ Files skipped from review due to trivial changes (1)
- code/core/src/telemetry/event-cache.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- code/core/src/telemetry/ai-setup-utils.ts
Sidnioulz
force-pushed
the
sidnioulz/remove-ai-setup-evidence
branch
from
8595235 to
a23b3e7
Compare
14 tasks
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.
We realised
ai-setup-evidenceis now redundant with the scoring events from Vitest, and is no longer being fired because it relied on prompts telling agents to rundevanddoctorcommands.This PR removes the event. We only lose the information whether or not
previewwas changed, which we found during the project has a near 100% success rate whenai setupwas run, so we're ok with that information loss.@yannbf is working on a PR that changes the vitest test report to produce a cumulative test count and pass rate from previous runs, which will be less dependent on whether prompts run every single test or only the last modified ones / currently failing ones.
Summary by CodeRabbit