feat(onboarding): route analytics through backend - #3131
Conversation
Merging this PR will not alter performance
Comparing 
Footnotes
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (16)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
💤 Files with no reviewable changes (1)
Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour. 📝 WalkthroughWalkthroughChangesThe PR adds shared onboarding tracking with browser context, local-host gating, enriched event properties, and failure suppression. Frontend onboarding flows use the service. Backend tracking normalizes timestamps and updates AI-copy event handling. Onboarding analytics service
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to This PR routes onboarding analytics through the authenticated backend while preserving existing event context and suppression behavior. No actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant OnboardingComponent
participant sendOnboardingEvent
participant PostHog
OnboardingComponent->>sendOnboardingEvent: send onboarding event and properties
sendOnboardingEvent->>sendOnboardingEvent: collect sanitized browser context
sendOnboardingEvent->>PostHog: dispatch enriched event
``
</details>
<!-- walkthrough_end -->
<!-- pre_merge_checks_walkthrough_start -->
<details>
<summary>🚥 Pre-merge checks | ✅ 4 | ❌ 1</summary>
### ❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
| :----------------: | :--------- | :----------------------------------------------------------------------------------- | :--------------------------------------------------------------------------------- |
| Docstring Coverage | ⚠️ Warning | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
<details>
<summary>✅ Passed checks (4 passed)</summary>
| Check name | Status | Explanation |
| :------------------------: | :------- | :----------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Title check | ✅ Passed | The title clearly identifies the main change: routing onboarding analytics through the backend. |
| Description check | ✅ Passed | The description explains the backend routing, preserved context, event behavior, and validation results, but omits the template checklist and detailed test steps. |
</details>
</details>
<!-- pre_merge_checks_walkthrough_end -->
<!-- finishing_touch_checkbox_start -->
<details>
<summary>✨ Finishing Touches 💡 1</summary>
<!-- finishing_touch_suggestion:docstrings -->
<details>
<summary>📝 Generate docstrings 💡</summary>
- [ ] <!-- {"checkboxId":"7962f53c-55bc-4827-bfbf-6a18da830691"} --> Create stacked PR
- [ ] <!-- {"checkboxId":"3e1879ae-f29b-4d0d-8e06-d12b7ba33d98"} --> Commit on current branch
</details>
</details>
<!-- finishing_touch_checkbox_end -->
<!-- tips_start -->
---
<sub>Comment `@coderabbitai help` to get the list of available commands.</sub>
<!-- tips_end -->
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 17 files
Confidence score: 3/5
captureOnboardingEventinsrc/utils/onboardingProgressAnalytics.tsand thesendOnboardingEventpath insrc/components/dashboard/StepsApp.vueno longer honor the local-host guard, so local and preview sessions may send authenticated analytics to/private/eventsand pollute production data; restore the environment check before dispatch.src/services/onboardingTracking.tsonly promotesapp_idforAI_INSTRUCTIONS_COPIED_EVENT, so other onboarding events can reach the backend without their app association; preserveapp_idtagging for every event that provides it.- The fire-and-forget tracking in
src/components/dashboard/AppOnboardingFlow.vuedrops the prior rejection handling, allowing analytics failures to become unhandled promise rejections; retain explicit error handling around the tracking call.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/components/dashboard/StepsApp.vue">
<violation number="1" location="src/components/dashboard/StepsApp.vue:97">
P3: The old pushEvent skipped tracking when the API host was local (isLocal(config.supaHost)); sendOnboardingEvent routes through sendEvent, which sends to /private/events for any authenticated session with no local-env suppression. If a developer runs the console against the production/real backend (or VITE_API_HOST points to a non-local host), onboarding steps now emit real PostHog/analytics events from a dev environment where they were previously suppressed. Confirm this is intended or keep a local guard in sendOnboardingEvent.</violation>
</file>
<file name="src/utils/onboardingProgressAnalytics.ts">
<violation number="1" location="src/utils/onboardingProgressAnalytics.ts:53">
P3: The new `captureOnboardingEvent` drops the `isLocal(supaHost)` guard that `pushEvent` previously applied. Onboarding analytics are now always dispatched through `sendOnboardingEvent`, so events from local/preview environments that were suppressed before now reach the analytics backend (and its downstream PostHog). If dev-noise suppression was intentional to keep, thread the locality check through `sendOnboardingEvent`/`sendEvent` or gate `captureOnboardingEvent` on `isLocal(supaHost)` instead of discarding it.</violation>
<violation number="2" location="src/utils/onboardingProgressAnalytics.ts:53">
P2: The previous default capture (`pushEvent`) suppressed analytics outside the production host with `if (isLocal(supaHost)) return`. The new `captureOnboardingEvent` discards `supaHost` and always calls `sendEvent`, which POSTs to `defaultApiHost/private/events` whenever an auth session exists. This silently re-enables onboarding event delivery in local/dev builds, sending test-session events (or rejected tokens) to the configured API host. Gate the new path with the same `isLocal(supaHost)` guard before calling `sendOnboardingEvent`.</violation>
</file>
<file name="src/services/onboardingTracking.ts">
<violation number="1" location="src/services/onboardingTracking.ts:20">
P3: The app association is sent to the backend only for the AI-copy event. `app_id` is promoted to `tags.app_id` solely when `event === AI_INSTRUCTIONS_COPIED_EVENT`; for all other onboarding events that carry `app_id`, it stays inside `nonPersonTags`, which the backend's `getAppId` in `private/events.ts` ignores. As a result those events reach the endpoint without app attribution or app-level access control, silently dropping the app association the properties intend to carry. Promote `app_id` to `tags` consistently (or document why only the copy event should carry it).</violation>
</file>
<file name="src/components/dashboard/AppOnboardingFlow.vue">
<violation number="1" location="src/components/dashboard/AppOnboardingFlow.vue:1513">
P3: The new fire-and-forget analytics call drops the rejection handling the replaced code had. The old `onboarding_intent_selected` path wrapped `pushEvent` in try/catch, and `trackSuccessfulCopy` called `sendEvent(...).catch(() => {})`. The new `void sendOnboardingEvent('onboarding_intent_selected', {...})` leaves the returned promise without a rejection handler. `sendOnboardingEvent` returns the `sendEvent` promise and is invoked as fire-and-forget at the component boundary, so preserve the convention of `void` plus an explicit rejection handler (`.catch(() => {})`). Currently `sendEvent` swallows its own errors and resolves `null`, so this is defensive, but it keeps the changed call consistent with the team convention for frontend telemetry.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| type AnalyticsProperties = Record<string, AnalyticsPrimitive> | ||
| type CaptureEvent = (name: string, supaHost: string, properties?: AnalyticsProperties) => void | ||
|
|
||
| const captureOnboardingEvent: CaptureEvent = (name, _supaHost, properties) => { |
There was a problem hiding this comment.
P2: The previous default capture (pushEvent) suppressed analytics outside the production host with if (isLocal(supaHost)) return. The new captureOnboardingEvent discards supaHost and always calls sendEvent, which POSTs to defaultApiHost/private/events whenever an auth session exists. This silently re-enables onboarding event delivery in local/dev builds, sending test-session events (or rejected tokens) to the configured API host. Gate the new path with the same isLocal(supaHost) guard before calling sendOnboardingEvent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/utils/onboardingProgressAnalytics.ts, line 53:
<comment>The previous default capture (`pushEvent`) suppressed analytics outside the production host with `if (isLocal(supaHost)) return`. The new `captureOnboardingEvent` discards `supaHost` and always calls `sendEvent`, which POSTs to `defaultApiHost/private/events` whenever an auth session exists. This silently re-enables onboarding event delivery in local/dev builds, sending test-session events (or rejected tokens) to the configured API host. Gate the new path with the same `isLocal(supaHost)` guard before calling `sendOnboardingEvent`.</comment>
<file context>
@@ -50,6 +50,10 @@ type AnalyticsPrimitive = string | number | boolean
type AnalyticsProperties = Record<string, AnalyticsPrimitive>
type CaptureEvent = (name: string, supaHost: string, properties?: AnalyticsProperties) => void
+const captureOnboardingEvent: CaptureEvent = (name, _supaHost, properties) => {
+ void sendOnboardingEvent(name, properties)
+}
</file context>
There was a problem hiding this comment.
Fixed centrally in 2e03d16. sendOnboardingEvent checks the configured Supabase host with isLocal before collecting browser context or dispatching.
| notify: false, | ||
| }).catch() | ||
| pushEvent(`user:onboarding-step-${stepToName(step.value)}`, config.supaHost, { org_id: orgId }) | ||
| void sendOnboardingEvent(`user:onboarding-step-${stepToName(step.value)}`, { org_id: orgId }) |
There was a problem hiding this comment.
P3: The old pushEvent skipped tracking when the API host was local (isLocal(config.supaHost)); sendOnboardingEvent routes through sendEvent, which sends to /private/events for any authenticated session with no local-env suppression. If a developer runs the console against the production/real backend (or VITE_API_HOST points to a non-local host), onboarding steps now emit real PostHog/analytics events from a dev environment where they were previously suppressed. Confirm this is intended or keep a local guard in sendOnboardingEvent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/dashboard/StepsApp.vue, line 97:
<comment>The old pushEvent skipped tracking when the API host was local (isLocal(config.supaHost)); sendOnboardingEvent routes through sendEvent, which sends to /private/events for any authenticated session with no local-env suppression. If a developer runs the console against the production/real backend (or VITE_API_HOST points to a non-local host), onboarding steps now emit real PostHog/analytics events from a dev environment where they were previously suppressed. Confirm this is intended or keep a local guard in sendOnboardingEvent.</comment>
<file context>
@@ -94,7 +94,7 @@ function setLog() {
notify: false,
}).catch()
- pushEvent(`user:onboarding-step-${stepToName(step.value)}`, config.supaHost, { org_id: orgId })
+ void sendOnboardingEvent(`user:onboarding-step-${stepToName(step.value)}`, { org_id: orgId })
}
}
</file context>
There was a problem hiding this comment.
Fixed centrally in 2e03d16. All onboarding calls now retain the previous non-production suppression through sendOnboardingEvent.
| type AnalyticsProperties = Record<string, AnalyticsPrimitive> | ||
| type CaptureEvent = (name: string, supaHost: string, properties?: AnalyticsProperties) => void | ||
|
|
||
| const captureOnboardingEvent: CaptureEvent = (name, _supaHost, properties) => { |
There was a problem hiding this comment.
P3: The new captureOnboardingEvent drops the isLocal(supaHost) guard that pushEvent previously applied. Onboarding analytics are now always dispatched through sendOnboardingEvent, so events from local/preview environments that were suppressed before now reach the analytics backend (and its downstream PostHog). If dev-noise suppression was intentional to keep, thread the locality check through sendOnboardingEvent/sendEvent or gate captureOnboardingEvent on isLocal(supaHost) instead of discarding it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/utils/onboardingProgressAnalytics.ts, line 53:
<comment>The new `captureOnboardingEvent` drops the `isLocal(supaHost)` guard that `pushEvent` previously applied. Onboarding analytics are now always dispatched through `sendOnboardingEvent`, so events from local/preview environments that were suppressed before now reach the analytics backend (and its downstream PostHog). If dev-noise suppression was intentional to keep, thread the locality check through `sendOnboardingEvent`/`sendEvent` or gate `captureOnboardingEvent` on `isLocal(supaHost)` instead of discarding it.</comment>
<file context>
@@ -50,6 +50,10 @@ type AnalyticsPrimitive = string | number | boolean
type AnalyticsProperties = Record<string, AnalyticsPrimitive>
type CaptureEvent = (name: string, supaHost: string, properties?: AnalyticsProperties) => void
+const captureOnboardingEvent: CaptureEvent = (name, _supaHost, properties) => {
+ void sendOnboardingEvent(name, properties)
+}
</file context>
There was a problem hiding this comment.
Fixed centrally in 2e03d16. The helper retains the old isLocal suppression for the configured Supabase host.
| void sendOnboardingEvent('onboarding_intent_selected', { | ||
| intent: selectedIntent.value, | ||
| estimated_mau: estimatedMau, | ||
| org_id: data.id, | ||
| }) |
There was a problem hiding this comment.
P3: The new fire-and-forget analytics call drops the rejection handling the replaced code had. The old onboarding_intent_selected path wrapped pushEvent in try/catch, and trackSuccessfulCopy called sendEvent(...).catch(() => {}). The new void sendOnboardingEvent('onboarding_intent_selected', {...}) leaves the returned promise without a rejection handler. sendOnboardingEvent returns the sendEvent promise and is invoked as fire-and-forget at the component boundary, so preserve the convention of void plus an explicit rejection handler (.catch(() => {})). Currently sendEvent swallows its own errors and resolves null, so this is defensive, but it keeps the changed call consistent with the team convention for frontend telemetry.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/dashboard/AppOnboardingFlow.vue, line 1513:
<comment>The new fire-and-forget analytics call drops the rejection handling the replaced code had. The old `onboarding_intent_selected` path wrapped `pushEvent` in try/catch, and `trackSuccessfulCopy` called `sendEvent(...).catch(() => {})`. The new `void sendOnboardingEvent('onboarding_intent_selected', {...})` leaves the returned promise without a rejection handler. `sendOnboardingEvent` returns the `sendEvent` promise and is invoked as fire-and-forget at the component boundary, so preserve the convention of `void` plus an explicit rejection handler (`.catch(() => {})`). Currently `sendEvent` swallows its own errors and resolves `null`, so this is defensive, but it keeps the changed call consistent with the team convention for frontend telemetry.</comment>
<file context>
@@ -1511,16 +1510,11 @@ async function createOrganizationAndApp() {
- catch (eventError) {
- console.error('Failed to track onboarding intent', eventError)
- }
+ void sendOnboardingEvent('onboarding_intent_selected', {
+ intent: selectedIntent.value,
+ estimated_mau: estimatedMau,
</file context>
| void sendOnboardingEvent('onboarding_intent_selected', { | |
| intent: selectedIntent.value, | |
| estimated_mau: estimatedMau, | |
| org_id: data.id, | |
| }) | |
| void sendOnboardingEvent('onboarding_intent_selected', { | |
| intent: selectedIntent.value, | |
| estimated_mau: estimatedMau, | |
| org_id: data.id, | |
| }).catch(() => {}) |
There was a problem hiding this comment.
Fixed centrally in 2e03d16. sendOnboardingEvent now returns void and owns the sendEvent promise with a rejection handler, so component callers cannot leak an unhandled rejection.
| ...getPostHogBrowserContext(), | ||
| }, | ||
| ...(orgId ? { org_id: orgId } : {}), | ||
| ...(event === AI_INSTRUCTIONS_COPIED_EVENT && appId ? { tags: { app_id: appId } } : {}), |
There was a problem hiding this comment.
P3: The app association is sent to the backend only for the AI-copy event. app_id is promoted to tags.app_id solely when event === AI_INSTRUCTIONS_COPIED_EVENT; for all other onboarding events that carry app_id, it stays inside nonPersonTags, which the backend's getAppId in private/events.ts ignores. As a result those events reach the endpoint without app attribution or app-level access control, silently dropping the app association the properties intend to carry. Promote app_id to tags consistently (or document why only the copy event should carry it).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/services/onboardingTracking.ts, line 20:
<comment>The app association is sent to the backend only for the AI-copy event. `app_id` is promoted to `tags.app_id` solely when `event === AI_INSTRUCTIONS_COPIED_EVENT`; for all other onboarding events that carry `app_id`, it stays inside `nonPersonTags`, which the backend's `getAppId` in `private/events.ts` ignores. As a result those events reach the endpoint without app attribution or app-level access control, silently dropping the app association the properties intend to carry. Promote `app_id` to `tags` consistently (or document why only the copy event should carry it).</comment>
<file context>
@@ -0,0 +1,24 @@
+ ...getPostHogBrowserContext(),
+ },
+ ...(orgId ? { org_id: orgId } : {}),
+ ...(event === AI_INSTRUCTIONS_COPIED_EVENT && appId ? { tags: { app_id: appId } } : {}),
+ timestamp: Date.now(),
+ tracking_version: 2,
</file context>
There was a problem hiding this comment.
Kept intentionally. app_id already remains in nonPersonTags for event attribution. Promoting every app_id to tags would make the backend PostHog adapter include it in the $set person properties, contrary to the event-only design. Only onboarding_ai_instructions_copied duplicates app_id into tags because the existing Bento mapper requires verified app context; that exception is now documented in code.
There was a problem hiding this comment.
All reported issues were addressed across 11 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
1 participant
Summary
/private/eventsendpoint while preserving the previous non-production suppressiononboarding_ai_instructions_copiedonce to both PostHog and the existing Bento mappingValidation
bun lint(zero errors; existing warnings only)bun lint:backendbun typecheckbun test:unit(260 files, 2,156 tests)Summary by CodeRabbit
Enhancements
Bug Fixes
Tests