Telemetry: Fix preview-first-load event

[shilman]

Closes N/A

What I did

Follow-up to #32770

• Fix typescript types
• Record whether the user is a new user

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

• stories
• unit tests
• integration tests
• end-to-end tests

Manual testing

See #32770

🦋 Canary release

This pull request has been released as version 0.0.0-pr-32859-sha-0ff3fd82. Try it out in a new sandbox by running npx [email protected] sandbox or in an existing project with npx [email protected] upgrade.

More information

Published version	0.0.0-pr-32859-sha-0ff3fd82
Triggered by	@shilman
Repository	storybookjs/storybook
Branch	shilman/first-load-new-user
Commit	0ff3fd82
Datetime	Tue Oct 28 08:52:12 UTC 2025 (1761641532)
Workflow run	18869205798

To request a new release of this pull request, mention the @storybookjs/core team.

core team members can create a new canary release here or locally with gh workflow run --repo storybookjs/storybook publish.yml --field pr=32859

Summary by CodeRabbit

• Tests

  • Added deterministic tests for preview initialization covering new vs. existing sessions, timing calculations, and user-agent preservation.

• Refactor

  • Centralized preview-init payload construction and improved telemetry type safety; strengthened event caching typings and error-sanitization handling to make telemetry reporting more reliable.

[coderabbitai]

📝 Walkthrough

Walkthrough

Extracted payload construction into exported makePayload; added telemetry types (TelemetryEvent, CacheEntry, InitPayload, Context); updated event-cache APIs to return CacheEntry; added tests for makePayload; adjusted initPreviewInitializedChannel signature. No runtime behavior change beyond payload construction and typings.

Changes

Cohort / File(s)	Summary
Preview init handler & helper code/core/src/core-server/server-channel/preview-initialized-channel.ts	Added and exported makePayload(userAgent, lastInit, sessionId) returning { userAgent, isNewUser, timeSinceInit }; replaced inline payload creation with makePayload; updated initPreviewInitializedChannel signature to include _coreConfig.
Tests for payload logic code/core/src/core-server/server-channel/preview-initialized-channel.test.ts	New test suite for makePayload covering new user, existing user, missing init, and different sessionId scenarios using fake timers and inline snapshots.
Event cache typing code/core/src/telemetry/event-cache.ts	Introduced exported CacheEntry { timestamp, body }; changed helpers and getters to use TelemetryEvent/CacheEntry; get and getLastEvents now return CacheEntry-typed results.
Telemetry types code/core/src/telemetry/types.ts	Added Context, TelemetryEvent, and InitPayload interfaces; removed Dependency export; extended payload-related typing to include event/session metadata.
Telemetry surface change code/core/src/telemetry/index.ts	Moved some error-related metadata properties from telemetryData.payload to the top-level payload and adjusted sanitization/conditional checks accordingly.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Client
  participant PreviewChannel as PreviewChannel
  participant EventCache as EventCache
  participant makePayload as makePayload
  participant Telemetry as telemetry()

  Client->>PreviewChannel: PREVIEW_INITIALIZED (userAgent, sessionId)
  PreviewChannel->>EventCache: getLastEvents()
  EventCache-->>PreviewChannel: lastInit (CacheEntry | undefined)
  PreviewChannel->>makePayload: makePayload(userAgent, lastInit, sessionId)
  makePayload-->>PreviewChannel: payload { userAgent, isNewUser, timeSinceInit? }
  PreviewChannel->>Telemetry: telemetry("preview-first-load", payload)
  Telemetry-->>PreviewChannel: ack
  PreviewChannel-->>Client: done

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Verify makePayload correctly compares sessionId and computes timeSinceInit (edge cases: undefined lastInit, differing sessionId).
• Check callers of event-cache APIs across the codebase for required type adjustments.
• Review telemetry.index changes to ensure error fields are handled consistently.

Possibly related PRs

• Core: Add preview-first-load telemetry #32770 — Adds PREVIEW_INITIALIZED event and preview-first-load channel wiring; overlaps with payload construction and preview-initialized channel changes.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch shilman/first-load-new-user

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5266eba and 0ff3fd8.

📒 Files selected for processing (5)

• code/core/src/core-server/server-channel/preview-initialized-channel.test.ts (1 hunks)
• code/core/src/core-server/server-channel/preview-initialized-channel.ts (2 hunks)
• code/core/src/telemetry/event-cache.ts (3 hunks)
• code/core/src/telemetry/index.ts (1 hunks)
• code/core/src/telemetry/types.ts (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

• code/core/src/telemetry/index.ts
• code/core/src/core-server/server-channel/preview-initialized-channel.ts

🧰 Additional context used

📓 Path-based instructions (7)

code/**/*.{test,spec}.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

code/**/*.{test,spec}.{ts,tsx}: Place all test files under the code/ directory
Name test files as *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx

Files:

• code/core/src/core-server/server-channel/preview-initialized-channel.test.ts

**/*.test.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)

**/*.test.{ts,tsx,js,jsx}: Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Use vi.mocked() to type and access mocked functions
Implement mock behaviors in beforeEach blocks
Mock all required dependencies that the test subject uses
Mock implementations should be placed in beforeEach blocks
Each mock implementation should return a Promise for async functions
Mock implementations should match the expected return type of the original function
Use vi.mocked() to access and implement mock behaviors
Mock all required properties and methods that the test subject uses
Avoid direct function mocking without vi.mocked()
Avoid mock implementations outside of beforeEach blocks
Avoid mocking without the spy: true option
Avoid inline mock implementations within test cases
Avoid mocking only a subset of required dependencies
Mock at the highest level of abstraction needed
Keep mock implementations simple and focused
Use type-safe mocking with vi.mocked()
Document complex mock behaviors
Group related mocks together

Files:

• code/core/src/core-server/server-channel/preview-initialized-channel.test.ts

**/*.{js,jsx,json,html,ts,tsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (use yarn lint:js:cmd <file>)

Files:

• code/core/src/core-server/server-channel/preview-initialized-channel.test.ts
• code/core/src/telemetry/event-cache.ts
• code/core/src/telemetry/types.ts

**/*.@(test|spec).{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.@(test|spec).{ts,tsx,js,jsx}: Unit tests should import and execute the functions under test rather than only asserting on syntax patterns
Mock external dependencies in tests using vi.mock() (e.g., filesystem, loggers)

Files:

• code/core/src/core-server/server-channel/preview-initialized-channel.test.ts

**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Export functions from modules when they need to be unit-tested

Files:

• code/core/src/core-server/server-channel/preview-initialized-channel.test.ts
• code/core/src/telemetry/event-cache.ts
• code/core/src/telemetry/types.ts

code/**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

In application code, use Storybook loggers instead of console.* (client code: storybook/internal/client-logger; server code: storybook/internal/node-logger)

Files:

• code/core/src/core-server/server-channel/preview-initialized-channel.test.ts
• code/core/src/telemetry/event-cache.ts
• code/core/src/telemetry/types.ts

{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size

Files:

• code/core/src/core-server/server-channel/preview-initialized-channel.test.ts
• code/core/src/telemetry/event-cache.ts
• code/core/src/telemetry/types.ts

🧠 Learnings (2)

📚 Learning: 2025-10-13T13:33:14.659Z

Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-13T13:33:14.659Z
Learning: Applies to **/*.@(test|spec).{ts,tsx,js,jsx} : Unit tests should import and execute the functions under test rather than only asserting on syntax patterns

Applied to files:

• code/core/src/core-server/server-channel/preview-initialized-channel.test.ts

📚 Learning: 2025-09-17T08:11:47.196Z

Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mock implementations outside of beforeEach blocks

Applied to files:

• code/core/src/core-server/server-channel/preview-initialized-channel.test.ts

🧬 Code graph analysis (2)

code/core/src/core-server/server-channel/preview-initialized-channel.test.ts (1)

code/core/src/core-server/server-channel/preview-initialized-channel.ts (1)

• makePayload (9-26)

code/core/src/telemetry/event-cache.ts (1)

code/core/src/telemetry/types.ts (2)

• TelemetryEvent (111-115)
• EventType (7-36)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: normal
• GitHub Check: Core Unit Tests, windows-latest

🔇 Additional comments (8)

code/core/src/core-server/server-channel/preview-initialized-channel.test.ts (1)

1-86: LGTM! Comprehensive test coverage for makePayload.

The test suite thoroughly covers all code paths:

• New user initialization with matching sessionId
• Existing user initialization with matching sessionId
• Missing initialization data
• Session ID mismatch scenarios

The use of fake timers ensures deterministic testing, and inline snapshots provide clear expectations. Previous review issues have been properly addressed.

code/core/src/telemetry/types.ts (3)

92-94: LGTM! Context interface added.

The flexible index signature appropriately supports arbitrary context data for telemetry events.

---

110-115: LGTM! TelemetryEvent interface properly extends TelemetryData.

The interface correctly adds the required event metadata (eventId, sessionId, context) while preserving the base telemetry data structure.

---

117-123: LGTM! InitPayload interface is well-structured.

The interface clearly defines the initialization payload shape with appropriate fields for tracking new users, project type, features, and CLI integration data.

code/core/src/telemetry/event-cache.ts (4)

3-3: LGTM! CacheEntry interface improves type safety.

The new CacheEntry interface properly types the cache structure with timestamp and TelemetryEvent body, improving type consistency across the codebase.

Also applies to: 12-15

---

19-19: LGTM! Internal setHelper properly typed.

The signature correctly requires TelemetryEvent while the public set function maintains backward compatibility with any. This is a sound gradual typing approach.

---

31-34: LGTM! Return types properly typed with CacheEntry.

Both get and getLastEvents now correctly return CacheEntry-typed values, ensuring type consistency throughout the cache API.

Also applies to: 36-38

---

40-48: LGTM! upgradeFields properly typed.

The function parameter is correctly typed as CacheEntry, and the implementation safely accesses the nested body properties with optional chaining.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[storybook-bot]

Failed to publish canary version of this pull request, triggered by @shilman. See the failed workflow run at: https://github.com/storybookjs/storybook/actions/runs/18868318743

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (3)

code/core/src/core-server/server-channel/preview-initialized-channel.test.ts (1)

16-35: Consider improving type safety in test mocks.

The test uses as any casting (line 28) because the mock lastInit object doesn't fully conform to the CacheEntry type. While this is functional, you could improve type safety by creating a test helper that constructs partial CacheEntry mocks with proper typing.

Example approach:

type PartialCacheEntry = Partial<CacheEntry> & {
  body?: Partial<TelemetryData>;
};

const createMockCacheEntry = (partial: PartialCacheEntry): CacheEntry => ({
  timestamp: partial.timestamp ?? Date.now(),
  body: {
    eventId: 'test-id',
    sessionId: partial.body?.sessionId ?? 'test-session',
    eventType: 'init',
    payload: partial.body?.payload ?? {},
    context: {},
    ...partial.body,
  } as TelemetryData,
});

code/core/src/telemetry/types.ts (1)

92-94: Consider strengthening Context type definition.

The Context interface uses any for values, which reduces type safety. Consider restricting to specific types:

 export interface Context {
-  [key: string]: any;
+  [key: string]: string | number | boolean | undefined | null | Context;
 }

This allows nested contexts while maintaining type safety for primitive values.

code/core/src/core-server/server-channel/preview-initialized-channel.ts (1)

9-25: Consider simplifying type annotation.

The explicit type annotation on lines 14-18 is redundant since TypeScript can infer the type from the initialization. The logic correctly handles edge cases with optional chaining and defaults.

-  const payload = {
+  const payload: {
+    userAgent: string;
+    isNewUser: boolean;
+    timeSinceInit: number | undefined;
+  } = {
     userAgent,
     isNewUser: false,
     timeSinceInit: undefined,
-  } as {
-    userAgent: string;
-    isNewUser: boolean;
-    timeSinceInit: number | undefined;
-  };
+  };

Or simply remove the annotation entirely:

   const payload = {
     userAgent,
     isNewUser: false,
-    timeSinceInit: undefined,
-  } as {
-    userAgent: string;
-    isNewUser: boolean;
-    timeSinceInit: number | undefined,
-  };
+    timeSinceInit: undefined as number | undefined,
+  };

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cece9dc and fc4134c.

📒 Files selected for processing (4)

• code/core/src/core-server/server-channel/preview-initialized-channel.test.ts (1 hunks)
• code/core/src/core-server/server-channel/preview-initialized-channel.ts (2 hunks)
• code/core/src/telemetry/event-cache.ts (3 hunks)
• code/core/src/telemetry/types.ts (2 hunks)

🧰 Additional context used

📓 Path-based instructions (7)

**/*.{js,jsx,json,html,ts,tsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (use yarn lint:js:cmd <file>)

Files:

• code/core/src/telemetry/event-cache.ts
• code/core/src/core-server/server-channel/preview-initialized-channel.ts
• code/core/src/core-server/server-channel/preview-initialized-channel.test.ts
• code/core/src/telemetry/types.ts

**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Export functions from modules when they need to be unit-tested

Files:

• code/core/src/telemetry/event-cache.ts
• code/core/src/core-server/server-channel/preview-initialized-channel.ts
• code/core/src/core-server/server-channel/preview-initialized-channel.test.ts
• code/core/src/telemetry/types.ts

code/**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

In application code, use Storybook loggers instead of console.* (client code: storybook/internal/client-logger; server code: storybook/internal/node-logger)

Files:

• code/core/src/telemetry/event-cache.ts
• code/core/src/core-server/server-channel/preview-initialized-channel.ts
• code/core/src/core-server/server-channel/preview-initialized-channel.test.ts
• code/core/src/telemetry/types.ts

{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size

Files:

• code/core/src/telemetry/event-cache.ts
• code/core/src/core-server/server-channel/preview-initialized-channel.ts
• code/core/src/core-server/server-channel/preview-initialized-channel.test.ts
• code/core/src/telemetry/types.ts

code/**/*.{test,spec}.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

code/**/*.{test,spec}.{ts,tsx}: Place all test files under the code/ directory
Name test files as *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx

Files:

• code/core/src/core-server/server-channel/preview-initialized-channel.test.ts

**/*.test.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)

**/*.test.{ts,tsx,js,jsx}: Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Use vi.mocked() to type and access mocked functions
Implement mock behaviors in beforeEach blocks
Mock all required dependencies that the test subject uses
Mock implementations should be placed in beforeEach blocks
Each mock implementation should return a Promise for async functions
Mock implementations should match the expected return type of the original function
Use vi.mocked() to access and implement mock behaviors
Mock all required properties and methods that the test subject uses
Avoid direct function mocking without vi.mocked()
Avoid mock implementations outside of beforeEach blocks
Avoid mocking without the spy: true option
Avoid inline mock implementations within test cases
Avoid mocking only a subset of required dependencies
Mock at the highest level of abstraction needed
Keep mock implementations simple and focused
Use type-safe mocking with vi.mocked()
Document complex mock behaviors
Group related mocks together

Files:

• code/core/src/core-server/server-channel/preview-initialized-channel.test.ts

**/*.@(test|spec).{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.@(test|spec).{ts,tsx,js,jsx}: Unit tests should import and execute the functions under test rather than only asserting on syntax patterns
Mock external dependencies in tests using vi.mock() (e.g., filesystem, loggers)

Files:

• code/core/src/core-server/server-channel/preview-initialized-channel.test.ts

🧬 Code graph analysis (3)

code/core/src/telemetry/event-cache.ts (1)

code/core/src/telemetry/types.ts (2)

• TelemetryData (105-112)
• EventType (7-36)

code/core/src/core-server/server-channel/preview-initialized-channel.ts (2)

code/core/src/telemetry/event-cache.ts (1)

• CacheEntry (12-15)

code/core/src/telemetry/types.ts (1)

• InitPayload (114-120)

code/core/src/core-server/server-channel/preview-initialized-channel.test.ts (1)

code/core/src/core-server/server-channel/preview-initialized-channel.ts (1)

• makePayload (9-25)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Core Unit Tests, windows-latest

🔇 Additional comments (8)

code/core/src/telemetry/event-cache.ts (3)

3-3: LGTM! Type safety improvements are well-structured.

The addition of TelemetryData import and the new exported CacheEntry interface properly type the cached event structure, replacing previous any types.

Also applies to: 12-15

---

19-19: LGTM! Parameter type strengthened.

Replacing any with TelemetryData improves type safety and aligns with the CacheEntry interface.

---

31-31: LGTM! Return types properly updated.

The return type updates for get, getLastEvents, and the upgradeFields parameter type all correctly use the new CacheEntry interface, ensuring consistency and type safety.

Also applies to: 36-36, 40-40

code/core/src/core-server/server-channel/preview-initialized-channel.test.ts (1)

58-70: LGTM! Edge case properly tested.

This test correctly validates the behavior when no init session exists, ensuring isNewUser defaults to false and timeSinceInit remains undefined.

code/core/src/telemetry/types.ts (2)

105-112: LGTM! TelemetryData properly extended.

The additions of eventId, sessionId, and context fields appropriately extend the telemetry data structure to support richer event tracking, aligning with the PR objectives.

---

114-120: LGTM! InitPayload interface is well-structured.

The interface properly types the initialization payload with appropriate required and optional fields, supporting the telemetry changes for tracking new users.

code/core/src/core-server/server-channel/preview-initialized-channel.ts (2)

3-3: LGTM! Imports properly updated.

The addition of InitPayload and CacheEntry types, along with getLastEvents function, supports the refactored payload construction logic.

Also applies to: 6-6

---

40-41: LGTM! Clean refactoring to use makePayload helper.

Extracting payload construction into the makePayload function improves testability and separation of concerns. The handler now cleanly orchestrates the telemetry call.

[nx-cloud]

View your CI Pipeline Execution ↗ for commit 0ff3fd8

Command	Status	Duration	Result
nx run-many -t build --parallel=3	✅ Succeeded	46s	View ↗

---

☁️ Nx Cloud last updated this comment at 2025-10-28 09:05:50 UTC