fix(analytics): simplify desktop analytics and fix funnels#502
fix(analytics): simplify desktop analytics and fix funnels#502saddlepaddle merged 1 commit intomainfrom
Conversation
|
Warning Rate limit exceeded@saddlepaddle has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 7 minutes and 44 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughSplits the single dashboard funnel into Activation and Marketing funnels with independent time ranges; adds a TRPC analytics router and setUserId mutation; centralizes PostHog client and setUserId usage in desktop main/renderer flows; renames and adds funnel procedures in the TRPC analytics backend. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Renderer
participant TRPC_Server
participant MainProcess
participant PostHog
rect rgba(150,200,255,0.08)
Note over Renderer,TRPC_Server: User identification flow (new)
Renderer->>PostHog: posthog.identify(user.id)
Renderer->>TRPC_Server: trpc.analytics.setUserId({ userId: user.id })
TRPC_Server->>MainProcess: (rpc) setUserId -> set module userId
MainProcess->>PostHog: posthog.identify / set internal user id
end
rect rgba(200,255,180,0.06)
Note over Renderer,TRPC_Server: Sign-out / reset flow
Renderer->>PostHog: posthog.reset()
Renderer->>TRPC_Server: trpc.analytics.setUserId({ userId: null })
TRPC_Server->>MainProcess: (rpc) setUserId(null)
MainProcess->>PostHog: posthog.reset / clear id
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
🧹 Nitpick comments (1)
apps/desktop/src/main/lib/analytics/index.ts (1)
4-5: Consider makingposthogread-only to prevent external reassignment.The
let posthogexport is mutable, allowing consumers to accidentally reassign it. TheuserIdis appropriately module-private, butposthogcould benefit from the same encapsulation.🔎 Alternative approach using getter
-export let posthog: PostHog | null = null; +let _posthog: PostHog | null = null; + +export function getPostHog(): PostHog | null { + return _posthog; +} let userId: string | null = null; function getClient(): PostHog | null { if (!env.NEXT_PUBLIC_POSTHOG_KEY) { return null; } - if (!posthog) { - posthog = new PostHog(env.NEXT_PUBLIC_POSTHOG_KEY, { + if (!_posthog) { + _posthog = new PostHog(env.NEXT_PUBLIC_POSTHOG_KEY, { host: env.NEXT_PUBLIC_POSTHOG_HOST, flushAt: 1, flushInterval: 0, }); } - return posthog; + return _posthog; }Then update
apps/desktop/src/main/index.tsto usegetPostHog()?.shutdown().
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/admin/src/app/(dashboard)/page.tsxapps/desktop/src/lib/trpc/routers/analytics/index.tsapps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/lib/trpc/routers/projects/projects.tsapps/desktop/src/main/index.tsapps/desktop/src/main/lib/analytics/index.tsapps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxpackages/trpc/src/router/analytics/analytics.ts
💤 Files with no reviewable changes (1)
- apps/desktop/src/lib/trpc/routers/auth/index.ts
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/main/lib/analytics/index.tsapps/desktop/src/lib/trpc/routers/analytics/index.tsapps/admin/src/app/(dashboard)/page.tsxapps/desktop/src/main/index.tsapps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/lib/trpc/routers/projects/projects.tspackages/trpc/src/router/analytics/analytics.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix
Files:
apps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/main/lib/analytics/index.tsapps/desktop/src/lib/trpc/routers/analytics/index.tsapps/admin/src/app/(dashboard)/page.tsxapps/desktop/src/main/index.tsapps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/lib/trpc/routers/projects/projects.tspackages/trpc/src/router/analytics/analytics.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/main/lib/analytics/index.tsapps/desktop/src/lib/trpc/routers/analytics/index.tsapps/desktop/src/main/index.tsapps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/lib/trpc/routers/projects/projects.ts
apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Files:
apps/desktop/src/main/lib/analytics/index.tsapps/desktop/src/main/index.tsapps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx
apps/desktop/src/main/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Accept object parameters in IPC handlers - do not use positional parameters in ipcMain.handle()
Files:
apps/desktop/src/main/lib/analytics/index.tsapps/desktop/src/main/index.ts
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + TailwindCSS v4 + shadcn/ui for UI development
Files:
apps/admin/src/app/(dashboard)/page.tsxapps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx
apps/desktop/src/main/index.ts
📄 CodeRabbit inference engine (AGENTS.md)
Load environment variables in src/main/index.ts and electron.vite.config.ts with override: true before other imports
Files:
apps/desktop/src/main/index.ts
**/{components,features}/**/[!.]*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Organize project structure with one folder per component: ComponentName/ComponentName.tsx with index.ts barrel export
Files:
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx
**/{components,features}/**/*.{ts,tsx,test.ts,test.tsx,stories.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Files:
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)
Files:
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx
**/{components,features}/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/{components,features}/**/*.tsx: Nest components in parent's components/ folder if used only once, promote to highest shared parent's components/ if used 2+ times
Use one component per file - do not combine multiple components in a single file
Files:
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx
🧬 Code graph analysis (7)
apps/desktop/src/lib/trpc/routers/index.ts (1)
apps/desktop/src/lib/trpc/routers/analytics/index.ts (1)
createAnalyticsRouter(5-13)
apps/desktop/src/main/lib/analytics/index.ts (1)
apps/desktop/src/main/env.main.ts (1)
env(12-44)
apps/desktop/src/lib/trpc/routers/analytics/index.ts (2)
apps/desktop/src/lib/trpc/index.ts (1)
router(47-47)apps/desktop/src/main/lib/analytics/index.ts (1)
setUserId(22-24)
apps/admin/src/app/(dashboard)/page.tsx (2)
apps/admin/src/app/(dashboard)/components/TimeRangePicker/TimeRangePicker.tsx (2)
TimeRange(5-5)TimeRangePicker(19-35)apps/admin/src/app/(dashboard)/components/FunnelChart/FunnelChart.tsx (1)
FunnelChart(42-126)
apps/desktop/src/main/index.ts (1)
apps/desktop/src/main/lib/analytics/index.ts (1)
posthog(4-4)
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (2)
apps/desktop/src/main/lib/analytics/index.ts (1)
posthog(4-4)apps/desktop/src/renderer/lib/posthog.ts (1)
posthog(28-28)
apps/desktop/src/lib/trpc/routers/projects/projects.ts (1)
apps/desktop/src/main/lib/analytics/index.ts (1)
track(26-44)
⏰ 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). (6)
- GitHub Check: Deploy Web
- GitHub Check: Deploy Docs
- GitHub Check: Deploy API
- GitHub Check: Deploy Marketing
- GitHub Check: Deploy Admin
- GitHub Check: Build
🔇 Additional comments (11)
packages/trpc/src/router/analytics/analytics.ts (2)
57-95: LGTM! Well-structured activation funnel.The funnel events flow logically: App Opened → Signed Up → Opened Project → Created Workspace. The input validation with Zod is correct, and the default value handling is consistent.
97-126: LGTM! Marketing funnel complements the activation funnel well.The marketing funnel captures the site-to-app journey (Site Visit → Download → App Opened), while activation tracks in-app progression. This separation provides clearer insights into each user journey phase.
apps/desktop/src/lib/trpc/routers/index.ts (1)
3-3: LGTM! Clean router integration.The analytics router is properly imported and integrated into the app router, following the established pattern of other domain-specific routers.
Also applies to: 29-29
apps/desktop/src/lib/trpc/routers/analytics/index.ts (1)
1-15: LGTM! Clean analytics router implementation.The router is minimal and focused. Using
publicProcedureis appropriate here since setting the analytics user ID is part of the app initialization flow and doesn't require authentication gating.apps/desktop/src/lib/trpc/routers/projects/projects.ts (3)
304-307: LGTM! Analytics tracking for project open.Tracking is correctly placed after successful project upsert, with appropriate
method: "open"property to distinguish the entry point.
359-362: LGTM! Analytics tracking for git init flow.Consistent pattern with
method: "init"to identify projects created via git initialization.
433-437: LGTM! Analytics tracking for clone flow.Both code paths (existing project reuse and new clone) correctly track
project_openedwithmethod: "clone".Also applies to: 481-484
apps/desktop/src/main/index.ts (1)
10-10: LGTM! Clean analytics lifecycle integration.The import change and shutdown call are correct. Using
posthog?.shutdown()with optional chaining properly handles the case where PostHog is not initialized (e.g., missing API key).Also applies to: 148-151
apps/admin/src/app/(dashboard)/page.tsx (2)
22-42: LGTM! Clean funnel state separation.The two funnel queries with independent time range controls provide users with granular control over each funnel's analysis period.
119-145: LGTM! Well-structured funnel chart components.Both funnels have appropriate titles and descriptions that clearly communicate their purpose. The data binding and loading/error states are correctly wired.
apps/desktop/src/main/lib/analytics/index.ts (1)
26-43: LGTM! Clean event tracking implementation.The guard on
userIdensures events aren't sent before user identification, and the property augmentation withapp_nameandplatformprovides consistent metadata across all events.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 Preview resources have been processed for cleanup |
saddlepaddle
force-pushed
the
mighty-earthworm-aeb0a3
branch
from
1cb1eb2 to
aee99d4
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
apps/desktop/src/main/lib/analytics/index.ts (2)
4-5: Consider exportingposthogas a getter function instead of a mutable variable.Exporting
posthogasletallows external code to reassign it, which could break the singleton pattern. Consider either:
- Exporting a getter function:
export const getPostHog = () => posthog- Using
export const posthogwith a getter that initializes lazilyThe current approach works but is unconventional and could lead to accidental reassignment.
</invoke_comment_end -->
26-44: Add error handling to prevent silent tracking failures.The
track()function silently fails if the PostHog client is unavailable or if an error occurs during capture. Consider adding error handling to log failures for debugging.🔎 Proposed fix with error handling
export function track( event: string, properties?: Record<string, unknown>, ): void { if (!userId) return; const client = getClient(); if (!client) return; - client.capture({ - distinctId: userId, - event, - properties: { - ...properties, - app_name: "desktop", - platform: process.platform, - }, - }); + try { + client.capture({ + distinctId: userId, + event, + properties: { + ...properties, + app_name: "desktop", + platform: process.platform, + }, + }); + } catch (error) { + console.error("[analytics] Failed to track event:", event, error); + } }apps/desktop/src/lib/trpc/routers/analytics/index.ts (1)
7-11: Consider returning a value from the mutation for better debugging.The
setUserIdmutation doesn't return anything. Consider returning a success indicator or void explicitly to make the API contract clearer.🔎 Proposed enhancement
setUserId: publicProcedure .input(z.object({ userId: z.string().nullable() })) .mutation(({ input }) => { setUserId(input.userId); + return { success: true }; }),
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/admin/src/app/(dashboard)/page.tsxapps/desktop/src/lib/trpc/routers/analytics/index.tsapps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/lib/trpc/routers/projects/projects.tsapps/desktop/src/main/index.tsapps/desktop/src/main/lib/analytics/index.tsapps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxpackages/trpc/src/router/analytics/analytics.ts
💤 Files with no reviewable changes (1)
- apps/desktop/src/lib/trpc/routers/auth/index.ts
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/src/lib/trpc/routers/projects/projects.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx
- apps/desktop/src/lib/trpc/routers/index.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/desktop/src/lib/trpc/routers/analytics/index.tsapps/desktop/src/main/index.tspackages/trpc/src/router/analytics/analytics.tsapps/admin/src/app/(dashboard)/page.tsxapps/desktop/src/main/lib/analytics/index.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix
Files:
apps/desktop/src/lib/trpc/routers/analytics/index.tsapps/desktop/src/main/index.tspackages/trpc/src/router/analytics/analytics.tsapps/admin/src/app/(dashboard)/page.tsxapps/desktop/src/main/lib/analytics/index.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/lib/trpc/routers/analytics/index.tsapps/desktop/src/main/index.tsapps/desktop/src/main/lib/analytics/index.ts
apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Files:
apps/desktop/src/main/index.tsapps/desktop/src/main/lib/analytics/index.ts
apps/desktop/src/main/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Accept object parameters in IPC handlers - do not use positional parameters in ipcMain.handle()
Files:
apps/desktop/src/main/index.tsapps/desktop/src/main/lib/analytics/index.ts
apps/desktop/src/main/index.ts
📄 CodeRabbit inference engine (AGENTS.md)
Load environment variables in src/main/index.ts and electron.vite.config.ts with override: true before other imports
Files:
apps/desktop/src/main/index.ts
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + TailwindCSS v4 + shadcn/ui for UI development
Files:
apps/admin/src/app/(dashboard)/page.tsx
🧬 Code graph analysis (3)
packages/trpc/src/router/analytics/analytics.ts (1)
packages/trpc/src/lib/posthog-client.ts (1)
executeFunnelQuery(190-210)
apps/admin/src/app/(dashboard)/page.tsx (2)
apps/admin/src/app/(dashboard)/components/TimeRangePicker/TimeRangePicker.tsx (2)
TimeRange(5-5)TimeRangePicker(19-35)apps/admin/src/app/(dashboard)/components/FunnelChart/FunnelChart.tsx (1)
FunnelChart(42-126)
apps/desktop/src/main/lib/analytics/index.ts (1)
apps/desktop/src/main/env.main.ts (1)
env(12-44)
⏰ 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). (6)
- GitHub Check: Deploy Admin
- GitHub Check: Deploy Docs
- GitHub Check: Deploy Web
- GitHub Check: Deploy Marketing
- GitHub Check: Deploy API
- GitHub Check: Build
🔇 Additional comments (8)
packages/trpc/src/router/analytics/analytics.ts (4)
138-183: LGTM: WAU trend calculation looks correct.The Weekly Active User (WAU) calculation with a 7-day lookback window and 3-day minimum activity threshold is properly implemented. The zero-filling logic ensures consistent data points across the date range.
</invoke_comment_end -->
189-189: Theterminal_openedevent is properly tracked in the desktop app.The event is tracked when terminals are created in
apps/desktop/src/main/lib/terminal/manager.ts(line 81) with relevant metadata (workspace_id, pane_id). The change to useterminal_openedas the returning event for retention analysis is consistent with the actual tracking implementation.
57-95: Breaking change: Procedure renamed without deprecation path.The procedure
getFullJourneyFunnelhas been renamed togetActivationFunnel. While this is a breaking change for external API consumers, all internal references within the codebase have been updated. This appears to be an intentional split as part of introducing separate activation and marketing funnels.
110-120: Marketing Funnel events are properly tracked.All three events used in the funnel are being tracked across the codebase:
$pageviewis enabled viacapture_pageview: "history_change"in the marketing app's PostHog configurationdownload_clickedis captured atapps/marketing/src/app/components/DownloadButton/DownloadButton.tsxdesktop_openedis captured atapps/desktop/src/renderer/screens/sign-in/index.tsxPostHog is properly configured and initialized across all apps using the same project credentials, so events are correctly routed to the same instance.
apps/admin/src/app/(dashboard)/page.tsx (2)
22-42: LGTM: Clean separation of funnel state and queries.The split of the single funnel into separate Activation and Marketing funnels with independent time ranges is well-structured. Each funnel has its own:
- State variable for time range
- TRPC query with proper options
- Loading and error states
This follows the same pattern used for other dashboard metrics.
</invoke_comment_end -->
119-145: LGTM: Funnel charts properly configured.Both funnel charts are correctly configured with:
- Descriptive titles and descriptions
- Proper data binding from queries
- Loading and error state handling
- Independent TimeRangePicker controls
The Activation Funnel focuses on in-app progression while the Marketing Funnel tracks the acquisition journey, which aligns with the PR objectives.
</invoke_comment_end -->
apps/desktop/src/main/index.ts (1)
150-150: The PostHog shutdown implementation is correct.The code properly awaits
posthog?.shutdown()viaPromise.all(). PostHog's shutdown() method returnsPromise<void>, which flushes queued events before exit. The pattern used here is correct and ensures proper cleanup.apps/desktop/src/lib/trpc/routers/analytics/index.ts (1)
1-1: The import path and process boundaries are correctly configured. The wildcard tsconfig path alias"*": ["src/*"]properly resolves"main/lib/analytics"to the actual file atsrc/main/lib/analytics/index.ts. ThecreateAnalyticsRouter()factory is instantiated only in the main process (viacreateAppRouter), ensuring that the Node.js module usage (PostHog, process.platform) remains within the main process context where it's safe. No action required.Likely an incorrect or invalid review comment.
| if (!posthog) { | ||
| posthog = new PostHog(env.NEXT_PUBLIC_POSTHOG_KEY, { | ||
| host: env.NEXT_PUBLIC_POSTHOG_HOST, | ||
| flushAt: 1, // Send events immediately for desktop app | ||
| flushAt: 1, | ||
| flushInterval: 0, | ||
| }); | ||
| } | ||
| return client; | ||
| return posthog; |
There was a problem hiding this comment.
Potential race condition in PostHog client initialization.
If getClient() is called concurrently before posthog is initialized, multiple PostHog instances could be created. While this is unlikely in a single-threaded Node.js environment, consider initializing the client once during module load or using a more explicit initialization pattern.
🔎 Alternative initialization pattern
-export let posthog: PostHog | null = null;
+export let posthog: PostHog | null = initializePostHog();
let userId: string | null = null;
-function getClient(): PostHog | null {
+function initializePostHog(): PostHog | null {
if (!env.NEXT_PUBLIC_POSTHOG_KEY) {
return null;
}
-
- if (!posthog) {
- posthog = new PostHog(env.NEXT_PUBLIC_POSTHOG_KEY, {
- host: env.NEXT_PUBLIC_POSTHOG_HOST,
- flushAt: 1,
- flushInterval: 0,
- });
- }
- return posthog;
+ return new PostHog(env.NEXT_PUBLIC_POSTHOG_KEY, {
+ host: env.NEXT_PUBLIC_POSTHOG_HOST,
+ flushAt: 1,
+ flushInterval: 0,
+ });
}📝 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.
| if (!posthog) { | |
| posthog = new PostHog(env.NEXT_PUBLIC_POSTHOG_KEY, { | |
| host: env.NEXT_PUBLIC_POSTHOG_HOST, | |
| flushAt: 1, // Send events immediately for desktop app | |
| flushAt: 1, | |
| flushInterval: 0, | |
| }); | |
| } | |
| return client; | |
| return posthog; | |
| export let posthog: PostHog | null = initializePostHog(); | |
| let userId: string | null = null; | |
| function initializePostHog(): PostHog | null { | |
| if (!env.NEXT_PUBLIC_POSTHOG_KEY) { | |
| return null; | |
| } | |
| return new PostHog(env.NEXT_PUBLIC_POSTHOG_KEY, { | |
| host: env.NEXT_PUBLIC_POSTHOG_HOST, | |
| flushAt: 1, | |
| flushInterval: 0, | |
| }); | |
| } | |
| function setUserId(id: string | null): void { | |
| userId = id; | |
| if (posthog && id) { | |
| posthog.identify(id); | |
| } | |
| } | |
| function track(event: string, properties?: Record<string, unknown>): void { | |
| if (!posthog || !userId) { | |
| return; | |
| } | |
| posthog.capture({ | |
| distinctId: userId, | |
| event, | |
| properties, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In apps/desktop/src/main/lib/analytics/index.ts around lines 12-19, the lazy
getClient() code can create multiple PostHog instances if called concurrently;
replace the lazy unchecked init with a single-module initialization or an
explicit init/get pattern: either instantiate posthog once at module load using
env.NEXT_PUBLIC_POSTHOG_KEY and env.NEXT_PUBLIC_POSTHOG_HOST, or add a
module-level initPromise and an init(env) function that creates the PostHog
client exactly once (store the instance on success and have getClient await
initPromise), ensuring concurrent callers share the same in-flight
initialization and only one PostHog instance is created.
| flushAt: 1, | ||
| flushInterval: 0, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/desktop/src/main/lib/analytics/index.tsRepository: superset-sh/superset
Length of output: 1181
🏁 Script executed:
# Search for any configuration or environment-based settings for PostHog
rg -i "flush|posthog" apps/desktop/src/main/lib/analytics/ -A 3 -B 3Repository: superset-sh/superset
Length of output: 1533
🏁 Script executed:
# Check if there are environment-specific configurations
rg "NODE_ENV|process.env" apps/desktop/src/main/lib/analytics/ -A 2 -B 2Repository: superset-sh/superset
Length of output: 46
🌐 Web query:
PostHog Electron desktop application recommended flush settings production
💡 Result:
Short answer
- Use the SDK defaults for a desktop Electron app: flushAt = 20 and flushInterval = 10000 ms (10s). These are the sane production defaults for long‑running apps. [1][2]
When you might change them
- Make flushAt smaller (eg. 1) and flushInterval = 0 only for short‑lived/serverless contexts to avoid losing events. [2]
- If you want fewer network calls (reduce bandwidth/battery), increase flushAt (e.g. 50) and/or flushInterval (e.g. 60000 ms). If you need near‑real‑time events, lower flushAt or flushInterval. [1][2]
References
- PostHog Node SDK options (defaults & serverless guidance). [1]
- PostHog library docs showing flushAt/flushInterval settings and description. [2]
Sources
[1] PostHog Node.js docs — options and serverless recommendations.
[2] PostHog library docs (flushInterval / flushAt description).
Use PostHog's default flush settings for long-running desktop applications.
flushAt: 1 and flushInterval: 0 cause every event to be flushed immediately. PostHog recommends these settings only for short-lived/serverless contexts. Since this is an Electron desktop application (a long-running process), use the SDK defaults: flushAt: 20 and flushInterval: 10000 (10 seconds). The current settings will create unnecessary network calls on every tracking event, impacting performance and battery life in production.
If different settings are needed for testing, configure them based on the environment rather than hardcoding aggressive values.
🤖 Prompt for AI Agents
In apps/desktop/src/main/lib/analytics/index.ts around lines 15-16, the PostHog
client is configured with flushAt: 1 and flushInterval: 0 which forces immediate
flushes; change these to use PostHog SDK defaults for long-running desktop apps
(flushAt: 20 and flushInterval: 10000) and make the values
environment-configurable so tests/dev can override (e.g., read from env or a
config flag and fall back to defaults) to avoid per-event network calls in
production.
| event: "desktop_opened", | ||
| name: "App Opened", | ||
| }, | ||
| { | ||
| kind: "EventsNode", | ||
| event: "desktop_opened", | ||
| name: "Desktop Opened", | ||
| event: "auth_completed", | ||
| name: "Signed Up", | ||
| }, | ||
| { | ||
| kind: "EventsNode", | ||
| event: "auth_completed", | ||
| name: "Auth Completed", | ||
| event: "project_opened", | ||
| name: "Opened Project", | ||
| }, | ||
| { | ||
| kind: "EventsNode", | ||
| event: "terminal_opened", | ||
| name: "Terminal Opened", | ||
| event: "workspace_created", | ||
| name: "Created Workspace", | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for where these events are tracked in the desktop app
echo "=== Searching for desktop_opened tracking ==="
rg -n "desktop_opened" apps/desktop --type ts --type tsx -C 2
echo -e "\n=== Searching for project_opened tracking ==="
rg -n "project_opened" apps/desktop --type ts --type tsx -C 2
echo -e "\n=== Searching for auth_completed tracking ==="
rg -n "auth_completed" apps/desktop --type ts --type tsx -C 2
echo -e "\n=== Searching for workspace_created tracking ==="
rg -n "workspace_created" apps/desktop --type ts --type tsx -C 2Repository: superset-sh/superset
Length of output: 382
🏁 Script executed:
#!/bin/bash
# Search for where these events are tracked in the desktop app
# Using just basic rg without problematic file types
echo "=== Checking directory structure ==="
ls -la apps/ 2>/dev/null | head -20
echo -e "\n=== Searching for desktop_opened tracking ==="
rg -n "desktop_opened" apps/desktop -C 2 2>/dev/null
echo -e "\n=== Searching for project_opened tracking ==="
rg -n "project_opened" apps/desktop -C 2 2>/dev/null
echo -e "\n=== Searching for auth_completed tracking ==="
rg -n "auth_completed" apps/desktop -C 2 2>/dev/null
echo -e "\n=== Searching for workspace_created tracking ==="
rg -n "workspace_created" apps/desktop -C 2 2>/dev/null
# Also search in the entire repo if not in apps/desktop
echo -e "\n=== Checking if events are tracked anywhere in codebase ==="
rg -n "desktop_opened" --type ts -C 1 2>/dev/null | head -20Repository: superset-sh/superset
Length of output: 4061
The Activation Funnel includes auth_completed (Signed Up) event that is not tracked in the desktop application.
Three events are correctly tracked:
desktop_opened✓ (apps/desktop/src/renderer/screens/sign-in/index.tsx:15)project_opened✓ (apps/desktop/src/lib/trpc/routers/projects/projects.ts)workspace_created✓ (apps/desktop/src/lib/trpc/routers/workspaces/workspaces.ts:199)
However, auth_completed has no corresponding tracking in the desktop application. Remove it from the funnel or add the tracking event using posthog.capture("auth_completed") or track("auth_completed") where signup completes.
🤖 Prompt for AI Agents
In packages/trpc/src/router/analytics/analytics.ts around lines 72-89 the
Activation Funnel lists an `auth_completed` ("Signed Up") event that the desktop
app does not emit; either remove this event from the funnel or add desktop
tracking: implement posthog.capture("auth_completed") or track("auth_completed")
at the point signup completes in the desktop app (e.g., the signup completion
handler in apps/desktop/src/renderer/screens/sign-in or the signup success
flow), ensuring the event name matches exactly and any user/context properties
required by analytics are passed.
- Simplify main process analytics to just setUserId + track - Add project_opened tracking for activation funnel - Split into activation funnel (app → signup → project → workspace) and marketing funnel (site → download → app) - Sync user ID between renderer and main via single tRPC call - Remove complex anonymous ID aliasing - just use user ID directly 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
saddlepaddle
force-pushed
the
mighty-earthworm-aeb0a3
branch
from
aee99d4 to
c7e012f
Compare
1 participant
Summary
setUserId+trackproject_openedtracking for activation funnelTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.