feat: posthog distinct id migration#2062
Merged
AndreyHirsa merged 5 commits intomainfrom Mar 31, 2026
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughMigrates PostHog tracking from email-based identifiers to a structured UserIdentity ({ email, id } | null), updating identity resolution, aliasing, tracking APIs, CLI command integrations, localizer auth shapes, SDK observability logic, and related tests. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI Command
participant Localizer as Localizer.checkAuth()
participant IdentityUtil as determineUserIdentity()
participant Tracker as trackEvent()
participant PostHog as PostHog
CLI->>Localizer: request auth status
Localizer-->>CLI: { authenticated, username?, userId? }
CLI->>IdentityUtil: determineUserIdentity(ctx)
IdentityUtil->>Localizer: read auth status
IdentityUtil-->>CLI: UserIdentity { email, id } or null
CLI->>Tracker: trackEvent(userIdentity, event, props)
Tracker->>Tracker: determineDistinctId(userIdentity)
Tracker->>PostHog: capture(distinct_id: user.id, properties: {..., $set.email?: user.email})
alt email present
Tracker->>PostHog: alias(distinct_id: user.id, alias: user.email)
end
PostHog-->>Tracker: ack
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/src/cli/cmd/status.ts (1)
359-376:⚠️ Potential issue | 🟡 MinorDuplicate console.log for missing keys.
Lines 369-376 and 373-376 both output the "Missing:" message, resulting in duplicate output when
missingKeys.length > 0.Proposed fix — remove the duplicate
if (flags.verbose) { if (missingKeys.length > 0) { console.log( ` ${chalk.red(`Missing:`)} ${missingKeys.length} keys, ~${wordsToTranslate} words`, ); - console.log( - ` ${chalk.red(`Missing:`)} ${ - missingKeys.length - } keys, ~${wordsToTranslate} words`, - ); console.log(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/cli/cmd/status.ts` around lines 359 - 376, The verbose branch in status.ts prints the "Missing:" line twice; inside the flags.verbose block remove the duplicate console.log that prints `Missing: ${missingKeys.length} keys, ~${wordsToTranslate} words` so only a single line is logged when missingKeys.length > 0 (the remaining call should use the existing variables missingKeys and wordsToTranslate and remain under the same if (missingKeys.length > 0) check after bucketOra.succeed).
🧹 Nitpick comments (1)
packages/cli/src/cli/utils/observability.ts (1)
141-142: Inconsistent error handling between main and alias requests.The main request logs errors in DEBUG mode (lines 113-117), but the alias request silently swallows errors. For consistency and to aid debugging migration issues, consider logging alias request errors in DEBUG mode as well.
🔧 Suggested fix for consistent error handling
aliasReq.on("timeout", () => aliasReq.destroy()); - aliasReq.on("error", () => {}); + aliasReq.on("error", (error) => { + if (process.env.DEBUG === "true") { + console.error("[Tracking] Alias error ignored:", error.message); + } + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/cli/utils/observability.ts` around lines 141 - 142, The alias request currently swallows errors (aliasReq.on("error", () => {})); change this to mirror the main request's debug logging by capturing the error object and logging it with the same logger used for the main request (e.g., processLogger.debug or logger.debug) when debug is enabled; update the aliasReq.on("error", ...) handler to log a descriptive message including the error (e.g., "Alias request error") rather than silently ignoring it so aliasReq's error handling matches the main request's behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/cli/cmd/i18n.ts`:
- Around line 156-159: The code constructs a UserIdentity using auth.id without
validating it; update the auth handling in the block after
validateAuth(settings) (the auth variable used to populate userIdentity) to
explicitly check that auth.id is present and non-empty before assigning to
userIdentity (same safeguard as in status.ts). If auth.id is missing, handle it
deterministically — e.g., log a clear error / fail fast or attempt to resolve
the identifier (re-run authenticator.whoami() or prompt re-authentication) —
rather than creating a UserIdentity with id: undefined; ensure the check
references validateAuth and userIdentity so the fix is applied to this exact
assignment site.
In `@packages/cli/src/cli/cmd/status.ts`:
- Around line 77-80: The code constructs a UserIdentity from tryAuthenticate()
without verifying auth.id; update the block that calls tryAuthenticate so you
only set userIdentity = { email: auth.email, id: auth.id } when both auth.email
and auth.id are present and non-empty (match the safe pattern used in
run/_utils.ts like checking auth?.email and auth?.id), otherwise treat auth as
absent (leave userIdentity null or handle as unauthenticated) and adjust the ora
messaging accordingly; edit the code around tryAuthenticate, the auth variable,
and the userIdentity assignment to add this validation.
---
Outside diff comments:
In `@packages/cli/src/cli/cmd/status.ts`:
- Around line 359-376: The verbose branch in status.ts prints the "Missing:"
line twice; inside the flags.verbose block remove the duplicate console.log that
prints `Missing: ${missingKeys.length} keys, ~${wordsToTranslate} words` so only
a single line is logged when missingKeys.length > 0 (the remaining call should
use the existing variables missingKeys and wordsToTranslate and remain under the
same if (missingKeys.length > 0) check after bucketOra.succeed).
---
Nitpick comments:
In `@packages/cli/src/cli/utils/observability.ts`:
- Around line 141-142: The alias request currently swallows errors
(aliasReq.on("error", () => {})); change this to mirror the main request's debug
logging by capturing the error object and logging it with the same logger used
for the main request (e.g., processLogger.debug or logger.debug) when debug is
enabled; update the aliasReq.on("error", ...) handler to log a descriptive
message including the error (e.g., "Alias request error") rather than silently
ignoring it so aliasReq's error handling matches the main request's behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 41bc0291-d17b-4f47-85ea-fd2a4995eb8a
📒 Files selected for processing (10)
.changeset/lazy-beds-lay.mdpackages/cli/src/cli/cmd/i18n.tspackages/cli/src/cli/cmd/run/_utils.tspackages/cli/src/cli/cmd/run/index.tspackages/cli/src/cli/cmd/status.tspackages/cli/src/cli/localizer/_types.tspackages/cli/src/cli/localizer/lingodotdev.tspackages/cli/src/cli/utils/observability.tspackages/sdk/src/utils/observability.spec.tspackages/sdk/src/utils/observability.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/sdk/src/utils/observability.ts`:
- Around line 57-77: Wrap the posthog calls in a try/finally so that
posthog.shutdown() is always executed: move the await posthog.shutdown() into a
finally block, keep await posthog.capture(...) and the conditional await
posthog.alias(...) inside the try (alias only if email), and after shutdown
rethrow any caught error (or let it propagate) so behavior and error semantics
remain unchanged; reference posthog.capture, posthog.alias, and posthog.shutdown
to locate the changes in observability.ts.
- Around line 60-63: The properties object in observability.ts currently
overwrites any incoming properties.$set by assigning $set: email ? { email } :
{}, which can drop caller-provided fields; instead, preserve and merge caller
$set with the email if present: compute $set as the spread merge of existing
properties.$set and the email ({ ...properties.$set, ...(email ? { email } : {})
}) so that provided fields are retained, and leave properties.$set untouched
when email is absent; keep the tracking_version: TRACKING_VERSION assignment
as-is.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d4e113a0-910e-4bef-8d0b-1e74a73170f2
📒 Files selected for processing (1)
packages/sdk/src/utils/observability.ts
vrcprl
approved these changes
Mar 31, 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.
Summary
Migrate PostHog distinct_id from email to database user ID
Changes
apikey-{hash}fallback — a transient/users/mefailure no longer poisons the identity cache for the entire process lifetimeTesting
Existing tests updated to reflect new identity source:
distinct_idis database user ID/users/mereturns noidaliasmethod to support new$create_aliascallVisuals
N/A - no UI changes, backend tracking only.
Checklist
Summary by CodeRabbit