Ale/eng 8365 enable email level breakdown for custom events in posthog#2672
Ale/eng 8365 enable email level breakdown for custom events in posthog#2672
Conversation
WalkthroughThe changes refactor identity and telemetry tracking across CLI and studio packages. The CLI now extracts email and organization identifiers from JWT tokens to support richer identity resolution in PostHog. The studio's track module refactors PostHog identity syncing into a dedicated, testable function and updates its type signatures. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
📝 Coding Plan
Comment Tip You can generate walkthrough in a markdown collapsible section to save space.Enable the |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2672 +/- ##
==========================================
- Coverage 63.01% 9.13% -53.89%
==========================================
Files 245 425 +180
Lines 26260 53349 +27089
Branches 0 894 +894
==========================================
- Hits 16549 4872 -11677
- Misses 8366 48191 +39825
+ Partials 1345 286 -1059
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cli/src/core/telemetry.ts (1)
160-203:⚠️ Potential issue | 🟠 MajorCache telemetry identity instead of calling
whoAmIon every event.With an API key, every
capture()now awaitsapiClient.platform.whoAmI(...)before enqueueing PostHog. That puts extra network latency on the hot path and can slow commands that emit several events. Memoize the resolved identity (or resolve it once duringinitTelemetry) and keep a timeout if this lookup remains network-backed.Also applies to: 205-211, 263-265
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/src/core/telemetry.ts` around lines 160 - 203, getIdentityFromApiKey currently calls apiClient.platform.whoAmI each time (used by capture), adding network latency; change it to cache/memoize the resolved TelemetryIdentity (or call once from initTelemetry) and return the cached value until a configurable TTL expires, and on cache miss asynchronously call apiClient.platform.whoAmI to refresh; update getIdentityFromApiKey to read/write the in-memory cache and ensure capture uses the cached identity, and apply the same caching pattern for the other callers referenced (the blocks around the code that use getIdentityFromApiKey at the other sites).
🧹 Nitpick comments (3)
studio/src/lib/track.ts (1)
21-33: Please convertIdentifyUserInputto an interface and add the missing: voidannotations.The new shape and helpers are otherwise clear; this just keeps the file aligned with the repo's TypeScript rules.
As per coding guidelines, "Prefer interfaces over type aliases for object shapes in TypeScript" and "Use explicit type annotations for function parameters and return types in TypeScript."
Also applies to: 54-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@studio/src/lib/track.ts` around lines 21 - 33, Convert the type alias IdentifyUserInput to an interface (interface IdentifyUserInput { ... }) and add explicit void return annotations to the relevant functions (e.g., annotate syncPostHogIdentity(...): void and any other functions in the same file referenced in the comment range around lines 54-70) so the function parameter and return types follow the repo rules; leave the existing parameter destructuring and implementation unchanged, only change the type declaration and add the : void return type annotations where missing.studio/src/__tests__/track.test.ts (1)
6-11: Use a typed mock instead ofas any.
as anyremoves the contract this suite is supposed to pin, so drift inget_distinct_id/alias/identify/groupwon't be caught at compile time. A narrow local interface or asatisfiescheck keeps these tests strict without much extra code.As per coding guidelines, "Avoid
anytype in TypeScript; use specific types or generics."Also applies to: 35-40
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@studio/src/__tests__/track.test.ts` around lines 6 - 11, Replace the untyped test mock that uses "as any" with a properly typed mock of the PostHog client: declare a narrow local interface (or use the PostHog client type with vi.Mocked<...> / "satisfies") that includes get_distinct_id, alias, identify, and group, and update the posthogClient fixture (and the similar mock at lines 35-40) to implement that interface so the compiler enforces the method signatures instead of silencing them with any.cli/src/core/telemetry.ts (1)
47-52: Use an interface forTelemetryIdentityand annotatesyncIdentity()explicitly.The new object shape is a better fit for an
interface, andsyncIdentityis the only new helper in this block still relying on an inferredvoid.As per coding guidelines, "Prefer interfaces over type aliases for object shapes in TypeScript" and "Use explicit type annotations for function parameters and return types in TypeScript."
Also applies to: 214-252
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/src/core/telemetry.ts` around lines 47 - 52, Replace the TelemetryIdentity type alias with an interface named TelemetryIdentity and add explicit type annotations to the syncIdentity helper: annotate its parameter(s) with TelemetryIdentity (or partial/nullable variants if applicable) and give the function an explicit return type (e.g., void or Promise<void> as appropriate); apply the same conversion and explicit annotation to the other occurrence of TelemetryIdentity and syncIdentity in the 214-252 block so both object shapes use interface and both functions have explicit parameter and return type declarations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/src/core/telemetry.ts`:
- Around line 152-157: Remove the problematic aliasing by deleting the
previousDistinctId assignment that uses organizationSlug in both
getIdentityFromLoginDetails and getIdentityFromApiKey: stop setting
previousDistinctId = organizationSlug (and any conditional variants) so no alias
is created from the shared org slug to the user email; only set
previousDistinctId when you have a true per-user legacy ID to map from.
---
Outside diff comments:
In `@cli/src/core/telemetry.ts`:
- Around line 160-203: getIdentityFromApiKey currently calls
apiClient.platform.whoAmI each time (used by capture), adding network latency;
change it to cache/memoize the resolved TelemetryIdentity (or call once from
initTelemetry) and return the cached value until a configurable TTL expires, and
on cache miss asynchronously call apiClient.platform.whoAmI to refresh; update
getIdentityFromApiKey to read/write the in-memory cache and ensure capture uses
the cached identity, and apply the same caching pattern for the other callers
referenced (the blocks around the code that use getIdentityFromApiKey at the
other sites).
---
Nitpick comments:
In `@cli/src/core/telemetry.ts`:
- Around line 47-52: Replace the TelemetryIdentity type alias with an interface
named TelemetryIdentity and add explicit type annotations to the syncIdentity
helper: annotate its parameter(s) with TelemetryIdentity (or partial/nullable
variants if applicable) and give the function an explicit return type (e.g.,
void or Promise<void> as appropriate); apply the same conversion and explicit
annotation to the other occurrence of TelemetryIdentity and syncIdentity in the
214-252 block so both object shapes use interface and both functions have
explicit parameter and return type declarations.
In `@studio/src/__tests__/track.test.ts`:
- Around line 6-11: Replace the untyped test mock that uses "as any" with a
properly typed mock of the PostHog client: declare a narrow local interface (or
use the PostHog client type with vi.Mocked<...> / "satisfies") that includes
get_distinct_id, alias, identify, and group, and update the posthogClient
fixture (and the similar mock at lines 35-40) to implement that interface so the
compiler enforces the method signatures instead of silencing them with any.
In `@studio/src/lib/track.ts`:
- Around line 21-33: Convert the type alias IdentifyUserInput to an interface
(interface IdentifyUserInput { ... }) and add explicit void return annotations
to the relevant functions (e.g., annotate syncPostHogIdentity(...): void and any
other functions in the same file referenced in the comment range around lines
54-70) so the function parameter and return types follow the repo rules; leave
the existing parameter destructuring and implementation unchanged, only change
the type declaration and add the : void return type annotations where missing.
🪄 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: ed20633c-ae93-4441-bdf3-b8a9f5fa6965
📒 Files selected for processing (3)
cli/src/core/telemetry.tsstudio/src/__tests__/track.test.tsstudio/src/lib/track.ts
| return { | ||
| distinctId: email ?? organizationSlug ?? 'anonymous', | ||
| email, | ||
| organizationSlug, | ||
| previousDistinctId: email && organizationSlug ? organizationSlug : undefined, | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -path "*/cli/src/core/telemetry.ts" -type fRepository: wundergraph/cosmo
Length of output: 88
🏁 Script executed:
cat -n ./cli/src/core/telemetry.tsRepository: wundergraph/cosmo
Length of output: 11256
Remove previousDistinctId assignment using shared organizationSlug.
When previousDistinctId is set to organizationSlug (lines 156, 189), the syncIdentity function at line 219 creates an alias from the organization slug to the user's email. Since organizationSlug is shared across all organization members, this causes a conflict: the first user to log in gets aliased from the org slug, consuming all pre-login org-wide telemetry. When subsequent users from the same organization log in, they create conflicting aliases for an already-mapped distinct ID.
Remove the previousDistinctId assignment in both getIdentityFromLoginDetails() and getIdentityFromApiKey() unless you have a true per-user legacy ID to alias from.
Suggested direction
return {
distinctId: email ?? organizationSlug ?? 'anonymous',
email,
organizationSlug,
- previousDistinctId: email && organizationSlug ? organizationSlug : undefined,
};Apply the same change at both locations.
📝 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.
| return { | |
| distinctId: email ?? organizationSlug ?? 'anonymous', | |
| email, | |
| organizationSlug, | |
| previousDistinctId: email && organizationSlug ? organizationSlug : undefined, | |
| }; | |
| return { | |
| distinctId: email ?? organizationSlug ?? 'anonymous', | |
| email, | |
| organizationSlug, | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cli/src/core/telemetry.ts` around lines 152 - 157, Remove the problematic
aliasing by deleting the previousDistinctId assignment that uses
organizationSlug in both getIdentityFromLoginDetails and getIdentityFromApiKey:
stop setting previousDistinctId = organizationSlug (and any conditional
variants) so no alias is created from the shared org slug to the user email;
only set previousDistinctId when you have a true per-user legacy ID to map from.
alepane21
deleted the
ale/eng-8365-enable-email-level-breakdown-for-custom-events-in-posthog
branch
1 participant
Summary by CodeRabbit
New Features
Tests
Checklist
Open Source AI Manifesto
This project follows the principles of the Open Source AI Manifesto. Please ensure your contribution aligns with its principles.